From 6b7f66a1eed5d01c312d125f016e48cf4e8ce0c9 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Mon, 10 Apr 2023 17:58:05 -0700 Subject: [PATCH 2/2] [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 the favored name, `erc-buffer-display', appears in the definition and in the Customize menu. (erc-query-display, erc-interactive-display): Make the former an alias for the latter 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--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. * 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-process-input-line, erc-select-read-args, erc-tls, erc--interactive): Change expected default value of `erc-interactive-display' from `buffer' to `window'. (Bug#60428.) --- etc/ERC-NEWS | 17 +- 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 | 13 +- 5 files changed, 211 insertions(+), 127 deletions(-) create mode 100644 test/lisp/erc/erc-scenarios-base-buffer-display.el diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS index 31f40e9d0d3..e9de48b2e34 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. Accompanying this addition are "display"-suffixed +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 reported as being difficult to discover and remember. diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index b38ef38e9f7..2088a755da5 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 from user interaction. +Examples include the \"slash\" commands /QUERY and /JOIN. See +`erc-buffer-display' for a full description of available choices. + +Note that this doesn't affect commands like `erc-cmd-JOIN' when +called from lisp code. Formerly known as `erc-query-display', +this option now applies to more than just /QUERY. See option +`erc-receive-display' in the same Custom group to decide how +buffers created as a result of server-initiated activity 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 . + +;;; 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: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-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..88b9babf206 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -1292,6 +1292,7 @@ erc-process-input-line (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 @@ 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 +1480,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 +1490,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 +1626,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 +1687,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 +1697,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