Subject: bug#60936: 30.0.50; ERC >5.5: Add erc-fill style based on visual-line-mode

From: "J.P." <[email-masked]>
Date: Mon, 23 Oct 2023 19:19:47 -0700

Some bugs have surfaced stemming from recent work on this initiative.
Most come down to sloppiness on my part. The worst of the bunch involves
`erc-insert-done-hook' being narrowed on date-stamp insertion, which
defies a tacit agreement to the contrary. A related bug concerns members
of the new internal date-stamp hook possibly running twice if the latter
has a buffer-local value.

I've also added a new helper for deleting inserted messages. It attempts
to respect user markers and invisibility props.

>From b1b473f23db097106fb250686c06f4e8ef5d536f Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email-masked]>
Date: Sat, 21 Oct 2023 13:53:46 -0700
Subject: [PATCH] [5.6] Ensure marker for max pos in erc--traverse-inserted

* lisp/erc/erc-stamp.el (erc-stamp--propertize-left-date-stamp):
Run `erc-stamp--insert-date-hook' here.
(erc-stamp--insert-date-stamp-as-phony-message): Don't include value
of `erc-stamp--insert-date-hook' in let-bound `erc-insert-modify-hook'
because it runs twice if buffer-local.  Also call getter for
`erc-stamp--current-time' and remove `erc-send-modify-hook' because
that only runs via `erc-display-msg'.
(erc-stamp--lr-date-on-pre-modify,
erc-insert-timestamp-left-and-right): Use function form of
`erc-stamp--current-time' for determining current time stamp.
* lisp/erc/erc.el (erc--traverse-inserted): Create temporary marker
when END is non-nil and not already a marker so that insertions and
deletions do not affect the position at which the loop should end.
(erc--delete-inserted-message): New function.
* test/lisp/erc/erc-tests.el (erc--delete-inserted-message): New test.
(erc--update-modules/unknown): Improve readability slightly.
* test/lisp/erc/resources/erc-d/erc-d-t.el (erc-d-t-make-expecter):
Indicate assertion flavor in error message.  (Bug#60936)
---
 lisp/erc/erc-stamp.el                    | 17 ++++---
 lisp/erc/erc.el                          | 33 +++++++++++--
 test/lisp/erc/erc-tests.el               | 61 +++++++++++++++++++++---
 test/lisp/erc/resources/erc-d/erc-d-t.el |  1 +
 4 files changed, 94 insertions(+), 18 deletions(-)

diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index b515513dcb7..56fa975c32d 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -638,7 +638,8 @@ erc-stamp--date-format-end
 (defun erc-stamp--propertize-left-date-stamp ()
   (add-text-properties (point-min) (1- (point-max))
                        '(field erc-timestamp erc-stamp-type date-left))
-  (erc--hide-message 'timestamp))
+  (erc--hide-message 'timestamp)
+  (run-hooks 'erc-stamp--insert-date-hook))
 
 ;; A kludge to pass state from insert hook to nested insert hook.
 (defvar erc-stamp--current-datestamp-left nil)
@@ -665,19 +666,17 @@ erc-stamp--insert-date-stamp-as-phony-message
   (cl-assert string)
   (let ((erc-stamp--skip t)
         (erc--msg-props (map-into `((erc-msg . datestamp)
-                                    (erc-ts . ,erc-stamp--current-time))
+                                    (erc-ts . ,(erc-stamp--current-time)))
                                   'hash-table))
-        (erc-send-modify-hook `(,@erc-send-modify-hook
-                                erc-stamp--propertize-left-date-stamp
-                                ,@erc-stamp--insert-date-hook))
         (erc-insert-modify-hook `(,@erc-insert-modify-hook
-                                  erc-stamp--propertize-left-date-stamp
-                                  ,@erc-stamp--insert-date-hook)))
+                                  erc-stamp--propertize-left-date-stamp))
+        ;; Don't run hooks that aren't expecting a narrowed buffer.
+        (erc-insert-done-hook nil))
     (erc-display-message nil nil (current-buffer) string)
     (setq erc-timestamp-last-inserted-left string)))
 
 (defun erc-stamp--lr-date-on-pre-modify (_)
-  (when-let ((ct (or erc-stamp--current-time (erc-stamp--current-time)))
+  (when-let ((ct (erc-stamp--current-time))
              (rendered (erc-stamp--format-date-stamp ct))
              ((not (string-equal rendered erc-timestamp-last-inserted-left)))
              (erc-stamp--current-datestamp-left rendered)
@@ -713,7 +712,7 @@ erc-insert-timestamp-left-and-right
       (narrow-to-region erc--insert-marker end-marker)
       (set-marker end-marker nil)
       (set-marker erc--insert-marker nil)))
-  (let* ((ct (or erc-stamp--current-time (erc-stamp--current-time)))
+  (let* ((ct (erc-stamp--current-time))
          (ts-right (with-suppressed-warnings
                        ((obsolete erc-timestamp-format-right))
                      (if erc-timestamp-format-right
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 7d75ec49ccd..92f6f1fcb1f 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -3006,8 +3006,12 @@ erc--with-inserted-msg
        ,@body)))
 
 (defun erc--traverse-inserted (beg end fn)
-  "Visit messages between BEG and END and run FN in narrowed buffer."
-  (setq end (min end (marker-position erc-insert-marker)))
+  "Visit messages between BEG and END and run FN in narrowed buffer.
+If END is a marker, possibly update its position."
+  (unless (markerp end)
+    (setq end (set-marker (make-marker) (or end erc-insert-marker))))
+  (unless (eq end erc-insert-marker)
+    (set-marker end (min erc-insert-marker end)))
   (save-excursion
     (goto-char beg)
     (let ((b (if (get-text-property (point) 'erc-msg)
@@ -3019,7 +3023,9 @@ erc--traverse-inserted
         (save-restriction
           (narrow-to-region b e)
           (funcall fn))
-        (setq b e)))))
+        (setq b e))))
+  (unless (eq end erc-insert-marker)
+    (set-marker end nil)))
 
 (defvar erc--insert-marker nil
   "Internal override for `erc-insert-marker'.")
@@ -3241,6 +3247,27 @@ erc--hide-message
           (cl-incf beg))
         (erc--merge-prop (1- beg) (1- end) 'invisible value)))))
 
+(defun erc--delete-inserted-message (beg-or-point &optional end)
+  "Remove message between BEG and END.
+Expect BEG and END to match bounds as returned by the macro
+`erc--get-inserted-msg-bounds'.  Ensure all markers residing at
+the start of the deleted message end up at the beginning of the
+subsequent message."
+  (let ((beg beg-or-point))
+    (save-restriction
+      (widen)
+      (unless end
+        (setq end (erc--get-inserted-msg-bounds nil beg-or-point)
+              beg (pop end)))
+      (with-silent-modifications
+        (if erc-legacy-invisible-bounds-p
+            (delete-region beg (1+ end))
+          (save-excursion
+            (goto-char beg)
+            (insert-before-markers
+             (substring (delete-and-extract-region (1- (point)) (1+ end))
+                        -1))))))))
+
 (defvar erc--ranked-properties '(erc-msg erc-ts erc-cmd))
 
 (defun erc--order-text-properties-from-hash (table)
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 57bf5860ac4..6429fce8861 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1432,6 +1432,57 @@ erc-process-input-line
 
           (should-not calls))))))
 
+(ert-deftest erc--delete-inserted-message ()
+  (erc-mode)
+  (erc--initialize-markers (point) nil)
+  ;; Put unique invisible properties on the line endings.
+  (erc-display-message nil 'notice nil "one")
+  (put-text-property (1- erc-insert-marker) erc-insert-marker 'invisible 'a)
+  (let ((erc--msg-prop-overrides '((erc-msg . datestamp) (erc-ts . 0))))
+    (erc-display-message nil nil nil
+                         (propertize "\n[date]" 'field 'erc-timestamp)))
+  (put-text-property (1- erc-insert-marker) erc-insert-marker 'invisible 'b)
+  (erc-display-message nil 'notice nil "two")
+
+  (ert-info ("Date stamp deleted cleanly")
+    (goto-char 11)
+    (should (looking-at (rx "\n[date]")))
+    (should (eq 'datestamp (get-text-property (point) 'erc-msg)))
+    (should (eq (point) (field-beginning (1+ (point)))))
+
+    (erc--delete-inserted-message (point))
+
+    ;; Preceding line ending clobbered, replaced by trailing.
+    (should (looking-back (rx "*** one\n")))
+    (should (looking-at (rx "*** two")))
+    (should (eq 'b (get-text-property (1- (point)) 'invisible))))
+
+  (ert-info ("Markers at pos-bol preserved")
+    (erc-display-message nil 'notice nil "three")
+    (should (looking-at (rx "*** two")))
+
+    (let ((m (point-marker))
+          (n (point-marker))
+          (p (point)))
+      (set-marker-insertion-type m t)
+      (goto-char (point-max))
+      (erc--delete-inserted-message p)
+      (should (= (marker-position n) p))
+      (should (= (marker-position m) p))
+      (goto-char p)
+      (set-marker m nil)
+      (set-marker n nil)
+      (should (looking-back (rx "*** one\n")))
+      (should (looking-at (rx "*** three")))))
+
+  (ert-info ("Compat")
+    (erc-display-message nil 'notice nil "four")
+    (should (looking-at (rx "*** three\n")))
+    (with-suppressed-warnings ((obsolete erc-legacy-invisible-bounds-p))
+      (let ((erc-legacy-invisible-bounds-p t))
+        (erc--delete-inserted-message (point))))
+    (should (looking-at (rx "*** four\n")))))
+
 (ert-deftest erc--order-text-properties-from-hash ()
   (let ((table (map-into '((a . 1)
                            (erc-ts . 0)
@@ -2617,8 +2668,8 @@ erc--update-modules/unknown
               (obarray (obarray-make))
               (err (should-error (erc--update-modules erc-modules))))
          (should (equal (cadr err) "`foo' is not a known ERC module"))
-         (should (equal (funcall get-calls)
-                        `((req . ,(intern-soft "erc-foo")))))))
+         (should (equal (mapcar #'prin1-to-string (funcall get-calls))
+                        '("(req . erc-foo)")))))
 
      ;; Module's mode command exists but lacks an associated file.
      (ert-info ("Bad autoload flagged as suspect")
@@ -2627,10 +2678,8 @@ erc--update-modules/unknown
               (obarray (obarray-make))
               (erc-modules (list (intern "foo"))))
 
-         ;; Create a mode activation command.
+         ;; Create a mode-activation command and make mode-var global.
          (funcall mk-cmd "foo")
-
-         ;; Make the mode var global.
          (funcall mk-global "foo")
 
          ;; No local modules to return.
@@ -2639,7 +2688,7 @@ erc--update-modules/unknown
                         '("foo")))
          ;; ERC requires the library via prefixed module name.
          (should (equal (mapcar #'prin1-to-string (funcall get-calls))
-                        `("(req . erc-foo)" "(erc-foo-mode . 1)"))))))))
+                        '("(req . erc-foo)" "(erc-foo-mode . 1)"))))))))
 
 ;; A local module (here, `lo2') lacks a mode toggle, so ERC tries to
 ;; load its defining library, first via the symbol property
diff --git a/test/lisp/erc/resources/erc-d/erc-d-t.el b/test/lisp/erc/resources/erc-d/erc-d-t.el
index cf869fb3c70..7126165fd91 100644
--- a/test/lisp/erc/resources/erc-d/erc-d-t.el
+++ b/test/lisp/erc/resources/erc-d/erc-d-t.el
@@ -157,6 +157,7 @@ erc-d-t-make-expecter
   (let (positions)
     (lambda (timeout text &optional reset-from)
       (let* ((pos (cdr (assq (current-buffer) positions)))
+             (erc-d-t--wait-message-prefix (and (< timeout 0) "Sustaining: "))
              (cb (lambda ()
                    (unless pos
                      (push (cons (current-buffer) (setq pos (make-marker)))
-- 
2.41.0