From: F. Jason Park <jp@neverwas.me> Date: Sat, 23 Dec 2023 20:46:33 +0000 (-0800) Subject: Improve multi-window erc-keep-place-indicator-mode X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=65735efdca017f2ec0aa1022b7e82f68fbe0084d;p=emacs.git 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. --- 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."))