master b7cdce09700 2/3: Fix local variable persistence in erc-stamp

[F. Jason Park]

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

    Fix local variable persistence in erc-stamp
    
    * etc/ERC-NEWS: Mention renaming of `erc-munge-invisible-property'.
    * lisp/erc/erc-stamp.el (erc-stamp-mode, erc-stamp-disable): Remove
    correct function from `erc-mode-hook'.
    (erc-stamp--recover-on-reconnect): Revise doc string.
    (erc-munge-invisibility-spec, erc-stamp--manage-local-options-state):
    Mark former name as obsolete and rename to latter.  Don't use helper
    macro meant only for local modules.  This bug originated from c68dc778
    "Manage some text props for ERC insertion-hook members", which stemmed
    from bug#60936.
    (erc-stamp--setup, erc-hide-timestamps, erc-show-timestamps)
    (erc-toggle-timestamps): Use new name for
    `erc-munge-invisibility-spec'.
    * lisp/erc/erc.el (erc--restore-initialize-priors): Raise error at
    runtime if mode var doesn't belong to a local module.
    * test/lisp/erc/erc-stamp-tests.el (erc-stamp-tests--insert-right)
    (erc-timestamp-intangible--left): Use new name for
    `erc-munge-invisibility-spec'.
    * test/lisp/erc/erc-tests.el (erc--refresh-prompt): Shadow
    `erc-last-input-time'.
    (erc--restore-initialize-priors): Add error form to expected
    expansion, and skip test on Emacs 27.
    * test/lisp/erc/resources/erc-scenarios-common.el
    (erc-scenarios-common--make-bindings): Shadow `erc-last-input-time'.
---
 etc/ERC-NEWS                                    |  9 ++++--
 lisp/erc/erc-stamp.el                           | 39 ++++++++++++++++---------
 lisp/erc/erc.el                                 |  4 ++-
 test/lisp/erc/erc-stamp-tests.el                |  4 +--
 test/lisp/erc/erc-tests.el                      |  5 ++++
 test/lisp/erc/resources/erc-scenarios-common.el |  1 +
 6 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index f91d3fcb351..1e88500d169 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -435,9 +435,12 @@ contains unique closures and thus no longer proves 
effective for
 traversing inserted messages.  For now, ERC only provides an internal
 means of visiting messages, but a public interface is forthcoming.
 Also affecting the 'stamp' module is the deprecation of the function
-'erc-insert-aligned' and its removal from client code.  Additionally,
-the module now merges its 'invisible' property with existing ones and
-includes all white space around stamps when doing so.
+'erc-insert-aligned' and its removal from the default client's code.
+In the same library, the function 'erc-munge-invisibility-spec' has
+been renamed to 'erc-stamp--manage-local-options-state' to better
+reflect its purpose.  Additionally, the module now merges its
+'invisible' property with existing ones and includes all white space
+around stamps when doing so.
 
 This "propertizing" of surrounding white space extends to all
 'stamp'-applied properties, like 'field', in all intervening space
diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index 558afd19427..a11739a4195 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -184,7 +184,7 @@ from entering them and instead jump over them."
    (add-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect)
    (add-hook 'erc--pre-clear-functions #'erc-stamp--reset-on-clear 40)
    (unless erc--updating-modules-p (erc-buffer-do #'erc-stamp--setup)))
-  ((remove-hook 'erc-mode-hook #'erc-munge-invisibility-spec)
+  ((remove-hook 'erc-mode-hook #'erc-stamp--setup)
    (remove-hook 'erc-insert-modify-hook #'erc-add-timestamp)
    (remove-hook 'erc-send-modify-hook #'erc-add-timestamp)
    (remove-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect)
@@ -198,6 +198,7 @@ from entering them and instead jump over them."
   "Escape hatch for omitting stamps when first char is invisible.")
 
 (defun erc-stamp--recover-on-reconnect ()
+  "Attempt to restore \"last-inserted\" snapshots from prior session."
   (when-let ((priors (or erc--server-reconnecting erc--target-priors)))
     (dolist (var '(erc-timestamp-last-inserted
                    erc-timestamp-last-inserted-left
@@ -854,12 +855,20 @@ Return the empty string if FORMAT is nil."
 
 (defvar-local erc-stamp--csf-props-updated-p nil)
 
-;; This function is used to munge `buffer-invisibility-spec' to an
-;; appropriate value. Currently, it only handles timestamps, thus its
-;; location.  If you add other features which affect invisibility,
-;; please modify this function and move it to a more appropriate
-;; location.
-(defun erc-munge-invisibility-spec ()
+(define-obsolete-function-alias 'erc-munge-invisibility-spec
+  #'erc-stamp--manage-local-options-state "30.1"
+  "Perform setup and teardown of `stamp'-owned options.
+
+Note that this function's role in practice has long defied its
+stated mandate as claimed in a now deleted comment, which
+envisioned it as evolving into a central toggle for modifying
+`buffer-invisibility-spec' on behalf of options and features
+ERC-wide.")
+(defun erc-stamp--manage-local-options-state ()
+  "Perform local setup and teardown for `stamp'-owned options.
+For `erc-timestamp-intangible', toggle `cursor-intangible-mode'.
+For `erc-echo-timestamps', integrate with `cursor-sensor-mode'.
+For `erc-hide-timestamps, modify `buffer-invisibility-spec'."
   (if erc-timestamp-intangible
       (cursor-intangible-mode +1) ; idempotent
     (when (bound-and-true-p cursor-intangible-mode)
@@ -869,10 +878,12 @@ Return the empty string if FORMAT is nil."
         (unless erc-stamp--permanent-cursor-sensor-functions
           (dolist (hook '(erc-insert-post-hook erc-send-post-hook))
             (add-hook hook #'erc-stamp--add-csf-on-post-modify nil t))
-          (erc--restore-initialize-priors erc-stamp-mode
-            erc-stamp--csf-props-updated-p nil)
+          (setq erc-stamp--csf-props-updated-p
+                (alist-get 'erc-stamp--csf-props-updated-p
+                           (or erc--server-reconnecting erc--target-priors)))
           (unless erc-stamp--csf-props-updated-p
             (setq erc-stamp--csf-props-updated-p t)
+            ;; Spoof `erc--ts' as being non-nil.
             (let ((erc--msg-props (map-into '((erc--ts . t)) 'hash-table)))
               (with-silent-modifications
                 (erc--traverse-inserted
@@ -902,9 +913,9 @@ Return the empty string if FORMAT is nil."
 (defun erc-stamp--setup ()
   "Enable or disable buffer-local `erc-stamp-mode' modifications."
   (if erc-stamp-mode
-      (erc-munge-invisibility-spec)
+      (erc-stamp--manage-local-options-state)
     (let (erc-echo-timestamps erc-hide-timestamps erc-timestamp-intangible)
-      (erc-munge-invisibility-spec))
+      (erc-stamp--manage-local-options-state))
     ;; Undo local mods from `erc-insert-timestamp-left-and-right'.
     (erc-stamp--date-mode -1) ; kills `erc-timestamp-last-inserted-left'
     (kill-local-variable 'erc-stamp--last-stamp)
@@ -916,7 +927,7 @@ Return the empty string if FORMAT is nil."
   "Hide timestamp information from display."
   (interactive)
   (setq erc-hide-timestamps t)
-  (erc-munge-invisibility-spec))
+  (erc-stamp--manage-local-options-state))
 
 (defun erc-show-timestamps ()
   "Show timestamp information on display.
@@ -924,7 +935,7 @@ This function only works if `erc-timestamp-format' was 
previously
 set, and timestamping is already active."
   (interactive)
   (setq erc-hide-timestamps nil)
-  (erc-munge-invisibility-spec))
+  (erc-stamp--manage-local-options-state))
 
 (defun erc-toggle-timestamps ()
   "Hide or show timestamps in ERC buffers.
@@ -938,7 +949,7 @@ enabled when the message was inserted."
     (setq erc-hide-timestamps t))
   (mapc (lambda (buffer)
          (with-current-buffer buffer
-           (erc-munge-invisibility-spec)))
+            (erc-stamp--manage-local-options-state)))
        (erc-buffer-list)))
 
 (defvar-local erc-stamp--last-stamp nil)
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 67c31d961e3..ef047201251 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1531,7 +1531,7 @@ 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."
+  "Restore local VARS for local minor MODE from a previous session."
   (declare (indent 1))
   (let ((priors (make-symbol "priors"))
         (initp (make-symbol "initp"))
@@ -1541,6 +1541,8 @@ buffer during local-module setup and `erc-mode-hook' 
activation.")
       (push `(,k (if ,initp (alist-get ',k ,priors) ,(pop vars))) forms))
     `(let* ((,priors (or erc--server-reconnecting erc--target-priors))
             (,initp (and ,priors (alist-get ',mode ,priors))))
+       (unless (local-variable-if-set-p ',mode)
+         (error "Not a local minor mode var: %s" ',mode))
        (setq ,@(mapcan #'identity (nreverse forms))))))
 
 (defun erc--target-from-string (string)
diff --git a/test/lisp/erc/erc-stamp-tests.el b/test/lisp/erc/erc-stamp-tests.el
index ef292ccb618..70ca224ac74 100644
--- a/test/lisp/erc/erc-stamp-tests.el
+++ b/test/lisp/erc/erc-stamp-tests.el
@@ -46,7 +46,7 @@
 
     (with-current-buffer (get-buffer-create "*erc-stamp-tests--insert-right*")
       (erc-mode)
-      (erc-munge-invisibility-spec)
+      (erc-stamp--manage-local-options-state)
       (erc--initialize-markers (point) nil)
       (erc-tests-common-init-server-proc "sleep" "1")
 
@@ -235,7 +235,7 @@
     (with-current-buffer (get-buffer-create "*erc-timestamp-intangible*")
       (erc-mode)
       (erc--initialize-markers (point) nil)
-      (erc-munge-invisibility-spec)
+      (erc-stamp--manage-local-options-state)
       (erc-display-message nil 'notice (current-buffer) "Welcome")
       ;;
       ;; Pretend `fill' is active and that these lines are
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index b51bd67ae04..7890049a325 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -302,6 +302,7 @@
                                (cl-incf counter))))
          erc-accidental-paste-threshold-seconds
          erc-insert-modify-hook
+         (erc-last-input-time 0)
          (erc-modules (remq 'stamp erc-modules))
          (erc-send-input-line-function #'ignore)
          (erc--input-review-functions erc--input-review-functions)
@@ -1189,12 +1190,16 @@
       (should (erc--valid-local-channel-p "&local")))))
 
 (ert-deftest erc--restore-initialize-priors ()
+  (unless (>= emacs-major-version 28)
+    (ert-skip "Lisp nesting exceeds `max-lisp-eval-depth'"))
   (should (pcase (macroexpand-1 '(erc--restore-initialize-priors erc-my-mode
                                    foo (ignore 1 2 3)
                                    bar #'spam
                                    baz nil))
             (`(let* ((,p (or erc--server-reconnecting erc--target-priors))
                      (,q (and ,p (alist-get 'erc-my-mode ,p))))
+                (unless (local-variable-if-set-p 'erc-my-mode)
+                  (error "Not a local minor mode var: %s" 'erc-my-mode))
                 (setq foo (if ,q (alist-get 'foo ,p) (ignore 1 2 3))
                       bar (if ,q (alist-get 'bar ,p) #'spam)
                       baz (if ,q (alist-get 'baz ,p) nil)))
diff --git a/test/lisp/erc/resources/erc-scenarios-common.el 
b/test/lisp/erc/resources/erc-scenarios-common.el
index 0ec48d766ef..042b3a8c05b 100644
--- a/test/lisp/erc/resources/erc-scenarios-common.el
+++ b/test/lisp/erc/resources/erc-scenarios-common.el
@@ -151,6 +151,7 @@
       (erc-autojoin-channels-alist nil)
       (erc-server-auto-reconnect nil)
       (erc-after-connect nil)
+      (erc-last-input-time 0)
       (erc-d-linger-secs 10)
       ,@bindings)))