bug#59943: 30.0.50; ERC 5.5+: Add visual indicator to ERC keep-place

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

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

From emacs -Q

  1. M-: (setopt erc-keep-place-indicator-follow t) RET
     M-x erc RET ... RET

  2. In the server buffer, do M-x erc-keep-place-indicator-mode RET

  3. Scroll "up" to hide the indicator but not so far that point moves
     beyond `erc-insert-marker'

  4. Switch away to another buffer and then back

  5. Notice the indicator fails to update

This happens because `keep-place-indicator' is a local module, and it
adds `erc--keep-place-indicator-on-window-buffer-change' to the hook
`window-buffer-change-functions' locally. But that function needs to run
in the buffer that's being switched to, which may not have the mode
enabled (and thus not have the hook member available). Subscribing to
the hook globally should fix the problem. There are also at least two
bugs affecting the integration between `keep-place-indicator' and
`keep-place'. The attached patch attempts to address these as well.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-5.6-Make-erc-send-input-lines-a-normal-function.patch --]
[-- Type: text/x-patch, Size: 1612 bytes --]

From abc5a18c96617c5040f2ffd2e286befec75c4e0f Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 1 Jan 2024 00:34:53 -0800
Subject: [PATCH 1/2] [5.6] Make erc--send-input-lines a normal function

* lisp/erc/erc.el (erc--send-input-lines): Revert portion of
174b3dd9bd78c662ce9fff78404dcfa02259d21b "Make nested input handling
more robust in ERC" that converted this from a function to a method.
Instead, defer change until it's needed, likely for bug#49860.  Also,
don't overload `insertp' because user code can legitimately set that
to a function, which we then blindly call.
---
 lisp/erc/erc.el | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 5b3d0d66941..b4f31a361c4 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -7878,14 +7878,12 @@ erc--run-send-hooks
     (user-error "Multiline command detected" ))
   lines-obj)
 
-(cl-defmethod erc--send-input-lines (lines-obj)
+(defun erc--send-input-lines (lines-obj)
   "Send lines in `erc--input-split-lines' object LINES-OBJ."
   (when (erc--input-split-sendp lines-obj)
     (dolist (line (erc--input-split-lines lines-obj))
       (when (erc--input-split-insertp lines-obj)
-        (if (functionp (erc--input-split-insertp lines-obj))
-            (funcall (erc--input-split-insertp lines-obj) line)
-          (erc-display-msg line)))
+        (erc-display-msg line))
       (erc-process-input-line (concat line "\n")
                               (null erc-flood-protect)
                               (not (erc--input-split-cmdp lines-obj))))))
-- 
2.42.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-5.6-Use-global-window-hook-for-erc-keep-place-indica.patch --]
[-- Type: text/x-patch, Size: 10490 bytes --]

From e910a462ce698d364c52438e60618fbe504d7dad Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 1 Jan 2024 23:18:54 -0800
Subject: [PATCH 2/2] [5.6] Use global window hook for erc-keep-place-indicator

* lisp/erc/erc-goodies.el
(erc--keep-place-indicator-on-window-buffer-change): Expect a frame
instead of a window argument for the only parameter, and exit early
when entering a minibuffer.
(erc--keep-place-indicator-setup): Remove function because local
modules don't need a separate setup function.
(erc-keep-place-indicator-mode, erc-keep-place-indicator-enable,
erc-keep-place-indicator-disable): Move functionality from
`erc--keep-place-indicator-setup' into enable body.  Use global
instead of local members for `erc-keep-place-mode-hook' and
`window-buffer-change-functions'.
(erc--keep-place-indicator-on-global-module): Perform necessary action
in all ERC buffers, not just the current one, where the user has
ostensibly disabled `erc-keep-place-mode'.
* test/lisp/erc/erc-goodies-tests.el
(erc-goodies-tests--assert-kp-indicator-on,
erc-goodies-tests--assert-kp-indicator-off): Change expected hook
membership from global to local.
(erc-goodies-tests--keep-place-indicator): Use new helpers from
erc-tests-common library.  (Bug#59943)
---
 lisp/erc/erc-goodies.el            | 83 ++++++++++++++++--------------
 test/lisp/erc/erc-goodies-tests.el | 18 +++----
 2 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index a0502a3b75c..ea97f2099bc 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -331,14 +331,15 @@ erc-keep-place-indicator-arrow
 (defvar-local erc--keep-place-indicator-overlay nil
   "Overlay for `erc-keep-place-indicator-mode'.")
 
-(defun erc--keep-place-indicator-on-window-buffer-change (window)
+(defun erc--keep-place-indicator-on-window-buffer-change (_)
   "Maybe sync `erc--keep-place-indicator-overlay'.
 Do so only when switching to a new buffer in the same window if
 the replaced buffer is no longer visible in another window and
 its `window-start' at the time of switching is strictly greater
 than the indicator's position."
   (when-let ((erc-keep-place-indicator-follow)
-             ((eq window (selected-window)))
+             (window (selected-window))
+             ((not (eq window (active-minibuffer-window))))
              (old-buffer (window-old-buffer window))
              ((buffer-live-p old-buffer))
              ((not (eq old-buffer (current-buffer))))
@@ -352,67 +353,69 @@ erc--keep-place-indicator-on-window-buffer-change
     (with-current-buffer old-buffer
       (erc-keep-place-move old-start))))
 
-(defun erc--keep-place-indicator-setup ()
-  "Initialize buffer for maintaining `erc--keep-place-indicator-overlay'."
-  (require 'fringe)
-  (erc--restore-initialize-priors erc-keep-place-indicator-mode
-    erc--keep-place-indicator-overlay (make-overlay 0 0))
-  (add-hook 'erc-keep-place-mode-hook
-            #'erc--keep-place-indicator-on-global-module nil t)
-  (add-hook 'window-buffer-change-functions
-            #'erc--keep-place-indicator-on-window-buffer-change 40 t)
-  (when-let* (((memq erc-keep-place-indicator-style '(t arrow)))
-              (ov-property (if (zerop (fringe-columns 'left))
-                               'after-string
-                             'before-string))
-              (display (if (zerop (fringe-columns 'left))
-                           `((margin left-margin) ,overlay-arrow-string)
-                         '(left-fringe right-triangle
-                                       erc-keep-place-indicator-arrow)))
-              (bef (propertize " " 'display display)))
-    (overlay-put erc--keep-place-indicator-overlay ov-property bef))
-  (when (memq erc-keep-place-indicator-style '(t face))
-    (overlay-put erc--keep-place-indicator-overlay 'face
-                 'erc-keep-place-indicator-line)))
-
 ;;;###autoload(put 'keep-place-indicator 'erc--feature 'erc-goodies)
 (define-erc-module keep-place-indicator nil
   "Buffer-local `keep-place' with fringe arrow and/or highlighted face.
 Play nice with global module `keep-place' but don't depend on it.
 Expect that users may want different combinations of `keep-place'
-and `keep-place-indicator' in different buffers.  Unlike global
-`keep-place', when `switch-to-buffer-preserve-window-point' is
-enabled, don't forcibly sync point in all windows where buffer
-has previously been shown because that defeats the purpose of
-having a placeholder."
+and `keep-place-indicator' in different buffers."
   ((cond (erc-keep-place-mode)
          ((memq 'keep-place erc-modules)
           (erc-keep-place-mode +1))
          ;; Enable a local version of `keep-place-mode'.
          (t (add-hook 'erc-insert-pre-hook  #'erc-keep-place 65 t)))
+   (require 'fringe)
+   (add-hook 'window-buffer-change-functions
+             #'erc--keep-place-indicator-on-window-buffer-change 40)
+   (add-hook 'erc-keep-place-mode-hook
+             #'erc--keep-place-indicator-on-global-module 40)
    (if (pcase erc-keep-place-indicator-buffer-type
          ('target erc--target)
          ('server (not erc--target))
          ('t t))
-       (erc--keep-place-indicator-setup)
+       (progn
+         (erc--restore-initialize-priors erc-keep-place-indicator-mode
+           erc--keep-place-indicator-overlay (make-overlay 0 0))
+         (when-let (((memq erc-keep-place-indicator-style '(t arrow)))
+                    (ov-property (if (zerop (fringe-columns 'left))
+                                     'after-string
+                                   'before-string))
+                    (display (if (zerop (fringe-columns 'left))
+                                 `((margin left-margin) ,overlay-arrow-string)
+                               '(left-fringe right-triangle
+                                             erc-keep-place-indicator-arrow)))
+                    (bef (propertize " " 'display display)))
+           (overlay-put erc--keep-place-indicator-overlay ov-property bef))
+         (when (memq erc-keep-place-indicator-style '(t face))
+           (overlay-put erc--keep-place-indicator-overlay 'face
+                        'erc-keep-place-indicator-line)))
      (erc-keep-place-indicator-mode -1)))
   ((when erc--keep-place-indicator-overlay
      (delete-overlay erc--keep-place-indicator-overlay))
-   (remove-hook 'window-buffer-change-functions
-                #'erc--keep-place-indicator-on-window-buffer-change t)
+   (let ((buffer (current-buffer)))
+     ;; Remove global hooks unless others exist with mode enabled.
+     (unless (erc-buffer-filter (lambda ()
+                                  (and (not (eq buffer (current-buffer)))
+                                       erc-keep-place-indicator-mode)))
+       (remove-hook 'erc-keep-place-mode-hook
+                    #'erc--keep-place-indicator-on-global-module)
+       (remove-hook 'window-buffer-change-functions
+                    #'erc--keep-place-indicator-on-window-buffer-change)))
+   (when (local-variable-p 'erc-insert-pre-hook)
+     (remove-hook 'erc-insert-pre-hook  #'erc-keep-place t))
    (remove-hook 'erc-keep-place-mode-hook
                 #'erc--keep-place-indicator-on-global-module t)
-   (remove-hook 'erc-insert-pre-hook  #'erc-keep-place t)
    (kill-local-variable 'erc--keep-place-indicator-overlay))
   'local)
 
 (defun erc--keep-place-indicator-on-global-module ()
-  "Ensure `keep-place-indicator' can cope with `erc-keep-place-mode'.
-That is, ensure the local module can survive a user toggling the
-global one."
-  (if erc-keep-place-mode
-      (remove-hook 'erc-insert-pre-hook  #'erc-keep-place t)
-    (add-hook 'erc-insert-pre-hook  #'erc-keep-place 65 t)))
+  "Ensure `keep-place-indicator' survives toggling `erc-keep-place-mode'.
+Do this by simulating `keep-place' in all buffers where
+`keep-place-indicator' is enabled."
+  (erc-with-all-buffers-of-server nil (lambda () erc-keep-place-indicator-mode)
+    (if erc-keep-place-mode
+        (remove-hook 'erc-insert-pre-hook  #'erc-keep-place t)
+      (add-hook 'erc-insert-pre-hook  #'erc-keep-place 65 t))))
 
 (defun erc-keep-place-move (pos)
   "Move keep-place indicator to current line or POS.
diff --git a/test/lisp/erc/erc-goodies-tests.el b/test/lisp/erc/erc-goodies-tests.el
index bdd197fa5cb..f3de4ba4dd4 100644
--- a/test/lisp/erc/erc-goodies-tests.el
+++ b/test/lisp/erc/erc-goodies-tests.el
@@ -251,15 +251,16 @@ erc-controls-highlight--motd
 
 (defun erc-goodies-tests--assert-kp-indicator-on ()
   (should erc--keep-place-indicator-overlay)
-  (should (local-variable-p 'window-buffer-change-functions))
-  (should window-configuration-change-hook)
+  (should (memq 'erc--keep-place-indicator-on-window-buffer-change
+                window-buffer-change-functions))
   (should (memq 'erc-keep-place erc-insert-pre-hook))
   (should (eq erc-keep-place-mode
               (not (local-variable-p 'erc-insert-pre-hook)))))
 
 (defun erc-goodies-tests--assert-kp-indicator-off ()
   (should-not (local-variable-p 'erc-insert-pre-hook))
-  (should-not (local-variable-p 'window-buffer-change-functions))
+  (should-not (memq 'erc--keep-place-indicator-on-window-buffer-change
+                    window-buffer-change-functions))
   (should-not erc--keep-place-indicator-overlay))
 
 (defun erc-goodies-tests--kp-indicator-populate ()
@@ -272,12 +273,9 @@ erc-goodies-tests--kp-indicator-populate
   (goto-char erc-input-marker))
 
 (defun erc-goodies-tests--keep-place-indicator (test)
-  (with-current-buffer (get-buffer-create "*erc-keep-place-indicator-mode*")
-    (erc-mode)
-    (erc--initialize-markers (point) nil)
-    (setq erc-server-process
-          (start-process "sleep" (current-buffer) "sleep" "1"))
-    (set-process-query-on-exit-flag erc-server-process nil)
+  (erc-keep-place-mode -1)
+  (with-current-buffer (erc-tests-common-make-server-buf
+                        "*erc-keep-place-indicator-mode*")
     (let (erc-connect-pre-hook
           erc-modules)
 
@@ -294,7 +292,7 @@ erc-goodies-tests--keep-place-indicator
       (should-not (member 'erc-keep-place
                           (default-value 'erc-insert-pre-hook)))
       (should-not (local-variable-p 'erc-insert-pre-hook))
-      (kill-buffer))))
+      (erc-tests-common-kill-buffers))))
 
 (ert-deftest erc-keep-place-indicator-mode--no-global ()
   (erc-goodies-tests--keep-place-indicator
-- 
2.42.0