From 828aac06bd5aefb575547b069f8abbc7425dea17 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Tue, 14 May 2024 21:09:55 -0700 Subject: [PATCH] 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) --- etc/ERC-NEWS | 8 ++ lisp/erc/erc-backend.el | 9 +- lisp/erc/erc-log.el | 4 +- lisp/erc/erc.el | 32 ++++--- .../erc/erc-scenarios-base-kill-on-part.el | 95 +++++++++++++++++++ 5 files changed, 133 insertions(+), 15 deletions(-) create mode 100644 test/lisp/erc/erc-scenarios-base-kill-on-part.el diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS index b66ea6a7a02..2c3c6c365a1 100644 --- 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 ab419d2b018..ef99d762a07 100644 --- 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 d5c56bcc2b3..66420662c23 100644 --- 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 5ac10165584..81ada55aba1 100644 --- 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 index 00000000000..0ca0b1ae054 --- /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 . + +;;; 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 " 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 " 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 -- 2.39.5