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

["J.P." <jp@neverwas.me>]

[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]

A couple more bugs stemming from this feature's introduction have
surfaced. The first involves stamp hiding when `erc-fill-wrap-mode' is
enabled. To reproduce from emacs -Q:

- Connect and join a channel
- In the channel buffer, set `erc-timestamp-last-inserted-left' to nil
- Say something and notice a new date stamp inserted
- Run M-x erc-toggle-timestamps RET
- Notice that the message after the stamp is dedented incorrectly

This problem occurs because date stamps are not well defined and
straddle roles occupied by normal stamps and standalone messages. The
remedy I've chosen retains compatibility at the cost of kicking the can
down the road WRT defining the precise role and expected behavior of
date stamps. (If still unclear, I say "date stamp" to mean a left-sided
stamp inserted by the function `erc-insert-timestamp-left-and-right' and
formatted using the string `erc-timestamp-format-left'.) This issue is
closely related to the interplay between normal right-hand stamps and
non-`fill-wrap' fill functions because the latter hard-wrap (i.e.,
"fill") messages, which results in a stamp often residing on its own
line.

The second issue comes down to the lack of an integration with
`text-scale-mode'. To reproduce from emacs -Q:

- Connect from a graphical Emacs
- In the server buffer, hit C-x C-=, and notice misaligned speaker tags
  among the upscaled text
- Run a command, like "/msg NickServ help", and notice the leading
  `erc-notice-prefix' portion of new messages correctly dedented
- Hit C-x C-0 and observe the just-inserted messages now looking mangled
  and the preexisting ones seemingly restored

The problem is that our `line-prefix' values use display specs with
pixel widths, which is needed for speakers with variable-width faces and
non-ascii chars. (Based on a cursory glance at relevant sections of the
manual, it doesn't look like there's an easy way to adjust these
automatically.) For now, I'm proposing we include a command to manually
traverse and refill target buffers. Luckily, this is much faster than
it'd be with some other `erc-fill-function' because no actual "filling"
takes place. We're just remeasuring speaker tags and replacing existing
display-spec values.

If you're affected by these bugs, please try these patches. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-5.6-Prefer-ticks-hz-pairs-for-erc-timestamp-values-o.patch --]
[-- Type: text/x-patch, Size: 1752 bytes --]

From c4d98ab82a9edac04abdde59df4055685f17b6cb Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 18 Sep 2023 22:50:28 -0700
Subject: [PATCH 1/3] [5.6] Prefer ticks/hz pairs for erc-timestamp values on
 <29

* lisp/erc/erc-compat.el (erc-compat--current-lisp-time): New macro to
prefer ticks/hz pairs on older Emacs versions.  They're easier to
compare at a glance when used as values for text properties.
* lisp/erc/erc-stamp.el (erc-stamp--current-time): Use compat macro.
(Bug#60936)
---
 lisp/erc/erc-compat.el | 6 ++++++
 lisp/erc/erc-stamp.el  | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/erc/erc-compat.el b/lisp/erc/erc-compat.el
index 109b5d245ab..4dae578de67 100644
--- a/lisp/erc/erc-compat.el
+++ b/lisp/erc/erc-compat.el
@@ -444,6 +444,12 @@ erc-compat--29-browse-url-irc
                  (cons '("\\`irc6?s?://" . erc-compat--29-browse-url-irc)
                        existing))))))
 
+(defmacro erc-compat--current-lisp-time ()
+  "Return `current-time' as a frequency pair."
+  (if (>= emacs-major-version 29)
+      '(let (current-time-list) (current-time))
+    '(time-convert nil t)))
+
 
 (provide 'erc-compat)
 
diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index f159b6d226f..0f3163bf68d 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -215,7 +215,7 @@ erc-stamp--current-time
 (cl-defgeneric erc-stamp--current-time ()
   "Return a lisp time object to associate with an IRC message.
 This becomes the message's `erc-timestamp' text property."
-  (let (current-time-list) (current-time)))
+  (erc-compat--current-lisp-time))
 
 (cl-defmethod erc-stamp--current-time :around ()
   (or erc-stamp--current-time (cl-call-next-method)))
-- 
2.41.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-5.6-Fix-date-stamp-invisibility-in-erc-fill-wrap.patch --]
[-- Type: text/x-patch, Size: 14131 bytes --]

From 0c2b76532490d85a5b622e57af5aa1320278a20c Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Thu, 21 Sep 2023 23:54:31 -0700
Subject: [PATCH 2/3] [5.6] Fix date-stamp invisibility in erc-fill-wrap

* lisp/erc/erc-fill.el (erc-fill--wrap-measure): New helper function,
factored out from common code shared by `erc-fill-wrap' and
`erc-fill--wrap-stamp-insert-prefixed-date'.
(erc-fill--wrap-stamp-insert-prefixed-date): Refactor for more general
use and decrement `invisible' bounds, when applicable.
(erc-fill-wrap): Use helper `erc-fill--wrap-measure'.
* lisp/erc/erc-stamp.el (erc-insert-timestamp-left-and-right): Mention
intervals of relevant text props in doc string.
* lisp/erc/erc.el (erc--hide-message): Don't bother offsetting start
of first message in a buffer.
(erc--own-property-names): Add `erc-stamp-type'.
* test/lisp/erc/erc-scenarios-match.el
(erc-scenarios-match--fill-wrap-stamp-dedented-p): New function.
(erc-scenarios-match--stamp-both-invisible-fill-wrap) New test.
(Bug#60936)
---
 lisp/erc/erc-fill.el                 |  54 ++++++++-----
 lisp/erc/erc-stamp.el                |   9 ++-
 lisp/erc/erc.el                      |   9 ++-
 test/lisp/erc/erc-scenarios-match.el | 112 ++++++++++++++++++++++++++-
 4 files changed, 162 insertions(+), 22 deletions(-)

diff --git a/lisp/erc/erc-fill.el b/lisp/erc/erc-fill.el
index f4835f71278..6d39bcb19b9 100644
--- a/lisp/erc/erc-fill.el
+++ b/lisp/erc/erc-fill.el
@@ -484,25 +484,45 @@ erc-fill--wrap-continued-message-p
               ((erc-nick-equal-p props nick))))
     (set-marker erc-fill--wrap-last-msg (point-min))))
 
-(defun erc-fill--wrap-stamp-insert-prefixed-date (&rest args)
-  "Apply `line-prefix' property to args."
-  (let* ((ts-left (car args))
-         (start)
+(defun erc-fill--wrap-measure (beg end)
+  "Return display spec width for inserted region between BEG and END.
+Ignore any `invisible' props that may be present when figuring."
+  (if (and erc-fill-wrap-use-pixels (fboundp 'buffer-text-pixel-size))
+      (save-restriction
+        (narrow-to-region beg end)
+        (let (buffer-invisibility-spec)
+          (list (car (buffer-text-pixel-size)))))
+    (- end beg)))
+
+(defun erc-fill--wrap-stamp-insert-prefixed-date (&rest _)
+  "Apply `line-prefix' property to args.
+Expect a multi-line \"date\" stamp, similar to that provided by
+the default value of `erc-timestamp-format-left'.  Add
+`erc-stamp-type' property with the symbol `date-left' as its
+value.  Possibly adjust invisibility interval to begin at the
+previous newline and extend until the end of the last line of the
+stamp, not including its line ending."
+  (let* ((beg)
          ;; Insert " " to simulate gap between <speaker> and msg beg.
          (end (save-excursion (skip-chars-backward "\n")
-                              (setq start (pos-bol))
+                              (setq beg (pos-bol))
                               (insert " ")
                               (point)))
-         (width (if (and erc-fill-wrap-use-pixels
-                         (fboundp 'buffer-text-pixel-size))
-                    (save-restriction (narrow-to-region start end)
-                                      (list (car (buffer-text-pixel-size))))
-                  (length (string-trim-left ts-left)))))
+         (width (erc-fill--wrap-measure beg end)))
     (delete-region (1- end) end)
-    ;; Use `point-min' instead of `start' to cover leading newilnes.
+    ;; Offset existing invisibility bounds by decrementing.  See
+    ;; `erc-legacy-invisible-bounds-p'.
+    (when-let ((invisible (get-text-property (point) 'invisible))
+               (min (point-min)))
+      (save-restriction
+        (widen)
+        (remove-text-properties (max 1 (1- min)) (1+ (point)) '(invisible nil))
+        (narrow-to-region min (1+ (point)))
+        (erc--hide-message invisible)))
+    (put-text-property (point-min) (point) 'erc-stamp-type 'date-left)
+    ;; Use `point-min' instead of `beg' to cover leading newilnes.
     (put-text-property (point-min) (point) 'line-prefix
-                       `(space :width (- erc-fill--wrap-value ,width))))
-  args)
+                       `(space :width (- erc-fill--wrap-value ,width)))))
 
 ;; An escape hatch for third-party code expecting speakers of ACTION
 ;; messages to be exempt from `line-prefix'.  This could be converted
@@ -536,12 +556,8 @@ erc-fill-wrap
                             (put-text-property (point-min) (point)
                                                'display "")
                             0)
-                           ((and erc-fill-wrap-use-pixels
-                                 (fboundp 'buffer-text-pixel-size))
-                            (save-restriction
-                              (narrow-to-region (point-min) (point))
-                              (list (car (buffer-text-pixel-size)))))
-                           (t (- (point) (point-min))))))))
+                           (t
+                            (erc-fill--wrap-measure (point-min) (point))))))))
       (erc-put-text-properties (point-min) (1- (point-max)) ; exclude "\n"
                                '(line-prefix wrap-prefix) nil
                                `((space :width (- erc-fill--wrap-value ,len))
diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index 0f3163bf68d..4e16906c550 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -609,7 +609,14 @@ erc-insert-timestamp-left-and-right
 When the deprecated option `erc-timestamp-format-right' is nil,
 use STRING, which originates from `erc-timestamp-format', for the
 right-hand stamp.  Use `erc-timestamp-format-left' for the
-left-hand stamp and expect it to change less frequently."
+left-hand stamp and expect it to change less frequently.  Include
+line endings present in `erc-timestamp-format-left' as part of
+the `erc-timestamp' field, which extends to the start of the
+message proper.  Do this so other code knows the stamp is part of
+the subsequent IRC message even though it may appear on its own
+line.  However, allow the stamp's `invisible' property to span a
+different interval, in order to satisfy newer folding
+requirements related to `erc-legacy-invisible-bounds-p'."
   (let* ((ct (or erc-stamp--current-time (erc-stamp--current-time)))
          (ts-left (erc-format-timestamp ct erc-timestamp-format-left))
          (ts-right (with-suppressed-warnings
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index ec4fae548c7..e4b0cd0ddbe 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -3046,7 +3046,11 @@ erc-legacy-invisible-bounds-p
 
 (defun erc--hide-message (value)
   "Apply `invisible' text-property with VALUE to current message.
-Expect to run in a narrowed buffer during message insertion."
+Expect to run in a narrowed buffer during message insertion.
+Begin the invisible interval at the previous message's trailing
+newline and end before the current message's.  If the preceding
+message ends in a double newline or there is no previous message,
+don't bother including the preceding newline."
   (if erc-legacy-invisible-bounds-p
       ;; Before ERC 5.6, this also used to add an `intangible'
       ;; property, but the docs say it's now obsolete.
@@ -3055,6 +3059,8 @@ erc--hide-message
           (end (point-max)))
       (save-restriction
         (widen)
+        (when (or (<= beg 4) (= ?\n (char-before (- beg 2))))
+          (cl-incf beg))
         (erc--merge-prop (1- beg) (1- end) 'invisible value)))))
 
 (defun erc-display-message-highlight (type string)
@@ -4770,6 +4776,7 @@ erc--own-property-names
      rear-nonsticky erc-prompt field front-sticky read-only
      ;; stamp
      cursor-intangible cursor-sensor-functions isearch-open-invisible
+     erc-stamp-type
      ;; match
      invisible intangible
      ;; button
diff --git a/test/lisp/erc/erc-scenarios-match.el b/test/lisp/erc/erc-scenarios-match.el
index cd899fddb98..bf74806207d 100644
--- a/test/lisp/erc/erc-scenarios-match.el
+++ b/test/lisp/erc/erc-scenarios-match.el
@@ -167,7 +167,6 @@ erc-scenarios-match--find-eol
 
 ;; In most cases, `erc-hide-fools' makes line endings invisible.
 (defun erc-scenarios-match--stamp-right-fools-invisible ()
-  :tags '(:expensive-test)
   (let ((erc-insert-timestamp-function #'erc-insert-timestamp-right))
     (erc-scenarios-match--invisible-stamp
 
@@ -271,6 +270,117 @@ erc-scenarios-match--stamp-right-invisible-fill-wrap
        (let ((inv-beg (next-single-property-change (1- (pos-bol)) 'invisible)))
          (should (eq (get-text-property inv-beg 'invisible) 'timestamp)))))))
 
+(defun erc-scenarios-match--fill-wrap-stamp-dedented-p (point)
+  (pcase (get-text-property point 'line-prefix)
+    (`(space :width (- erc-fill--wrap-value (,n)))
+     (if (display-graphic-p) (< 100 n 200) (< 10 n 30)))
+    (`(space :width (- erc-fill--wrap-value ,n))
+     (< 10 n 30))))
+
+(ert-deftest erc-scenarios-match--stamp-both-invisible-fill-wrap ()
+
+  ;; Rewind the clock to known date artificially.
+  (let ((erc-stamp--current-time 704591940)
+        (erc-stamp--tz t)
+        (erc-fill-function #'erc-fill-wrap)
+        (bob-utterance-counter 0))
+
+    (erc-scenarios-match--invisible-stamp
+
+     (lambda ()
+       (ert-info ("Baseline check")
+         ;; False date printed initially before anyone speaks.
+         (when (zerop bob-utterance-counter)
+           (save-excursion
+             (goto-char (point-min))
+             (search-forward "[Wed Apr 29 1992]")
+             ;; First stamp in a buffer is not invisible from previous
+             ;; newline (before stamp's own leading newline).
+             (should (= 4 (match-beginning 0)))
+             (should (get-text-property 3 'invisible))
+             (should-not (get-text-property 2 'invisible))
+             (should (erc-scenarios-match--fill-wrap-stamp-dedented-p 4))
+             (search-forward "[23:59]"))))
+
+       (ert-info ("Line endings in Bob's messages are invisible")
+         ;; The message proper has the `invisible' property `match-fools'.
+         (should (eq (get-text-property (pos-bol) 'invisible) 'match-fools))
+         (let* ((mbeg (or (and (get-text-property (pos-bol) 'erc-command)
+                               (pos-bol))
+                          (next-single-property-change (pos-bol)
+                                                       'erc-command)))
+                (mend (text-property-not-all
+                       mbeg (point-max) 'erc-command
+                       (get-text-property mbeg 'erc-command))))
+
+           (if (/= 1 bob-utterance-counter)
+               (should-not (field-at-pos mend))
+             ;; For Bob's stamped message, check newline after stamp.
+             (should (eq (field-at-pos mend) 'erc-timestamp))
+             (setq mend (field-end mend)))
+
+           ;; The `erc-timestamp' property spans entire messages,
+           ;; including stamps and filled text, which makes for
+           ;; convenient traversal when `erc-stamp-mode' is enabled.
+           (should (get-text-property (pos-bol) 'erc-timestamp))
+           (should (= (next-single-property-change (pos-bol) 'erc-timestamp)
+                      mend))
+
+           ;; Line ending has the `invisible' property `match-fools'.
+           (should (= (char-after mend) ?\n))
+           (with-suppressed-warnings ((obsolete erc-legacy-invisible-bounds-p))
+             (if erc-legacy-invisible-bounds-p
+                 (should (eq (get-text-property mend 'invisible) 'match-fools))
+               (should (eq (get-text-property mbeg 'invisible) 'match-fools))
+               (should-not (get-text-property mend 'invisible))))))
+
+       ;; Only the message right after Alice speaks contains stamps.
+       (when (= 1 bob-utterance-counter)
+
+         (ert-info ("Date stamp occupying previous line is invisible")
+           (save-excursion
+             (forward-line -1)
+             (goto-char (pos-bol))
+             (should (looking-at (rx "[Mon May  4 1992]")))
+             (should (erc-scenarios-match--fill-wrap-stamp-dedented-p (point)))
+             ;; Date stamp has a combined `invisible' property value
+             ;; that starts at the previous message's trailing newline
+             ;; and extends until the start of the message proper.
+             (should (equal ?\n (char-before (point))))
+             (should (equal ?\n (char-before (1- (point)))))
+             (let ((val (get-text-property (- (point) 2) 'invisible)))
+               (should (equal val '(timestamp match-fools)))
+               (should (= (text-property-not-all (- (point) 2) (point-max)
+                                                 'invisible val)
+                          (pos-eol))))))
+
+         (ert-info ("Current message's RHS stamp is hidden")
+           ;; Right stamp has `match-fools' property.
+           (save-excursion
+             (should-not (field-at-pos (point)))
+             (should (eq (field-at-pos (1- (pos-eol))) 'erc-timestamp)))
+
+           ;; Stamp invisibility starts where message's ends.
+           (let ((msgend (next-single-property-change (pos-bol) 'invisible)))
+             ;; Stamp has a combined `invisible' property value.
+             (should (equal (get-text-property msgend 'invisible)
+                            '(timestamp match-fools)))
+
+             ;; Combined `invisible' property spans entire timestamp.
+             (should (= (next-single-property-change msgend 'invisible)
+                        (pos-eol))))))
+
+       (cl-incf bob-utterance-counter))
+
+     ;; Alice.
+     (lambda ()
+       ;; Set clock ahead a week or so.
+       (setq erc-stamp--current-time 704962800)
+
+       ;; This message has no time stamp and is completely visible.
+       (should-not (eq (field-at-pos (1- (pos-eol))) 'erc-timestamp))
+       (should-not (next-single-property-change (pos-bol) 'invisible))))))
+
 (defun erc-scenarios-match--stamp-both-invisible-fill-static ()
   (should (eq erc-insert-timestamp-function
               #'erc-insert-timestamp-left-and-right))
-- 
2.41.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-5.6-Add-command-to-refill-buffer-with-erc-fill-wrap-.patch --]
[-- Type: text/x-patch, Size: 3381 bytes --]

From 2dd2c5c00e5a405f74ee0c7d61b35ba2f1e633e1 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Thu, 21 Sep 2023 06:54:27 -0700
Subject: [PATCH 3/3] [5.6] Add command to refill buffer with
 erc-fill-wrap-mode

* lisp/erc/erc-fill.el (erc-fill--wrap-rejigger-last-message):
New internal variable.
(erc-fill--wrap-rejigger-region,
erc-fill-wrap-refill-buffer): New command and helper function.
(Bug#60936)
---
 lisp/erc/erc-fill.el | 51 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/lisp/erc/erc-fill.el b/lisp/erc/erc-fill.el
index 6d39bcb19b9..78b29b51cf7 100644
--- a/lisp/erc/erc-fill.el
+++ b/lisp/erc/erc-fill.el
@@ -563,6 +563,57 @@ erc-fill-wrap
                                `((space :width (- erc-fill--wrap-value ,len))
                                  (space :width erc-fill--wrap-value))))))
 
+(defvar erc-fill--wrap-rejigger-last-message nil
+  "Temporary working instance of `erc-fill--wrap-last-msg'.")
+
+(defun erc-fill--wrap-rejigger-region (start finish on-next)
+  "Recalculate `line-prefix' from START to FINISH.
+After refilling each message, call ON-NEXT with no args.  But
+stash and restore `erc-fill--wrap-last-msg' before doing so, in
+case this module's insert hooks run by way of the process filter."
+  (goto-char start)
+  (cl-assert (null erc-fill--wrap-rejigger-last-message))
+  (let (erc-fill--wrap-rejigger-last-message)
+    (while-let
+        (((< (point) finish))
+         (beg (if (get-text-property (point) 'line-prefix)
+                  (point)
+                (next-single-property-change (point) 'line-prefix)))
+         (val (get-text-property beg 'line-prefix))
+         (end (text-property-not-all beg finish 'line-prefix val)))
+      ;; If this is a left-side stamp on its own line.
+      (remove-text-properties beg (1+ end) '(line-prefix nil wrap-prefix nil))
+      (save-restriction
+        (narrow-to-region beg (1+ end))
+        (if-let (((eq 'erc-timestamp (field-at-pos beg)))
+                 ((eq 'date-left (get-text-property beg 'erc-stamp-type))))
+            (progn
+              (goto-char (field-end beg))
+              (erc-fill--wrap-stamp-insert-prefixed-date))
+          (let ((erc-fill--wrap-last-msg erc-fill--wrap-rejigger-last-message))
+            (erc-fill-wrap)
+            (setq erc-fill--wrap-rejigger-last-message
+                  erc-fill--wrap-last-msg))))
+      (when on-next
+        (funcall on-next))
+      (goto-char end))))
+
+(defun erc-fill-wrap-refill-buffer ()
+  "Recalculate all `fill-wrap' prefixes in the current buffer."
+  (interactive)
+  (unless erc-fill-wrap-mode
+    (user-error "Module `fill-wrap' not active in current buffer."))
+  (save-excursion
+    (with-silent-modifications
+      (let* ((rep (make-progress-reporter
+                   "Rewrap" 0 (line-number-at-pos erc-insert-marker) 1))
+             (seen 0)
+             (callback (lambda ()
+                         (progress-reporter-update rep (cl-incf seen))
+                         (accept-process-output nil 0.000001))))
+        (erc-fill--wrap-rejigger-region (point-min) erc-insert-marker callback)
+        (progress-reporter-done rep)))))
+
 ;; FIXME use own text property to avoid false positives.
 (defun erc-fill--wrap-merged-button-p (point)
   (equal "" (get-text-property point 'display)))
-- 
2.41.0