Re: bug#60428: 30.0.50; ERC >5.5: Make M-x erc a more welcoming entry point

From 3658e117ca66a11ef8bc4a071e98c1d0130c3101 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Mon, 10 Apr 2023 17:58:05 -0700
Subject: [PATCH] [5.6] Extend erc-interactive-display to cover /JOINs

* lisp/erc/erc.el (erc-buffer-display, erc-join-buffer): Swap alias
and aliased so that `erc-buffer-display' appears in Customize.
(erc-query-display, erc-interactive-display): Make former an alias for
latter because they're redundant and quite possibly confusing.  Change
the default value to match the longstanding default of
`erc-query-display' because it has seniority.
(erc--called-as-input-p): New variable for "slash"
commands, like `erc-cmd-FOO', to detect whether they're being called
"interactively" as a result of input given at ERC's prompt.
(erc-process-input-line): Bind `erc--called-as-input-p' when running
slash commands.
(erc-cmd-JOIN): Schedule callback to control how new buffer is
displayed when called interactively.
* test/lisp/erc/erc-scenarios-base-buffer-display.el: New file.
* test/lisp/erc/erc-scenarios-base-reconnect.el
(erc-scenarios-common--base-reconnect-options,
ert-deftest erc-scenarios-base-reconnect-options--buffer,
erc-scenarios-base-reconnect-options--default): Move to new file
erc-scenarios-base-buffer-display.el and rename.
* test/lisp/erc/erc-tests.el (erc-select-read-args, erc-tls,
erc--interactive): Change expected default value of
`erc-interactive-display' from `buffer' to `window'.  (Bug#60428.)
---
 lisp/erc/erc.el                               |  61 ++++---
 .../erc/erc-scenarios-base-buffer-display.el  | 158 ++++++++++++++++++
 test/lisp/erc/erc-scenarios-base-reconnect.el |  89 ----------
 test/lisp/erc/erc-tests.el                    |  12 +-
 4 files changed, 200 insertions(+), 120 deletions(-)
 create mode 100644 test/lisp/erc/erc-scenarios-base-buffer-display.el

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 284990e2d43..a3a9b03a93c 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1507,8 +1507,8 @@ erc-default-port-tls
   "IRC port to use for encrypted connections if it cannot be \
 detected otherwise.")
 
-(defvaralias 'erc-buffer-display 'erc-join-buffer)
-(defcustom erc-join-buffer 'bury
+(defvaralias 'erc-join-buffer 'erc-buffer-display)
+(defcustom erc-buffer-display 'bury
   "Determines how to display a newly created IRC buffer.
 
 The available choices are:
@@ -1528,13 +1528,21 @@ erc-join-buffer
                  (const :tag "Use current buffer" buffer)
                  (const :tag "Use current buffer" t)))
 
-(defcustom erc-interactive-display 'buffer
-  "How and whether to display server buffers for M-x erc.
-See `erc-buffer-display' and friends for a description of
-possible values."
+(defvaralias 'erc-query-display 'erc-interactive-display)
+(defcustom erc-interactive-display 'window
+  "How to display buffers created by issuing a /SLASH command.
+Examples include /QUERY and /JOIN.  See `erc-buffer-display' for
+a full description of available choices.
+
+Note that this does not affect the behavior of commands like
+`erc-cmd-JOIN' when called from lisp code.  Also, this option
+used to be called `erc-query-display' but now applies to more
+than just /QUERY.  See option `erc-receive-query-display' in the
+Custom group `erc-query' to decide how private messages from
+other people should be displayed."
   :package-version '(ERC . "5.6") ; FIXME sync on release
   :group 'erc-buffers
-  :type '(choice (const :tag "Use value of `erc-join-buffer'" nil)
+  :type '(choice (const :tag "Use value of `erc-buffer-display'" nil)
                  (const :tag "Split window and select" window)
                  (const :tag "Split window, don't select" window-noselect)
                  (const :tag "New frame" frame)
@@ -3057,6 +3065,10 @@ erc-message-type-member
   (let ((prop-val (erc-get-parsed-vector position)))
     (and prop-val (member (erc-response.command prop-val) list))))
 
+(defvar erc--called-as-input-p nil
+  "Non-nil when a user types a \"/slash\" command.
+Remains bound until `erc-cmd-SLASH' returns.")
+
 (defvar-local erc-send-input-line-function 'erc-send-input-line
   "Function for sending lines lacking a leading user command.
 When a line typed into a buffer contains an explicit command, like /msg,
@@ -3110,7 +3122,8 @@ erc-process-input-line
     (if (and command-list
              (not no-command))
         (let* ((cmd  (nth 0 command-list))
-               (args (nth 1 command-list)))
+               (args (nth 1 command-list))
+               (erc--called-as-input-p t))
           (condition-case nil
               (if (listp args)
                   (apply cmd args)
@@ -3584,6 +3597,21 @@ erc-cmd-JOIN
                    (erc-get-channel-user (erc-current-nick)))))
           (switch-to-buffer existing)
         (setq erc--server-last-reconnect-count 0)
+        (when-let* ; bind `erc-join-buffer' when /JOIN issued
+            ((erc--called-as-input-p)
+             (fn (lambda (proc parsed)
+                   (when-let* ; `fn' wrapper already removed from hook
+                       (((equal (car (erc-response.command-args parsed))
+                                channel))
+                        (sn (erc-extract-nick (erc-response.sender parsed)))
+                        ((erc-nick-equal-p sn (erc-current-nick)))
+                        (erc-join-buffer (or erc-interactive-display
+                                             erc-join-buffer)))
+                     (run-hook-with-args-until-success
+                      'erc-server-JOIN-functions proc parsed)
+                     t))))
+          (erc-with-server-buffer
+            (erc-once-with-server-event "JOIN" fn)))
         (erc-server-join-channel nil chnl key))))
   t)
 
@@ -3947,23 +3975,6 @@ erc-cmd-QUOTE
    (t nil)))
 (put 'erc-cmd-QUOTE 'do-not-parse-args t)
 
-(defcustom erc-query-display 'window
-  "How to display query buffers when using the /QUERY command to talk to someone.
-
-The default behavior is to display the message in a new window
-and bring it to the front.  See the documentation for
-`erc-join-buffer' for a description of the available choices.
-
-See also `erc-auto-query' to decide how private messages from
-other people should be displayed."
-  :group 'erc-query
-  :type '(choice (const :tag "Split window and select" window)
-                 (const :tag "Split window, don't select" window-noselect)
-                 (const :tag "New frame" frame)
-                 (const :tag "Bury in new buffer" bury)
-                 (const :tag "Use current buffer" buffer)
-                 (const :tag "Use current buffer" t)))
-
 (defun erc-cmd-QUERY (&optional user)
   "Open a query with USER.
 How the query is displayed (in a new window, frame, etc.) depends
diff --git a/test/lisp/erc/erc-scenarios-base-buffer-display.el b/test/lisp/erc/erc-scenarios-base-buffer-display.el
new file mode 100644
index 00000000000..3ed7a83653e
--- /dev/null
+++ b/test/lisp/erc/erc-scenarios-base-buffer-display.el
@@ -0,0 +1,158 @@
+;;; erc-scenarios-base-buffer-display.el --- Buffer display scenarios -*- lexical-binding: t -*-
+
+;; Copyright (C) 2023 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert-x)
+(eval-and-compile
+  (let ((load-path (cons (ert-resource-directory) load-path)))
+    (require 'erc-scenarios-common)))
+
+(eval-when-compile (require 'erc-join))
+
+;; These first couple `erc-reconnect-display' tests used to live in
+;; erc-scenarios-base-reconnect but have since been renamed.
+
+(defun erc-scenarios-base-buffer-display--reconnect-common (test)
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "base/reconnect")
+       (dumb-server (erc-d-run "localhost" t 'options 'options-again))
+       (port (process-contact dumb-server :service))
+       (expect (erc-d-t-make-expecter))
+       (erc-server-flood-penalty 0.1)
+       (erc-server-auto-reconnect t)
+       erc-autojoin-channels-alist
+       erc-server-buffer)
+
+    (should (memq 'autojoin erc-modules))
+
+    (ert-info ("Connect to foonet")
+      (setq erc-server-buffer (erc :server "127.0.0.1"
+                                   :port port
+                                   :nick "tester"
+                                   :password "changeme"
+                                   :full-name "tester"))
+      (with-current-buffer erc-server-buffer
+        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))
+        (funcall expect 10 "debug mode")))
+
+    (ert-info ("Wait for some output in channels")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+        (funcall expect 10 "welcome")))
+
+    (ert-info ("Server buffer shows connection failed")
+      (with-current-buffer erc-server-buffer
+        (funcall expect 10 "Connection failed!  Re-establishing")))
+
+    (should (equal erc-autojoin-channels-alist '((FooNet "#chan"))))
+
+    (funcall test)
+
+    ;; A manual /JOIN command tells ERC we're done auto-reconnecting
+    (with-current-buffer "FooNet" (erc-cmd-JOIN "#spam"))
+
+    (erc-d-t-ensure-for 1 "Newly joined chan ignores `erc-reconnect-display'"
+      (not (eq (window-buffer) (get-buffer "#spam"))))
+
+    (ert-info ("Wait for auto reconnect")
+      (with-current-buffer erc-server-buffer
+        (funcall expect 10 "still in debug mode")))
+
+    (ert-info ("Wait for activity to recommence in channels")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+        (funcall expect 10 "forest of Arden"))
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam"))
+        (funcall expect 10 "her elves come here anon")))))
+
+(ert-deftest erc-scenarios-base-reconnect-options--buffer ()
+  :tags '(:expensive-test)
+  (should (eq erc-join-buffer 'bury))
+  (should-not erc-reconnect-display)
+
+  ;; FooNet (the server buffer) is not switched to because it's
+  ;; already current (but not shown) when `erc-open' is called.  See
+  ;; related conditional guard towards the end of that function.
+
+  (let ((erc-reconnect-display 'buffer))
+    (erc-scenarios-base-buffer-display--reconnect-common
+     (lambda ()
+       (pop-to-buffer-same-window "*Messages*")
+
+       (erc-d-t-ensure-for 1 "Server buffer not shown"
+         (not (eq (window-buffer) (get-buffer "FooNet"))))
+
+       (erc-d-t-wait-for 5 "Channel #chan shown when autojoined"
+         (eq (window-buffer) (get-buffer "#chan")))))))
+
+(ert-deftest erc-scenarios-base-reconnect-options--default ()
+  :tags '(:expensive-test)
+  (should (eq erc-join-buffer 'bury))
+  (should-not erc-reconnect-display)
+
+  (erc-scenarios-base-buffer-display--reconnect-common
+
+   (lambda ()
+     (pop-to-buffer-same-window "*Messages*")
+
+     (erc-d-t-ensure-for 1 "Server buffer not shown"
+       (not (eq (window-buffer) (get-buffer "FooNet"))))
+
+     (erc-d-t-ensure-for 3 "Channel #chan not shown"
+       (not (eq (window-buffer) (get-buffer "#chan"))))
+
+     (should (eq (window-buffer) (messages-buffer))))))
+
+
+;; This shows that the option `erc-interactive-display' overrides
+;; `erc-join-buffer' during cold opens and interactive /JOINs.
+
+(ert-deftest erc-scenarios-base-buffer-display--interactive-default ()
+  :tags '(:expensive-test)
+  (should (eq erc-join-buffer 'bury))
+  (should (eq erc-interactive-display 'window))
+
+  (erc-scenarios-common-with-cleanup
+      ((erc-scenarios-common-dialog "join/legacy")
+       (dumb-server (erc-d-run "localhost" t 'foonet))
+       (port (process-contact dumb-server :service))
+       (url (format "tester:[email protected]:%d\r\r" port))
+       (expect (erc-d-t-make-expecter))
+       (erc-server-flood-penalty 0.1)
+       (erc-server-auto-reconnect t)
+       (erc-user-full-name "tester"))
+
+    (ert-info ("Connect to foonet")
+      (with-current-buffer (let (inhibit-interaction)
+                             (ert-simulate-keys url
+                               (call-interactively #'erc)))
+        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))
+
+        (erc-d-t-wait-for 10 "Server buffer shown"
+          (eq (window-buffer) (current-buffer)))
+        (funcall expect 10 "debug mode")
+        (erc-scenarios-common-say "/JOIN #chan")))
+
+    (ert-info ("Wait for output in #chan")
+      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
+        (funcall expect 10 "welcome")
+        (erc-d-t-ensure-for 3 "Channel #chan shown"
+          (eq (window-buffer) (current-buffer)))
+        (funcall expect 10 "be prosperous")))))
+
+;;; erc-scenarios-base-buffer-display.el ends here
diff --git a/test/lisp/erc/erc-scenarios-base-reconnect.el b/test/lisp/erc/erc-scenarios-base-reconnect.el
index 5b4dc549042..7bd16d1ed14 100644
--- a/test/lisp/erc/erc-scenarios-base-reconnect.el
+++ b/test/lisp/erc/erc-scenarios-base-reconnect.el
@@ -65,95 +65,6 @@ erc-scenarios-base-reconnect-timer
       (should (equal (list (get-buffer (format "127.0.0.1:%d" port)))
                      (erc-scenarios-common-buflist "127.0.0.1"))))))
 
-(defun erc-scenarios-common--base-reconnect-options (test)
-  (erc-scenarios-common-with-cleanup
-      ((erc-scenarios-common-dialog "base/reconnect")
-       (dumb-server (erc-d-run "localhost" t 'options 'options-again))
-       (port (process-contact dumb-server :service))
-       (expect (erc-d-t-make-expecter))
-       (erc-server-flood-penalty 0.1)
-       (erc-server-auto-reconnect t)
-       erc-autojoin-channels-alist
-       erc-server-buffer)
-
-    (should (memq 'autojoin erc-modules))
-
-    (ert-info ("Connect to foonet")
-      (setq erc-server-buffer (erc :server "127.0.0.1"
-                                   :port port
-                                   :nick "tester"
-                                   :password "changeme"
-                                   :full-name "tester"))
-      (with-current-buffer erc-server-buffer
-        (should (string= (buffer-name) (format "127.0.0.1:%d" port)))
-        (funcall expect 10 "debug mode")))
-
-    (ert-info ("Wait for some output in channels")
-      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
-        (funcall expect 10 "welcome")))
-
-    (ert-info ("Server buffer shows connection failed")
-      (with-current-buffer erc-server-buffer
-        (funcall expect 10 "Connection failed!  Re-establishing")))
-
-    (should (equal erc-autojoin-channels-alist '((FooNet "#chan"))))
-
-    (funcall test)
-
-    ;; A manual /JOIN command tells ERC we're done auto-reconnecting
-    (with-current-buffer "FooNet" (erc-cmd-JOIN "#spam"))
-
-    (erc-d-t-ensure-for 1 "Newly joined chan ignores `erc-reconnect-display'"
-      (not (eq (window-buffer) (get-buffer "#spam"))))
-
-    (ert-info ("Wait for auto reconnect")
-      (with-current-buffer erc-server-buffer
-        (funcall expect 10 "still in debug mode")))
-
-    (ert-info ("Wait for activity to recommence in channels")
-      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#chan"))
-        (funcall expect 10 "forest of Arden"))
-      (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam"))
-        (funcall expect 10 "her elves come here anon")))))
-
-(ert-deftest erc-scenarios-base-reconnect-options--buffer ()
-  :tags '(:expensive-test)
-  (should (eq erc-join-buffer 'bury))
-  (should-not erc-reconnect-display)
-
-  ;; FooNet (the server buffer) is not switched to because it's
-  ;; already current (but not shown) when `erc-open' is called.  See
-  ;; related conditional guard towards the end of that function.
-
-  (let ((erc-reconnect-display 'buffer))
-    (erc-scenarios-common--base-reconnect-options
-     (lambda ()
-       (pop-to-buffer-same-window "*Messages*")
-
-       (erc-d-t-ensure-for 1 "Server buffer not shown"
-         (not (eq (window-buffer) (get-buffer "FooNet"))))
-
-       (erc-d-t-wait-for 5 "Channel #chan shown when autojoined"
-         (eq (window-buffer) (get-buffer "#chan")))))))
-
-(ert-deftest erc-scenarios-base-reconnect-options--default ()
-  :tags '(:expensive-test)
-  (should (eq erc-join-buffer 'bury))
-  (should-not erc-reconnect-display)
-
-  (erc-scenarios-common--base-reconnect-options
-
-   (lambda ()
-     (pop-to-buffer-same-window "*Messages*")
-
-     (erc-d-t-ensure-for 1 "Server buffer not shown"
-       (not (eq (window-buffer) (get-buffer "FooNet"))))
-
-     (erc-d-t-ensure-for 3 "Channel #chan not shown"
-       (not (eq (window-buffer) (get-buffer "#chan"))))
-
-     (eq (window-buffer) (messages-buffer)))))
-
 ;; Upon reconnecting, playback for channel and target buffers is
 ;; routed correctly.  Autojoin is irrelevant here, but for the
 ;; skeptical, see `erc-scenarios-common--join-network-id', which
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 29bda7e742d..b516b885895 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1469,7 +1469,7 @@ erc-select-read-args
                          :nick (user-login-name)
                          '&interactive-env
                          '((erc-server-connect-function . erc-open-tls-stream)
-                           (erc-join-buffer . buffer))))))
+                           (erc-join-buffer . window))))))
 
   (ert-info ("Switches to TLS when port matches default TLS port")
     (should (equal (ert-simulate-keys "irc.gnu.org\r6697\r\r\r"
@@ -1479,7 +1479,7 @@ erc-select-read-args
                          :nick (user-login-name)
                          '&interactive-env
                          '((erc-server-connect-function . erc-open-tls-stream)
-                           (erc-join-buffer . buffer))))))
+                           (erc-join-buffer . window))))))
 
   (ert-info ("Switches to TLS when URL is ircs://")
     (should (equal (ert-simulate-keys "ircs://irc.gnu.org\r\r\r\r"
@@ -1489,7 +1489,7 @@ erc-select-read-args
                          :nick (user-login-name)
                          '&interactive-env
                          '((erc-server-connect-function . erc-open-tls-stream)
-                           (erc-join-buffer . buffer))))))
+                           (erc-join-buffer . window))))))
 
   (setq-local erc-interactive-display nil) ; cheat to save space
 
@@ -1625,7 +1625,7 @@ erc-tls
                        '("localhost" 6667 "nick" "unknown" t "sesame"
                          nil nil nil nil "user" nil)))
         (should (equal (pop env)
-                       '((erc-join-buffer buffer)
+                       '((erc-join-buffer window)
                          (erc-server-connect-function erc-open-tls-stream)))))
 
       (ert-info ("Custom connect function")
@@ -1686,7 +1686,7 @@ erc--interactive
                        '("irc.libera.chat" 6697 "tester" "unknown" t nil
                          nil nil nil nil "user" nil)))
         (should (equal (pop env)
-                       '((erc-join-buffer buffer) (erc-server-connect-function
+                       '((erc-join-buffer window) (erc-server-connect-function
                                                    erc-open-tls-stream)))))
 
       (ert-info ("Nick supplied, decline TLS upgrade")
@@ -1696,7 +1696,7 @@ erc--interactive
                        '("irc.libera.chat" 6667 "dummy" "unknown" t nil
                          nil nil nil nil "user" nil)))
         (should (equal (pop env)
-                       '((erc-join-buffer buffer)
+                       '((erc-join-buffer window)
                          (erc-server-connect-function
                           erc-open-network-stream))))))))
 
-- 
2.39.2