master 5de90fa9611 03/14: Extend erc-interactive-display to cover /JOINs

[F. Jason Park]

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

    Extend erc-interactive-display to cover /JOINs
    
    * lisp/erc/erc.el (erc-display): Mention that buffer-related display
    options live in the customization group `erc-buffers'.
    (erc-buffer-display, erc-join-buffer): Swap alias and aliased so that
    the favored name, `erc-buffer-display', appears in the definition and
    in the Customize menu.  Also note related buffer-display options in
    the doc string.
    (erc-query-display, erc-interactive-display): Make the former an alias
    of the latter, new in ERC 5.6, because their roles were functionally
    redundant and thus confusing.  Inherit the default value from
    `erc-query-display' because users are more familiar with the pop-up
    window behavior than a single-window replacement.
    (erc-reconnect-display): Use preferred name for cross-referencing
    fallback option `erc-buffer-display' in doc string, and explain how
    /reconnect handling differs.
    (erc--setup-buffer-hook): Add new internal hook for modules that
    operate on windows and frames, such as erc-speedbar and
    erc-status-sidebar.
    (erc-open): Run `erc--setup-buffer-hook' after `erc-setup-buffer' so
    hook members know their code isn't tied to `erc-setup-buffer' itself,
    which may be used in other contexts, but rather to a new ERC buffer on
    which some display-related action has just been performed.
    (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): When called interactively, schedule a callback to wrap
    the response handler and control how new buffers are thus displayed.
    (erc-cmd-QUERY): Use preferred alias for `erc-query-display'.
    * test/lisp/erc/erc-scenarios-base-buffer-display.el:
    (erc-scenarios-base-buffer-display--interactive-default): New test.
    * test/lisp/erc/erc-tests.el (erc-process-input-line,
    erc-select-read-args, erc-tls, erc--interactive): Change expected
    default value of `erc-interactive-display' from `buffer' to
    `window'.  (Bug#62833)
---
 etc/ERC-NEWS                                       | 17 +++--
 lisp/erc/erc.el                                    | 88 +++++++++++++---------
 test/lisp/erc/erc-scenarios-base-buffer-display.el | 37 +++++++++
 test/lisp/erc/erc-tests.el                         | 13 ++--
 4 files changed, 105 insertions(+), 50 deletions(-)

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index 6897993c628..57dce501760 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -37,15 +37,18 @@ decade overdue, this is no longer the case.  Other UX 
improvements in
 this area aim to make the process of connecting interactively slightly
 more streamlined and less repetitive, even for veteran users.
 
-** New buffer-display option 'erc-interactive-display'.
+** Revised buffer-display handling for interactive commands.
 A point of friction for new users and one only just introduced with
 ERC 5.5 has been the lack of visual feedback when first connecting via
-M-x erc.  As explained below in the news for 5.5, the discovery of a
-security issue led to new ERC buffers being "buried" on creation.  On
-further reflection, this was judged to have been an overcorrection in
-the case of interactive invocations, hence the new option
-'erc-interactive-display', which is set to 'buffer' (i.e., "take me
-there") by default.
+M-x erc or when issuing a "/JOIN" command at the prompt.  As explained
+below, in the news for 5.5, the discovery of a security issue led to
+most new ERC buffers being "buried" on creation.  On further
+reflection, this was judged to have been an overcorrection in the case
+of interactive invocations, hence the borrowing of an old option,
+'erc-query-display', and the bestowing of a new alias,
+'erc-interactive-display', which better describes its expanded role as
+a more general buffer-display knob for interactive commands ("/QUERY"
+still among them).
 
 Accompanying this addition are "display"-suffixed aliases for related
 options 'erc-join-buffer' and 'erc-auto-query', which users have
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 22b92a0d31b..13f6da2d5be 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -98,7 +98,9 @@
   :group 'erc)
 
 (defgroup erc-display nil
-  "Settings for how various things are displayed."
+  "Settings controlling how various things are displayed.
+See the customization group `erc-buffers' for display options
+concerning buffers."
   :group 'erc)
 
 (defgroup erc-mode-line-and-header nil
@@ -1507,9 +1509,9 @@ Defaults to the server buffer."
   "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
-  "Determines how to display a newly created IRC buffer.
+(defvaralias 'erc-join-buffer 'erc-buffer-display)
+(defcustom erc-buffer-display 'bury
+  "How to display a newly created ERC buffer.
 
 The available choices are:
 
@@ -1518,7 +1520,9 @@ The available choices are:
   `frame'           - in another frame,
   `bury'            - bury it in a new buffer,
   `buffer'          - in place of the current buffer,
-  any other value  - in place of the current buffer."
+
+See related options `erc-interactive-display',
+`erc-reconnect-display', and `erc-receive-query-display'."
   :package-version '(ERC . "5.5")
   :group 'erc-buffers
   :type '(choice (const :tag "Split window and select" window)
@@ -1528,13 +1532,17 @@ The available choices are:
                  (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 as a result of user interaction.
+This affects commands like /QUERY and /JOIN when issued
+interactively at the prompt.  It does not apply when calling a
+handler for such a command, like `erc-cmd-JOIN', from lisp code.
+See `erc-buffer-display' for a full description of available
+values."
   :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)
@@ -1542,15 +1550,14 @@ possible values."
                  (const :tag "Use current buffer" buffer)))
 
 (defcustom erc-reconnect-display nil
-  "How (and whether) to display a channel buffer upon reconnecting.
-
-This only affects automatic reconnections and is ignored when
-issuing a /reconnect command or reinvoking `erc-tls' with the
-same args (assuming success, of course).  See `erc-join-buffer'
-for a description of possible values."
+  "How and whether to display a channel buffer when auto-reconnecting.
+This only affects automatic reconnections and is ignored, like
+all other buffer-display options, when issuing a /RECONNECT or
+successfully reinvoking `erc-tls' with similar arguments.  See
+`erc-buffer-display' for a description of possible values."
   :package-version '(ERC . "5.5")
   :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)
@@ -2044,6 +2051,9 @@ to display-buffer machinery."
         (display-buffer-use-some-frame buffer
                                        `((frame-predicate . ,ercp) ,@alist)))))
 
+(defvar erc--setup-buffer-hook nil
+  "Internal hook for module setup involving windows and frames.")
+
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
   (pcase (if (zerop (erc-with-server-buffer
@@ -2251,7 +2261,8 @@ Returns the buffer for the given server or channel."
         ;; we can't log to debug buffer, it may not exist yet
         (message "erc: old buffer %s, switching to %s"
                  old-buffer buffer))
-      (erc-setup-buffer buffer))
+      (erc-setup-buffer buffer)
+      (run-hooks 'erc--setup-buffer-hook))
 
     buffer))
 
@@ -3057,6 +3068,10 @@ present."
   (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 +3125,8 @@ this function from interpreting the line as a command."
     (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 +3600,21 @@ were most recently invited.  See also `invitation'."
                    (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,27 +3978,10 @@ just as you provided it.  Use this command with care!"
    (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
-on the value of `erc-query-display'."
+on the value of `erc-interactive-display'."
   ;; FIXME: The doc string used to say at the end:
   ;; "If USER is omitted, close the current query buffer if one exists
   ;; - except this is broken now ;-)"
diff --git a/test/lisp/erc/erc-scenarios-base-buffer-display.el 
b/test/lisp/erc/erc-scenarios-base-buffer-display.el
index d511c8ff738..3ed7a83653e 100644
--- a/test/lisp/erc/erc-scenarios-base-buffer-display.el
+++ b/test/lisp/erc/erc-scenarios-base-buffer-display.el
@@ -118,4 +118,41 @@
 
      (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:changeme@127.0.0.1:%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-tests.el b/test/lisp/erc/erc-tests.el
index 29bda7e742d..88b9babf206 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1292,6 +1292,7 @@
       (cl-letf (((symbol-function 'erc-cmd-MSG)
                  (lambda (line)
                    (push line calls)
+                   (should erc--called-as-input-p)
                    (funcall orig-erc-cmd-MSG line)))
                 ((symbol-function 'erc-server-buffer)
                  (lambda () (current-buffer)))
@@ -1469,7 +1470,7 @@
                          :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 +1480,7 @@
                          :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 +1490,7 @@
                          :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 +1626,7 @@
                        '("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 +1687,7 @@
                        '("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 +1697,7 @@
                        '("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))))))))