bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups

["J.P." <jp@neverwas.me>]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

Seems I was under the mistaken impression that all customizations with a
"CHANGED" state get saved when hitting C-x C-s (`Custom-save'), which is
obviously not the case. Thus, I think it's best not to mislead users by
spoofing the modification state because the only benefit of doing so now
is that carats aren't automatically thrown open on module-var widgets,
which is a comparatively minor annoyance. The attached patch reverts
this spoofing nonsense and also restores a working toggle button so
users again have the option of dismissing the warning.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-5.6-Restore-module-var-toggles-in-ERC-s-Custom-buffe.patch --]
[-- Type: text/x-patch, Size: 7512 bytes --]

From f8ec0e7646f8d1ed0c30ec6d11e22235edb5a316 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 14 Apr 2023 00:07:31 -0700
Subject: [PATCH] [5.6] Restore module var toggles in ERC's Custom buffers

* lisp/erc/erc-common.el (erc--neuter-custom-variable-state): Remove
function.  ERC famously toggles global minor-mode vars during normal
operations, which adds noise to its customization buffers because
`customize-variable-state' always sees an activated module's mode
variable as having "CHANGED".  To suppress this annoyance, a
workaround was employed that used a dishonest `:get' function to
simply return the "saved value," if any.  While this improved the
Customize experience it also misled users, which likely wasn't
justified.
(erc--make-show-me-widget): Add helper to avoid forward declarations.
(erc--prepare-custom-module-type): Don't deprive users of a working
minor-mode toggle.
(erc--find-feature): New function to guess the feature of a module's
containing library.
(define-erc-module): Remove `:get' keyword.  Specify `:require'
instead, whose value may be nil.  Users who currently have mode vars
in their `custom-file' won't be impacted by this addition because
those `custom-set-variables' entries will still lack a REQUEST list
and hence won't incur a startup penalty.  And new users intent on
using the toggle will hopefully do so with the knowledge they're
opting in to requiring ERC on startup, which is not the case if they
follow the recommended practice of using `erc-modules' instead.
* test/lisp/erc/erc-tests.el (define-erc-module--global): Change
expected expansion.  (Bug#60935.)
---
 lisp/erc/erc-common.el     | 62 ++++++++++++++++++++++++--------------
 test/lisp/erc/erc-tests.el |  2 +-
 2 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 6c015c71ff9..7b63a4b1eca 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -40,6 +40,9 @@ erc-session-server
 (declare-function erc-server-buffer "erc" nil)
 (declare-function widget-apply-action "wid-edit" (widget &optional event))
 (declare-function widget-at "wid-edit" (&optional pos))
+(declare-function widget-create-child-and-convert "wid-edit"
+                  (parent type &rest args))
+(declare-function widget-default-format-handler "wid-edit" (widget escape))
 (declare-function widget-get-sibling "wid-edit" (widget))
 (declare-function widget-move "wid-edit" (arg &optional suppress-echo))
 (declare-function widget-type "wid-edit" (widget))
@@ -195,16 +198,6 @@ erc--find-group
         (throw 'found found)))
     'erc))
 
-(defun erc--neuter-custom-variable-state (variable)
-  "Lie to Customize about VARIABLE's true state.
-Do so by always returning its standard value, namely nil."
-  ;; Make a module's global minor-mode toggle blind to Customize, so
-  ;; that `customize-variable-state' never sees it as "changed",
-  ;; regardless of its value.  This snippet is
-  ;; `custom--standard-value' from Emacs 28+.
-  (cl-assert (null (eval (car (get variable 'standard-value)) t)))
-  nil)
-
 ;; This exists as a separate, top-level function to prevent the byte
 ;; compiler from warning about widget-related dependencies not being
 ;; loaded at runtime.
@@ -230,25 +223,42 @@ erc--tick-module-checkbox
              (substitute-command-keys "\\[Custom-set]")
              (substitute-command-keys "\\[Custom-save]"))))
 
+;; This stands apart to avoid needing forward declarations for
+;; `wid-edit' functions in every file requiring `erc-common'.
+(defun erc--make-show-me-widget (widget escape &rest plist)
+  (if (eq escape ?i)
+      (apply #'widget-create-child-and-convert widget 'push-button plist)
+    (widget-default-format-handler widget escape)))
+
 (defun erc--prepare-custom-module-type (name)
   `(let* ((name (erc--normalize-module-symbol ',name))
           (fmtd (format " `%s' " name)))
      `(boolean
-       :button-face '(custom-variable-obsolete custom-button)
-       :format "%{%t%}: %[Deprecated Toggle%] \n%h\n"
+       :format "%{%t%}: %i %[Deprecated Toggle%] %v \n%h\n"
+       :format-handler
+       ,(lambda (widget escape)
+          (erc--make-show-me-widget
+           widget escape
+           :button-face '(custom-variable-obsolete custom-button)
+           :tag "Show Me"
+           :action (apply-partially #'erc--tick-module-checkbox name)
+           :help-echo (lambda (_)
+                        (let ((hasp (memq name erc-modules)))
+                          (concat (if hasp "Remove" "Add") fmtd
+                                  (if hasp "from" "to")
+                                  " `erc-modules'.")))))
+       :action widget-toggle-action
        :documentation-property
        ,(lambda (_)
           (let ((hasp (memq name erc-modules)))
-            (concat "Setting a module's minor-mode variable is "
-                    (propertize "ineffective" 'face 'error)
-                    ".\nPlease " (if hasp "remove" "add") fmtd
-                    (if hasp "from" "to") " `erc-modules' directly instead.\n"
-                    "You can do so now by clicking the scary button above.")))
-       :help-echo ,(lambda (_)
-                     (let ((hasp (memq name erc-modules)))
-                       (concat (if hasp "Remove" "Add") fmtd
-                               (if hasp "from" "to") " `erc-modules'.")))
-       :action ,(apply-partially #'erc--tick-module-checkbox name))))
+            (concat
+             "Setting a module's minor-mode variable is "
+             (propertize "ineffective" 'face 'error)
+             ".\nPlease " (if hasp "remove" "add") fmtd
+             (if hasp "from" "to") " `erc-modules' directly instead.\n"
+             "You can do so now by clicking "
+             (propertize "Show Me" 'face 'custom-variable-obsolete)
+             " above."))))))
 
 (defun erc--fill-module-docstring (&rest strings)
   (with-temp-buffer
@@ -264,6 +274,12 @@ erc--fill-module-docstring
     (goto-char (point-min))
     (nth 3 (read (current-buffer)))))
 
+(defmacro erc--find-feature (name alias)
+  `(pcase (erc--find-group ',name ,(and alias (list 'quote alias)))
+     ('erc (and-let* ((file (or (macroexp-file-name) buffer-file-name)))
+             (intern (file-name-base file))))
+     (v v)))
+
 (defmacro define-erc-module (name alias doc enable-body disable-body
                                   &optional local-p)
   "Define a new minor mode using ERC conventions.
@@ -310,7 +326,7 @@ define-erc-module
 \n%s" name name doc))
          :global ,(not local-p)
          :group (erc--find-group ',name ,(and alias (list 'quote alias)))
-         ,@(unless local-p '(:get #'erc--neuter-custom-variable-state))
+         ,@(unless local-p `(:require ',(erc--find-feature name alias)))
          ,@(unless local-p `(:type ,(erc--prepare-custom-module-type name)))
          (if ,mode
              (,enable)
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 5aaf7e499e3..7fc885c7b9d 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -2018,7 +2018,7 @@ define-erc-module--global
 Some docstring."
                         :global t
                         :group (erc--find-group 'mname 'malias)
-                        :get #'erc--neuter-custom-variable-state
+                        :require 'nil
                         :type "mname"
                         (if erc-mname-mode
                             (erc-mname-enable)
-- 
2.39.2