Prefer erc-target to erc-default-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 when applicable.
(erc-server-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".  Note that this will always be nil when
disconnected.
(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 assertion.
* test/lisp/erc/erc-tests.el (erc-with-all-buffers-of-server):
Replace `erc-default-recipients' with `erc--target'.
(erc--target-from-string): Update expected shape of
`erc--target-channel' struct with new `joined-p' slot.
(erc-message): Set `erc--target' in buffer "#chan".  (Bug#66578)

8 files changed, 72 insertions, 76 deletions

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index e47c42a51a6..41ab9cc4c5e 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -425,6 +425,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 d3094b0b955..4b5edaa77d2 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -1103,7 +1103,7 @@ Change value of property `erc-prompt' from t to `hidden'."
             (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 @@ Change value of property `erc-prompt' from t to `hidden'."
                               (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)
@@ -1731,6 +1733,7 @@ add things to `%s' instead."
                         (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 @@
   (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 @@ nil."
                             ,@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 @@
 (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 @@ object."
                                       (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 877478690af..7d75ec49ccd 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -534,6 +534,8 @@ See also: `erc-remove-server-user' and
 
 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 @@ buffer during local-module setup and `erc-mode-hook' activation.")
              #'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 @@ If BUFFER is nil, the current buffer is used."
   "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) '(?# ?& ?+ ?!)))))))
@@ -2492,10 +2484,13 @@ in here get called with three parameters, SERVER, PORT and NICK."
   :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))
 
@@ -5749,9 +5744,7 @@ See also: `erc-echo-notice-in-user-buffers',
         (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
@@ -5770,14 +5763,11 @@ Set user modes and run `erc-after-connect' hook."
         (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.
@@ -7065,25 +7055,11 @@ See also `erc-downcase'."
 ;; 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.
@@ -8315,6 +8291,7 @@ See also `kill-buffer'."
   :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'.
@@ -8328,7 +8305,7 @@ or `erc-kill-buffer-hook' if any other buffer."
     (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 45ef0d10a6e..d0f1dddf6b3 100644
--- a/test/lisp/erc/erc-networks-tests.el
+++ b/test/lisp/erc/erc-networks-tests.el
@@ -623,11 +623,6 @@
                                :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 @@ Adapted from scenario clash-of-chans/uniquify described in Bug#48598:
         (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 @@ Adapted from scenario clash-of-chans/uniquify described in Bug#48598:
       (get-buffer "#chan/127.0.0.1<3>"))
 
     (ert-info ("Activity continues in new, <n>-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 02dfc55b6d5..4d2f880b46f 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -69,26 +69,25 @@
     (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 @@
            (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 @@
 
 (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 @@
   (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-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)