Don't kill server buffer with erc-kill-buffer-on-part

* etc/ERC-NEWS: Mention new flag `erc-killing-buffer-on-part-p' and
the renaming of `erc-kill-channel'.
* lisp/erc/erc-backend.el (erc-server-PART): Only kill a buffer on
behalf of `erc-kill-buffer-on-part' when the buffer hasn't already
been killed, and bind `erc-killing-buffer-on-part-p' to t when doing
so.
* lisp/erc/erc-log.el (erc-conditional-save-buffer): Don't save logs
when the buffer parameter is nil because that causes the server buffer
to be saved out.  It's possible that user code relying on this
longstanding bug will be affected, however, by default, the server
buffer will also be saved out independently at designated junctures.
* lisp/erc/erc.el (erc-part-hook): Redo doc string.
(erc-killing-buffer-on-part-p): New variable, a flag to prevent
redundant execution of `erc-kill-channel-hook' members concerned with
parted channels.
(erc-kill-buffer-on-part): Tweak doc string.
(erc-kill-channel-hook): Use new name for `erc-kill-channel',
`erc-part-channel-on-kill'.
(erc-kill-channel, erc-part-channel-on-kill): Rename former to latter,
and inhibit execution when `erc-killing-buffer-on-part-p' is non-nil.
* test/lisp/erc/erc-scenarios-base-kill-on-part.el: New file.
(Bug#70840)

(cherry picked from commit cf7cc4c630a2cd71c52069237053cb1476104572)

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index b66ea6a7a02075e8d26be1a779df9dc18d987c0e..2c3c6c365a1ed4c7b0aefe7d6d322360743b50ee 100644 (file)
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -648,6 +648,14 @@ release lacks a similar solution for detecting "joinedness" directly,
 but users can turn to 'xor'-ing 'erc-default-target' and 'erc-target'
 as a makeshift kludge.
 
+*** Function 'erc-kill-channel' renamed to 'erc-part-channel-on-kill'.
+This function, which normally emits a 'PART' when ERC kills a channel
+buffer, has been renamed for clarity.  Moreover, this and all other
+members of 'erc-kill-channel-hook' can now take comfort in knowing
+that the killing of buffers done on behalf of the option
+'erc-kill-buffer-on-part' has been made more detectable by the flag
+'erc-killing-buffer-on-part-p'.
+
 *** Channel-mode handling has become stricter and more predictable.
 ERC has always processed channel modes using "standardized" letters
 and popular status prefixes.  Starting with this release, ERC will

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index ab419d2b018ddb5c11688e451a0c90c88cb26826..ef99d762a074c0d1abb63c2b66e40ef4456f7d41 100644 (file)
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -1883,6 +1883,9 @@ add things to `%s' instead."
          (buffer (erc-get-buffer chnl proc)))
     (pcase-let ((`(,nick ,login ,host)
                  (erc-parse-user (erc-response.sender parsed))))
+      ;; When `buffer' is nil, `erc-remove-channel-member' and
+      ;; `erc-remove-channel-users' do almost nothing, and the message
+      ;; is displayed in the server buffer.
       (erc-remove-channel-member buffer nick)
       (erc-display-message parsed 'notice buffer
                            'PART ?n nick ?u login
@@ -1896,8 +1899,10 @@ add things to `%s' instead."
           (erc-delete-default-channel chnl buffer))
         (erc-update-mode-line buffer)
         (defvar erc-kill-buffer-on-part)
-        (when erc-kill-buffer-on-part
-          (kill-buffer buffer))))))
+        (when (and erc-kill-buffer-on-part buffer)
+          (defvar erc-killing-buffer-on-part-p)
+          (let ((erc-killing-buffer-on-part-p t))
+            (kill-buffer buffer)))))))
 
 (define-erc-response-handler (PING)
   "Handle ping messages." nil

diff --git a/lisp/erc/erc-log.el b/lisp/erc/erc-log.el
index d5c56bcc2b3449dbba5b8f3924e8a353f1837de7..66420662c23344798f6e29697a936e656e1c3ba7 100644 (file)
--- a/lisp/erc/erc-log.el
+++ b/lisp/erc/erc-log.el
@@ -289,8 +289,8 @@ Return nil if BUFFER is a server buffer."
                                  (erc-save-buffer-in-logs)))
 
 (defun erc-conditional-save-buffer (buffer)
-  "Save Query BUFFER if `erc-save-queries-on-quit' is t."
-  (when erc-save-buffer-on-part
+  "Save channel BUFFER if it and `erc-save-buffer-on-part' are non-nil."
+  (when (and buffer erc-save-buffer-on-part)
     (erc-save-buffer-in-logs buffer)))
 
 (defun erc-conditional-save-queries (process)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 5ac1016558489147681eebf92e6845fcfe34ce89..81ada55aba180b366a4634a4108a194fb38a9c63 100644 (file)
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -470,13 +470,22 @@ See also `erc-server-QUIT-functions' and `erc-disconnected-hook'."
 
 (defcustom erc-part-hook nil
   "Hook run when processing a PART message directed at our nick.
-
-The hook receives one argument, the current BUFFER.
-See also `erc-server-QUIT-functions', `erc-quit-hook' and
-`erc-disconnected-hook'."
+Called in the server buffer with a single argument: the channel buffer
+being parted.  For historical reasons, the buffer argument may be nil if
+it's been killed or otherwise can't be found.  This typically happens
+when the \"PART\" response being handled comes by way of a channel
+buffer being killed, which, by default, tells `erc-part-channel-on-kill'
+to emit a \"PART\".  Users needing to operate on a parted channel buffer
+before it's killed in this manner should use `erc-kill-channel-hook' and
+condition their code on `erc-killing-buffer-on-part-p' being nil."
   :group 'erc-hooks
   :type 'hook)
 
+(defvar erc-killing-buffer-on-part-p nil
+  "Non-nil when killing a target buffer while handling a \"PART\" response.
+Useful for preventing the redundant execution of code designed to run
+once when parting a channel.")
+
 (defcustom erc-kick-hook nil
   "Hook run when processing a KICK message directed at our nick.
 
@@ -1114,8 +1123,7 @@ directory in the list."
 
 (defcustom erc-kill-buffer-on-part nil
   "Kill the channel buffer on PART.
-This variable should probably stay nil, as ERC can reuse buffers if
-you rejoin them later."
+Nil by default because ERC can reuse buffers later re-joined."
   :group 'erc-quit-and-part
   :type 'boolean)
 
@@ -9597,7 +9605,7 @@ See also `format-spec'."
   :type 'hook)
 
 (defcustom erc-kill-channel-hook
-  '(erc-kill-channel
+  '(erc-part-channel-on-kill
     erc-networks-shrink-ids-and-buffer-names
     erc-networks-rename-surviving-target-buffer)
   "Invoked whenever a channel-buffer is killed via `kill-buffer'."
@@ -9658,10 +9666,12 @@ This function should be on `erc-kill-server-hook'."
     (setq erc-server-quitting t)
     (erc-server-send (format "QUIT :%s" (funcall erc-quit-reason nil)))))
 
-(defun erc-kill-channel ()
-  "Sends a PART command to the server when the channel buffer is killed.
-This function should be on `erc-kill-channel-hook'."
-  (when (erc-server-process-alive)
+(define-obsolete-function-alias 'erc-kill-channel #'erc-part-channel-on-kill
+  "30.1")
+(defun erc-part-channel-on-kill ()
+  "Send a \"PART\" when killing a channel buffer."
+  (when (and (not erc-killing-buffer-on-part-p)
+             (erc-server-process-alive))
     (let ((tgt (erc-default-target)))
       (if tgt
          (erc-server-send (format "PART %s :%s" tgt

diff --git a/test/lisp/erc/erc-scenarios-base-kill-on-part.el b/test/lisp/erc/erc-scenarios-base-kill-on-part.el
new file mode 100644 (file)
index 0000000..0ca0b1a
--- /dev/null
+++ b/test/lisp/erc/erc-scenarios-base-kill-on-part.el
@@ -0,0 +1,95 @@
+;;; erc-scenarios-base-kill-on-part.el --- killing buffers on part -*- lexical-binding: t -*-
+
+;; Copyright (C) 2024 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)))
+
+;; Assert channel buffer is killed when `erc-kill-buffer-on-part' is
+;; enabled and a user issues a /part.  Also assert that code in
+;; `erc-kill-channel-hook' can detect when `erc-response-PART' is
+;; killing a buffer on behalf of that option.
+(ert-deftest erc-scenarios-base-kill-on-part--enabled ()
+  :tags '(:expensive-test)
+  (should-not erc-kill-buffer-on-part)
+
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "base/reuse-buffers/channel")
+       (erc-server-flood-penalty 0.1)
+       (dumb-server (erc-d-run "localhost" t 'foonet))
+       (port (process-contact dumb-server :service))
+       (erc-kill-buffer-on-part t)
+       (calls nil)
+       (erc-part-hook (lambda (b) (push (buffer-name b) calls)))
+       (erc-kill-channel-hook
+        (cons (lambda () (push erc-killing-buffer-on-part-p calls))
+              erc-kill-channel-hook))
+       (expect (erc-d-t-make-expecter)))
+
+    (ert-info ("Connect to foonet")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester"
+                                :password "foonet:changeme"
+                                :full-name "tester")
+        (funcall expect 10 "This server is in debug mode")))
+
+    (with-current-buffer (erc-d-t-wait-for 20 (get-buffer "#chan"))
+      (funcall expect 10 "<alice> bob: Whilst I can shake")
+      (erc-scenarios-common-say "/part"))
+
+    (erc-d-t-wait-for 20 (null (get-buffer "#chan")))
+    (should (equal calls '(t "#chan")))))
+
+;; When `erc-kill-buffer-on-part' is non-nil, and the parted buffer has
+;; already been killed, don't kill the server buffer.  Bug#70840
+(ert-deftest erc-scenarios-base-kill-on-part--enabled/killed ()
+  :tags '(:expensive-test)
+  (should-not erc-kill-buffer-on-part)
+
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "base/reuse-buffers/channel")
+       (erc-server-flood-penalty 0.1)
+       (dumb-server (erc-d-run "localhost" t 'foonet))
+       (port (process-contact dumb-server :service))
+       (erc-kill-buffer-on-part t)
+       (calls nil)
+       (erc-part-hook (lambda (b) (push b calls)))
+       (expect (erc-d-t-make-expecter)))
+
+    (ert-info ("Connect to foonet")
+      (with-current-buffer (erc :server "127.0.0.1"
+                                :port port
+                                :nick "tester"
+                                :password "foonet:changeme"
+                                :full-name "tester")
+        (funcall expect 10 "This server is in debug mode")))
+
+    (with-current-buffer (erc-d-t-wait-for 20 (get-buffer "#chan"))
+      (funcall expect 10 "<alice> bob: Whilst I can shake")
+      (kill-buffer))
+
+    (erc-d-t-wait-for 20 (null (get-buffer "#chan")))
+    (erc-d-t-wait-for 10 (equal calls '(nil)))
+    (erc-d-t-ensure-for 0.1 (get-buffer "foonet"))))
+
+;;; erc-scenarios-base-kill-on-part.el ends here