>From 016fda6c9836a326d251c4343880da1a99861ded Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 19 May 2024 23:04:49 -0700 Subject: [PATCH 1/7] [5.6] Return nil from more ERC response handlers * etc/ERC-NEWS: Mention that some non-nil returning default response handlers now return nil. * lisp/erc/erc-backend.el (define-erc-response-handler): Mention that body should explicitly return nil. (erc-server-PING): Return nil. * lisp/erc/erc-sasl.el (erc-sasl--destroy): Return nil. * lisp/erc/erc.el (erc-display-message): Mention in doc string that the return value is undefined. (erc-kill-channel-hook): Fix package-version. * test/lisp/erc/erc-networks-tests.el (erc-networks--set-name): Ensure `erc--route-insertion' returns nil because this influences whether response-handler hooks continue running. * test/lisp/erc/erc-tests.el (erc-message): Don't return non-nil in mocked `erc-display-message'. (erc-send-modify-hook): Shadow `erc-send-modify-hook' because erc-stamp--date-mode' adds to it locally. --- etc/ERC-NEWS | 8 ++++++++ lisp/erc/erc-backend.el | 20 ++++++++++++++++---- lisp/erc/erc-sasl.el | 3 ++- lisp/erc/erc.el | 6 ++++-- test/lisp/erc/erc-networks-tests.el | 2 +- test/lisp/erc/erc-tests.el | 3 ++- 6 files changed, 33 insertions(+), 9 deletions(-) diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS index 62970f52396..07e9608c836 100644 --- a/etc/ERC-NEWS +++ b/etc/ERC-NEWS @@ -685,6 +685,14 @@ The option 'erc-format-nick-function' has been renamed to actual role. So too has the related function 'erc-format-nick', which is now 'erc-determine-speaker-from user'. +*** All default response handlers return nil. +Actually, this isn't yet true, but ERC is moving in this direction, +with the aim of guaranteeing all response-handler hook members +directly following a default handler always run. In service of this +goal, default handlers like 'erc-server-PONG' and 'erc-server-904' +that may previously have returned non-nil have been updated to return +nil in all cases. User-defined default handlers should do the same. + *** A template-based approach to formatting inserted chat messages. Predicting and influencing how ERC formats messages containing a leading "" has never been straightforward. The characters diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el index ef99d762a07..90c46eadaf4 100644 --- a/lisp/erc/erc-backend.el +++ b/lisp/erc/erc-backend.el @@ -1566,13 +1566,23 @@ define-erc-response-handler `erc-server-NAME'. - a function `erc-server-NAME' with body FN-BODY. +\(Note that here, NAME merely refers to the parameter NAME rather than +an actual IRC response or server-sent command.) + If ALIASES is non-nil, each alias in ALIASES is `defalias'ed to `erc-server-NAME'. Alias hook variables are created as `erc-server-ALIAS-functions' and initialized to the same default value as `erc-server-NAME-functions'. -FN-BODY is the body of `erc-server-NAME' it may refer to the two -function arguments PROC and PARSED. +ERC uses FN-BODY as the body of the default response handler +`erc-server-NAME', which handles all incoming IRC \"NAME\" responses, +unless overridden (see below). ERC calls the function with two +arguments, PROC and PARSED, whose symbols (lowercase) are bound to the +current `erc-server-process' and `erc-response' instance within FN-BODY. +Implementers should take care not to shadow them inadvertently. In all +cases, FN-BODY should return nil to allow third parties to run code +after `erc-server-NAME' returns. For historical reasons, ERC does not +currently enforce this, however future versions very well may. If EXTRA-FN-DOC is non-nil, it is inserted at the beginning of the defined function's docstring. @@ -1902,7 +1912,8 @@ erc--server-determine-join-display-context (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))))))) + (kill-buffer buffer)))))) + nil) (define-erc-response-handler (PING) "Handle ping messages." nil @@ -1914,7 +1925,8 @@ erc--server-determine-join-display-context (erc-display-message parsed 'error proc 'PING ?s (erc-time-diff erc-server-last-ping-time (erc-current-time)))) - (setq erc-server-last-ping-time (erc-current-time)))) + (setq erc-server-last-ping-time (erc-current-time))) + nil) (define-erc-response-handler (PONG) "Handle pong messages." nil diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el index f1cc68e2620..1998e4f129b 100644 --- a/lisp/erc/erc-sasl.el +++ b/lisp/erc/erc-sasl.el @@ -373,7 +373,8 @@ erc-sasl--destroy "Destroy process PROC and warn user that their settings are likely faulty." (delete-process proc) (erc--lwarn 'erc-sasl :error - "Disconnected from %s; please review SASL settings" proc)) + "Disconnected from %s; please review SASL settings" proc) + nil) (define-erc-response-handler (902) "Handle an ERR_NICKLOCKED response." nil diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index 9100ab5577d..3d73c33312a 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -3987,7 +3987,9 @@ erc-display-message instead of lower-level ones, like `erc-insert-line', to insert arbitrary informative messages as if sent by the server. That is, tell modules to treat a \"local\" message for which PARSED is -nil like any other server-sent message." +nil like any other server-sent message. Finally, expect users to +treat the return value of this function as undefined even though +various default response handlers may appear to presume nil." (let* ((erc--msg-props (or erc--msg-props (let ((table (make-hash-table)) @@ -9626,7 +9628,7 @@ erc-kill-channel-hook erc-networks-shrink-ids-and-buffer-names erc-networks-rename-surviving-target-buffer) "Invoked whenever a channel-buffer is killed via `kill-buffer'." - :package-version '(ERC . "5.5") + :package-version '(ERC . "5.6") ; FIXME sync on release :group 'erc-hooks :type 'hook) diff --git a/test/lisp/erc/erc-networks-tests.el b/test/lisp/erc/erc-networks-tests.el index 90d6f13f2f6..f0a7c37ddf2 100644 --- a/test/lisp/erc/erc-networks-tests.el +++ b/test/lisp/erc/erc-networks-tests.el @@ -1199,7 +1199,7 @@ erc-networks--set-name (erc-mode) (cl-letf (((symbol-function 'erc--route-insertion) - (lambda (&rest r) (push r calls)))) + (lambda (&rest r) (ignore (push r calls))))) (ert-info ("Signals when `erc-server-announced-name' unset") (should-error (erc-networks--set-name nil (make-erc-response))) diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index 999d9f100c9..a6e6d58cf9d 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -330,6 +330,7 @@ erc--refresh-prompt (cl-incf counter)))) erc-accidental-paste-threshold-seconds erc-insert-modify-hook + erc-send-modify-hook (erc-last-input-time 0) (erc-modules (remq 'stamp erc-modules)) (erc-send-input-line-function #'ignore) @@ -2533,7 +2534,7 @@ erc-message erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) (cl-letf (((symbol-function 'erc-display-message) (lambda (_ _ _ msg &rest args) - (push (apply #'erc-format-message msg args) calls))) + (ignore (push (apply #'erc-format-message msg args) calls)))) ((symbol-function 'erc-server-send) (lambda (line _) (push line calls))) ((symbol-function 'erc-server-buffer) -- 2.45.1