>From 1b064c410393843e7d07f2b3ac70d42c729872fb Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sun, 7 May 2023 19:43:57 -0700 Subject: [PATCH 1/3] [5.6] Add helper for restoring local session vars in ERC * lisp/erc/erc-goodies.el (erc--keep-place-indicator-setup): Use macro `erc--restore-initialize-priors'. (erc-keep-place-indicator-mode, erc-keep-place-indicator-enable, erc-keep-place-indicator-disable): Use convenience function to show missing-dependency notice. * lisp/erc/erc-sasl.el (erc-sasl-auth-source-password-as-host): Merge redundant `when' forms. (erc-sasl--init): Remove unused function. (erc-sasl-mode, erc-sasl-enable, erc-sasl-disable): Use helper `erc--restore-initialize-priors' to restore `erc-sasl--options' * lisp/erc/erc.el (erc--restore-initialize-priors): New macro. (erc--warn-once-before-connect): Add helper to display messages, ideally just after module setup. * test/lisp/erc/erc-tests.el (erc--restore-initialize-priors): New test. Also see test/lisp/erc/erc-scenarios-base-local-modules.el for more realistic exercising of this functionality. * test/lisp/erc/erc-goodies-tests.el (erc-controls-highlight--examples, erc-controls-highlight--inverse, erc-controls-highlight--motd, erc-keep-place-indicator-mode): Remove feature check. For the latter, also start fake process and shadow `erc-connect-pre-hook'. (Bug#60936) --- lisp/erc/erc-goodies.el | 24 ++++++---------- lisp/erc/erc-sasl.el | 38 +++++++++++-------------- lisp/erc/erc.el | 45 ++++++++++++++++++++++++++++++ test/lisp/erc/erc-goodies-tests.el | 19 ++++--------- test/lisp/erc/erc-tests.el | 15 ++++++++++ 5 files changed, 90 insertions(+), 51 deletions(-) diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el index cc60ba0018b..4558ff7c076 100644 --- a/lisp/erc/erc-goodies.el +++ b/lisp/erc/erc-goodies.el @@ -154,21 +154,21 @@ erc-keep-place-indicator-style `face', ERC adds the face `erc-keep-place-indicator-line' to the appropriate line. A value of t does both." :group 'erc - :package-version '(ERC . "5.6") + :package-version '(ERC . "5.6") ; FIXME sync on release :type '(choice (const t) (const server) (const target))) (defcustom erc-keep-place-indicator-buffer-type t "ERC buffer type in which to display `keep-place-indicator'. A value of t means \"all\" ERC buffers." :group 'erc - :package-version '(ERC . "5.6") + :package-version '(ERC . "5.6") ; FIXME sync on release :type '(choice (const t) (const server) (const target))) (defcustom erc-keep-place-indicator-follow nil "Whether to sync visual kept place to window's top when reading. For use with `erc-keep-place-indicator-mode'." :group 'erc - :package-version '(ERC . "5.6") + :package-version '(ERC . "5.6") ; FIXME sync on release :type 'boolean) (defface erc-keep-place-indicator-line @@ -209,11 +209,8 @@ erc--keep-place-indicator-on-window-configuration-change (defun erc--keep-place-indicator-setup () "Initialize buffer for maintaining `erc--keep-place-indicator-overlay'." (require 'fringe) - (setq erc--keep-place-indicator-overlay - (if-let* ((vars (or erc--server-reconnecting erc--target-priors)) - ((alist-get 'erc-keep-place-indicator-mode vars))) - (alist-get 'erc--keep-place-indicator-overlay vars) - (make-overlay 0 0))) + (erc--restore-initialize-priors erc-keep-place-indicator-mode + erc--keep-place-indicator-overlay (make-overlay 0 0)) (add-hook 'window-configuration-change-hook #'erc--keep-place-indicator-on-window-configuration-change nil t) (when-let* (((memq erc-keep-place-indicator-style '(t arrow))) @@ -232,13 +229,10 @@ keep-place-indicator "`keep-place' with a fringe arrow and/or highlighted face." ((unless erc-keep-place-mode (unless (memq 'keep-place erc-modules) - ;; FIXME use `erc-button--display-error-notice-with-keys' - ;; to display this message when bug#60933 is ready. - (erc-display-error-notice - nil (concat - "Local module `keep-place-indicator' needs module `keep-place'." - " Enabling now. This will affect \C-]all\C-] ERC sessions." - " Add `keep-place' to `erc-modules' to silence this message."))) + (erc--warn-once-before-connect 'erc-keep-place-mode + "Local module `keep-place-indicator' needs module `keep-place'." + " Enabling now. This will affect \C-]all\C-] ERC sessions." + " Add `keep-place' to `erc-modules' to silence this message.")) (erc-keep-place-mode +1)) (if (pcase erc-keep-place-indicator-buffer-type ('target erc--target) diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el index bfe17285a68..c6922b1b26b 100644 --- a/lisp/erc/erc-sasl.el +++ b/lisp/erc/erc-sasl.el @@ -137,12 +137,12 @@ erc-sasl-auth-source-password-as-host `erc-session-password' instead. Otherwise, just defer to `erc-auth-source-search' to pick a suitable `:host'. Expect PLIST to contain keyword params known to `auth-source-search'." - (when erc-sasl-password - (when-let ((host (if (eq :password erc-sasl-password) - (and (not (functionp erc-session-password)) - erc-session-password) - erc-sasl-password))) - (setq plist `(,@plist :host ,(format "%s" host))))) + (when-let* ((erc-sasl-password) + (host (if (eq :password erc-sasl-password) + (and (not (functionp erc-session-password)) + erc-session-password) + erc-sasl-password))) + (setq plist `(,@plist :host ,(format "%s" host)))) (apply #'erc-auth-source-search plist)) (defun erc-sasl--read-password (prompt) @@ -297,21 +297,6 @@ erc-sasl--create-client (sasl-client-set-property client 'ecdsa-keyfile keyfile) client))))) -;; This stands alone because it's also used by bug#49860. -(defun erc-sasl--init () - (setq erc-sasl--state (make-erc-sasl--state)) - ;; If the previous attempt failed during registration, this may be - ;; non-nil and contain erroneous values, but how can we detect that? - ;; What if the server dropped the connection for some other reason? - (setq erc-sasl--options - (or (and erc--server-reconnecting - (alist-get 'erc-sasl--options erc--server-reconnecting)) - `((user . ,erc-sasl-user) - (password . ,erc-sasl-password) - (mechanism . ,erc-sasl-mechanism) - (authfn . ,erc-sasl-auth-source-function) - (authzid . ,erc-sasl-authzid))))) - (defun erc-sasl--mechanism-offered-p (offered) "Return non-nil when OFFERED appears among a list of mechanisms." (string-match-p (rx-to-string @@ -334,7 +319,16 @@ sasl This doesn't solicit or validate a suite of supported mechanisms." ;; See bug#49860 for a CAP 3.2-aware WIP implementation. ((unless erc--target - (erc-sasl--init) + (setq erc-sasl--state (make-erc-sasl--state)) + ;; If the previous attempt failed during registration, this may be + ;; non-nil and contain erroneous values, but how can we detect that? + ;; What if the server dropped the connection for some other reason? + (erc--restore-initialize-priors erc-sasl-mode + erc-sasl--options `((user . ,erc-sasl-user) + (password . ,erc-sasl-password) + (mechanism . ,erc-sasl-mechanism) + (authfn . ,erc-sasl-auth-source-function) + (authzid . ,erc-sasl-authzid))) (let* ((mech (alist-get 'mechanism erc-sasl--options)) (client (erc-sasl--create-client mech))) (unless client diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index dbf413bac74..249b9963cea 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -1363,6 +1363,20 @@ erc--target-priors Bound to local variables from an existing (logical) session's buffer during local-module setup and `erc-mode-hook' activation.") +(defmacro erc--restore-initialize-priors (mode &rest vars) + "Restore local VARS for MODE from a previous session." + (declare (indent 1)) + (let ((existing (make-symbol "existing")) + ;; + restore initialize) + (while-let ((k (pop vars)) (v (pop vars))) + (push `(,k (alist-get ',k ,existing)) restore) + (push `(,k ,v) initialize)) + `(if-let* ((,existing (or erc--server-reconnecting erc--target-priors)) + ((alist-get ',mode ,existing))) + (setq ,@(mapcan #'identity (nreverse restore))) + (setq ,@(mapcan #'identity (nreverse initialize)))))) + (defun erc--target-from-string (string) "Construct an `erc--target' variant from STRING." (funcall (if (erc-channel-p string) @@ -1412,6 +1426,37 @@ erc-once-with-server-event (add-hook hook fun nil t) fun)) +(defun erc--warn-once-before-connect (mode-var &rest args) + "Display an \"error notice\" once. +Expect ARGS to be `erc-button--display-error-notice-with-keys' +compatible parameters, except without any leading buffers or +processes. If we're in an ERC buffer with a network process when +called, print the notice immediately. Otherwise, if we're in a +server buffer, arrange to do so after local modules have been set +up and mode hooks have run. Otherwise, if MODE-VAR is a global +module, try again at most once the next time `erc-mode-hook' +runs." + (declare (indent 1)) + (cl-assert (stringp (car args))) + (if (derived-mode-p 'erc-mode) + (unless (or (erc-with-server-buffer ; needs `erc-server-process' + (apply #'erc-button--display-error-notice-with-keys + (current-buffer) args) + t) + erc--target) ; unlikely + (let (hook) + (setq hook + (lambda (_) + (remove-hook 'erc-connect-pre-hook hook t) + (apply #'erc-button--display-error-notice-with-keys args))) + (add-hook 'erc-connect-pre-hook hook nil t))) + (when (custom-variable-p mode-var) + (let (hook) + (setq hook (lambda () + (remove-hook 'erc-mode-hook hook) + (apply #'erc--warn-once-before-connect 'erc-fake args))) + (add-hook 'erc-mode-hook hook))))) + (defun erc-server-buffer () "Return the server buffer for the current buffer's process. The buffer-local variable `erc-server-process' is used to find diff --git a/test/lisp/erc/erc-goodies-tests.el b/test/lisp/erc/erc-goodies-tests.el index a1f53c5bf88..bd40df4b680 100644 --- a/test/lisp/erc/erc-goodies-tests.el +++ b/test/lisp/erc/erc-goodies-tests.el @@ -21,7 +21,6 @@ ;;; Code: (require 'ert-x) (require 'erc-goodies) -(declare-function erc--initialize-markers "erc" (old-point continued) t) (defun erc-goodies-tests--assert-face (beg end-str present &optional absent) (setq beg (+ beg (point-min))) @@ -44,9 +43,6 @@ erc-goodies-tests--assert-face ;; https://modern.ircdocs.horse/formatting.html (ert-deftest erc-controls-highlight--examples () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (should (eq t erc-interpret-controls-p)) (let ((erc-insert-modify-hook '(erc-controls-highlight)) erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) @@ -130,9 +126,6 @@ erc-controls-highlight--examples ;; in a high-contrast face. (ert-deftest erc-controls-highlight--inverse () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (should (eq t erc-interpret-controls-p)) (let ((erc-insert-modify-hook '(erc-controls-highlight)) erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) @@ -212,9 +205,6 @@ erc-goodies-tests--motd (":- "))) (ert-deftest erc-controls-highlight--motd () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (should (eq t erc-interpret-controls-p)) (let ((erc-insert-modify-hook '(erc-controls-highlight)) erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) @@ -256,12 +246,12 @@ erc-controls-highlight--motd ;; needed. (ert-deftest erc-keep-place-indicator-mode () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (with-current-buffer (get-buffer-create "*erc-keep-place-indicator-mode*") (erc-mode) (erc--initialize-markers (point) nil) + (setq erc-server-process + (start-process "sleep" (current-buffer) "sleep" "1")) + (set-process-query-on-exit-flag erc-server-process nil) (let ((assert-off (lambda () (should-not erc-keep-place-indicator-mode) @@ -275,6 +265,7 @@ erc-keep-place-indicator-mode (should erc-keep-place-mode))) ;; erc-insert-pre-hook + erc-connect-pre-hook erc-modules) (funcall assert-off) @@ -284,7 +275,7 @@ erc-keep-place-indicator-mode (erc-keep-place-indicator-mode +1) (funcall assert-on) (goto-char (point-min)) - (should (search-forward "Enabling" nil t)) + (should (search-forward "Enabling now" nil t)) (should (memq 'keep-place erc-modules))) (erc-keep-place-indicator-mode -1) diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index be5a566a268..b624186d88d 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -868,6 +868,21 @@ erc--valid-local-channel-p (should-not (erc--valid-local-channel-p "#chan")) (should (erc--valid-local-channel-p "&local"))))) +(ert-deftest erc--restore-initialize-priors () + ;; This `pcase' expands to 100+k. Guess we could do something like + ;; (and `(,_ ((,e . ,_) . ,_) . ,_) v) first and then return a + ;; (equal `(if-let* ((,e ...)...)...) v) to cut it down to < 1k. + (should (pcase (macroexpand-1 '(erc--restore-initialize-priors erc-my-mode + foo (ignore 1 2 3) + bar #'spam)) + (`(if-let* ((,e (or erc--server-reconnecting erc--target-priors)) + ((alist-get 'erc-my-mode ,e))) + (setq foo (alist-get 'foo ,e) + bar (alist-get 'bar ,e)) + (setq foo (ignore 1 2 3) + bar #'spam)) + t)))) + (ert-deftest erc--target-from-string () (should (equal (erc--target-from-string "#chan") #s(erc--target-channel "#chan" \#chan))) -- 2.40.0