From d880a08f9592e51ada5749d10b472396683fb6ee Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Mon, 5 Jun 2023 03:49:44 -0700 Subject: [PATCH] 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)) -- 2.39.2