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

"J.P." <[email protected]>
Newsgroups gmane.emacs.erc.general
Message-ID <[email protected]>
"J.P." <[email protected]> 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).
0000-v1-v2.diff (text/x-patch, 5.8 KB)
From c1ac9cfa3919ec0e7fc2633d611872088d017432 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
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
0003-5.6-Ignore-killed-buffers-when-switching-in-erc-trac.patch (text/x-patch, 3.2 KB)
From c1ac9cfa3919ec0e7fc2633d611872088d017432 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.