Subject: bug#60560: 29.0.60; ERC 5.5: erc-track should account for killed buffers

From: "J.P." <[email-masked]>
Date: Tue, 10 Jan 2023 06:35:57 -0800

"J.P." <[email-masked]> writes:

> Case 1
[...]
>
>   Here, `erc-track--switch-buffer' and friends do not fully account for
>   killed buffers hanging around in `erc-modified-channels-alist'. This
>   could potentially be addressed by adding a maintenance function to
>   `erc-kill-server-hook' on module init (er, major-mode init). However,
>   since the hook itself is a public option and there's a race between it
>   and `erc-window-configuration-change' (which sees the killed buffer as
>   still being alive), I've modified the command instead. If this is
>   potentially problematic, someone please enlighten me.
>
>
> Case 2
>
[...]
>
>   This highlights the need for other modules to perform chores when
>   erc-networks kills a server buffer. Basically, when merging contexts,
>   it doesn't run the normal `erc-kill-server-hook' because that would
>   invite a host of complications. I've chosen to address this by adding
>   a separate, internal hook expressly for this purpose. If that's dumb,
>   please someone say so.
>
> The fix for the second case belongs on emacs-29 (IMO) because the bug is
> caused by changes it introduces.

This has been carried out.

> The fix for the first could arguably go on master because it's been
> with ERC since the beginning.

This has indeed been held back in a separate patch destined for master
(attached).

>From c1ac9cfa3919ec0e7fc2633d611872088d017432 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email-masked]>
Date: Tue, 10 Jan 2023 00:31:42 -0800
Subject: [PATCH 0/3] *** NOT A PATCH ***

*** BLURB HERE ***

F. Jason Park (3):
  ; Fix wrong type in erc-ignore hide-list options
  Remove obsolete server buffers on MOTD in erc-track
  [5.6] Ignore killed buffers when switching in erc-track

 lisp/erc/erc-networks.el                      |  6 +++
 lisp/erc/erc-track.el                         | 18 +++++--
 lisp/erc/erc.el                               |  6 ++-
 .../erc/erc-scenarios-base-association.el     | 49 +++++++++++++++++++
 test/lisp/erc/erc-scenarios-misc.el           | 34 +++++++++++++
 .../resources/networks/merge-server/track.eld | 44 +++++++++++++++++
 6 files changed, 152 insertions(+), 5 deletions(-)
 create mode 100644 test/lisp/erc/resources/networks/merge-server/track.eld

Range-diff:
-:  ---------- > 1:  e2e608e330 ; Fix wrong type in erc-ignore hide-list options
1:  da8d5def8f ! 2:  6e31bac624 Remove obsolete server buffers on MOTD in erc-track
    @@ Commit message
         membership in `erc-networks--copy-server-buffer-functions' hook.
         (erc-track--replace-killed-buffer): New function to replace server
         buffer being killed in `erc-modified-channels-alist'.
    -    (erc-track--switch-buffer): If recommended buffer has been killed,
    -    remove it from the `erc-modified-channels-alist', and try another one.
    -    * test/lisp/erc/erc-scenarios-misc.el
    -    (erc-scenarios-base-kill-server-track) : New test.
         * test/lisp/erc/erc-scenarios-base-association.el
         (erc-scenarios-networks-merge-server-track): New test.
         * test/lisp/erc/resources/networks/merge-server/track.eld: New test
    -    data.
    +    data.  (Bug#60560.)
     
      ## lisp/erc/erc-networks.el ##
     @@ lisp/erc/erc-networks.el: erc-networks--reclaim-orphaned-target-buffers
    @@ lisp/erc/erc-networks.el: erc-networks--reclaim-orphaned-target-buffers
      
     +(defvar erc-networks--copy-server-buffer-functions nil
     +  "Abnormal hook run in new server buffers when deduping.
    -+Passed the existing buffer slated to be killed, whose contents
    -+have already been copied over to the current buffer, its
    -+replacement.")
    ++Passed the existing buffer to be killed, whose contents have
    ++already been copied over to the current, replacement buffer.")
     +
      (defun erc-networks--copy-over-server-buffer-contents (existing name)
        "Kill off existing server buffer after copying its contents.
    @@ lisp/erc/erc-track.el: track
      
      (defcustom erc-track-when-inactive nil
        "Enable channel tracking even for visible buffers, if you are inactive."
    -@@ lisp/erc/erc-track.el: erc-track--switch-buffer
    - 	   (unless (eq major-mode 'erc-mode)
    - 	     (setq erc-track-last-non-erc-buffer (current-buffer)))
    - 	   ;; and jump to the next active channel
    --	   (funcall fun (erc-track-get-active-buffer arg)))
    -+           (if-let ((buf (erc-track-get-active-buffer arg))
    -+                    ((buffer-live-p buf)))
    -+               (funcall fun buf)
    -+             (erc-modified-channels-update)
    -+             (erc-track--switch-buffer fun arg)))
    - 	  ;; if no active channels, switch back to what we were doing before
    - 	  ((and erc-track-last-non-erc-buffer
    - 	        erc-track-switch-from-erc
     @@ lisp/erc/erc-track.el: erc-track-switch-buffer-other-window
        (interactive "p")
        (erc-track--switch-buffer 'switch-to-buffer-other-window arg))
    @@ test/lisp/erc/erc-scenarios-base-association.el: erc-scenarios-base-association-
     +
      ;;; erc-scenarios-base-association.el ends here
     
    - ## test/lisp/erc/erc-scenarios-misc.el ##
    -@@ test/lisp/erc/erc-scenarios-misc.el: erc-scenarios-handle-irc-url
    -       (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
    -         (funcall expect 10 "welcome")))))
    - 
    -+;; Ensure that ERC does not attempt to switch to a killed server
    -+;; buffer via `erc-track-switch-buffer'.
    -+
    -+(declare-function erc-track-switch-buffer "erc-track" (arg))
    -+(defvar erc-track-mode)
    -+
    -+(ert-deftest erc-scenarios-base-kill-server-track ()
    -+  :tags '(:expensive-test)
    -+  (erc-scenarios-common-with-cleanup
    -+      ((erc-scenarios-common-dialog "networks/merge-server")
    -+       (dumb-server (erc-d-run "localhost" t 'track))
    -+       (port (process-contact dumb-server :service))
    -+       (erc-server-flood-penalty 0.1)
    -+       (expect (erc-d-t-make-expecter)))
    -+
    -+    (ert-info ("Connect")
    -+      (with-current-buffer (erc :server "127.0.0.1"
    -+                                :port port
    -+                                :nick "tester")
    -+        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))
    -+        (should erc-track-mode)
    -+        (funcall expect 5 "changed mode for tester")
    -+        (erc-cmd-JOIN "#chan")))
    -+
    -+    (ert-info ("Join channel and kill server buffer")
    -+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
    -+        (funcall expect 5 "The hour that fools should ask"))
    -+      (with-current-buffer "FooNet"
    -+        (set-process-query-on-exit-flag erc-server-process nil)
    -+        (kill-buffer))
    -+      (should-not (eq (current-buffer) (get-buffer "#chan"))) ; *temp*
    -+      (ert-simulate-command '(erc-track-switch-buffer 1)) ; No longer signals
    -+      (should (eq (current-buffer) (get-buffer "#chan"))))))
    -+
    - ;;; erc-scenarios-misc.el ends here
    -
      ## test/lisp/erc/resources/networks/merge-server/track.eld (new) ##
     @@
     +;; -*- mode: lisp-data; -*-
-:  ---------- > 3:  c1ac9cfa39 [5.6] Ignore killed buffers when switching in erc-track
-- 
2.38.1

>From c1ac9cfa3919ec0e7fc2633d611872088d017432 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email-masked]>
Date: Tue, 3 Jan 2023 23:10:53 -0800
Subject: [PATCH 3/3] [5.6] Ignore killed buffers when switching in erc-track

* lisp/erc/erc-track.el (erc-track--switch-buffer): If the chosen
buffer has been killed, remove it from `erc-modified-channels-alist'
and try again.
* test/lisp/erc/erc-scenarios-misc.el
(erc-scenarios-base-kill-server-track) : New test.  (Bug#60560.)
---
 lisp/erc/erc-track.el               |  6 ++++-
 test/lisp/erc/erc-scenarios-misc.el | 34 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
index 7fd7b53602..e060b7039b 100644
--- a/lisp/erc/erc-track.el
+++ b/lisp/erc/erc-track.el
@@ -921,7 +921,11 @@ erc-track--switch-buffer
 	   (unless (eq major-mode 'erc-mode)
 	     (setq erc-track-last-non-erc-buffer (current-buffer)))
 	   ;; and jump to the next active channel
-	   (funcall fun (erc-track-get-active-buffer arg)))
+           (if-let ((buf (erc-track-get-active-buffer arg))
+                    ((buffer-live-p buf)))
+               (funcall fun buf)
+             (erc-modified-channels-update)
+             (erc-track--switch-buffer fun arg)))
 	  ;; if no active channels, switch back to what we were doing before
 	  ((and erc-track-last-non-erc-buffer
 	        erc-track-switch-from-erc
diff --git a/test/lisp/erc/erc-scenarios-misc.el b/test/lisp/erc/erc-scenarios-misc.el
index 5927eee48f..bb925eed83 100644
--- a/test/lisp/erc/erc-scenarios-misc.el
+++ b/test/lisp/erc/erc-scenarios-misc.el
@@ -205,4 +205,38 @@ erc-scenarios-handle-irc-url
       (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
         (funcall expect 10 "welcome")))))
 
+;; Ensure that ERC does not attempt to switch to a killed server
+;; buffer via `erc-track-switch-buffer'.
+
+(declare-function erc-track-switch-buffer "erc-track" (arg))
+(defvar erc-track-mode)
+
+(ert-deftest erc-scenarios-base-kill-server-track ()
+  :tags '(:expensive-test)
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "networks/merge-server")
+       (dumb-server (erc-d-run "localhost" t 'track))
+       (port (process-contact dumb-server :service))
+       (erc-server-flood-penalty 0.1)
+       (expect (erc-d-t-make-expecter)))
+
+    (ert-info ("Connect")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester")
+        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))
+        (should erc-track-mode)
+        (funcall expect 5 "changed mode for tester")
+        (erc-cmd-JOIN "#chan")))
+
+    (ert-info ("Join channel and kill server buffer")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+        (funcall expect 5 "The hour that fools should ask"))
+      (with-current-buffer "FooNet"
+        (set-process-query-on-exit-flag erc-server-process nil)
+        (kill-buffer))
+      (should-not (eq (current-buffer) (get-buffer "#chan"))) ; *temp*
+      (ert-simulate-command '(erc-track-switch-buffer 1)) ; No longer signals
+      (should (eq (current-buffer) (get-buffer "#chan"))))))
+
 ;;; erc-scenarios-misc.el ends here
-- 
2.38.1