master 0d3ccdbde44 16/37: Don't associate ERC modules with undefined groups

[F. Jason Park]

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

    Don't associate ERC modules with undefined groups
    
    * lisp/erc/erc-capab.el: Add property crutch to help ERC find module's
    Custom group.
    * lisp/erc/erc-common.el (erc--find-group): Add new function, a helper
    for finding an existing ERC module's Custom group based on
    `define-erc-module' params.  Prefer `group-documentation' as a
    sentinel over symbol properties owned by Customize because they might
    not be present if the group isn't yet associated with any custom
    variables.
    (define-erc-module): Set `:group' keyword value more accurately,
    falling back to `erc' when no associated group has been defined.
    * 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.  (Bug#60935.)
---
 lisp/erc/erc-capab.el      |  1 +
 lisp/erc/erc-common.el     | 38 +++++++++++++++++++++++++++++++++-----
 test/lisp/erc/erc-tests.el | 37 +++++++++++++++++++++++++++++++++++--
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/lisp/erc/erc-capab.el b/lisp/erc/erc-capab.el
index 650c5fa84ac..bb0921da7f0 100644
--- a/lisp/erc/erc-capab.el
+++ b/lisp/erc/erc-capab.el
@@ -89,6 +89,7 @@ character not found in IRC nicknames to avoid confusion."
 ;;; Define module:
 
 ;;;###autoload(autoload 'erc-capab-identify-mode "erc-capab" nil t)
+(put 'capab-identify 'erc-group 'erc-capab)
 (define-erc-module capab-identify nil
   "Handle dancer-ircd's CAPAB IDENTIFY-MSG and IDENTIFY-CTCP."
   ;; append so that `erc-server-parameters' is already set by `erc-server-005'
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index b8f6a06b76c..91ddce8fbd9 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -118,6 +118,37 @@
              (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 appears 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 in which this fails or the catch-all of `erc' is
+;; more inappropriate, (global) modules can declare a top-level
+;;
+;;   (put 'foo 'erc-group 'erc-bar)
+;;
+;; where `erc-bar' is the group and `foo' is the normalized module.
+;; Do this *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
+    (dolist (s symbols)
+      (let* ((downed (downcase (symbol-name s)))
+             (known (intern-soft (concat "erc-" downed))))
+        (when (and known
+                   (or (get known 'group-documentation)
+                       (rassq known custom-current-group-alist)))
+          (throw 'found known))
+        (when (setq known (intern-soft (concat "erc-" downed "-mode")))
+          (when-let ((found (custom-group-of-mode known)))
+            (throw 'found found))))
+      (when-let ((found (get (erc--normalize-module-symbol s) 'erc-group)))
+        (throw 'found found)))
+    'erc))
+
 (defmacro define-erc-module (name alias doc enable-body disable-body
                                   &optional local-p)
   "Define a new minor mode using ERC conventions.
@@ -152,7 +183,6 @@ Example:
   (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
@@ -163,10 +193,8 @@ With a prefix argument ARG, enable %s if ARG is positive,
 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/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index acd470a1e17..45d8cae5125 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1353,6 +1353,39 @@
   ;; 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) "")
+            ((get 'baz 'erc-group) 'erc-foo))
+    (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))
+    (should (eq (erc--find-group 'fake 'baz) 'erc-foo))))
+
+(ert-deftest erc--find-group--real ()
+  :tags '(:unstable)
+  (require 'erc-services)
+  (require 'erc-stamp)
+  (require 'erc-sound)
+  (require 'erc-page)
+  (require 'erc-join)
+  (require 'erc-capab)
+  (require 'erc-pcomplete)
+  (should (eq (erc--find-group 'services 'nickserv) 'erc-services))
+  (should (eq (erc--find-group 'stamp 'timestamp) 'erc-stamp))
+  (should (eq (erc--find-group 'sound 'ctcp-sound) 'erc-sound))
+  (should (eq (erc--find-group 'page 'ctcp-page) 'erc-page))
+  (should (eq (erc--find-group 'autojoin) 'erc-autojoin))
+  (should (eq (erc--find-group 'pcomplete 'Completion) 'erc-pcomplete))
+  (should (eq (erc--find-group 'capab-identify) 'erc-capab))
+  ;; No group specified.
+  (should (eq (erc--find-group 'smiley nil) 'erc))
+  (should (eq (erc--find-group 'unmorse nil) 'erc)))
+
 (ert-deftest erc--update-modules ()
   (let (calls
         erc-modules
@@ -1453,7 +1486,7 @@ and disable it otherwise.  If called from Lisp, enable 
the mode
 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)))
@@ -1499,7 +1532,7 @@ and disable it otherwise.  If called from Lisp, enable 
the mode
 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)))