From 3d81ecf0a95374793f70a19da81ea75da84d0be1 Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sat, 4 Feb 2023 06:24:59 -0800 Subject: [PATCH] Leverage loaddefs for migrating ERC modules * lisp/erc/erc-common.el (erc--features-to-modules, erc--modules-to-features, erc--module-name-migrations): Remove unused internal functions. (erc--normalize-module-symbol): Make aware of new migration scheme based on symbol properties. * lisp/erc/erc-page.el: Add autoload cookie for module migration. * lisp/erc/erc-pcomplete.el: Add autoload cookies for module migration. * lisp/erc/erc-services.el: Add autoload cookie for module migration. * lisp/erc/erc-sound.el: Add autoload cookie for module migration. * lisp/erc/erc-stamp.el: Add autoload cookie for module migration. * lisp/erc/erc.el (erc-modules): Reorder default value, sorted by `string<' so that Customize does not consider the value to have been edited. Remove non-existent module `hecomplete' from lineup and swap a couple more to maintain sorted order. Change `:initialize' function to tag all symbols for built-in modules with an `erc--module' property. In the `:set' function, ensure third-party modules appear after the sorted and normalized built-ins, but in user-defined order. Do this to prevent all modules, built-ins included, from ending up as populated form fields for the "other" checkbox in the Customize interface. (erc--find-mode): Add helper function for `erc--update-modules'. (erc--update-modules): Always resolve module names and only conditionally attempt to require corresponding features. * test/lisp/erc/erc-tests.el (erc-tests--modules): Add manifest for asserting built-in modules and features. This is easier to verify visually than looking at the custom-type set for `erc-modules'. (erc-modules--initialize): New test. (erc-modules--internal-property): Add test. (erc--normalize-module-symbol): New test. (erc--find-mode): New test. (erc--update-modules) Adapt to new paradigm and make more comprehensive. (Bug#60954.) --- lisp/erc/erc-common.el | 39 ++--------- lisp/erc/erc-page.el | 1 + lisp/erc/erc-pcomplete.el | 2 + lisp/erc/erc-services.el | 1 + lisp/erc/erc-sound.el | 1 + lisp/erc/erc-stamp.el | 4 ++ lisp/erc/erc.el | 63 ++++++++++++------ test/lisp/erc/erc-tests.el | 130 ++++++++++++++++++++++++++++++++----- 8 files changed, 173 insertions(+), 68 deletions(-) diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el index 0279b0a0bc4..b8f6a06b76c 100644 --- a/lisp/erc/erc-common.el +++ b/lisp/erc/erc-common.el @@ -85,40 +85,13 @@ (contents "" :type string) (tags '() :type list)) -;; TODO move goodies modules here after 29 is released. -(defconst erc--features-to-modules - '((erc-pcomplete completion pcomplete) - (erc-capab capab-identify) - (erc-join autojoin) - (erc-page page ctcp-page) - (erc-sound sound ctcp-sound) - (erc-stamp stamp timestamp) - (erc-services services nickserv)) - "Migration alist mapping a library feature to module names. -Keys need not be unique: a library may define more than one -module. Sometimes a module's downcased alias will be its -canonical name.") - -(defconst erc--modules-to-features - (let (pairs) - (pcase-dolist (`(,feature . ,names) erc--features-to-modules) - (dolist (name names) - (push (cons name feature) pairs))) - (nreverse pairs)) - "Migration alist mapping a module's name to its home library feature.") - -(defconst erc--module-name-migrations - (let (pairs) - (pcase-dolist (`(,_ ,canonical . ,rest) erc--features-to-modules) - (dolist (obsolete rest) - (push (cons obsolete canonical) pairs))) - pairs) - "Association list of obsolete module names to canonical names.") - +;; After dropping 28, we can use prefixed "erc-autoload" cookies. (defun erc--normalize-module-symbol (symbol) - "Return preferred SYMBOL for `erc-modules'." - (setq symbol (intern (downcase (symbol-name symbol)))) - (or (cdr (assq symbol erc--module-name-migrations)) symbol)) + "Return preferred SYMBOL for `erc--modules'." + (while-let ((canonical (get symbol 'erc--module)) + ((not (eq canonical symbol)))) + (setq symbol canonical)) + symbol) (defun erc--assemble-toggle (localp name ablsym mode val body) (let ((arg (make-symbol "arg"))) diff --git a/lisp/erc/erc-page.el b/lisp/erc/erc-page.el index 308b3784ca5..6cba59c6946 100644 --- a/lisp/erc/erc-page.el +++ b/lisp/erc/erc-page.el @@ -34,6 +34,7 @@ "React to CTCP PAGE messages." :group 'erc) +;;;###autoload(put 'ctcp-page 'erc--module 'page) ;;;###autoload(autoload 'erc-page-mode "erc-page") (define-erc-module page ctcp-page "Process CTCP PAGE requests from IRC." diff --git a/lisp/erc/erc-pcomplete.el b/lisp/erc/erc-pcomplete.el index 0bce856018c..7eb7431fb91 100644 --- a/lisp/erc/erc-pcomplete.el +++ b/lisp/erc/erc-pcomplete.el @@ -56,6 +56,8 @@ add this string to nicks completed." "If t, order nickname completions with the most recent speakers first." :type 'boolean) +;;;###autoload(put 'Completion 'erc--module 'completion) +;;;###autoload(put 'pcomplete 'erc--module 'completion) ;;;###autoload(autoload 'erc-completion-mode "erc-pcomplete" nil t) (define-erc-module pcomplete Completion "In ERC Completion mode, the TAB key does completion whenever possible." diff --git a/lisp/erc/erc-services.el b/lisp/erc/erc-services.el index 2e6959cc3f0..5408ba405db 100644 --- a/lisp/erc/erc-services.el +++ b/lisp/erc/erc-services.el @@ -102,6 +102,7 @@ You can also use \\[erc-nickserv-identify-mode] to change modes." (when (featurep 'erc-services) (erc-nickserv-identify-mode val)))) +;;;###autoload(put 'nickserv 'erc--module 'services) ;;;###autoload(autoload 'erc-services-mode "erc-services" nil t) (define-erc-module services nickserv "This mode automates communication with services." diff --git a/lisp/erc/erc-sound.el b/lisp/erc/erc-sound.el index 0abdbfd959c..9da9202f0cf 100644 --- a/lisp/erc/erc-sound.el +++ b/lisp/erc/erc-sound.el @@ -47,6 +47,7 @@ (require 'erc) +;;;###autoload(put 'ctcp-sound 'erc--module 'sound) ;;;###autoload(autoload 'erc-sound-mode "erc-sound") (define-erc-module sound ctcp-sound "In ERC sound mode, the client will respond to CTCP SOUND requests diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el index 0aa1590f801..d1a1507f700 100644 --- a/lisp/erc/erc-stamp.el +++ b/lisp/erc/erc-stamp.el @@ -147,6 +147,10 @@ from entering them and instead jump over them." "ERC timestamp face." :group 'erc-faces) +;; New libraries should only autoload the minor mode for a module's +;; preferred name (rather than its alias). + +;;;###autoload(put 'timestamp 'erc--module 'stamp) ;;;###autoload(autoload 'erc-timestamp-mode "erc-stamp" nil t) (define-erc-module stamp timestamp "This mode timestamps messages in the channel buffers." diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index ef51f100f8b..27e9ec81b98 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -1839,9 +1839,9 @@ buffer rather than a server buffer.") ;; each item is in the format '(old . new) (delete-dups (mapcar #'erc--normalize-module-symbol mods))) -(defcustom erc-modules '(netsplit fill button match track completion readonly - networks ring autojoin noncommands irccontrols - move-to-prompt stamp menu list) +(defcustom erc-modules '( autojoin button completion fill irccontrols + list match menu move-to-prompt netsplit + networks noncommands readonly ring stamp track) "A list of modules which ERC should enable. If you set the value of this without using `customize' remember to call \(erc-update-modules) after you change it. When using `customize', modules @@ -1849,12 +1849,20 @@ removed from the list will be disabled." :get (lambda (sym) ;; replace outdated names with their newer equivalents (erc-migrate-modules (symbol-value sym))) - :initialize #'custom-initialize-default + ;; Expect every built-in module to have the symbol property + ;; `erc--module' set to its canonical symbol (often itself). + :initialize (lambda (symbol exp) + ;; Use `cdddr' because (set :greedy t . ,entries) + (dolist (entry (cdddr (get 'erc-modules 'custom-type))) + (when-let* (((eq (car entry) 'const)) + (s (cadddr entry))) ; (const :tag "..." ,s) + (put s 'erc--module s))) + (custom-initialize-reset symbol exp)) :set (lambda (sym val) ;; disable modules which have just been removed (when (and (boundp 'erc-modules) erc-modules val) (dolist (module erc-modules) - (unless (member module val) + (unless (memq module val) (let ((f (intern-soft (format "erc-%s-mode" module)))) (when (and (fboundp f) (boundp f)) (when (symbol-value f) @@ -1866,7 +1874,15 @@ removed from the list will be disabled." (when (symbol-value f) (funcall f 0)) (kill-local-variable f))))))))) - (set sym val) + (let (built-in third-party) + (dolist (v val) + (setq v (erc--normalize-module-symbol v)) + (if (get v 'erc--module) + (push v built-in) + (push v third-party))) + ;; Calling `set-default-toplevel-value' complicates testing + (set sym (append (sort built-in #'string-lessp) + (nreverse third-party)))) ;; this test is for the case where erc hasn't been loaded yet (when (fboundp 'erc-update-modules) (erc-update-modules))) @@ -1880,7 +1896,6 @@ removed from the list will be disabled." capab-identify) (const :tag "completion: Complete nicknames and commands (programmable)" completion) - (const :tag "hecomplete: Complete nicknames and commands (obsolete, use \"completion\")" hecomplete) (const :tag "dcc: Provide Direct Client-to-Client support" dcc) (const :tag "fill: Wrap long lines" fill) (const :tag "identd: Launch an identd server on port 8113" identd) @@ -1897,11 +1912,11 @@ removed from the list will be disabled." (const :tag "networks: Provide data about IRC networks" networks) (const :tag "noncommands: Don't display non-IRC commands after evaluation" noncommands) + (const :tag "notifications: Desktop alerts on PRIVMSG or mentions" + notifications) (const :tag "notify: Notify when the online status of certain users changes" notify) - (const :tag "notifications: Send notifications on PRIVMSG or nickname mentions" - notifications) (const :tag "page: Process CTCP PAGE requests from IRC" page) (const :tag "readonly: Make displayed lines read-only" readonly) (const :tag "replace: Replace text in messages" replace) @@ -1914,8 +1929,8 @@ removed from the list will be disabled." (const :tag "smiley: Convert smileys to pretty icons" smiley) (const :tag "sound: Play sounds when you receive CTCP SOUND requests" sound) - (const :tag "stamp: Add timestamps to messages" stamp) (const :tag "spelling: Check spelling" spelling) + (const :tag "stamp: Add timestamps to messages" stamp) (const :tag "track: Track channel activity in the mode-line" track) (const :tag "truncate: Truncate buffers to a certain size" truncate) (const :tag "unmorse: Translate morse code in messages" unmorse) @@ -1929,18 +1944,28 @@ Except ignore all local modules, which were introduced in ERC 5.5." (erc--update-modules) nil) +(defun erc--find-mode (sym) + (setq sym (erc--normalize-module-symbol sym)) + (if-let* ((mode (intern-soft (concat "erc-" (symbol-name sym) "-mode"))) + ((or (boundp mode) + (and (fboundp mode) + (autoload-do-load (symbol-function mode) mode))))) + mode + (and (require (or (get sym 'erc--feature) + (intern (concat "erc-" (symbol-name sym)))) + nil 'noerror) + (setq mode (intern-soft (concat "erc-" (symbol-name sym) "-mode"))) + (fboundp mode) + mode))) + (defun erc--update-modules () (let (local-modes) (dolist (module erc-modules local-modes) - (require (or (alist-get module erc--modules-to-features) - (intern (concat "erc-" (symbol-name module)))) - nil 'noerror) ; some modules don't have a corresponding feature - (let ((mode (intern-soft (concat "erc-" (symbol-name module) "-mode")))) - (unless (and mode (fboundp mode)) - (error "`%s' is not a known ERC module" module)) - (if (custom-variable-p mode) - (funcall mode 1) - (push mode local-modes)))))) + (if-let ((mode (erc--find-mode module))) + (if (custom-variable-p mode) + (funcall mode 1) + (push mode local-modes)) + (error "`%s' is not a known ERC module" module))))) (defun erc-setup-buffer (buffer) "Consults `erc-join-buffer' to find out how to display `BUFFER'." diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index bbf3269161d..81381a0c800 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -1224,6 +1224,85 @@ (kill-buffer "baznet") (kill-buffer "#chan"))) +(defconst erc-tests--modules + '( autoaway autojoin button capab-identify completion dcc fill identd + irccontrols keep-place list log match menu move-to-prompt netsplit + networks noncommands notifications notify page readonly + replace ring sasl scrolltobottom services smiley sound + spelling stamp track truncate unmorse xdcc)) + +;; Ensure that `:initialize' doesn't change the ordering of the +;; members because otherwise the widget's state is "edited". + +(ert-deftest erc-modules--initialize () + ;; This is `custom--standard-value' from Emacs 28. + (should (equal (eval (car (get 'erc-modules 'standard-value)) t) + erc-modules))) + +;; Ensure the `:initialize' function for `erc-modules' successfully +;; tags all built-in modules with the internal property `erc--module'. + +(ert-deftest erc-modules--internal-property () + (let (ours) + (mapatoms (lambda (s) + (when-let ((v (get s 'erc--module)) + ((eq v s))) + (push s ours)))) + (should (equal (sort ours #'string-lessp) erc-tests--modules)))) + +(ert-deftest erc--normalize-module-symbol () + (dolist (mod erc-tests--modules) + (should (eq (erc--normalize-module-symbol mod) mod))) + (should (eq (erc--normalize-module-symbol 'pcomplete) 'completion)) + (should (eq (erc--normalize-module-symbol 'Completion) 'completion)) + (should (eq (erc--normalize-module-symbol 'ctcp-page) 'page)) + (should (eq (erc--normalize-module-symbol 'ctcp-sound) 'sound)) + (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 () + (let* ((package (if-let* ((found (getenv "ERC_PACKAGE_NAME")) + ((string-prefix-p "erc-" found))) + (intern found) + 'erc)) + (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))))) + (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)))) + (ert-deftest erc-migrate-modules () (should (equal (erc-migrate-modules '(autojoin timestamp button)) '(autojoin stamp button))) @@ -1234,17 +1313,28 @@ (let (calls erc-modules erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) + + ;; This `lbaz' module is unknown, so ERC looks for it via the + ;; symbol proerty `erc--feature' and, failing that, by + ;; `require'ing its "erc-" prefixed symbol. + (should-not (intern-soft "erc-lbaz-mode")) + (cl-letf (((symbol-function 'require) - (lambda (s &rest _) (push s calls))) + (lambda (s &rest _) + (when (eq s 'erc--lbaz-feature) + (fset (intern "erc-lbaz-mode") ; local module + (lambda (n) (push (cons 'lbaz n) calls)))) + (push s calls))) ;; Local modules - ((symbol-function 'erc-fake-bar-mode) - (lambda (n) (push (cons 'fake-bar n) calls))) + ((symbol-function 'erc-lbar-mode) + (lambda (n) (push (cons 'lbar n) calls))) + ((get 'lbaz 'erc--feature) 'erc--lbaz-feature) ;; Global modules - ((symbol-function 'erc-fake-foo-mode) - (lambda (n) (push (cons 'fake-foo n) calls))) - ((get 'erc-fake-foo-mode 'standard-value) 'ignore) + ((symbol-function 'erc-gfoo-mode) + (lambda (n) (push (cons 'gfoo n) calls))) + ((get 'erc-gfoo-mode 'standard-value) 'ignore) ((symbol-function 'erc-autojoin-mode) (lambda (n) (push (cons 'autojoin n) calls))) ((get 'erc-autojoin-mode 'standard-value) 'ignore) @@ -1255,20 +1345,28 @@ (lambda (n) (push (cons 'completion n) calls))) ((get 'erc-completion-mode 'standard-value) 'ignore)) + (ert-info ("Unknown module") + (setq erc-modules '(lfoo)) + (should-error (erc--update-modules)) + (should (equal (pop calls) 'erc-lfoo)) + (should-not calls)) + (ert-info ("Local modules") - (setq erc-modules '(fake-foo fake-bar)) - (should (equal (erc--update-modules) '(erc-fake-bar-mode))) - ;; Bar the feature is still required but the mode is not activated - (should (equal (nreverse calls) - '(erc-fake-foo (fake-foo . 1) erc-fake-bar))) + (setq erc-modules '(gfoo lbar lbaz)) + ;; Don't expose the mode here + (should (equal (mapcar #'symbol-name (erc--update-modules)) + '("erc-lbaz-mode" "erc-lbar-mode"))) + ;; Lbaz required because unknown. + (should (equal (nreverse calls) '((gfoo . 1) erc--lbaz-feature))) + (fmakunbound (intern "erc-lbaz-mode")) + (unintern (intern "erc-lbaz-mode") obarray) (setq calls nil)) - (ert-info ("Module name overrides") - (setq erc-modules '(completion autojoin networks)) + (ert-info ("Global modules") ; `pcomplete' resolved to `completion' + (setq erc-modules '(pcomplete autojoin networks)) (should-not (erc--update-modules)) ; no locals - (should (equal (nreverse calls) '( erc-pcomplete (completion . 1) - erc-join (autojoin . 1) - erc-networks (networks . 1)))) + (should (equal (nreverse calls) + '((completion . 1) (autojoin . 1) (networks . 1)))) (setq calls nil))))) (ert-deftest erc--merge-local-modes () -- 2.39.2