From 8ec6095d2f433e505765874c043903d0975a70e7 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 15 Oct 2023 13:20:07 -0700 Subject: [PATCH 2/2] [5.6] Add function erc-target * etc/ERC-NEWS: Mention `erc-target' and new `erc-server-buffer-p' alias. * lisp/erc/erc-backend.el (erc-process-sentinel): Set `joined-p' slot of `erc--target-channel' object to nil if applicable. (JOIN): Mark `erc--target-channel' object as being joined. * lisp/erc/erc-common.el (erc--target-channel): Add `joined-p' slot. Use hyphenated name so accessor function's name ends in "joined-p" rather than "joinedp". (erc--target): Relocate here from erc.el. (erc-target): New public API function to return the current buffer's target as a string even in channels that have been unjoined. * lisp/erc/erc-networks.el (erc--default-target): Remove forward declaration. (erc-networks--id-reload): Use `erc-target' instead of `erc--default-target' as predicate for visiting target buffers. * lisp/erc/erc.el (erc-remove-channel-users): Set channel "joinedness" to nil in `erc--target-channel' object, when applicable. (erc--target): Move to erc-common. (erc--default-target): Remove, replaced by new function `erc-target'. (erc-query-buffer-p): Use `erc-target'. (erc-after-connect): Revise doc string. (erc-connection-established): Revise doc string and move `erc-unhide-query-prompt' business before hook. (erc--current-buffer-joined-p): Remove comment and use new `joined-p' slot of `erc--target-channel' for determining "joinedness" of channel. (erc-kill-buffer-function): Use `erc--target-channel-p' for detecting whether the buffer is a channel buffer. * test/lisp/erc/erc-networks-tests.el (erc-networks--shrink-ids-and-buffer-names--hook-collapse-target): Remove comment. * test/lisp/erc/erc-scenarios-base-reuse-buffers.el (erc-scenarios-common--base-reuse-buffers-channel-buffers): Clarify invariant. * test/lisp/erc/erc-tests.el (erc-with-all-buffers-of-server): Replace `erc-default-recipients' with `erc--target'. (erc--target-from-string): Fix expected shape of `erc--target-channel' struct. (erc-message): Set `erc--target' in buffer "#chan". --- etc/ERC-NEWS | 18 +++++ lisp/erc/erc-backend.el | 5 +- lisp/erc/erc-common.el | 22 ++++--- lisp/erc/erc-networks.el | 12 ++-- lisp/erc/erc.el | 65 ++++++------------- test/lisp/erc/erc-networks-tests.el | 5 -- .../erc/erc-scenarios-base-reuse-buffers.el | 2 + test/lisp/erc/erc-tests.el | 19 +++--- 8 files changed, 72 insertions(+), 76 deletions(-) diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS index 2e56539f210..4b261254bda 100644 --- a/etc/ERC-NEWS +++ b/etc/ERC-NEWS @@ -402,6 +402,24 @@ use of 'insert-before-markers' instead of 'insert'. As always, users feeling unduly inconvenienced by these changes are encouraged to voice their concerns on the bug list. +*** Introducing new ways to detect ERC buffer types. +The old standby 'erc-default-target' has served ERC well for over two +decades. But a lesser known gotcha affecting its use has always +haunted an unlucky few, that is, the function has always returned +non-nil in "unjoined" channel buffers (those that the client has +parted with or been kicked from). While perhaps not itself a major +footgun, recessive pitfalls rooted in this subtlety continue to affect +dependent functions, like 'erc-get-buffer'. + +To discourage misuse of 'erc-default-target', ERC 5.6 offers an +alternative in the function 'erc-target', which is identical to the +former except for its disregard for "joinedness." As a related bonus, +the dependent function 'erc-server-buffer-p' is being rebranded as +'erc-server-or-unjoined-channel-buffer-p'. Unfortunately, this +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. + *** Miscellaneous changes Two helper macros from GNU ELPA's Compat library are now available to third-party modules as 'erc-compat-call' and 'erc-compat-function'. diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el index 3d34fc97d00..29b69fad0e6 100644 --- a/lisp/erc/erc-backend.el +++ b/lisp/erc/erc-backend.el @@ -1103,7 +1103,7 @@ erc-process-sentinel (erc--register-connection) ;; assume event is 'failed (erc-with-all-buffers-of-server cproc nil - (setq erc-server-connected nil)) + (setq erc-server-connected nil)) (when erc-server-ping-handler (progn (cancel-timer erc-server-ping-handler) (setq erc-server-ping-handler nil))) @@ -1111,6 +1111,8 @@ erc-process-sentinel (erc-current-nick) (system-name) "") (dolist (buf (erc-buffer-filter (lambda () (boundp 'erc-channel-users)) cproc)) (with-current-buffer buf + (when (erc--target-channel-p erc--target) + (setf (erc--target-channel-joined-p erc--target) nil)) (setq erc-channel-users (make-hash-table :test 'equal)))) ;; Hide the prompt (erc--hide-prompt cproc) @@ -1729,6 +1731,7 @@ erc--server-determine-join-display-context (with-suppressed-warnings ((obsolete erc-add-default-channel)) (erc-add-default-channel chnl)) + (setf (erc--target-channel-joined-p erc--target) t) (erc-server-send (format "MODE %s" chnl))) (erc-with-buffer (chnl proc) (erc-channel-begin-receiving-names)) diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el index 8d896e663b5..930e8032f6d 100644 --- a/lisp/erc/erc-common.el +++ b/lisp/erc/erc-common.el @@ -81,16 +81,13 @@ erc--target (string "" :type string :documentation "Received name of target.") (symbol nil :type symbol :documentation "Case-mapped name as symbol.")) -;; At some point, it may make sense to add a query type with an -;; account field, which may help support reassociation across -;; reconnects and nick changes (likely requires v3 extensions). -;; -;; These channel variants should probably take on a `joined' field to -;; track "joinedness", which `erc-server-JOIN', `erc-server-PART', -;; etc. should toggle. Functions like `erc--current-buffer-joined-p' -;; may find it useful. +;; At some point, it may make sense to add a separate query type, +;; possibly with an account field to help reassociation across +;; reconnects and nick changes. + +(cl-defstruct (erc--target-channel (:include erc--target)) + (joined-p nil :type boolean :documentation "Whether channel is joined.")) -(cl-defstruct (erc--target-channel (:include erc--target))) (cl-defstruct (erc--target-channel-local (:include erc--target-channel))) ;; Beginning in 5.5/29.1, the `tags' field may take on one of two @@ -427,6 +424,13 @@ erc-with-all-buffers-of-server ,@forms)) ,process))) +(defvar-local erc--target nil + "A permanent `erc--target' struct instance in channel and query buffers.") + +(define-inline erc-target () + "Return target of current buffer, if any, as a string." + (inline-quote (and erc--target (erc--target-string erc--target)))) + (defun erc-log-aux (string) "Do the debug logging of STRING." (let ((cb (current-buffer)) diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el index d73d715db2c..dd047243a3c 100644 --- a/lisp/erc/erc-networks.el +++ b/lisp/erc/erc-networks.el @@ -53,7 +53,6 @@ erc-server-parameters (defvar erc-server-process) (defvar erc-session-server) -(declare-function erc--default-target "erc" nil) (declare-function erc--get-isupport-entry "erc-backend" (key &optional single)) (declare-function erc-buffer-filter "erc" (predicate &optional proc)) (declare-function erc-current-nick "erc" nil) @@ -991,12 +990,11 @@ erc-networks--id-reload (erc-networks--id-qualifying-len nid)) (erc-networks--rename-server-buffer (or proc erc-server-process) parsed) (erc-networks--shrink-ids-and-buffer-names-any) - (erc-with-all-buffers-of-server - erc-server-process #'erc--default-target - (when-let* ((new-name (erc-networks--reconcile-buffer-names erc--target - nid)) - ((not (equal (buffer-name) new-name)))) - (rename-buffer new-name 'unique)))) + (erc-with-all-buffers-of-server erc-server-process #'erc-target + (when-let + ((new-name (erc-networks--reconcile-buffer-names erc--target nid)) + ((not (equal (buffer-name) new-name)))) + (rename-buffer new-name 'unique)))) (cl-defgeneric erc-networks--id-ensure-comparable (self other) "Take measures to ensure two net identities are in comparable states.") diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index 7144e81397a..26f7f5bf188 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -534,6 +534,8 @@ erc-remove-channel-users Removes all users in the current channel. This is called by `erc-server-PART' and `erc-server-QUIT'." + (when (erc--target-channel-p erc--target) + (setf (erc--target-channel-joined-p erc--target) nil)) (when (and erc-server-connected (erc-server-process-alive) (hash-table-p erc-channel-users)) @@ -1391,16 +1393,6 @@ erc--target-from-string #'make-erc--target) :string string :symbol (intern (erc-downcase string)))) -(defvar-local erc--target nil - "Info about a buffer's target, if any.") - -;; Temporary internal getter to ease transition to `erc--target' -;; everywhere. Will be replaced by updated `erc-default-target'. -(defun erc--default-target () - "Return target string or nil." - (when erc--target - (erc--target-string erc--target))) - (defun erc-once-with-server-event (event f) "Run function F the next time EVENT occurs in the `current-buffer'. @@ -1504,7 +1496,7 @@ erc-query-buffer-p "Return non-nil if BUFFER is an ERC query buffer. If BUFFER is nil, the current buffer is used." (with-current-buffer (or buffer (current-buffer)) - (let ((target (erc-default-target))) + (let ((target (erc-target))) (and (eq major-mode 'erc-mode) target (not (memq (aref target 0) '(?# ?& ?+ ?!))))))) @@ -2480,10 +2472,13 @@ erc-before-connect :type '(repeat function)) (defcustom erc-after-connect nil - "Functions called after connecting to a server. -This functions in this variable gets executed when an end of MOTD -has been received. All functions in here get called with the -parameters SERVER and NICK." + "Abnormal hook run upon establishing a logical IRC connection. +Runs on MOTD's end when `erc-server-connected' becomes non-nil. +ERC calls members with `erc-server-announced-name', falling back +to the 376/422 message's \"sender\", as well as the current nick, +as given by the 376/422 message's \"target\" parameter, which is +typically the same as that reported by `erc-current-nick'." + :package-version '(ERC . "5.6") ; FIXME sync on release :group 'erc-hooks :type '(repeat function)) @@ -5701,9 +5696,7 @@ erc-handle-login (erc-load-script f))))) (defun erc-connection-established (proc parsed) - "Run just after connection. - -Set user modes and run `erc-after-connect' hook." + "Set user mode and run `erc-after-connect' hook in server buffer." (with-current-buffer (process-buffer proc) (unless erc-server-connected ; only once per session (let ((server (or erc-server-announced-name @@ -5722,14 +5715,11 @@ erc-connection-established (erc-update-mode-line) (erc-set-initial-user-mode nick buffer) (erc-server-setup-periodical-ping buffer) - (run-hook-with-args 'erc-after-connect server nick)))) - - (when erc-unhide-query-prompt - (erc-with-all-buffers-of-server proc - nil ; FIXME use `erc--target' after bug#48598 - (when (and (erc-default-target) - (not (erc-channel-p (car erc-default-recipients)))) - (erc--unhide-prompt))))) + (when erc-unhide-query-prompt + (erc-with-all-buffers-of-server erc-server-process nil + (when (and erc--target (not (erc--target-channel-p erc--target))) + (erc--unhide-prompt)))) + (run-hook-with-args 'erc-after-connect server nick))))) (defun erc-set-initial-user-mode (nick buffer) "If `erc-user-mode' is non-nil for NICK, set the user modes. @@ -7017,25 +7007,11 @@ erc-nick-equal-p ;; default target handling (defun erc--current-buffer-joined-p () - "Return whether the current target buffer is joined." - ;; This may be a reliable means of detecting subscription status, - ;; but it's also roundabout and awkward. Perhaps it's worth - ;; discussing adding a joined slot to `erc--target' for this. + "Return non-nil if the current buffer is a channel and is joined." (cl-assert erc--target) (and (erc--target-channel-p erc--target) - (erc-get-channel-user (erc-current-nick)) t)) - -;; While `erc-default-target' happens to return nil in channel buffers -;; you've parted or from which you've been kicked, using it to detect -;; whether a channel is currently joined may become unreliable in the -;; future. For now, third-party code can use -;; -;; (erc-get-channel-user (erc-current-nick)) -;; -;; A predicate may be provided eventually. For retrieving a target's -;; name regardless of subscription or connection status, new library -;; code should use `erc--default-target'. Third-party code should -;; continue to use `erc-default-target'. + (erc--target-channel-joined-p erc--target) + t)) (defun erc-default-target () "Return the current channel or query target, if any. @@ -8267,6 +8243,7 @@ erc-kill-buffer-hook :group 'erc-hooks :type 'hook) +;; FIXME alias and deprecate current *-function suffixed name. (defun erc-kill-buffer-function () "Function to call when an ERC buffer is killed. This function should be on `kill-buffer-hook'. @@ -8280,7 +8257,7 @@ erc-kill-buffer-function (cond ((eq (erc-server-buffer) (current-buffer)) (run-hooks 'erc-kill-server-hook)) - ((erc-channel-p (or (erc-default-target) (buffer-name))) + ((erc--target-channel-p erc--target) (run-hooks 'erc-kill-channel-hook)) (t (run-hooks 'erc-kill-buffer-hook))))) diff --git a/test/lisp/erc/erc-networks-tests.el b/test/lisp/erc/erc-networks-tests.el index e95d99c128f..e5069880bc5 100644 --- a/test/lisp/erc/erc-networks-tests.el +++ b/test/lisp/erc/erc-networks-tests.el @@ -623,11 +623,6 @@ erc-networks--shrink-ids-and-buffer-names--hook-collapse-target :symbol 'foonet/dummy :parts [foonet "dummy"] :len 2) - ;; `erc-kill-buffer-function' uses legacy target detection - ;; but falls back on buffer name, so no need for: - ;; - ;; erc-default-recipients '("#a") - ;; erc--target (erc--target-from-string "#a") erc-server-process (with-temp-buffer (erc-networks-tests--create-dead-proc))) diff --git a/test/lisp/erc/erc-scenarios-base-reuse-buffers.el b/test/lisp/erc/erc-scenarios-base-reuse-buffers.el index 71027a0c138..af483bb1a52 100644 --- a/test/lisp/erc/erc-scenarios-base-reuse-buffers.el +++ b/test/lisp/erc/erc-scenarios-base-reuse-buffers.el @@ -124,6 +124,7 @@ erc-scenarios-common--base-reuse-buffers-channel-buffers (erc-d-t-search-for 1 "shake my sword") (erc-cmd-PART "#chan") (funcall expect 3 "You have left channel #chan") + (should-not (erc-get-channel-user (erc-current-nick))) (erc-cmd-JOIN "#chan"))) (ert-info ("Part #chan@barnet") @@ -139,6 +140,7 @@ erc-scenarios-common--base-reuse-buffers-channel-buffers (get-buffer "#chan/127.0.0.1<3>")) (ert-info ("Activity continues in new, -suffixed #chan@foonet buffer") + ;; The first /JOIN did not cause the same buffer to be reused. (with-current-buffer "#chan/127.0.0.1" (should-not (erc-get-channel-user (erc-current-nick)))) (with-current-buffer "#chan/127.0.0.1<3>" diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index 4f4662f5075..5155a8fc724 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -69,26 +69,25 @@ erc-with-all-buffers-of-server (with-current-buffer (get-buffer-create "#foo") (erc-mode) (setq erc-server-process proc-exnet) - (setq erc-default-recipients '("#foo"))) + (setq erc--target (erc--target-from-string "#foo"))) (with-current-buffer (get-buffer-create "#spam") (erc-mode) (setq erc-server-process proc-onet) - (setq erc-default-recipients '("#spam"))) + (setq erc--target (erc--target-from-string "#spam"))) (with-current-buffer (get-buffer-create "#bar") (erc-mode) (setq erc-server-process proc-onet) - (setq erc-default-recipients '("#bar"))) + (setq erc--target (erc--target-from-string "#bar"))) (with-current-buffer (get-buffer-create "#baz") (erc-mode) (setq erc-server-process proc-exnet) - (setq erc-default-recipients '("#baz"))) + (setq erc--target (erc--target-from-string "#baz"))) (should (eq (get-buffer-process "ExampleNet") proc-exnet)) - (erc-with-all-buffers-of-server (get-buffer-process "ExampleNet") - nil + (erc-with-all-buffers-of-server (get-buffer-process "ExampleNet") nil (kill-buffer)) (should-not (get-buffer "ExampleNet")) @@ -102,8 +101,7 @@ erc-with-all-buffers-of-server (calls 0) (get-test (lambda () (cl-incf calls) test))) - (erc-with-all-buffers-of-server proc-onet - (funcall get-test) + (erc-with-all-buffers-of-server proc-onet (funcall get-test) (kill-buffer)) (should (= calls 1))) @@ -812,7 +810,7 @@ erc--restore-initialize-priors (ert-deftest erc--target-from-string () (should (equal (erc--target-from-string "#chan") - #s(erc--target-channel "#chan" \#chan))) + #s(erc--target-channel "#chan" \#chan nil))) (should (equal (erc--target-from-string "Bob") #s(erc--target "Bob" bob))) @@ -820,7 +818,7 @@ erc--target-from-string (let ((erc--isupport-params (make-hash-table))) (puthash 'CHANTYPES '("&#") erc--isupport-params) (should (equal (erc--target-from-string "&Bitlbee") - #s(erc--target-channel-local "&Bitlbee" &bitlbee))))) + #s(erc--target-channel-local "&Bitlbee" &bitlbee nil))))) (ert-deftest erc--modify-local-map () (when (and (bound-and-true-p erc-irccontrols-mode) @@ -1846,6 +1844,7 @@ erc-message (erc-mode) (setq erc-server-process (buffer-local-value 'erc-server-process (get-buffer "ExampleNet")) + erc--target (erc--target-from-string "#chan") erc-default-recipients '("#chan") erc-channel-users (make-hash-table :test 'equal) erc-network 'ExampleNet) -- 2.41.0