bug#64855: 30.0.50; ERC 5.6: Make scrolltobottom less erratic

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

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

"J.P." <jp@neverwas.me> writes:

> v5. Make new behavior hinge on new option, disabled by default. Assign
> some module functions explicit hook depths. Add news and tests.

I've added a version of this as

  commit 617ddb808999a71c925b68f5369d77aebfcd9254
  Consider all windows in erc-scrolltobottom-mode

But it comes with a few known problems.

The first can be observed during the rapid insertion of newly arriving
messages, like you get with history playback or large swaths of help
text. Basically, when point is away from the prompt and you issue a
command that changes `window-start' significantly (e.g., M-<), point
hardly moves at all, maybe half a screenful at most.

Another issue is only noticeable if you have code running that
suppresses message insertion after prompt submissions (see bug#49860's
`echo-message' for one example). Basically, if you've got
`erc-scrolltobottom-relaxed' enabled and you stare long enough, you'll
notice that the prompt drifts downward with every submitted round of
multi-line input.

Attached is a patch that attempts to address both of these issues, along
with a couple more to get at some unrelated odds and ends. I'm also sort
of thinking we ought to temporarily change the default of the new option
`erc-scrolltobottom-all' to t for a few weeks to help flush out any
other glaring bugs introduced by this feature. If anyone thinks that's a
bad idea, please say so. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-doc-misc-erc.texi-Fix-display-buffer-example.patch --]
[-- Type: text/x-patch, Size: 769 bytes --]

From 88430ddf866031366ccda5439d007fef6b579d8b Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sun, 10 Sep 2023 22:55:16 -0700
Subject: [PATCH 1/4] ; * doc/misc/erc.texi: Fix display-buffer example.

---
 doc/misc/erc.texi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 6d7785a9b54..1cbecfa891d 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -1821,7 +1821,7 @@ display-buffer
 
 (defun my-erc-disp-chan-p (_ action)
   (or (assq 'erc-autojoin-mode action)
-      (and (memq (cdr (assq 'erc-buffer-display alist)) 'JOIN)
+      (and (eq (cdr (assq 'erc-buffer-display action)) 'JOIN)
            (member (erc-default-target) '("#emacs" "#fsf")))))
 @end lisp
 
-- 
2.41.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-5.6-Simplify-erc-fill-module-docstring.patch --]
[-- Type: text/x-patch, Size: 1679 bytes --]

From 118e2e4aea66d0c54f0966d5e658086654e9807a Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Wed, 13 Sep 2023 02:50:28 -0700
Subject: [PATCH 2/4] [5.6] Simplify erc--fill-module-docstring

* lisp/erc/erc-common.el (erc--fill-module-docstring): Don't run hooks
for major mode when filling.  Prefer `lisp-data-mode' to
`emacs-lisp-mode'.
---
 lisp/erc/erc-common.el | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 85971797c2f..67c2cf8535b 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -270,18 +270,20 @@ erc--prepare-custom-module-type
              " above."))))))
 
 (defun erc--fill-module-docstring (&rest strings)
+  "Concatenate STRINGS and fill as a doc string."
+  ;; Perhaps it's better to mimic `internal--format-docstring-line'
+  ;; and use basic filling instead of applying a major mode?
   (with-temp-buffer
-    (emacs-lisp-mode)
-    (insert "(defun foo ()\n"
-            (format "%S" (apply #'concat strings))
-            "\n(ignore))")
+    (delay-mode-hooks
+      (if (fboundp 'lisp-data-mode) (lisp-data-mode) (emacs-lisp-mode)))
+    (insert (format "%S" (apply #'concat strings)))
     (goto-char (point-min))
-    (forward-line 2)
-    (let ((emacs-lisp-docstring-fill-column 65)
+    (forward-line)
+    (let ((fill-column 65)
           (sentence-end-double-space t))
       (fill-paragraph))
     (goto-char (point-min))
-    (nth 3 (read (current-buffer)))))
+    (read (current-buffer))))
 
 (defmacro erc--find-feature (name alias)
   `(pcase (erc--find-group ',name ,(and alias (list 'quote alias)))
-- 
2.41.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-5.6-Skip-erc-ignored-user-p-when-erc-ignore-list-is-.patch --]
[-- Type: text/x-patch, Size: 4568 bytes --]

From 801306861920124a4ca68d925829f957d748aa37 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 11 Sep 2023 21:21:42 -0700
Subject: [PATCH 3/4] [5.6] Skip erc-ignored-user-p when erc-ignore-list is
 empty

* lisp/erc/erc-backend.el (erc-server-PRIVMSG): Don't bother with
`erc-ignored-user-p' and `erc-ignored-reply-p' when their associated
options are null.
* lisp/erc/erc-common.el (erc-get-server-user): Rearrange so
`erc-with-server-buffer' doesn't have to switch to the server buffer
because `erc-downcase' can run in channels as well.
* lisp/erc/erc.el (erc-ignored-user-p): Add comment.
* test/lisp/erc/erc-scenarios-base-buffer-display.el
(erc-scenarios-base-buffer-display--defwin-recbury-intbuf): Extend
timeout.
---
 lisp/erc/erc-backend.el                            | 7 ++++---
 lisp/erc/erc-common.el                             | 5 +++--
 lisp/erc/erc.el                                    | 2 ++
 test/lisp/erc/erc-scenarios-base-buffer-display.el | 2 +-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 9e121ec1e92..596e504b39f 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -113,6 +113,8 @@ erc-ensure-target-buffer-on-privmsg
 (defvar erc-format-nick-function)
 (defvar erc-format-query-as-channel-p)
 (defvar erc-hide-prompt)
+(defvar erc-ignore-list)
+(defvar erc-ignore-reply-list)
 (defvar erc-input-marker)
 (defvar erc-insert-marker)
 (defvar erc-invitation)
@@ -1902,8 +1904,8 @@ erc--server-determine-join-display-context
         (cmd (erc-response.command parsed))
         (tgt (car (erc-response.command-args parsed)))
         (msg (erc-response.contents parsed)))
-    (if (or (erc-ignored-user-p sender-spec)
-            (erc-ignored-reply-p msg tgt proc))
+    (if (or (and erc-ignore-list (erc-ignored-user-p sender-spec))
+            (and erc-ignore-reply-list (erc-ignored-reply-p msg tgt proc)))
         (when erc-minibuffer-ignored
           (message "Ignored %s from %s to %s" cmd sender-spec tgt))
       (let* ((sndr (erc-parse-user sender-spec))
@@ -1918,7 +1920,6 @@ erc--server-determine-join-display-context
                                      ,@erc--display-context))
              s buffer
              fnick)
-        (setf (erc-response.contents parsed) msg)
         (setq buffer (erc-get-buffer (if privp nick tgt) proc))
         ;; Even worth checking for empty target here? (invalid anyway)
         (unless (or buffer noticep (string-empty-p tgt) (eq ?$ (aref tgt 0))
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 67c2cf8535b..e680666156b 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -472,8 +472,9 @@ erc-get-channel-user
 (define-inline erc-get-server-user (nick)
   "Find NICK in the current server's `erc-server-users' hash table."
   (inline-letevals (nick)
-    (inline-quote (erc-with-server-buffer
-                    (gethash (erc-downcase ,nick) erc-server-users)))))
+    (inline-quote
+     (gethash (erc-downcase ,nick)
+              (erc-with-server-buffer erc-server-users)))))
 
 (defmacro erc--with-dependent-type-match (type &rest features)
   "Massage Custom :type TYPE with :match function that pre-loads FEATURES."
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 07ba32d1cca..a65739cf861 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -6832,6 +6832,8 @@ erc-delete-query
         (setq erc-default-recipients d2)
       (error "Current target is not a QUERY"))))
 
+;; FIXME spin this ignore stuff off into its own module, enabled by
+;; default until some major version change.
 (defun erc-ignored-user-p (spec)
   "Return non-nil if SPEC matches something in `erc-ignore-list'.
 
diff --git a/test/lisp/erc/erc-scenarios-base-buffer-display.el b/test/lisp/erc/erc-scenarios-base-buffer-display.el
index df292a8c113..ef544b4dcd0 100644
--- a/test/lisp/erc/erc-scenarios-base-buffer-display.el
+++ b/test/lisp/erc/erc-scenarios-base-buffer-display.el
@@ -109,7 +109,7 @@ erc-scenarios-base-buffer-display--defwin-recbury-intbuf
          (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam"))
            (should (eq (window-buffer) (get-buffer "#spam")))
            ;; Option `buffer' replaces entire window (no split)
-           (erc-d-t-wait-for 5 (frame-root-window-p (selected-window)))))))))
+           (erc-d-t-wait-for 10 (frame-root-window-p (selected-window)))))))))
 
 (ert-deftest erc-scenarios-base-buffer-display--defwino-recbury-intbuf ()
   :tags '(:expensive-test)
-- 
2.41.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-5.6-Run-erc-scrolltobottom-on-pre-insert-uncondition.patch --]
[-- Type: text/x-patch, Size: 2535 bytes --]

From 88681f79c2a6fcfb9343c85e2d8ba58ed8090680 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Wed, 13 Sep 2023 05:42:24 -0700
Subject: [PATCH 4/4] [5.6] Run erc--scrolltobottom-on-pre-insert
 unconditionally

* lisp/erc/erc-goodies.el (erc--scrolltobottom-all): Pass `no-force'
argument to `set-window-start'.
(erc--scrolltobottom-on-pre-insert): Convert function from generic to
normal and drop `erc-input' method completely.  A non-nil `insertp'
slot means a message is marked for insertion in the read-only portion
of the buffer, above the prompt.  But conditioning the restoring of
window parameters on the latter is not enough: the window still needs
adjusting whenever input is typed, regardless of whether it's erased
or "inserted."  (Bug#64855)
---
 lisp/erc/erc-goodies.el | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index 6353b813805..6eb015fdd64 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -223,7 +223,7 @@ erc--scrolltobottom-all
           ((erc--scrolltobottom-window-info)
            (found (assq window erc--scrolltobottom-window-info))
            ((not (erc--scrolltobottom-confirm (nth 2 found)))))
-        (setf (window-start window) (cadr found)))))
+        (set-window-start window (cadr found) 'no-force))))
   ;; Necessary unless we're sure `erc--scrolltobottom-on-pre-insert'
   ;; always runs between calls to this function.
   (setq erc--scrolltobottom-window-info nil))
@@ -280,7 +280,7 @@ erc--scrolltobottom-setup
     (kill-local-variable 'erc--scrolltobottom-relaxed-commands)
     (kill-local-variable 'erc--scrolltobottom-window-info)))
 
-(cl-defmethod erc--scrolltobottom-on-pre-insert (_input-or-string)
+(defun erc--scrolltobottom-on-pre-insert (_)
   "Remember the `window-start' before inserting a message."
   (setq erc--scrolltobottom-window-info
         (mapcar (lambda (w)
@@ -293,11 +293,6 @@ erc--scrolltobottom-on-pre-insert
                           (if (= ?\n (char-before (point-max))) (1+ c) c))))
                 (get-buffer-window-list nil nil 'visible))))
 
-(cl-defmethod erc--scrolltobottom-on-pre-insert ((input erc-input))
-  "Remember the `window-start' before inserting a message."
-  (when (erc-input-insertp input)
-    (cl-call-next-method)))
-
 (defun erc--scrolltobottom-confirm (&optional scroll-to)
   "Like `erc-scroll-to-bottom', but use `window-point'.
 Position current line (with `recenter') SCROLL-TO lines below
-- 
2.41.0