Improve multi-window erc-keep-place-indicator-mode

* lisp/erc/erc-goodies.el (erc-keep-place-indicator-follow): Describe
condition causing an indicator update.
(erc--keep-place-indicator-on-window-configuration-change,
erc--keep-place-indicator-on-window-buffer-change): Rename former to
latter, add required WINDOW parameter, and don't move indicator if
buffer appears in multiple windows.  Also, don't bother checking
whether either buffer is a mini because the manual says window change
functions don't run for minibuffer replacements.
(erc--keep-place-indicator-setup): Hook on
`window-buffer-change-functions' instead of
`window-configuration-change-hook'.
(erc-keep-place-mode, erc-keep-place-disable): Remove member from
`window-buffer-change-functions' instead of
`window-configuration-change-hook'.
(erc-keep-place): Use `visible' FRAME arg of `get-buffer-window'.
Don't twiddle `window-prev-buffers' when
`erc-keep-place-indicator-mode' is non-nil.  This feature was
originally introduced by bug#59943.
* test/lisp/erc/erc-goodies-tests.el
(erc-goodies-tests--assert-kp-indicator-on,
erc-goodies-tests--assert-kp-indicator-off): Update hook name.
* test/lisp/erc/erc-scenarios-keep-place-indicator.el: New file.
* test/lisp/erc/resources/keep-place/follow.eld: New file.

4 files changed, 244 insertions, 21 deletions

diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index e10f047b187..9d385b628dc 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -300,7 +300,10 @@ A value of t means \"all\" ERC buffers."
 
 (defcustom erc-keep-place-indicator-follow nil
   "Whether to sync visual kept place to window's top when reading.
-For use with `erc-keep-place-indicator-mode'."
+For use with `erc-keep-place-indicator-mode'.  When enabled, the
+indicator updates when the last window displaying the same buffer
+switches away, but only if the indicator resides earlier in the
+buffer than the window's start."
   :group 'erc
   :package-version '(ERC . "5.6")
   :type 'boolean)
@@ -328,17 +331,26 @@ For use with `erc-keep-place-indicator-mode'."
 (defvar-local erc--keep-place-indicator-overlay nil
   "Overlay for `erc-keep-place-indicator-mode'.")
 
-(defun erc--keep-place-indicator-on-window-configuration-change ()
+(defun erc--keep-place-indicator-on-window-buffer-change (window)
   "Maybe sync `erc--keep-place-indicator-overlay'.
-Specifically, do so unless switching to or from another window in
-the active frame."
-  (when erc-keep-place-indicator-follow
-    (unless (or (minibuffer-window-active-p (minibuffer-window))
-                (eq (window-old-buffer) (current-buffer)))
-      (when (< (overlay-end erc--keep-place-indicator-overlay)
-               (window-start)
-               erc-insert-marker)
-        (erc-keep-place-move (window-start))))))
+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)))
+             (old-buffer (window-old-buffer window))
+             ((buffer-live-p old-buffer))
+             ((not (eq old-buffer (current-buffer))))
+             (ov (buffer-local-value 'erc--keep-place-indicator-overlay
+                                     old-buffer))
+             ((not (get-buffer-window old-buffer 'visible)))
+             (prev (assq old-buffer (window-prev-buffers window)))
+             (old-start (nth 1 prev))
+             (old-inmkr (buffer-local-value 'erc-insert-marker old-buffer))
+             ((< (overlay-end ov) old-start old-inmkr)))
+    (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'."
@@ -347,8 +359,8 @@ the active frame."
     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-configuration-change-hook
-            #'erc--keep-place-indicator-on-window-configuration-change 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
@@ -368,7 +380,11 @@ the active frame."
   "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."
+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."
   ((cond (erc-keep-place-mode)
          ((memq 'keep-place erc-modules)
           (erc-keep-place-mode +1))
@@ -382,8 +398,8 @@ and `keep-place-indicator' in different buffers."
      (erc-keep-place-indicator-mode -1)))
   ((when erc--keep-place-indicator-overlay
      (delete-overlay erc--keep-place-indicator-overlay))
-   (remove-hook 'window-configuration-change-hook
-                #'erc--keep-place-indicator-on-window-configuration-change t)
+   (remove-hook 'window-buffer-change-functions
+                #'erc--keep-place-indicator-on-window-buffer-change 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)
@@ -450,13 +466,13 @@ For use with `keep-place-indicator' module."
     (forward-line -1)
     (when erc-keep-place-indicator-mode
       (unless (or (minibuffer-window-active-p (selected-window))
-                  (and (frame-visible-p (selected-frame))
-                       (get-buffer-window (current-buffer) (selected-frame))))
+                  (get-buffer-window nil 'visible))
         (erc-keep-place-move nil)))
     ;; if `switch-to-buffer-preserve-window-point' is set,
     ;; we cannot rely on point being saved, and must commit
     ;; it to window-prev-buffers.
-    (when switch-to-buffer-preserve-window-point
+    (when (and switch-to-buffer-preserve-window-point
+               (not erc-keep-place-indicator-mode))
       (dolist (frame (frame-list))
         (walk-window-tree
          (lambda (window)
diff --git a/test/lisp/erc/erc-goodies-tests.el b/test/lisp/erc/erc-goodies-tests.el
index cdf861e2018..ca02089eb7c 100644
--- a/test/lisp/erc/erc-goodies-tests.el
+++ b/test/lisp/erc/erc-goodies-tests.el
@@ -247,7 +247,7 @@
 
 (defun erc-goodies-tests--assert-kp-indicator-on ()
   (should erc--keep-place-indicator-overlay)
-  (should (local-variable-p 'window-configuration-change-hook))
+  (should (local-variable-p 'window-buffer-change-functions))
   (should window-configuration-change-hook)
   (should (memq 'erc-keep-place erc-insert-pre-hook))
   (should (eq erc-keep-place-mode
@@ -255,7 +255,7 @@
 
 (defun erc-goodies-tests--assert-kp-indicator-off ()
   (should-not (local-variable-p 'erc-insert-pre-hook))
-  (should-not (local-variable-p 'window-configuration-change-hook))
+  (should-not (local-variable-p 'window-buffer-change-functions))
   (should-not erc--keep-place-indicator-overlay))
 
 (defun erc-goodies-tests--kp-indicator-populate ()
diff --git a/test/lisp/erc/erc-scenarios-keep-place-indicator.el b/test/lisp/erc/erc-scenarios-keep-place-indicator.el
new file mode 100644
index 00000000000..7566288066e
--- /dev/null
+++ b/test/lisp/erc/erc-scenarios-keep-place-indicator.el
@@ -0,0 +1,134 @@
+;;; erc-scenarios-keep-place-indicator.el --- erc-keep-place-indicator-mode -*- lexical-binding: t -*-
+
+;; Copyright (C) 2023 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert-x)
+(eval-and-compile
+  (let ((load-path (cons (ert-resource-directory) load-path)))
+    (require 'erc-scenarios-common)))
+
+(require 'erc-goodies)
+
+;; This test shows that the indicator does not update when at least
+;; one window remains.  When the last window showing a buffer switches
+;; away, the indicator is updated if it's earlier in the buffer.
+(ert-deftest erc-scenarios-keep-place-indicator--follow ()
+  :tags `(:expensive-test
+          ,@(and (getenv "ERC_TESTS_GRAPHICAL") '(:erc--graphical)))
+  (when (version< emacs-version "29") (ert-skip "Times out"))
+  ;; XXX verify that this continues to be the case ^.
+
+  (should-not erc-scrolltobottom-all)
+  (should-not erc-scrolltobottom-mode)
+  (should-not erc-keep-place-mode)
+
+  (erc-scenarios-common-with-noninteractive-in-term
+      ((erc-scenarios-common-dialog "keep-place")
+       (dumb-server (erc-d-run "localhost" t 'follow))
+       (port (process-contact dumb-server :service))
+       (erc-modules `( keep-place-indicator scrolltobottom fill-wrap
+                       ,@erc-modules))
+       (erc-keep-place-indicator-follow t)
+       (erc-scrolltobottom-all t)
+       (erc-server-flood-penalty 0.1)
+       (erc-autojoin-channels-alist '((foonet "#chan" "#spam")))
+       (expect (erc-d-t-make-expecter)))
+
+    (ert-info ("Connect")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :full-name "tester"
+                                :nick "tester"
+                                :user "tester")
+        (funcall expect 10 "debug mode")))
+
+    (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+      (set-window-buffer nil (current-buffer))
+      (delete-other-windows)
+      (split-window-below)
+      (funcall expect 10 "<bob> tester, welcome!")
+      (recenter 0)
+      (other-window 1)
+      (funcall expect 10 "<alice> tester, welcome!")
+      (recenter 0)
+      (should (= 2 (length (window-list))))
+
+      (ert-info ("Last window to switch away has point earlier in buffer")
+        ;; Lower window, with point later in buffer, switches away first.
+        (switch-to-buffer (erc-d-t-wait-for 10 (get-buffer "#spam"))) ; lower
+        (other-window 1)
+        (switch-to-buffer "#spam") ; upper
+        (erc-scenarios-common-say "one")
+        (funcall expect 10 "Ay, the heads")
+
+        ;; Overlay has moved to upper window start.
+        (switch-to-buffer "#chan")
+        (redisplay) ; force overlay to update
+        (save-excursion
+          (goto-char (window-point))
+          (should (looking-back (rx "<bob> tester, welcome!")))
+          (should (= (pos-bol) (window-start)))
+          (should (= (overlay-start erc--keep-place-indicator-overlay)
+                     (pos-bol))))
+        ;; Lower window is still centered at start.
+        (other-window 1)
+        (switch-to-buffer "#chan")
+        (save-excursion
+          (goto-char (window-point))
+          (should (looking-back (rx "<alice> tester, welcome!")))
+          (should (= (pos-bol) (window-start)))))
+
+      (ert-info ("Last window to switch away has point later in buffer")
+        ;; Lower window advances.
+        (funcall expect 10 "<bob> alice: Since you can cog")
+        (recenter 0)
+        (redisplay) ; force ^ to appear on first line
+
+        (other-window 1) ; upper still at indicator, swtiches first
+        (switch-to-buffer "#spam")
+        (other-window 1)
+        (switch-to-buffer "#spam") ; lower follows, speaks to sync
+        (erc-scenarios-common-say "two")
+        (funcall expect 10 "<bob> Cause they take")
+
+        ;; Upper switches back first, finds indicator gone.
+        (other-window 1)
+        (switch-to-buffer "#chan")
+        (save-excursion
+          (goto-char (window-point))
+          (should (looking-back (rx "<bob> tester, welcome!")))
+          (should (= (pos-bol) (window-start)))
+          (should (> (overlay-start erc--keep-place-indicator-overlay)
+                     (pos-eol))))
+
+        ;; Lower window follows, window-start preserved.
+        (other-window 1)
+        (switch-to-buffer "#chan")
+        (save-excursion
+          (goto-char (window-point))
+          (should (looking-back (rx "you can cog")))
+          (should (= (pos-bol) (window-start)))
+          (should (= (overlay-start erc--keep-place-indicator-overlay)
+                     (pos-bol))))))
+
+    (erc-keep-place-mode -1)
+    (erc-scrolltobottom-mode -1)))
+
+;;; erc-scenarios-keep-place-indicator.el ends here
diff --git a/test/lisp/erc/resources/keep-place/follow.eld b/test/lisp/erc/resources/keep-place/follow.eld
new file mode 100644
index 00000000000..e857c17175d
--- /dev/null
+++ b/test/lisp/erc/resources/keep-place/follow.eld
@@ -0,0 +1,73 @@
+;; -*- mode: lisp-data; -*-
+((nick 10 "NICK tester"))
+((user 10 "USER tester 0 * :tester")
+ (0.00 ":irc.foonet.org 001 tester :Welcome to the foonet IRC Network tester")
+ (0.01 ":irc.foonet.org 002 tester :Your host is irc.foonet.org, running version ergo-v2.11.1")
+ (0.01 ":irc.foonet.org 003 tester :This server was created Tue, 26 Dec 2023 08:36:35 UTC")
+ (0.01 ":irc.foonet.org 004 tester irc.foonet.org ergo-v2.11.1 BERTZios CEIMRUabefhiklmnoqstuv Iabefhkloqv")
+ (0.00 ":irc.foonet.org 005 tester AWAYLEN=390 BOT=B CASEMAPPING=ascii CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# CHATHISTORY=1000 ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX :are supported by this server")
+ (0.01 ":irc.foonet.org 005 tester KICKLEN=390 MAXLIST=beI:60 MAXTARGETS=4 MODES MONITOR=100 NETWORK=foonet NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ TARGMAX=NAMES:1,LIST:1,KICK:,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100 TOPICLEN=390 UTF8ONLY WHOX :are supported by this server")
+ (0.01 ":irc.foonet.org 005 tester draft/CHATHISTORY=1000 :are supported by this server")
+ (0.01 ":irc.foonet.org 251 tester :There are 0 users and 4 invisible on 1 server(s)")
+ (0.01 ":irc.foonet.org 252 tester 0 :IRC Operators online")
+ (0.00 ":irc.foonet.org 253 tester 0 :unregistered connections")
+ (0.00 ":irc.foonet.org 254 tester 2 :channels formed")
+ (0.00 ":irc.foonet.org 255 tester :I have 4 clients and 0 servers")
+ (0.00 ":irc.foonet.org 265 tester 4 4 :Current local users 4, max 4")
+ (0.00 ":irc.foonet.org 266 tester 4 4 :Current global users 4, max 4")
+ (0.03 ":irc.foonet.org 422 tester :MOTD File is missing")
+ (0.01 ":irc.foonet.org NOTICE tester :This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect."))
+
+((mode 10 "MODE tester +i"))
+
+((join 10 "JOIN #chan")
+ (0.01 ":irc.foonet.org 221 tester +i")
+ (0.01 ":tester!~u@p64eqfwvvbxrk.irc JOIN #chan")
+ (0.03 ":irc.foonet.org 353 tester = #chan :@fsbot bob alice tester")
+ (0.01 ":irc.foonet.org 366 tester #chan :End of NAMES list")
+ (0.00 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :tester, welcome!")
+ (0.01 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :tester, welcome!"))
+
+((join 10 "JOIN #spam")
+ (0.00 ":tester!~u@p64eqfwvvbxrk.irc JOIN #spam")
+ (0.06 ":irc.foonet.org 353 tester = #spam :@fsbot bob alice tester")
+ (0.01 ":irc.foonet.org 366 tester #spam :End of NAMES list")
+ (0.03 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #spam :tester, welcome!")
+ (0.01 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #spam :tester, welcome!"))
+
+((mode 10 "MODE #chan")
+ (0.00 ":irc.foonet.org 324 tester #chan +Cnt")
+ (0.02 ":irc.foonet.org 329 tester #chan 1703579802")
+ (0.02 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :bob: Madam, my lord is gone, for ever gone.")
+ (0.10 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :The kinder we, to give them thanks for nothing."))
+
+((mode 10 "MODE #spam")
+ (0.00 ":irc.foonet.org 324 tester #spam +Cnt")
+ (0.02 ":irc.foonet.org 329 tester #spam 1703579805")
+ (0.02 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :Most manifest, and not denied by himself.")
+ (0.02 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :alice: To bed, to bed: there's knocking at the gate. Come, come, come, come, give me your hand. What's done cannot be undone.")
+ (0.02 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :bob: And what I spake, I spake it to my face.")
+ (0.08 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :alice: Since you can cog, I'll play no more with you.")
+ (0.06 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :bob: The little casket bring me hither.")
+ (0.01 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :alice: Not to-night, good Iago: I have very poor and unhappy brains for drinking: I could well wish courtesy would invent some other custom of entertainment.")
+ (0.02 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :Yes, faith will I, Fridays and Saturdays and all."))
+
+((privmsg 10 "PRIVMSG #spam :one")
+ (0.03 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :bob: This is the first truth that e'er thine own tongue was guilty of.")
+ (0.02 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :alice: Drown the lamenting fool in sea-salt tears.")
+
+ ;; Insert some lines ^ before rendezvous, so #chan can update scrolltobottom.
+ (0.01 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #spam :Ay, the heads of the maids, or their maidenheads; take it in what sense thou wilt.")
+
+ (0.05 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :alice: And work confusion on his enemies.")
+ (0.06 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :bob: Truly, she must be given, or the marriage is not lawful."))
+
+((privmsg 10 "PRIVMSG #spam :two")
+ (0.02 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :To be whipped; and yet a better love than my master.")
+ (0.06 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :And duty in his service perishing.")
+
+ ;; Second check point.
+ (0.01 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #spam :Cause they take vengeance of such kind of men.")
+
+ (0.03 ":bob!~u@2q6ysndq32az6.irc PRIVMSG #chan :alice: No egma, no riddle, no l'envoy; no salve in the mail, sir. O! sir, plantain, a plain plantain: no l'envoy, no l'envoy: no salve, sir, but a plantain.")
+ (0.03 ":alice!~u@2q6ysndq32az6.irc PRIVMSG #chan :Signior Iachimo will not from it. Pray, let us follow 'em."))