From 92a006069823e7d13625f886ab5081f9d6e15f59 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 13 Nov 2022 01:52:48 -0800 Subject: [PATCH 2/8] Don't set erc-networks--id until network is known * lisp/erc/erc-networks.el (erc-networks--id-given): Accept a null argument. (erc-networks--id-on-connect): Remove unused function. (erc-networks--id-equal-p): Add method for comparing initialized and unset IDs. (erc-networks--update-server-identity): Ensure `erc-networks--id' is set before continuing search. (erc-networks--init-identity): Don't assume `erc-networks--id' is non-nil. Add branch condition to reload ID on non-nil case. (erc-networks-on-MOTD-end): Let init-ID function handle renaming of server buffer. * lisp/erc/erc.el (erc-open): For continued sessions, try copying over the last network ID. (erc--auth-source-determine-params-default): Don't expect a network ID to have been initialized. (erc-set-current-nick): When connected, reload network ID. Leave comment warning that it may be unneeded. * lisp/erc/erc-backend.el (erc-server-NICK, erc-server-433): Unless already connected, schedule ID reload when server rejects or mandates a nick change. * test/lisp/erc/erc-scenarios-base-association-nick.el (erc-scenarios-base-association-nick-bumped, erc-scenarios-base-association-nick-bumped-mandated-renick): Update to reflect more liberal association behavior when renamed by IRCd. --- lisp/erc/erc-backend.el | 6 +- lisp/erc/erc-networks.el | 53 +++++------- lisp/erc/erc.el | 21 +++-- .../erc-scenarios-base-association-nick.el | 84 +++++++++++-------- 4 files changed, 90 insertions(+), 74 deletions(-) diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el index 15fd6ac50f..f899b866f0 100644 --- a/lisp/erc/erc-backend.el +++ b/lisp/erc/erc-backend.el @@ -1619,7 +1619,7 @@ define-erc-response-handler (cl-pushnew (erc-server-buffer) bufs) (erc-set-current-nick nn) ;; Rename session, possibly rename server buf and all targets - (when (erc-network) + (when erc-server-connected (erc-networks--id-reload erc-networks--id proc parsed)) (erc-update-mode-line) (setq erc-nick-change-attempt-count 0) @@ -1629,6 +1629,8 @@ define-erc-response-handler 'NICK-you ?n nick ?N nn) (run-hook-with-args 'erc-nick-changed-functions nn nick)) (t + (when erc-server-connected + (erc-networks--id-reload erc-networks--id proc parsed)) (erc-handle-user-status-change 'nick (list nick login host) (list nn)) (erc-display-message parsed 'notice bufs 'NICK ?n nick ?u login ?h host ?N nn)))))) @@ -2255,6 +2257,8 @@ erc-server-322-message (define-erc-response-handler (433) "Login-time \"nick in use\"." nil + (when erc-server-connected + (erc-networks--id-reload erc-networks--id proc parsed)) (erc-nickname-in-use (cadr (erc-response.command-args parsed)) "already in use")) diff --git a/lisp/erc/erc-networks.el b/lisp/erc/erc-networks.el index b3e5fcf1a3..19a7ab8643 100644 --- a/lisp/erc/erc-networks.el +++ b/lisp/erc/erc-networks.el @@ -826,12 +826,11 @@ erc-networks--id ;; For now, please use this instead of `erc-networks--id-fixed-p'. (cl-defgeneric erc-networks--id-given (net-id) - "Return the preassigned identifier for a network presence, if any. -This may have originated from an `:id' arg to entry-point commands -`erc-tls' or `erc'.") + "Return the preassigned identifier for a network context, if any. +When non-nil, assume NET-ID originated from an `:id' argument to +entry-point commands `erc-tls' or `erc'.") -(cl-defmethod erc-networks--id-given ((_ erc-networks--id)) - nil) +(cl-defmethod erc-networks--id-given (_) nil) ; _ may be nil (cl-defmethod erc-networks--id-given ((nid erc-networks--id-fixed)) (erc-networks--id-symbol nid)) @@ -866,22 +865,15 @@ erc-networks--id-create ((_ symbol) &context (erc-obsolete-var erc-reuse-buffers null)) (erc-networks--id-fixed-create (intern (buffer-name)))) -(cl-defgeneric erc-networks--id-on-connect (net-id) - "Update NET-ID `erc-networks--id' after connection params known. -This is typically during or just after MOTD.") - -(cl-defmethod erc-networks--id-on-connect ((_ erc-networks--id)) - nil) - -(cl-defmethod erc-networks--id-on-connect ((id erc-networks--id-qualifying)) - (erc-networks--id-qualifying-update id (erc-networks--id-qualifying-create))) - (cl-defgeneric erc-networks--id-equal-p (self other) - "Return non-nil when two network identities exhibit underlying equality. -SELF and OTHER are `erc-networks--id' struct instances. This -should normally be used only for ID recovery or merging, after -which no two identities should be `equal' (timestamps aside) that -aren't also `eq'.") + "Return non-nil when two network IDs exhibit underlying equality. +Expect SELF and OTHER to be `erc-networks--id' struct instances +and that this will only be called for ID recovery or merging, +after which no two identities should be `equal' (timestamps +aside) that aren't also `eq'.") + +(cl-defmethod erc-networks--id-equal-p ((_ null) (_ erc-networks--id)) nil) +(cl-defmethod erc-networks--id-equal-p ((_ erc-networks--id) (_ null)) nil) (cl-defmethod erc-networks--id-equal-p ((self erc-networks--id) (other erc-networks--id)) @@ -1382,7 +1374,8 @@ erc-networks--update-server-identity (let* ((identity erc-networks--id) (buffer (current-buffer)) (f (lambda () - (unless (or (eq (current-buffer) buffer) + (unless (or (not erc-networks--id) + (eq (current-buffer) buffer) (eq erc-networks--id identity)) (if (erc-networks--id-equal-p identity erc-networks--id) (throw 'buffer erc-networks--id) @@ -1397,16 +1390,17 @@ erc-networks--update-server-identity ;; server buffer, whereas `erc-networks--rename-server-buffer' can run ;; mid-session, after an identity's core components have changed. -(defun erc-networks--init-identity (_proc _parsed) +(defun erc-networks--init-identity (proc parsed) "Update identity with real network name." ;; Initialize identity for real now that we know the network (cl-assert erc-network) - (unless (erc-networks--id-symbol erc-networks--id) ; unless just reconnected - (erc-networks--id-on-connect erc-networks--id)) - ;; Find duplicate identities or other conflicting ones and act - ;; accordingly. - (erc-networks--update-server-identity) - ;; + (if erc-networks--id + (erc-networks--id-reload erc-networks--id proc parsed) + (setq erc-networks--id (erc-networks--id-create nil)) + ;; Find duplicate identities or other conflicting ones and act + ;; accordingly. + (erc-networks--update-server-identity) + (erc-networks--rename-server-buffer proc parsed)) nil) (defun erc-networks--rename-server-buffer (new-proc &optional _parsed) @@ -1474,8 +1468,7 @@ erc-networks-on-MOTD-end ;; For now, retain compatibility with erc-server-NNN-functions. (or (erc-networks--ensure-announced proc parsed) (erc-networks--set-name proc parsed) - (erc-networks--init-identity proc parsed) - (erc-networks--rename-server-buffer proc parsed))) + (erc-networks--init-identity proc parsed))) (define-erc-module networks nil "Provide data about IRC networks." diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index 2312246e3e..1052c8c4c0 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -2017,10 +2017,12 @@ erc-open (setq erc-default-nicks (if (consp erc-nick) erc-nick (list erc-nick))) ;; client certificate (only useful if connecting over TLS) (setq erc-session-client-certificate client-certificate) - (setq erc-networks--id (if connect - (erc-networks--id-create id) - (buffer-local-value 'erc-networks--id - old-buffer))) + (setq erc-networks--id + (if connect + (or (and continued-session + (buffer-local-value 'erc-networks--id old-buffer)) + (and id (erc-networks--id-create id))) + (buffer-local-value 'erc-networks--id old-buffer))) ;; debug output buffer (setq erc-dbuf (when erc-log-p @@ -3179,7 +3181,8 @@ erc-auth-source-join-function function)) (defun erc--auth-source-determine-params-defaults () - (let* ((net (and-let* ((esid (erc-networks--id-symbol erc-networks--id)) + (let* ((net (and-let* ((erc-networks--id) + (esid (erc-networks--id-symbol erc-networks--id)) ((symbol-name esid))))) (localp (and erc--target (erc--target-channel-local-p erc--target))) (hosts (if localp @@ -5904,7 +5907,13 @@ erc-set-current-nick (with-current-buffer (if (buffer-live-p (erc-server-buffer)) (erc-server-buffer) (current-buffer)) - (setq erc-server-current-nick nick))) + (unless (equal erc-server-current-nick nick) + (setq erc-server-current-nick nick) + ;; This seems sensible but may well be superfluous. Should + ;; really prove that it's actually needed via test scenario. + (when erc-server-connected + (erc-networks--id-reload erc-networks--id))) + nick)) (defun erc-current-nick () "Return the current nickname." diff --git a/test/lisp/erc/erc-scenarios-base-association-nick.el b/test/lisp/erc/erc-scenarios-base-association-nick.el index 3e848be4df..b46c996bc0 100644 --- a/test/lisp/erc/erc-scenarios-base-association-nick.el +++ b/test/lisp/erc/erc-scenarios-base-association-nick.el @@ -25,13 +25,24 @@ (eval-when-compile (require 'erc-join)) -;; You register a new nick, disconnect, and log back in, but your nick -;; is not granted, so ERC obtains a backtick'd version. You open a -;; query buffer for NickServ, and ERC names it using the net-ID (which -;; includes the backtick'd nick) as a suffix. The original -;; (disconnected) NickServ buffer gets renamed with *its* net-ID as -;; well. You then identify to NickServ, and the dead session is no -;; longer considered distinct. +;; You register a new nick in a dedicated query buffer, disconnect, +;; and log back in, but your nick is not granted (maybe you just +;; turned off SASL). In any case, ERC obtains a backtick'd version. +;; You open a query buffer for NickServ, and ERC gives you the +;; existing one. And after you identify, all buffers retain their +;; names, although your net ID has changed internally. +;; +;; If ERC would've instead failed (or intentionally refused) to make +;; the association, you would've ended up with a new NickServ buffer +;; named after the new net ID as a suffix (based on the backtick'd +;; nick), for example, NickServ@foonet/tester`. And the original +;; (disconnected) NickServ buffer would've gotten suffixed with *its* +;; net-ID as well, e.g., NickServ@foonet/tester. And after +;; identifying, you would've seen ERC merge the two as well as their +;; server buffers. While this alternate behavior may arguably be a +;; more honest reflection of reality, it's also quite inconvenient. +;; For a clearer example, see the original version of this file +;; introduced by "Add user-oriented test scenarios for ERC". (ert-deftest erc-scenarios-base-association-nick-bumped () :tags '(:expensive-test) @@ -67,30 +78,29 @@ erc-scenarios-base-association-nick-bumped (funcall expect 5 "ERC finished")))) (with-current-buffer "foonet" - (erc-cmd-RECONNECT)) + (erc-cmd-RECONNECT) + (funcall expect 10 "User modes for tester`")) - (erc-d-t-wait-for 10 "Nick request rejection prevents reassociation (good)" - (get-buffer "foonet/tester`")) + (ert-info ("Server buffer reassociated with new nick") + (should-not (get-buffer "foonet/tester`"))) (ert-info ("Ask NickServ to change nick") - (with-current-buffer "foonet/tester`" - (funcall expect 3 "already in use") + (with-current-buffer "foonet" (funcall expect 3 "debug mode") (erc-cmd-QUERY "NickServ")) - (erc-d-t-wait-for 1 "Dead NickServ query buffer renamed, now qualified" - (get-buffer "NickServ@foonet/tester")) + (ert-info ( "NickServ buffer reassociated") + (should-not (get-buffer "NickServ@foonet/tester`")) + (should-not (get-buffer "NickServ@foonet/tester"))) - (with-current-buffer "NickServ@foonet/tester`" ; new one + (with-current-buffer "NickServ" ; new one (erc-scenarios-common-say "IDENTIFY tester changeme") - (funcall expect 5 "You're now logged in as tester") - (ert-info ("Original buffer found, reused") - (erc-d-t-wait-for 2 (equal (buffer-name) "NickServ"))))) + (funcall expect 5 "You're now logged in as tester"))) - (ert-info ("Ours is the only NickServ buffer that remains") + (ert-info ("Still just one NickServ buffer") (should-not (cdr (erc-scenarios-common-buflist "NickServ")))) - (ert-info ("Visible network ID truncated to one component") + (ert-info ("As well as one server buffer") (should (not (get-buffer "foonet/tester`"))) (should (not (get-buffer "foonet/tester"))) (should (get-buffer "foonet"))))) @@ -135,29 +145,29 @@ erc-scenarios-base-association-nick-bumped-mandated-renick ;; Since we use reconnect, a new buffer won't be created ;; TODO add variant with clean `erc' invocation (with-current-buffer "foonet" - (erc-cmd-RECONNECT)) + (erc-cmd-RECONNECT) + (funcall expect 10 "User modes for dummy")) - (ert-info ("Server-initiated renick") - (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "foonet/dummy")) - (should-not (get-buffer "foonet/tester")) - (funcall expect 15 "debug mode")) + (ert-info ("Server-initiated renick associated correctly") + (with-current-buffer "foonet" + (funcall expect 15 "debug mode") + (should-not (get-buffer "foonet/dummy")) + (should-not (get-buffer "foonet/tester"))) - (erc-d-t-wait-for 1 "Old query renamed, now qualified" - (get-buffer "bob@foonet/tester")) + (ert-info ("Old query reassociated") + (should (get-buffer "bob")) + (should-not (get-buffer "bob@foonet/tester")) + (should-not (get-buffer "bob@foonet/dummy"))) - (with-current-buffer (erc-d-t-wait-for 5 (get-buffer "bob@foonet/dummy")) + (with-current-buffer "foonet" (erc-cmd-NICK "tester") - (ert-info ("Buffers combined") - (erc-d-t-wait-for 2 (equal (buffer-name) "bob"))))) + (funcall expect 5 "You're now logged in as tester"))) - (with-current-buffer "foonet" - (funcall expect 5 "You're now logged in as tester")) - - (ert-info ("Ours is the only bob buffer that remains") + (ert-info ("Ours is still the only bob buffer that remains") (should-not (cdr (erc-scenarios-common-buflist "bob")))) - (ert-info ("Visible network ID truncated to one component") - (should (not (get-buffer "foonet/dummy"))) - (should (get-buffer "foonet"))))) + (ert-info ("Visible network ID still truncated to one component") + (should (not (get-buffer "foonet/tester"))) + (should (not (get-buffer "foonet/dummy")))))) ;;; erc-scenarios-base-association-nick.el ends here -- 2.38.1