>From e504a84e698982cec6830450bb2554be2f50e30d Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Sat, 14 Jan 2023 19:05:59 -0800 Subject: [PATCH 1/3] [5.6] Don't associate ERC modules with undefined groups * lisp/erc/erc-capab.el (erc-capab-mode): Add new alias for `erc-capab-identify-mode'. * lisp/erc/erc-common.el (erc--features-to-modules): Update `erc-capab' entry to prefer `capab' as the canonical name because it matches the library feature. (erc--find-group): Add new function, a helper for finding an existing ERC custom group based on `define-erc-module' params. Use `group-documentation' as a sentinel instead of `custom-group', which may not be present if it isn't yet associated with any custom variables. (define-erc-module): Set `:group' keyword value more accurately, falling back to `erc' group when no associated group has been defined. * lisp/erc/erc.el (erc-modules): Remove entry for nonexistent module `hecomplete' and change choice value for `erc-capab' from `capab-identify' to the now canonical `capab'. * test/lisp/erc/erc-tests.el (erc--find-group, erc--find-group--real): New tests. (define-erc-module--global, define-erc-module--local): Expect the :group keyword to be the unevaluated `erc--find-group' form. (Depends on bug#60784.) --- lisp/erc/erc-capab.el | 2 +- lisp/erc/erc-common.el | 36 ++++++++++++++++++++++++++++++------ lisp/erc/erc.el | 3 +-- test/lisp/erc/erc-tests.el | 33 +++++++++++++++++++++++++++++++-- 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/lisp/erc/erc-capab.el b/lisp/erc/erc-capab.el index 650c5fa84a..25cdea9e77 100644 --- a/lisp/erc/erc-capab.el +++ b/lisp/erc/erc-capab.el @@ -89,7 +89,7 @@ erc-capab-identify-unidentified ;;; Define module: ;;;###autoload(autoload 'erc-capab-identify-mode "erc-capab" nil t) -(define-erc-module capab-identify nil +(define-erc-module capab-identify capab "Handle dancer-ircd's CAPAB IDENTIFY-MSG and IDENTIFY-CTCP." ;; append so that `erc-server-parameters' is already set by `erc-server-005' ((add-hook 'erc-server-005-functions #'erc-capab-identify-setup t) diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el index 994555acec..60bb5dd6fc 100644 --- a/lisp/erc/erc-common.el +++ b/lisp/erc/erc-common.el @@ -91,7 +91,7 @@ erc--target ;; TODO move goodies modules here after 29 is released. (defconst erc--features-to-modules '((erc-pcomplete completion pcomplete) - (erc-capab capab-identify) + (erc-capab capab capab-identify) (erc-join autojoin) (erc-page page ctcp-page) (erc-sound sound ctcp-sound) @@ -148,6 +148,33 @@ erc--assemble-toggle (setq ,mode ,val) ,@body))))) +;; This is a migration helper that determines a module's `:group' +;; keyword argument from its name or alias. A (global) module's minor +;; mode variable will appear under the group's Custom menu. Like +;; `erc--normalize-module-symbol', it must run when the module's +;; definition (rather than that of `define-erc-module') is expanded. +;; For corner cases where this fails and where the catch-all of `erc' +;; is inappropriate, (global) modules can declare a top-level +;; +;; (custom-add-to-group 'erc-foo 'erc-bar-mode 'custom-variable) +;; +;; *before* the module's definition. If `define-erc-module' ever +;; accepts arbitrary keywords, passing an explicit `:group' will +;; obviously be preferable. + +(defun erc--find-group (&rest symbols) + (catch 'found + (while symbols + (when-let* ((s (pop symbols)) + (downed (concat "erc-" (downcase (symbol-name s)))) + (known (intern-soft downed))) + (when-let ((found (custom-group-of-mode known))) + (throw 'found found)) + (when (or (get known 'group-documentation) + (rassq known custom-current-group-alist)) + (throw 'found known)))) + 'erc)) + (defmacro define-erc-module (name alias doc enable-body disable-body &optional local-p) "Define a new minor mode using ERC conventions. @@ -182,7 +209,6 @@ define-erc-module (declare (doc-string 3) (indent defun)) (let* ((sn (symbol-name name)) (mode (intern (format "erc-%s-mode" (downcase sn)))) - (group (intern (format "erc-%s" (downcase sn)))) (enable (intern (format "erc-%s-enable" (downcase sn)))) (disable (intern (format "erc-%s-disable" (downcase sn))))) `(progn @@ -193,10 +219,8 @@ define-erc-module and disable it otherwise. If called from Lisp, enable the mode if ARG is omitted or nil. %s" name name doc) - ;; FIXME: We don't know if this group exists, so this `:group' may - ;; actually just silence a valid warning about the fact that the var - ;; is not associated with any group. - :global ,(not local-p) :group (quote ,group) + :global ,(not local-p) + :group (erc--find-group ',name ,(and alias (list 'quote alias))) (if ,mode (,enable) (,disable))) diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index 7f51b7bfb2..3bf6019783 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -1854,10 +1854,9 @@ erc-modules (const :tag "autojoin: Join channels automatically" autojoin) (const :tag "button: Buttonize URLs, nicknames, and other text" button) (const :tag "capab: Mark unidentified users on servers supporting CAPAB" - capab-identify) + capab) (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) diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index 40a2d2de65..c07f249343 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -1209,6 +1209,35 @@ erc-migrate-modules ;; Default unchanged (should (equal (erc-migrate-modules erc-modules) erc-modules))) +(ert-deftest erc--find-group () + ;; These two are loaded by default + (should (eq (erc--find-group 'keep-place nil) 'erc)) + (should (eq (erc--find-group 'networks nil) 'erc-networks)) + ;; These are fake + (cl-letf (((get 'erc-bar 'group-documentation) "")) + (should (eq (erc--find-group 'foo 'bar) 'erc-bar)) + (should (eq (erc--find-group 'bar 'foo) 'erc-bar)) + (should (eq (erc--find-group 'bar nil) 'erc-bar)) + (should (eq (erc--find-group 'foo nil) 'erc)))) + +(ert-deftest erc--find-group--real () + :tags '(:unstable) + (let (out) + (pcase-dolist (`(,feat . ,syms) erc--features-to-modules) + (require feat) + (push (cons syms (apply #'erc--find-group syms)) out)) + ;; Remove local modules, which are mapped to the catch-all + (while (and-let* ((m (rassq 'erc out))) ; while-let + (setq out (remq m out)))) + (should (equal out + '(((services nickserv) . erc-services) + ((stamp timestamp) . erc-stamp) + ((sound ctcp-sound) . erc-sound) + ((page ctcp-page) . erc-page) + ((autojoin) . erc-autojoin) + ((capab capab-identify) . erc-capab) + ((completion pcomplete) . erc-pcomplete)))))) + (ert-deftest erc--update-modules () (let (calls erc-modules @@ -1290,7 +1319,7 @@ define-erc-module--global if ARG is omitted or nil. Some docstring" :global t - :group 'erc-mname + :group (erc--find-group 'mname 'malias) (if erc-mname-mode (erc-mname-enable) (erc-mname-disable))) @@ -1336,7 +1365,7 @@ define-erc-module--local if ARG is omitted or nil. Some docstring" :global nil - :group 'erc-mname + :group (erc--find-group 'mname nil) (if erc-mname-mode (erc-mname-enable) (erc-mname-disable))) -- 2.38.1