Add helper for restoring local session vars in ERC

* lisp/erc/erc-common.el (erc--input-split): Suppress warning for
obsolete variable `erc-send-this' in init form.
* lisp/erc/erc-goodies.el (erc--keep-place-indicator-setup): Use macro
`erc--restore-initialize-priors' to preserve last session's indicator
position, if any.
(erc-keep-place-indicator-mode, erc-keep-place-indicator-enable): Use
convenience function to show missing-dependency notice.
* lisp/erc/erc-sasl.el (erc-sasl-auth-source-password-as-host): Merge
redundant `when' forms for clarity.
(erc-sasl--init): Remove unused function.
(erc-sasl-mode, erc-sasl-enable): Use helper to restore
`erc-sasl--options', essentially inlining the body of the now defunct
`erc-sasl--init'.
* lisp/erc/erc.el (erc--restore-initialize-priors): New macro to help
local modules and mode hooks prefer existing state over initializing
anew.
(erc--warn-once-before-connect): Add helper function to display
an "error notice" just after module setup.
(erc-accidental-paste-threshold-seconds) Improve doc string.
* 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'.
* test/lisp/erc/erc-tests.el (erc--restore-initialize-priors): New
test.  Also see test/lisp/erc/erc-scenarios-base-local-modules.el for
a more realistic exercising of this functionality.  (Bug#60936)

6 files changed, 107 insertions, 51 deletions

diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 86d78768374..f152a1a32d9 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -56,7 +56,9 @@
 (cl-defstruct (erc--input-split (:include erc-input
                                           (string :read-only)
                                           (insertp erc-insert-this)
-                                          (sendp erc-send-this)))
+                                          (sendp (with-suppressed-warnings
+                                                     ((obsolete erc-send-this))
+                                                   erc-send-this))))
   (lines nil :type (list-of string))
   (cmdp nil :type boolean))
 
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 @@ displays an arrow in the left fringe or margin.  When it's
 `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 @@ the active frame."
 (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 @@ the active frame."
   "`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 @@ that symbol is `:password', in which case, use a non-nil
 `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 @@ If necessary, pass PROMPT to `read-passwd'."
              (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 @@ If necessary, pass PROMPT to `read-passwd'."
 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..5738ee92578 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1363,6 +1363,20 @@ See also `erc-show-my-nick'."
 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 @@ capabilities."
     (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
@@ -6094,6 +6139,21 @@ if its previous invocation was fewer than this many seconds ago.
 This is useful so that if you accidentally enter large amounts of text
 into the ERC buffer, that text is not sent to the IRC server.
 
+This option only concerns the rapid submission of successive
+lines of prompt input from an \"external\" source, such as GNU
+screen or a desktop-automation script.  For example, typing
+
+  \\[kmacro-start-macro-or-insert-counter] \
+one \\`RET' two \\`RET' three \\`RET'
+  \\[kmacro-end-or-call-macro] in the \"*scratch*\" buffer, \
+followed by a
+  \\[kmacro-end-or-call-macro] again in a channel buffer,
+
+will send \"one\" to the server, leave \"two\" at the prompt, and
+insert \"three\" in an \"overflow\" buffer.  For suppression
+involving input yanked from the clipboard or the kill ring, see
+`erc-inhibit-multiline-input' and `erc-warn-about-blank-lines'.
+
 If the value is nil, `erc-send-current-line' always considers any
 submitted line to be intentional."
   :group 'erc
diff --git a/test/lisp/erc/erc-goodies-tests.el b/test/lisp/erc/erc-goodies-tests.el
index a1f53c5bf88..7acacb319f1 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 @@
 ;; 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 @@
 ;; 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 @@
     (":- ")))
 
 (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 @@
 ;; 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 @@
              (should erc-keep-place-mode)))
           ;;
           erc-insert-pre-hook
+          erc-connect-pre-hook
           erc-modules)
 
       (funcall assert-off)
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 @@
       (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)))