master e958a2b726 14/25: Discourage ill-defined use of buffer targets in ERC

[F. Jason Park]

branch: master
commit e958a2b726fdcb5a4f58169e6f4f384f5786f86a
Author: F. Jason Park <jp@neverwas.me>
Commit: F. Jason Park <jp@neverwas.me>

    Discourage ill-defined use of buffer targets in ERC
    
    * lisp/erc/erc.el (erc-default-recipients, erc-default-target):
    Explain that the variable has fallen out of favor and that the
    function may have been used historically by third-party code for
    detecting channel subscription status, even though that's never been
    the case internally since at least the adoption of version control.
    Recommend newer alternatives.
    
    (erc--current-buffer-joined-p): Add possibly temporary predicate for
    detecting whether a buffer's target is a joined channel.  The existing
    means are inconsistent, as discussed in bug#48598.  The mere fact that
    they are disparate is unfriendly to new contributors.  For example, in
    the function `erc-autojoin-channels', the `process-status' of the
    `erc-server-process' is used to detect whether a buffer needs joining.
    That's fine in that specific situation, but it won't work elsewhere.
    And neither will checking whether `erc-default-target' is nil, so
    long as `erc-delete-default-channel' and friends remain in play.
    
    (erc-add-default-channel, erc-delete-default-channel, erc-add-query,
    erc-delete-query): Deprecate these helpers, which rely on an unused
    usage variant of `erc-default-recipients'.
    
    * lisp/erc/erc-services.el: remove stray `erc-default-recipients'
    declaration.
    
    * lisp/erc/erc-backend.el (erc-server-NICK, erc-server-JOIN,
    erc-server-KICK, erc-server-PART): wrap deprecated helpers to suppress
    warnings.
    
    * lisp/erc/erc-join.el (erc-autojoin-channels): Use helper to detect
    whether a buffer needs joining.  Prefer this to server liveliness, as
    explained above.
---
 lisp/erc/erc-backend.el | 10 +++++++---
 lisp/erc/erc-join.el    |  2 +-
 lisp/erc/erc-track.el   |  2 --
 lisp/erc/erc.el         | 41 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index bb423eadc0..305422195b 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -1339,7 +1339,9 @@ add things to `%s' instead."
                                              erc-server-process))
                       (when buffer
                         (set-buffer buffer)
-                        (erc-add-default-channel chnl)
+                        (with-suppressed-warnings
+                            ((obsolete erc-add-default-channel))
+                          (erc-add-default-channel chnl))
                         (erc-server-send (format "MODE %s" chnl)))
                       (erc-with-buffer (chnl proc)
                         (erc-channel-begin-receiving-names))
@@ -1376,7 +1378,8 @@ add things to `%s' instead."
         (erc-with-buffer
             (buffer)
           (erc-remove-channel-users))
-        (erc-delete-default-channel ch buffer)
+        (with-suppressed-warnings ((obsolete erc-delete-default-channel))
+          (erc-delete-default-channel ch buffer))
         (erc-update-mode-line buffer))
        ((string= nick (erc-current-nick))
         (erc-display-message
@@ -1465,7 +1468,8 @@ add things to `%s' instead."
         (erc-with-buffer
             (buffer)
           (erc-remove-channel-users))
-        (erc-delete-default-channel chnl buffer)
+        (with-suppressed-warnings ((obsolete erc-delete-default-channel))
+          (erc-delete-default-channel chnl buffer))
         (erc-update-mode-line buffer)
         (when erc-kill-buffer-on-part
           (kill-buffer buffer))))))
diff --git a/lisp/erc/erc-join.el b/lisp/erc/erc-join.el
index b9788c192b..425de4dc56 100644
--- a/lisp/erc/erc-join.el
+++ b/lisp/erc/erc-join.el
@@ -176,7 +176,7 @@ This function is run from `erc-nickserv-identified-hook'."
                                                 (erc-downcase current)))))))))
              (when (or (not buffer)
                        (not (with-current-buffer buffer
-                              (erc-server-process-alive))))
+                               (erc--current-buffer-joined-p))))
                (erc-server-join-channel server chan))))))))
   ;; Return nil to avoid stomping on any other hook funcs.
   nil)
diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
index 9118d7b994..e8117f9a89 100644
--- a/lisp/erc/erc-track.el
+++ b/lisp/erc/erc-track.el
@@ -353,8 +353,6 @@ of `erc-track-shorten-start' characters."
      (> (length s) erc-track-shorten-cutoff))
    erc-track-shorten-start))
 
-(defvar erc-default-recipients)
-
 (defun erc-all-buffer-names ()
   "Return all channel or query buffer names.
 Note that we cannot use `erc-channel-list' with a nil argument,
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 078a446a1c..9f17816b8d 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1907,6 +1907,21 @@ all channel buffers on all servers."
 
 ;; Some local variables
 
+;; TODO eventually deprecate this variable
+;;
+;; In the ancient, pre-CVS days (prior to June 2001), this list may
+;; have been used for supporting the changing of a buffer's target on
+;; the fly (mid-session).  Such usage, which allowed cons cells like
+;; (QUERY . bob) to serve as the list's head, was either never fully
+;; integrated or was partially clobbered prior to the introduction of
+;; version control.  But vestiges remain (see `erc-dcc-chat-mode').
+;; And despite appearances, no evidence has emerged that ERC ever
+;; supported one-to-many target buffers.  If such a thing was aspired
+;; to, it was never realized.
+;;
+;; New library code should use the `erc--target' struct instead.
+;; Third-party code can continue to use this until a getter for
+;; `erc--target' (or whatever replaces it) is exported.
 (defvar-local erc-default-recipients nil
   "List of default recipients of the current buffer.")
 
@@ -5868,6 +5883,27 @@ 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.
+  (cl-assert erc--target)
+  (and (erc--target-channel-p erc--target)
+       (erc-get-channel-user (erc-current-nick)) t))
+
+;; This function happens to return nil in channel buffers previously
+;; parted or those from which a user had been kicked.  While this
+;; "works" for detecting whether a channel is currently subscribed to,
+;; new code should consider using
+;;
+;;   (erc-get-channel-user (erc-current-nick))
+;;
+;; instead.  For retrieving a target regardless of subscription or
+;; connection status, use replacements based on `erc--target'.
+;; (Coming soon.)
+;;
+;; TODO deprecate this
 (defun erc-default-target ()
   "Return the current default target (as a character string) or nil if none."
   (let ((tgt (car erc-default-recipients)))
@@ -5878,12 +5914,14 @@ See also `erc-downcase'."
 
 (defun erc-add-default-channel (channel)
   "Add CHANNEL to the default channel list."
+  (declare (obsolete "use `erc-cmd-JOIN' or similar instead" "29.1"))
   (let ((chl (downcase channel)))
     (setq erc-default-recipients
           (cons chl erc-default-recipients))))
 
 (defun erc-delete-default-channel (channel &optional buffer)
   "Delete CHANNEL from the default channel list."
+  (declare (obsolete "use `erc-cmd-PART' or similar instead" "29.1"))
   (with-current-buffer (if (and buffer
                                 (bufferp buffer))
                            buffer
@@ -5895,6 +5933,7 @@ See also `erc-downcase'."
   "Add QUERY'd NICKNAME to the default channel list.
 
 The previous default target of QUERY type gets removed."
+  (declare (obsolete "use `erc-cmd-QUERY' or similar instead" "29.1"))
   (let ((d1 (car erc-default-recipients))
         (d2 (cdr erc-default-recipients))
         (qt (cons 'QUERY (downcase nickname))))
@@ -5905,7 +5944,7 @@ The previous default target of QUERY type gets removed."
 
 (defun erc-delete-query ()
   "Delete the topmost target if it is a QUERY."
-
+  (declare (obsolete "use one query buffer per target instead" "29.1"))
   (let ((d1 (car erc-default-recipients))
         (d2 (cdr erc-default-recipients)))
     (if (and (listp d1)