master d880a08f959 08/11: Cement ordering of essential hook members in ERC

[F. Jason Park]

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

    Cement ordering of essential hook members in ERC
    
    * etc/ERC-NEWS: Add new section explaining the pinning of certain hook
    members owned by built-in modules to fixed depths.
    * lisp/erc/erc-button.el (erc-button-mode, erc-button-enable): Change
    hook depth for `erc-button-add-buttons' from 90 to 30.
    * lisp/erc/erc-fill.el (erc-fill-mode, erc-fill-enable): Change hook
    depth for `erc-fill' from 0 to 40.
    * lisp/erc/erc-match.el (erc-match-mode, erc-match-enable): Change
    hook depth for `erc-match-message' from 90 to 60.
    * lisp/erc/erc-stamp.el (erc-stamp-mode, erc-stamp-enable): Change
    hook depth for `erc-add-timestamp' from 90 to 50.
    * test/lisp/erc/erc-tests.el
    (erc-tests--assert-printed-in-subprocess): Add fixture for testing a
    form printed from a subprocess.
    (erc--find-mode, erc--essential-hook-ordering): Use helper in existing
    and new tests, respectively.  (Bug#60936)
---
 etc/ERC-NEWS               | 20 ++++++++++
 lisp/erc/erc-button.el     |  4 +-
 lisp/erc/erc-fill.el       |  4 +-
 lisp/erc/erc-match.el      |  2 +-
 lisp/erc/erc-stamp.el      |  4 +-
 test/lisp/erc/erc-tests.el | 97 ++++++++++++++++++++++++++++++----------------
 6 files changed, 90 insertions(+), 41 deletions(-)

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index 1534f0adc25..e4947d0c64d 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -178,6 +178,26 @@ impactfully, the value of the 'field' property for ERC's 
prompt has
 changed from 't' to the more useful 'erc-prompt', although the
 property of the same name has been retained.
 
+*** Members of insert- and send-related hooks have been reordered.
+Built-in and third-party modules rely on certain hooks for adjusting
+incoming and outgoing messages upon insertion.  And some modules only
+want to do so after others have done their damage.  Traditionally,
+this required various hacks and finagling to achieve.  And while this
+release makes an effort to load modules in a more consistent order,
+that alone isn't enough to ensure similar predictability among
+essential members of important hooks.
+
+Luckily, ERC now leverages a feature introduced in Emacs 27, "hook
+depth," to secure the positions of a few key members of
+'erc-insert-modify-hook' and 'erc-send-modify-hook'.  So far, this
+includes the functions 'erc-button-add-buttons', 'erc-fill',
+'erc-add-timestamp', and 'erc-match-message', which now appear in that
+order, when present, at depths beginning at 20 and ending below 80.
+Of most interest to module authors is the new relative positioning of
+the first two, 'erc-button-add-buttons' and 'erc-fill', which have
+been swapped with respect to their previous places in recent ERC
+versions.
+
 *** ERC now manages timestamp-related properties a bit differently.
 For starters, the 'cursor-sensor-functions' property no longer
 contains unique closures and thus no longer proves effective for
diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index 931236891bf..08610860630 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -52,8 +52,8 @@
 ;;;###autoload(autoload 'erc-button-mode "erc-button" nil t)
 (define-erc-module button nil
   "This mode buttonizes all messages according to `erc-button-alist'."
-  ((add-hook 'erc-insert-modify-hook #'erc-button-add-buttons 'append)
-   (add-hook 'erc-send-modify-hook #'erc-button-add-buttons 'append)
+  ((add-hook 'erc-insert-modify-hook #'erc-button-add-buttons 30)
+   (add-hook 'erc-send-modify-hook #'erc-button-add-buttons 30)
    (add-hook 'erc-mode-hook #'erc-button-setup 91)
    (unless erc--updating-modules-p (erc-buffer-do #'erc-button-setup))
    (add-hook 'erc--tab-functions #'erc-button-next)
diff --git a/lisp/erc/erc-fill.el b/lisp/erc/erc-fill.el
index 306d74a0741..5115e45210d 100644
--- a/lisp/erc/erc-fill.el
+++ b/lisp/erc/erc-fill.el
@@ -49,8 +49,8 @@ the channel buffers are filled."
   ;; other modules.  Ideally, this module's processing should happen
   ;; after "morphological" modifications to a message's text but
   ;; before superficial decorations.
-  ((add-hook 'erc-insert-modify-hook #'erc-fill)
-   (add-hook 'erc-send-modify-hook #'erc-fill))
+  ((add-hook 'erc-insert-modify-hook #'erc-fill 40)
+   (add-hook 'erc-send-modify-hook #'erc-fill 40))
   ((remove-hook 'erc-insert-modify-hook #'erc-fill)
    (remove-hook 'erc-send-modify-hook #'erc-fill)))
 
diff --git a/lisp/erc/erc-match.el b/lisp/erc/erc-match.el
index 0c58524cd9f..6ba524ef9a8 100644
--- a/lisp/erc/erc-match.el
+++ b/lisp/erc/erc-match.el
@@ -52,7 +52,7 @@ they are hidden or highlighted.  This is controlled via the 
variables
 `erc-current-nick-highlight-type'.  For all these highlighting types,
 you can decide whether the entire message or only the sending nick is
 highlighted."
-  ((add-hook 'erc-insert-modify-hook #'erc-match-message 'append)
+  ((add-hook 'erc-insert-modify-hook #'erc-match-message 60)
    (add-hook 'erc-mode-hook #'erc-match--modify-invisibility-spec)
    (unless erc--updating-modules-p
      (erc-buffer-do #'erc-match--modify-invisibility-spec))
diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index 78a8b1bc100..aac51135a07 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -163,8 +163,8 @@ from entering them and instead jump over them."
 (define-erc-module stamp timestamp
   "This mode timestamps messages in the channel buffers."
   ((add-hook 'erc-mode-hook #'erc-munge-invisibility-spec)
-   (add-hook 'erc-insert-modify-hook #'erc-add-timestamp t)
-   (add-hook 'erc-send-modify-hook #'erc-add-timestamp t)
+   (add-hook 'erc-insert-modify-hook #'erc-add-timestamp 50)
+   (add-hook 'erc-send-modify-hook #'erc-add-timestamp 50)
    (add-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect)
    (add-hook 'erc--pre-clear-functions #'erc-stamp--reset-on-clear)
    (unless erc--updating-modules-p
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 1c75f35e1b5..7d1e20132e1 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -2085,48 +2085,77 @@
   (should (eq (erc--normalize-module-symbol 'timestamp) 'stamp))
   (should (eq (erc--normalize-module-symbol 'nickserv) 'services)))
 
-;; Worrying about which library a module comes from is mostly not
-;; worth the hassle so long as ERC can find its minor mode.  However,
-;; bugs involving multiple modules living in the same library may slip
-;; by because a module's loading problems may remain hidden on account
-;; of its place in the default ordering.
-
-(ert-deftest erc--find-mode ()
+(defun erc-tests--assert-printed-in-subprocess (code expected)
   (let* ((package (if-let* ((found (getenv "ERC_PACKAGE_NAME"))
                             ((string-prefix-p "erc-" found)))
                       (intern found)
                     'erc))
+         ;; This is for integrations testing with managed configs
+         ;; ("starter kits") that use a different package manager.
+         (init (and-let* ((found (getenv "ERC_TESTS_INIT"))
+                          (files (split-string found ","))
+                          ((seq-every-p #'file-exists-p files)))
+                 (mapcan (lambda (f) (list "-l" f)) files)))
          (prog
-          `(,@(and (featurep 'compat)
-                   `((progn
-                       (require 'package)
-                       (let ((package-load-list '((compat t) (,package t))))
-                         (package-initialize)))))
-            (require 'erc)
-            (let ((mods (mapcar #'cadddr
-                                (cdddr (get 'erc-modules 'custom-type))))
-                  moded)
-              (setq mods
-                    (sort mods (lambda (a b) (if (zerop (random 2)) a b))))
-              (dolist (mod mods)
-                (unless (keywordp mod)
-                  (push (if-let ((mode (erc--find-mode mod)))
-                            mod
-                          (list :missing mod))
-                        moded)))
-              (message "%S"
-                       (sort moded
-                             (lambda (a b)
-                               (string< (symbol-name a) (symbol-name b))))))))
-         (proc (start-process "erc--module-mode-autoloads"
-                              (current-buffer)
-                              (concat invocation-directory invocation-name)
-                              "-batch" "-Q"
-                              "-eval" (format "%S" (cons 'progn prog)))))
+          `(progn
+             ,@(and (not init) (featurep 'compat)
+                    `((require 'package)
+                      (let ((package-load-list '((compat t) (,package t))))
+                        (package-initialize))))
+             (require 'erc)
+             (cl-assert (equal erc-version ,erc-version) t)
+             ,code))
+         (proc (apply #'start-process
+                      (symbol-name (ert-test-name (ert-running-test)))
+                      (current-buffer)
+                      (concat invocation-directory invocation-name)
+                      `("-batch" ,@(or init '("-Q"))
+                        "-eval" ,(format "%S" prog)))))
     (set-process-query-on-exit-flag proc t)
     (while (accept-process-output proc 10))
     (goto-char (point-min))
-    (should (equal (read (current-buffer)) erc-tests--modules))))
+    (unless (equal (read (current-buffer)) expected)
+      (message "Exepcted: %S\nGot: %s" expected (buffer-string))
+      (ert-fail "Mismatch"))))
+
+;; Worrying about which library a module comes from is mostly not
+;; worth the hassle so long as ERC can find its minor mode.  However,
+;; bugs involving multiple modules living in the same library may slip
+;; by because a module's loading problems may remain hidden on account
+;; of its place in the default ordering.
+
+(ert-deftest erc--find-mode ()
+  (erc-tests--assert-printed-in-subprocess
+   `(let ((mods (mapcar #'cadddr (cdddr (get 'erc-modules 'custom-type))))
+          moded)
+      (setq mods (sort mods (lambda (a b) (if (zerop (random 2)) a b))))
+      (dolist (mod mods)
+        (unless (keywordp mod)
+          (push (if-let ((mode (erc--find-mode mod))) mod (list :missing mod))
+                moded)))
+      (message "%S"
+               (sort moded (lambda (a b)
+                             (string< (symbol-name a) (symbol-name b))))))
+   erc-tests--modules))
+
+(ert-deftest erc--essential-hook-ordering ()
+  (erc-tests--assert-printed-in-subprocess
+   '(progn
+      (erc-update-modules)
+      (message "%S"
+               (list :erc-insert-modify-hook erc-insert-modify-hook
+                     :erc-send-modify-hook erc-send-modify-hook)))
+
+   '( :erc-insert-modify-hook (erc-controls-highlight ; 0
+                               erc-button-add-buttons ; 30
+                               erc-fill ; 40
+                               erc-add-timestamp ; 50
+                               erc-match-message) ; 60
+
+      :erc-send-modify-hook ( erc-controls-highlight ; 0
+                              erc-button-add-buttons ; 30
+                              erc-fill ; 40
+                              erc-add-timestamp)))) ; 50
 
 (ert-deftest erc-migrate-modules ()
   (should (equal (erc-migrate-modules '(autojoin timestamp button))