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

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

Tags: patch

Case 1

  - From emacs -Q, do an M-x erc.
  - Join a channel, but stay in the server buffer.
  - C-x k and answer "yes". You should end up in the channel buffer, and
    the mode line should contain an indicator referencing the killed
    server buffer.
  - Issuing a C-c C-SPC produces an error saying "attempt to display
    deleted buffer".

  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

  - From emacs -Q, do an M-x erc.
  - Join a channel, switch to it, and /quit.
  - From any buffer other than the server buffer (current is fine),
    do another M-x erc (instead of just /reconnect'ing).
  - Wait a few seconds until the mode line updates.
  - Issue a C-c C-SPC and get an error saying "attempt to display
    deleted buffer".

  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. The fix for the first could arguably go
on master because it's been with ERC since the beginning. If anyone has
opinions on this or other aspects of this patch, please make them known
in the coming days. Thanks.

>From 9041ff4b40e720655bddb49e8a14ef1de1dfac44 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email-masked]>
Date: Mon, 2 Jan 2023 18:13:08 -0800
Subject: [PATCH 1/2] ; Fix wrong type in erc-ignore hide-list options

* lisp/erc/erc.el (erc-network-hide-list, erc-channel-hide-list):
Fix type in custom definition.
---
 lisp/erc/erc.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 6315d5aa48..ba7db15cf8 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -323,7 +323,8 @@ erc-network-hide-list
   \(\"OFTC\" \"JOIN\" \"QUIT\"))."
   :version "25.1"
   :group 'erc-ignore
-  :type 'erc-message-type)
+  :type '(alist :key-type string :value-type erc-message-type
+                :options ("Libera.Chat")))
 
 (defcustom erc-channel-hide-list nil
   "A list of IRC channels to hide message types from.
@@ -331,7 +332,8 @@ erc-channel-hide-list
   \(\"#erc\" \"NICK\")."
   :version "25.1"
   :group 'erc-ignore
-  :type 'erc-message-type)
+  :type '(alist :key-type string :value-type erc-message-type
+                :options ("#emacs")))
 
 (defcustom erc-disconnected-hook nil
   "Run this hook with arguments (NICK IP REASON) when disconnected.
-- 
2.38.1

>From 8570b0de7ccd13d344a2cac40159486a16a75224 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email-masked]>
Date: Tue, 3 Jan 2023 23:10:53 -0800
Subject: [PATCH 2/2] Remove obsolete server buffers on MOTD in erc-track

* lisp/erc/erc-networks.el
(erc-networks--copy-server-buffer-functions): New internal hook
through which modules can perform housekeeping when server buffers
belonging to the same network context are merged.
(erc-networks--copy-over-server-buffer-contents): Run new internal
hook `erc-networks--copy-server-buffer-functions'.
* lisp/erc/erc-track.el (erc-track-enable, erc-track-disable): Manage
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.
---
 lisp/erc/erc-networks.el                      |  7 +++
 lisp/erc/erc-track.el                         | 18 +++++--
 .../erc/erc-scenarios-base-association.el     | 49 +++++++++++++++++++
 test/lisp/erc/erc-scenarios-misc.el           | 34 +++++++++++++
 .../resources/networks/merge-server/track.eld | 44 +++++++++++++++++
 5 files changed, 149 insertions(+), 3 deletions(-)
 create mode 100644 test/lisp/erc/resources/networks/merge-server/track.eld

diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el
index 4044be08f9..26d879998a 100644
--- a/lisp/erc/erc-networks.el
+++ b/lisp/erc/erc-networks.el
@@ -1366,6 +1366,12 @@ erc-networks--reclaim-orphaned-target-buffers
                erc-server-connected t
                erc-networks--id nid))))))
 
+(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.")
+
 (defun erc-networks--copy-over-server-buffer-contents (existing name)
   "Kill off existing server buffer after copying its contents.
 Must be called from the replacement buffer."
@@ -1386,6 +1392,7 @@ erc-networks--copy-over-server-buffer-contents
         erc-kill-server-hook
         erc-kill-buffer-hook)
     (erc-networks--insert-transplanted-content text)
+    (run-hook-with-args 'erc-networks--copy-server-buffer-functions existing)
     (kill-buffer name)))
 
 ;; This stands alone for testing purposes
diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
index 61c0c66abf..e060b7039b 100644
--- a/lisp/erc/erc-track.el
+++ b/lisp/erc/erc-track.el
@@ -521,7 +521,9 @@ track
        (add-hook 'erc-disconnected-hook #'erc-modified-channels-update))
      ;; enable the tracking keybindings
      (add-hook 'erc-connect-pre-hook #'erc-track-minor-mode-maybe)
-     (erc-track-minor-mode-maybe)))
+     (erc-track-minor-mode-maybe))
+   (add-hook 'erc-networks--copy-server-buffer-functions
+             #'erc-track--replace-killed-buffer))
   ;; Disable:
   ((when (boundp 'erc-track-when-inactive)
      (erc-track-remove-from-mode-line)
@@ -539,7 +541,9 @@ track
      ;; disable the tracking keybindings
      (remove-hook 'erc-connect-pre-hook #'erc-track-minor-mode-maybe)
      (when erc-track-minor-mode
-       (erc-track-minor-mode -1)))))
+       (erc-track-minor-mode -1)))
+   (remove-hook 'erc-networks--copy-server-buffer-functions
+                #'erc-track--replace-killed-buffer)))
 
 (defcustom erc-track-when-inactive nil
   "Enable channel tracking even for visible buffers, if you are inactive."
@@ -917,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
@@ -942,6 +950,10 @@ erc-track-switch-buffer-other-window
   (interactive "p")
   (erc-track--switch-buffer 'switch-to-buffer-other-window arg))
 
+(defun erc-track--replace-killed-buffer (existing)
+  (when-let ((found (assq existing erc-modified-channels-alist)))
+    (setcar found (current-buffer))))
+
 (provide 'erc-track)
 
 ;;; erc-track.el ends here
diff --git a/test/lisp/erc/erc-scenarios-base-association.el b/test/lisp/erc/erc-scenarios-base-association.el
index 1e280d0fdd..a40a4cb755 100644
--- a/test/lisp/erc/erc-scenarios-base-association.el
+++ b/test/lisp/erc/erc-scenarios-base-association.el
@@ -26,7 +26,9 @@
 
 (declare-function erc-network-name "erc-networks")
 (declare-function erc-network "erc-networks")
+(declare-function erc-track-get-active-buffer "erc-track" (arg))
 (defvar erc-autojoin-channels-alist)
+(defvar erc-track-mode)
 (defvar erc-network)
 
 ;; Two networks, same channel name, no confusion (no bouncer).  Some
@@ -190,4 +192,51 @@ erc-scenarios-base-association-bouncer-history
       (with-current-buffer "#chan@barnet"
         (erc-d-t-search-for 10 "I'll bid adieu")))))
 
+;; Some modules may need to perform housekeeping when a newly
+;; connected server buffer is deemed a duplicate after its persistent
+;; network context is discovered on MOTD end.  One such module is
+;; `track', which needs to rid its list of modified channels of the
+;; buffer being killed.  Without this, a user may encounter an
+;; "Attempt to display deleted buffer" error when they try switching
+;; to it.
+
+(ert-deftest erc-scenarios-networks-merge-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 '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 quit")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+        (funcall expect 5 "The hour that fools should ask")
+        (erc-cmd-QUIT ""))
+      (with-current-buffer "FooNet"
+        (funcall expect 5 "finished")))
+
+    (ert-info ("Reconnect")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester")
+        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))
+        (funcall expect 5 "changed mode for tester")))
+
+    (with-current-buffer "#chan"
+      (funcall expect 5 "The hour that fools should ask")
+      ;; Simulate the old `erc-track-switch-buffer'
+      (switch-to-buffer (erc-track-get-active-buffer 1))
+      (erc-d-t-wait-for 10 (eq (get-buffer "FooNet") (current-buffer)))
+      (erc-cmd-QUIT ""))))
+
 ;;; erc-scenarios-base-association.el ends here
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
diff --git a/test/lisp/erc/resources/networks/merge-server/track.eld b/test/lisp/erc/resources/networks/merge-server/track.eld
new file mode 100644
index 0000000000..4a97f92f72
--- /dev/null
+++ b/test/lisp/erc/resources/networks/merge-server/track.eld
@@ -0,0 +1,44 @@
+;; -*- mode: lisp-data; -*-
+((nick 10 "NICK tester"))
+((user 10 "USER user 0 * :unknown")
+ (0.00 ":irc.example.net NOTICE * :*** Looking up your hostname...")
+ (0.01 ":irc.example.net NOTICE tester :*** Could not resolve your hostname: Domain not found; using your IP address (10.0.2.100) instead.")
+ (0.10 ":irc.example.net 001 tester :Welcome to the FooNet IRC Network [email-masked]")
+ (0.02 ":irc.example.net 002 tester :Your host is irc.example.net, running version InspIRCd-3")
+ (0.02 ":irc.example.net 003 tester :This server was created 05:58:57 Jan 04 2023")
+ (0.01 ":irc.example.net 004 tester irc.example.net InspIRCd-3 BIRcgikorsw ACHIKMORTXabcefghijklmnopqrstvz :HIXabefghjkloqv")
+ (0.00 ":irc.example.net 005 tester ACCEPT=30 AWAYLEN=200 BOT=B CALLERID=g CASEMAPPING=ascii CHANLIMIT=#:20 CHANMODES=IXbeg,k,Hfjl,ACKMORTcimnprstz CHANNELLEN=64 CHANTYPES=# ELIST=CMNTU ESILENCE=CcdiNnPpTtx EXCEPTS=e :are supported by this server")
+ (0.02 ":irc.example.net 005 tester EXTBAN=,ACORTUacjrwz HOSTLEN=64 INVEX=I KEYLEN=32 KICKLEN=255 LINELEN=512 MAXLIST=I:100,X:100,b:100,e:100,g:100 MAXTARGETS=20 MODES=20 MONITOR=30 NAMELEN=128 NAMESX NETWORK=FooNet :are supported by this server")
+ (0.01 ":irc.example.net 005 tester NICKLEN=30 PREFIX=(qaohv)~&@%+ SAFELIST SILENCE=32 STATUSMSG=~&@%+ TOPICLEN=307 UHNAMES USERIP USERLEN=10 USERMODES=,,s,BIRcgikorw WHOX :are supported by this server")
+ (0.01 ":irc.example.net 251 tester :There are 2 users and 0 invisible on 2 servers")
+ (0.01 ":irc.example.net 253 tester 1 :unknown connections")
+ (0.01 ":irc.example.net 254 tester 1 :channels formed")
+ (0.00 ":irc.example.net 255 tester :I have 2 clients and 1 servers")
+ (0.00 ":irc.example.net 265 tester :Current local users: 2  Max: 3")
+ (0.00 ":irc.example.net 266 tester :Current global users: 2  Max: 3")
+ (0.00 ":irc.example.net 375 tester :irc.example.net message of the day")
+ (0.00 ":irc.example.net 372 tester :  Have fun with the image!")
+ (0.00 ":irc.example.net 376 tester :End of message of the day."))
+
+((mode 10 "MODE tester +i")
+ (0.00 ":irc.example.net 501 tester x :is not a recognised user mode.")
+ (0.00 ":[email-masked] NOTICE tester :Welcome to FooNet, tester! Here on FooNet, we provide services to enable the registration of nicknames and channels! For details, type \2/msg NickServ help\2 and \2/msg ChanServ help\2.")
+ (0.02 ":[email-masked] MODE tester :+i"))
+
+((join 10 "JOIN #chan")
+ (0.01 ":[email-masked] JOIN :#chan"))
+
+((mode 10 "MODE #chan")
+ (0.01 ":irc.example.net 353 tester = #chan :@alice bob tester")
+ (0.01 ":irc.example.net 366 tester #chan :End of /NAMES list.")
+ (0.00 ":alice!alice@0::1 PRIVMSG #chan :tester, welcome!")
+ (0.02 ":bob!bob@0::1 PRIVMSG #chan :tester, welcome!")
+ (0.02 ":irc.example.net 324 tester #chan :+nt")
+ (0.01 ":irc.example.net 329 tester #chan :1672811954")
+ (0.07 ":alice!alice@0::1 PRIVMSG #chan :bob: This afternoon, sir ? well, she shall be there.")
+ (0.05 ":bob!bob@0::1 PRIVMSG #chan :alice: The hour that fools should ask."))
+
+((quit 10 "QUIT :\2ERC\2")
+ (0.04 "ERROR :Closing link: ([email-masked]) [Quit: \2ERC\2]"))
+
+((drop 1 DROP))
-- 
2.38.1