* bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups
2023-01-18 14:50 bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups J.P.
@ 2023-02-07 15:23 ` J.P.
2023-02-19 15:03 ` J.P.
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: J.P. @ 2023-02-07 15:23 UTC (permalink / raw)
To: 60935; +Cc: emacs-erc
[-- Attachment #1: Type: text/plain, Size: 105 bytes --]
v2. Use autoloads for resolving groups in corner cases. Don't change
decades-old preferred module name.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v1-v2.diff --]
[-- Type: text/x-patch, Size: 6867 bytes --]
From e4e270a4bf556ce238bc42c8549d4c443f3a969a Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 7 Feb 2023 00:08:34 -0800
Subject: [PATCH 0/3] *** NOT A PATCH ***
*** BLURB HERE ***
F. Jason Park (3):
[5.6] Don't associate ERC modules with undefined groups
[5.6] Warn when setting minor-mode vars for ERC modules
[5.6] Fill doc strings for ERC modules.
lisp/erc/erc-capab.el | 1 +
lisp/erc/erc-common.el | 136 +++++++++++++++++++++++++++++++++----
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 78 ++++++++++++++++-----
4 files changed, 187 insertions(+), 31 deletions(-)
Interdiff:
diff --git a/lisp/erc/erc-capab.el b/lisp/erc/erc-capab.el
index 25cdea9e777..bb0921da7f0 100644
--- a/lisp/erc/erc-capab.el
+++ b/lisp/erc/erc-capab.el
@@ -89,7 +89,8 @@ erc-capab-identify-unidentified
;;; Define module:
;;;###autoload(autoload 'erc-capab-identify-mode "erc-capab" nil t)
-(define-erc-module capab-identify capab
+(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'
((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 f488244ef9c..cc278428d5d 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -97,7 +97,7 @@ erc--target
;; TODO move goodies modules here after 29 is released.
(defconst erc--features-to-modules
'((erc-pcomplete completion pcomplete)
- (erc-capab capab capab-identify)
+ (erc-capab capab-identify)
(erc-join autojoin)
(erc-page page ctcp-page)
(erc-sound sound ctcp-sound)
@@ -173,23 +173,27 @@ erc--assemble-toggle
;; 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)
+;; (put 'foo 'erc-group 'erc-bar)
;;
-;; *before* the module's definition. If `define-erc-module' ever
-;; accepts arbitrary keywords, passing an explicit `:group' will
+;; 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
- (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))))
+ (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))
(defun erc--custom-set-minor-mode (variable value)
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 70ba82fd4bf..5e971f15f5d 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1854,9 +1854,10 @@ 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)
+ 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)
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 703b26dcea0..e5db78ef2e1 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1214,29 +1214,33 @@ erc--find-group
(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) ""))
+ (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 'foo nil) 'erc))
+ (should (eq (erc--find-group 'fake 'baz) 'erc-foo))))
(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))))))
+ (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
--
2.39.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-5.6-Don-t-associate-ERC-modules-with-undefined-group.patch --]
[-- Type: text/x-patch, Size: 7148 bytes --]
From 380ea8edc391aa75be9b7c7019bfc8c3db57e4f4 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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 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 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' group 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 @@ erc-capab-identify-unidentified
;;; 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 994555acecf..bf3bce74cfd 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -148,6 +148,37 @@ 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
+;;
+;; (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.
@@ -182,7 +213,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 +223,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/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 40a2d2de657..24a4f9d0639 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1209,6 +1209,39 @@ 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) "")
+ ((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
@@ -1290,7 +1323,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 +1369,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.39.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-5.6-Warn-when-setting-minor-mode-vars-for-ERC-module.patch --]
[-- Type: text/x-patch, Size: 9563 bytes --]
From 84bdae341ec2e19ba45c6dd9547dc3a5db43cbea Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sat, 14 Jan 2023 19:08:11 -0800
Subject: [PATCH 2/3] [5.6] Warn when setting minor-mode vars for ERC modules
(erc--inside-mode-toggle-p): Add global var to inhibit mode toggles
from being run by `erc-update-modules'. It must be non-nil inside
custom-set functions for mode toggles created by `define-erc-module'.
(erc--assemble-toggle): Don't modify `erc-modules' when run from
custom-set function.
(erc--custom-set-minor-mode): Add new custom-set function for module
minor modes. Update `erc-modules' before calling a minor-mode toggle
via `custom-set-minor-mode'.
(erc--tick-module-checkbox): Add helper to toggle the appropriate
checkbox in the `erc-modules' widget when a user interactively toggles
a minor-mode state variable.
(erc--prepare-custom-module-type): Create spec for minor-mode custom
`:type', deferring various aspects until module-definition time.
(define-erc-module): Add `:set' and `:type' keywords for global
modules.
* lisp/erc/erc.el (erc-modules): Inhibit `erc-update-modules' when run
from a minor-mode toggle's custom-set function.
* test/lisp/erc/erc-tests.el
(define-erc-module--global, define-erc-module--local): Update
`erc-modules' mutations with `erc--inside-mode-toggle-p' guard
conditions. (Bug#60935.)
---
lisp/erc/erc-common.el | 78 +++++++++++++++++++++++++++++++++++---
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 13 +++++--
3 files changed, 85 insertions(+), 9 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index bf3bce74cfd..ec3a7cf9e85 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -29,6 +29,7 @@
(defvar erc--casemapping-rfc1459)
(defvar erc--casemapping-rfc1459-strict)
(defvar erc-channel-users)
+(defvar erc-modules)
(defvar erc-dbuf)
(defvar erc-log-p)
(defvar erc-server-users)
@@ -37,6 +38,11 @@ erc-session-server
(declare-function erc--get-isupport-entry "erc-backend" (key &optional single))
(declare-function erc-get-buffer "erc" (target &optional proc))
(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-get-sibling "wid-edit" (widget))
+(declare-function widget-move "wid-edit" (arg &optional suppress-echo))
+(declare-function widget-type "wid-edit" (widget))
(cl-defstruct erc-input
string insertp sendp)
@@ -123,6 +129,15 @@ erc--normalize-module-symbol
(setq symbol (intern (downcase (symbol-name symbol))))
(or (cdr (assq symbol erc--module-name-migrations)) symbol))
+(defvar erc--inside-mode-toggle-p nil
+ "Non-nil when a mode toggle is updating module membership.
+This serves as a flag to inhibit the mutual recursion that
+otherwise occurs between an ERC-defined minor-mode function, such
+as `erc-services-mode', and the custom-set function for
+`erc-modules'. For historical reasons, the latter calls
+`erc-update-modules', which, in turn, enables the minor-mode
+functions for all member modules.")
+
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
@@ -140,11 +155,13 @@ erc--assemble-toggle
(,ablsym))
(setq ,mode ,val)
,@body)))
- `(,(if val
- `(cl-pushnew ',(erc--normalize-module-symbol name)
- erc-modules)
- `(setq erc-modules (delq ',(erc--normalize-module-symbol name)
- erc-modules)))
+ `((unless erc--inside-mode-toggle-p
+ ,(if val
+ `(cl-pushnew ',(erc--normalize-module-symbol name)
+ erc-modules)
+ `(setq erc-modules
+ (delq ',(erc--normalize-module-symbol name)
+ erc-modules))))
(setq ,mode ,val)
,@body)))))
@@ -179,6 +196,55 @@ erc--find-group
(throw 'found found)))
'erc))
+(defun erc--custom-set-minor-mode (variable value)
+ (let ((name (get variable 'erc-module))
+ (erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules
+ (if value (cl-pushnew name erc-modules) (delq name erc-modules)))
+ (custom-set-minor-mode variable value)))
+
+;; This exists as a separate, top-level function to prevent the byte
+;; compiler from warning about widget-related dependencies not being
+;; loaded at runtime.
+
+(defun erc--tick-module-checkbox (name &rest _) ; `name' must be normalized
+ (customize-variable-other-window 'erc-modules)
+ ;; Move to `erc-modules' section.
+ (while (not (eq (widget-type (widget-at)) 'checkbox))
+ (widget-move 1 t))
+ ;; This search for a checkbox can fail when `name' refers to a
+ ;; third-party module that modifies `erc-modules' (improperly) on
+ ;; load.
+ (let (w)
+ (while (and (eq (widget-type (widget-at)) 'checkbox)
+ (not (and (setq w (widget-get-sibling (widget-at)))
+ (eq (widget-value w) name))))
+ (setq w nil)
+ (widget-move 1 t)) ; the `suppress-echo' arg exists in 27.2
+ (if w
+ (progn (widget-apply-action (widget-at))
+ (message "Hit %s to apply or %s to apply and save."
+ (substitute-command-keys "\\[Custom-set]")
+ (substitute-command-keys "\\[Custom-save]")))
+ (error "Failed to find %s in `erc-modules' checklist" name))))
+
+(defun erc--prepare-custom-module-type (name)
+ `(let* ((name (erc--normalize-module-symbol ',name))
+ (fmtd (format " `%s' " name)))
+ `(boolean
+ :button-face '(error custom-button)
+ :format "%{%t%}: %[Deprecated Toggle%] \n%d\n"
+ :doc ,(concat "Setting a module's minor-mode variable is "
+ (propertize "ineffective" 'face 'error) ".\nPlease add"
+ fmtd "directly to `erc-modules' instead.\nYou 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))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -225,6 +291,8 @@ define-erc-module
%s" name name doc)
:global ,(not local-p)
:group (erc--find-group ',name ,(and alias (list 'quote alias)))
+ ,@(unless local-p '(:set #'erc--custom-set-minor-mode))
+ ,@(unless local-p `(:type ,(erc--prepare-custom-module-type name)))
(if ,mode
(,enable)
(,disable)))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index ff1820cfaf2..5e971f15f5d 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1845,7 +1845,8 @@ erc-modules
(set sym val)
;; this test is for the case where erc hasn't been loaded yet
(when (fboundp 'erc-update-modules)
- (erc-update-modules)))
+ (unless erc--inside-mode-toggle-p
+ (erc-update-modules))))
:type
'(set
:greedy t
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 24a4f9d0639..e254b00454b 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1313,7 +1313,10 @@ define-erc-module--global
((ignore a) (ignore b))
((ignore c) (ignore d)))))
- (should (equal (macroexpand global-module)
+ (should (equal (cl-letf (((symbol-function
+ 'erc--prepare-custom-module-type)
+ #'symbol-name))
+ (macroexpand global-module))
`(progn
(define-minor-mode erc-mname-mode
@@ -1324,6 +1327,8 @@ define-erc-module--global
Some docstring"
:global t
:group (erc--find-group 'mname 'malias)
+ :set #'erc--custom-set-minor-mode
+ :type "mname"
(if erc-mname-mode
(erc-mname-enable)
(erc-mname-disable)))
@@ -1331,14 +1336,16 @@ define-erc-module--global
(defun erc-mname-enable ()
"Enable ERC mname mode."
(interactive)
- (cl-pushnew 'mname erc-modules)
+ (unless erc--inside-mode-toggle-p
+ (cl-pushnew 'mname erc-modules))
(setq erc-mname-mode t)
(ignore a) (ignore b))
(defun erc-mname-disable ()
"Disable ERC mname mode."
(interactive)
- (setq erc-modules (delq 'mname erc-modules))
+ (unless erc--inside-mode-toggle-p
+ (setq erc-modules (delq 'mname erc-modules)))
(setq erc-mname-mode nil)
(ignore c) (ignore d))
--
2.39.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0003-5.6-Fill-doc-strings-for-ERC-modules.patch --]
[-- Type: text/x-patch, Size: 5781 bytes --]
From e4e270a4bf556ce238bc42c8549d4c443f3a969a Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 16 Jan 2023 20:18:32 -0800
Subject: [PATCH 3/3] [5.6] Fill doc strings for ERC modules.
* lisp/erc/erc-common.el (erc--fill-module-docstring): Add helper to
fill doc strings.
(erc--assemble-toggle, define-erc-module): Use helper to fill doc
string.
* test/lisp/erc/erc-tests.el (define-minor-mode--global,
define-minor-mode--local): Adjust expected output for generated doc
strings. (Bug#60935.)
---
lisp/erc/erc-common.el | 20 +++++++++++++++++---
test/lisp/erc/erc-tests.el | 28 ++++++++++++++++------------
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index ec3a7cf9e85..cc278428d5d 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -141,7 +141,7 @@ erc--inside-mode-toggle-p
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
- ,(concat
+ ,(erc--fill-module-docstring
(if val "Enable" "Disable")
" ERC " (symbol-name name) " mode."
(when localp
@@ -245,6 +245,20 @@ erc--prepare-custom-module-type
(if hasp "from" "to") " `erc-modules'.")))
:action ,(apply-partially #'erc--tick-module-checkbox name))))
+(defun erc--fill-module-docstring (&rest strings)
+ (with-temp-buffer
+ (emacs-lisp-mode)
+ (insert "(defun foo ()\n"
+ (format "%S" (apply #'concat strings))
+ "\n(ignore))")
+ (goto-char (point-min))
+ (forward-line 2)
+ (let ((emacs-lisp-docstring-fill-column 65)
+ (sentence-end-double-space t))
+ (fill-paragraph))
+ (goto-char (point-min))
+ (nth 3 (read (current-buffer)))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -284,11 +298,11 @@ define-erc-module
`(progn
(define-minor-mode
,mode
- ,(format "Toggle ERC %S mode.
+ ,(erc--fill-module-docstring (format "Toggle ERC %s mode.
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)
+\n%s" name name doc))
:global ,(not local-p)
:group (erc--find-group ',name ,(and alias (list 'quote alias)))
,@(unless local-p '(:set #'erc--custom-set-minor-mode))
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index e254b00454b..e5db78ef2e1 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1309,7 +1309,7 @@ erc--merge-local-modes
(ert-deftest define-erc-module--global ()
(let ((global-module '(define-erc-module mname malias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d)))))
@@ -1321,10 +1321,11 @@ define-erc-module--global
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global t
:group (erc--find-group 'mname 'malias)
:set #'erc--custom-set-minor-mode
@@ -1359,7 +1360,7 @@ define-erc-module--global
(ert-deftest define-erc-module--local ()
(let* ((global-module '(define-erc-module mname nil ; no alias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d))
'local))
@@ -1371,10 +1372,11 @@ define-erc-module--local
`(progn
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global nil
:group (erc--find-group 'mname nil)
(if erc-mname-mode
@@ -1383,7 +1385,8 @@ define-erc-module--local
(defun erc-mname-enable (&optional ,arg-en)
"Enable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-en
@@ -1395,7 +1398,8 @@ define-erc-module--local
(defun erc-mname-disable (&optional ,arg-dis)
"Disable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-dis
--
2.39.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups
2023-01-18 14:50 bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups J.P.
2023-02-07 15:23 ` J.P.
@ 2023-02-19 15:03 ` J.P.
2023-03-14 13:43 ` J.P.
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: J.P. @ 2023-02-19 15:03 UTC (permalink / raw)
To: 60935; +Cc: emacs-erc
[-- Attachment #1: Type: text/plain, Size: 74 bytes --]
v3. Run custom :set function inside toggles. (Now depends on bug#60954.)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v2-v3.diff --]
[-- Type: text/x-patch, Size: 5384 bytes --]
From 23b193168c03c9d36307986f87f2789cafae9b41 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Thu, 16 Feb 2023 22:51:42 -0800
Subject: [PATCH 0/3] *** NOT A PATCH ***
*** BLURB HERE ***
F. Jason Park (3):
[5.6] Don't associate ERC modules with undefined groups
[5.6] Warn when setting minor-mode vars for ERC modules
[5.6] Fill doc strings for ERC modules.
lisp/erc/erc-capab.el | 1 +
lisp/erc/erc-common.el | 143 +++++++++++++++++++++++++++++++++----
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 84 +++++++++++++++++-----
4 files changed, 200 insertions(+), 31 deletions(-)
Interdiff:
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index cc278428d5d..6d09ac25a0a 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -130,13 +130,14 @@ erc--normalize-module-symbol
(or (cdr (assq symbol erc--module-name-migrations)) symbol))
(defvar erc--inside-mode-toggle-p nil
- "Non-nil when a mode toggle is updating module membership.
-This serves as a flag to inhibit the mutual recursion that
-otherwise occurs between an ERC-defined minor-mode function, such
+ "Non-nil when a module's mode toggle is updating module membership.
+This serves as a flag to inhibit the mutual recursion that would
+otherwise occur between an ERC-defined minor-mode function, such
as `erc-services-mode', and the custom-set function for
`erc-modules'. For historical reasons, the latter calls
`erc-update-modules', which, in turn, enables the minor-mode
-functions for all member modules.")
+functions for all member modules. Also non-nil when a mode's
+widget runs its set function.")
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
@@ -155,23 +156,29 @@ erc--assemble-toggle
(,ablsym))
(setq ,mode ,val)
,@body)))
- `((unless erc--inside-mode-toggle-p
- ,(if val
- `(cl-pushnew ',(erc--normalize-module-symbol name)
- erc-modules)
- `(setq erc-modules
- (delq ',(erc--normalize-module-symbol name)
- erc-modules))))
+ ;; No need for `default-value', etc. because a buffer-local
+ ;; `erc-modules' only influences the next session and
+ ;; doesn't survive the major-mode reset that soon follows.
+ `((unless
+ (or erc--inside-mode-toggle-p
+ ,@(let ((v `(memq ',(erc--normalize-module-symbol name)
+ erc-modules)))
+ `(,(if val v `(not ,v)))))
+ (let ((erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules (,(if val 'cons 'delq)
+ ',(erc--normalize-module-symbol name)
+ erc-modules))))
(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
+;; 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 where this fails and where the catch-all of `erc'
-;; is inappropriate, (global) modules can declare a top-level
+;; 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)
;;
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index e5db78ef2e1..8e41d428ce9 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1337,16 +1337,22 @@ define-erc-module--global
(defun erc-mname-enable ()
"Enable ERC mname mode."
(interactive)
- (unless erc--inside-mode-toggle-p
- (cl-pushnew 'mname erc-modules))
+ (unless (or erc--inside-mode-toggle-p
+ (memq 'mname erc-modules))
+ (let ((erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules (cons 'mname erc-modules))))
(setq erc-mname-mode t)
(ignore a) (ignore b))
(defun erc-mname-disable ()
"Disable ERC mname mode."
(interactive)
- (unless erc--inside-mode-toggle-p
- (setq erc-modules (delq 'mname erc-modules)))
+ (unless (or erc--inside-mode-toggle-p
+ (not (memq 'mname erc-modules)))
+ (let ((erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules (delq 'mname erc-modules))))
(setq erc-mname-mode nil)
(ignore c) (ignore d))
--
2.39.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-5.6-Don-t-associate-ERC-modules-with-undefined-group.patch --]
[-- Type: text/x-patch, Size: 7145 bytes --]
From f976c06bd1bff9921ecef743076d0c4c5cfeb9fd Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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 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 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' group 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 @@ erc-capab-identify-unidentified
;;; 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 994555acecf..1503dc99d72 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -148,6 +148,37 @@ 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 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.
@@ -182,7 +213,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 +223,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/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 40a2d2de657..24a4f9d0639 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1209,6 +1209,39 @@ 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) "")
+ ((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
@@ -1290,7 +1323,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 +1369,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.39.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-5.6-Warn-when-setting-minor-mode-vars-for-ERC-module.patch --]
[-- Type: text/x-patch, Size: 10412 bytes --]
From c790bfcac0275222cc5d54c95db4717bb67c6bfc Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sat, 14 Jan 2023 19:08:11 -0800
Subject: [PATCH 2/3] [5.6] Warn when setting minor-mode vars for ERC modules
(erc--inside-mode-toggle-p): Add global var to inhibit mode toggles
from being run by `erc-update-modules'. It must be non-nil inside
custom-set functions for mode toggles created by `define-erc-module'.
(erc--assemble-toggle): Don't modify `erc-modules' when run from
custom-set function.
(erc--custom-set-minor-mode): Add new custom-set function for module
minor modes. Update `erc-modules' before calling a minor-mode toggle
via `custom-set-minor-mode'.
(erc--tick-module-checkbox): Add helper to toggle the appropriate
checkbox in the `erc-modules' widget when a user interactively toggles
a minor-mode state variable.
(erc--prepare-custom-module-type): Create spec for minor-mode custom
`:type', deferring various aspects until module-definition time.
(define-erc-module): Add `:set' and `:type' keywords for global
modules.
* lisp/erc/erc.el (erc-modules): Inhibit `erc-update-modules' when run
from a minor-mode toggle's custom-set function.
* test/lisp/erc/erc-tests.el
(define-erc-module--global, define-erc-module--local): Update
`erc-modules' mutations with `erc--inside-mode-toggle-p' guard
conditions. (Bug#60935.)
---
lisp/erc/erc-common.el | 85 +++++++++++++++++++++++++++++++++++---
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 19 +++++++--
3 files changed, 98 insertions(+), 9 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 1503dc99d72..bc67bf94a67 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -29,6 +29,7 @@
(defvar erc--casemapping-rfc1459)
(defvar erc--casemapping-rfc1459-strict)
(defvar erc-channel-users)
+(defvar erc-modules)
(defvar erc-dbuf)
(defvar erc-log-p)
(defvar erc-server-users)
@@ -37,6 +38,11 @@ erc-session-server
(declare-function erc--get-isupport-entry "erc-backend" (key &optional single))
(declare-function erc-get-buffer "erc" (target &optional proc))
(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-get-sibling "wid-edit" (widget))
+(declare-function widget-move "wid-edit" (arg &optional suppress-echo))
+(declare-function widget-type "wid-edit" (widget))
(cl-defstruct erc-input
string insertp sendp)
@@ -123,6 +129,16 @@ erc--normalize-module-symbol
(setq symbol (intern (downcase (symbol-name symbol))))
(or (cdr (assq symbol erc--module-name-migrations)) symbol))
+(defvar erc--inside-mode-toggle-p nil
+ "Non-nil when a module's mode toggle is updating module membership.
+This serves as a flag to inhibit the mutual recursion that would
+otherwise occur between an ERC-defined minor-mode function, such
+as `erc-services-mode', and the custom-set function for
+`erc-modules'. For historical reasons, the latter calls
+`erc-update-modules', which, in turn, enables the minor-mode
+functions for all member modules. Also non-nil when a mode's
+widget runs its set function.")
+
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
@@ -140,11 +156,19 @@ erc--assemble-toggle
(,ablsym))
(setq ,mode ,val)
,@body)))
- `(,(if val
- `(cl-pushnew ',(erc--normalize-module-symbol name)
- erc-modules)
- `(setq erc-modules (delq ',(erc--normalize-module-symbol name)
- erc-modules)))
+ ;; No need for `default-value', etc. because a buffer-local
+ ;; `erc-modules' only influences the next session and
+ ;; doesn't survive the major-mode reset that soon follows.
+ `((unless
+ (or erc--inside-mode-toggle-p
+ ,@(let ((v `(memq ',(erc--normalize-module-symbol name)
+ erc-modules)))
+ `(,(if val v `(not ,v)))))
+ (let ((erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules (,(if val 'cons 'delq)
+ ',(erc--normalize-module-symbol name)
+ erc-modules))))
(setq ,mode ,val)
,@body)))))
@@ -179,6 +203,55 @@ erc--find-group
(throw 'found found)))
'erc))
+(defun erc--custom-set-minor-mode (variable value)
+ (let ((name (get variable 'erc-module))
+ (erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules
+ (if value (cl-pushnew name erc-modules) (delq name erc-modules)))
+ (custom-set-minor-mode variable value)))
+
+;; This exists as a separate, top-level function to prevent the byte
+;; compiler from warning about widget-related dependencies not being
+;; loaded at runtime.
+
+(defun erc--tick-module-checkbox (name &rest _) ; `name' must be normalized
+ (customize-variable-other-window 'erc-modules)
+ ;; Move to `erc-modules' section.
+ (while (not (eq (widget-type (widget-at)) 'checkbox))
+ (widget-move 1 t))
+ ;; This search for a checkbox can fail when `name' refers to a
+ ;; third-party module that modifies `erc-modules' (improperly) on
+ ;; load.
+ (let (w)
+ (while (and (eq (widget-type (widget-at)) 'checkbox)
+ (not (and (setq w (widget-get-sibling (widget-at)))
+ (eq (widget-value w) name))))
+ (setq w nil)
+ (widget-move 1 t)) ; the `suppress-echo' arg exists in 27.2
+ (if w
+ (progn (widget-apply-action (widget-at))
+ (message "Hit %s to apply or %s to apply and save."
+ (substitute-command-keys "\\[Custom-set]")
+ (substitute-command-keys "\\[Custom-save]")))
+ (error "Failed to find %s in `erc-modules' checklist" name))))
+
+(defun erc--prepare-custom-module-type (name)
+ `(let* ((name (erc--normalize-module-symbol ',name))
+ (fmtd (format " `%s' " name)))
+ `(boolean
+ :button-face '(error custom-button)
+ :format "%{%t%}: %[Deprecated Toggle%] \n%d\n"
+ :doc ,(concat "Setting a module's minor-mode variable is "
+ (propertize "ineffective" 'face 'error) ".\nPlease add"
+ fmtd "directly to `erc-modules' instead.\nYou 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))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -225,6 +298,8 @@ define-erc-module
%s" name name doc)
:global ,(not local-p)
:group (erc--find-group ',name ,(and alias (list 'quote alias)))
+ ,@(unless local-p '(:set #'erc--custom-set-minor-mode))
+ ,@(unless local-p `(:type ,(erc--prepare-custom-module-type name)))
(if ,mode
(,enable)
(,disable)))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index d35907a1677..0087c7a09a1 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1845,7 +1845,8 @@ erc-modules
(set sym val)
;; this test is for the case where erc hasn't been loaded yet
(when (fboundp 'erc-update-modules)
- (erc-update-modules)))
+ (unless erc--inside-mode-toggle-p
+ (erc-update-modules))))
:type
'(set
:greedy t
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 24a4f9d0639..26d986823b3 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1313,7 +1313,10 @@ define-erc-module--global
((ignore a) (ignore b))
((ignore c) (ignore d)))))
- (should (equal (macroexpand global-module)
+ (should (equal (cl-letf (((symbol-function
+ 'erc--prepare-custom-module-type)
+ #'symbol-name))
+ (macroexpand global-module))
`(progn
(define-minor-mode erc-mname-mode
@@ -1324,6 +1327,8 @@ define-erc-module--global
Some docstring"
:global t
:group (erc--find-group 'mname 'malias)
+ :set #'erc--custom-set-minor-mode
+ :type "mname"
(if erc-mname-mode
(erc-mname-enable)
(erc-mname-disable)))
@@ -1331,14 +1336,22 @@ define-erc-module--global
(defun erc-mname-enable ()
"Enable ERC mname mode."
(interactive)
- (cl-pushnew 'mname erc-modules)
+ (unless (or erc--inside-mode-toggle-p
+ (memq 'mname erc-modules))
+ (let ((erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules (cons 'mname erc-modules))))
(setq erc-mname-mode t)
(ignore a) (ignore b))
(defun erc-mname-disable ()
"Disable ERC mname mode."
(interactive)
- (setq erc-modules (delq 'mname erc-modules))
+ (unless (or erc--inside-mode-toggle-p
+ (not (memq 'mname erc-modules)))
+ (let ((erc--inside-mode-toggle-p t))
+ (customize-set-variable
+ 'erc-modules (delq 'mname erc-modules))))
(setq erc-mname-mode nil)
(ignore c) (ignore d))
--
2.39.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0003-5.6-Fill-doc-strings-for-ERC-modules.patch --]
[-- Type: text/x-patch, Size: 5781 bytes --]
From 23b193168c03c9d36307986f87f2789cafae9b41 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 16 Jan 2023 20:18:32 -0800
Subject: [PATCH 3/3] [5.6] Fill doc strings for ERC modules.
* lisp/erc/erc-common.el (erc--fill-module-docstring): Add helper to
fill doc strings.
(erc--assemble-toggle, define-erc-module): Use helper to fill doc
string.
* test/lisp/erc/erc-tests.el (define-minor-mode--global,
define-minor-mode--local): Adjust expected output for generated doc
strings. (Bug#60935.)
---
lisp/erc/erc-common.el | 20 +++++++++++++++++---
test/lisp/erc/erc-tests.el | 28 ++++++++++++++++------------
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index bc67bf94a67..6d09ac25a0a 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -142,7 +142,7 @@ erc--inside-mode-toggle-p
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
- ,(concat
+ ,(erc--fill-module-docstring
(if val "Enable" "Disable")
" ERC " (symbol-name name) " mode."
(when localp
@@ -252,6 +252,20 @@ erc--prepare-custom-module-type
(if hasp "from" "to") " `erc-modules'.")))
:action ,(apply-partially #'erc--tick-module-checkbox name))))
+(defun erc--fill-module-docstring (&rest strings)
+ (with-temp-buffer
+ (emacs-lisp-mode)
+ (insert "(defun foo ()\n"
+ (format "%S" (apply #'concat strings))
+ "\n(ignore))")
+ (goto-char (point-min))
+ (forward-line 2)
+ (let ((emacs-lisp-docstring-fill-column 65)
+ (sentence-end-double-space t))
+ (fill-paragraph))
+ (goto-char (point-min))
+ (nth 3 (read (current-buffer)))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -291,11 +305,11 @@ define-erc-module
`(progn
(define-minor-mode
,mode
- ,(format "Toggle ERC %S mode.
+ ,(erc--fill-module-docstring (format "Toggle ERC %s mode.
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)
+\n%s" name name doc))
:global ,(not local-p)
:group (erc--find-group ',name ,(and alias (list 'quote alias)))
,@(unless local-p '(:set #'erc--custom-set-minor-mode))
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 26d986823b3..8e41d428ce9 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1309,7 +1309,7 @@ erc--merge-local-modes
(ert-deftest define-erc-module--global ()
(let ((global-module '(define-erc-module mname malias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d)))))
@@ -1321,10 +1321,11 @@ define-erc-module--global
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global t
:group (erc--find-group 'mname 'malias)
:set #'erc--custom-set-minor-mode
@@ -1365,7 +1366,7 @@ define-erc-module--global
(ert-deftest define-erc-module--local ()
(let* ((global-module '(define-erc-module mname nil ; no alias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d))
'local))
@@ -1377,10 +1378,11 @@ define-erc-module--local
`(progn
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global nil
:group (erc--find-group 'mname nil)
(if erc-mname-mode
@@ -1389,7 +1391,8 @@ define-erc-module--local
(defun erc-mname-enable (&optional ,arg-en)
"Enable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-en
@@ -1401,7 +1404,8 @@ define-erc-module--local
(defun erc-mname-disable (&optional ,arg-dis)
"Disable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-dis
--
2.39.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups
2023-01-18 14:50 bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups J.P.
2023-02-07 15:23 ` J.P.
2023-02-19 15:03 ` J.P.
@ 2023-03-14 13:43 ` J.P.
2023-03-15 14:05 ` J.P.
[not found] ` <87edpqglf1.fsf@neverwas.me>
4 siblings, 0 replies; 7+ messages in thread
From: J.P. @ 2023-03-14 13:43 UTC (permalink / raw)
To: 60935; +Cc: emacs-erc
[-- Attachment #1: Type: text/plain, Size: 192 bytes --]
v4. Use `custom-set-variables' instead of `customize-set-variable'.
Munge `standard-value' of deprecated `erc-foo-mode' variables when
called by `erc-update-modules' (but not interactively).
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v3-v4.diff --]
[-- Type: text/x-patch, Size: 4065 bytes --]
From a6662c04e156b1dd37661991a045acaf3c5cb9b4 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 14 Mar 2023 06:25:32 -0700
Subject: [PATCH 0/3] *** NOT A PATCH ***
*** BLURB HERE ***
F. Jason Park (3):
[5.6] Don't associate ERC modules with undefined groups
[5.6] Warn when setting minor-mode vars for ERC modules
[5.6] Fill doc strings for ERC modules.
lisp/erc/erc-capab.el | 1 +
lisp/erc/erc-common.el | 146 +++++++++++++++++++++++++++++++++----
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 88 +++++++++++++++++-----
4 files changed, 207 insertions(+), 31 deletions(-)
Interdiff:
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 4332ed00cfb..522803b91e2 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -162,11 +162,14 @@ erc--assemble-toggle
erc-modules)))
`(,(if val v `(not ,v)))))
(let ((erc--inside-mode-toggle-p t))
- (customize-set-variable
- 'erc-modules (,(if val 'cons 'delq)
- ',(erc--normalize-module-symbol name)
- erc-modules))))
+ (custom-set-variables
+ `(erc-modules ',(,(if val 'cons 'delq)
+ ',(erc--normalize-module-symbol name)
+ erc-modules)))))
(setq ,mode ,val)
+ ;; Avoid "changed" state from `erc-update-modules'
+ (unless (called-interactively-p 'any)
+ (put ',mode 'standard-value (list ,val)))
,@body)))))
;; This is a migration helper that determines a module's `:group'
@@ -203,9 +206,9 @@ erc--find-group
(defun erc--custom-set-minor-mode (variable value)
(let ((name (get variable 'erc-module))
(erc--inside-mode-toggle-p t))
- (customize-set-variable
- 'erc-modules
- (if value (cl-pushnew name erc-modules) (delq name erc-modules)))
+ (custom-set-variables
+ `(erc-modules
+ ',(if value (cl-pushnew name erc-modules) (delq name erc-modules))))
(custom-set-minor-mode variable value)))
;; This exists as a separate, top-level function to prevent the byte
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 62480c0604e..ef742c853d6 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1340,9 +1340,11 @@ define-erc-module--global
(unless (or erc--inside-mode-toggle-p
(memq 'mname erc-modules))
(let ((erc--inside-mode-toggle-p t))
- (customize-set-variable
- 'erc-modules (cons 'mname erc-modules))))
+ (custom-set-variables
+ `(erc-modules ',(cons 'mname erc-modules)))))
(setq erc-mname-mode t)
+ (unless (called-interactively-p 'any)
+ (put 'erc-mname-mode 'standard-value (list t)))
(ignore a) (ignore b))
(defun erc-mname-disable ()
@@ -1351,9 +1353,11 @@ define-erc-module--global
(unless (or erc--inside-mode-toggle-p
(not (memq 'mname erc-modules)))
(let ((erc--inside-mode-toggle-p t))
- (customize-set-variable
- 'erc-modules (delq 'mname erc-modules))))
+ (custom-set-variables
+ `(erc-modules ',(delq 'mname erc-modules)))))
(setq erc-mname-mode nil)
+ (unless (called-interactively-p 'any)
+ (put 'erc-mname-mode 'standard-value (list nil)))
(ignore c) (ignore d))
(defalias 'erc-malias-mode #'erc-mname-mode)
--
2.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-5.6-Don-t-associate-ERC-modules-with-undefined-group.patch --]
[-- Type: text/x-patch, Size: 7145 bytes --]
From 12dd0075b751a9dbdfae57a500425f177158bb73 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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 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 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' group 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 @@ erc-capab-identify-unidentified
;;; 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 0279b0a0bc4..0eabd3a2fe9 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -145,6 +145,37 @@ 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 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.
@@ -179,7 +210,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
@@ -190,10 +220,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/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index d6c63934163..13ef99be167 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1209,6 +1209,39 @@ 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) "")
+ ((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
@@ -1290,7 +1323,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 +1369,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.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-5.6-Warn-when-setting-minor-mode-vars-for-ERC-module.patch --]
[-- Type: text/x-patch, Size: 11018 bytes --]
From 764dc63582a440495ab53a9986003d5d1a794bcf Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sat, 14 Jan 2023 19:08:11 -0800
Subject: [PATCH 2/3] [5.6] Warn when setting minor-mode vars for ERC modules
(erc--inside-mode-toggle-p): Add global var to inhibit mode toggles
from being run by `erc-update-modules'. It must be non-nil inside
custom-set functions for mode toggles created by `define-erc-module'.
(erc--assemble-toggle): Don't modify `erc-modules' when run from
custom-set function.
(erc--custom-set-minor-mode): Add new custom-set function for module
minor modes. Update `erc-modules' before calling a minor-mode toggle
via `custom-set-minor-mode'.
(erc--tick-module-checkbox): Add helper to toggle the appropriate
checkbox in the `erc-modules' widget when a user interactively toggles
a minor-mode state variable.
(erc--prepare-custom-module-type): Create spec for minor-mode custom
`:type', deferring various aspects until module-definition time.
(define-erc-module): Add `:set' and `:type' keywords for global
modules.
* lisp/erc/erc.el (erc-modules): Inhibit `erc-update-modules' when run
from a minor-mode toggle's custom-set function.
* test/lisp/erc/erc-tests.el
(define-erc-module--global, define-erc-module--local): Update
`erc-modules' mutations with `erc--inside-mode-toggle-p' guard
conditions. (Bug#60935.)
---
lisp/erc/erc-common.el | 88 +++++++++++++++++++++++++++++++++++---
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 23 ++++++++--
3 files changed, 105 insertions(+), 9 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 0eabd3a2fe9..cd0fc79e568 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -29,6 +29,7 @@
(defvar erc--casemapping-rfc1459)
(defvar erc--casemapping-rfc1459-strict)
(defvar erc-channel-users)
+(defvar erc-modules)
(defvar erc-dbuf)
(defvar erc-log-p)
(defvar erc-server-users)
@@ -37,6 +38,11 @@ erc-session-server
(declare-function erc--get-isupport-entry "erc-backend" (key &optional single))
(declare-function erc-get-buffer "erc" (target &optional proc))
(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-get-sibling "wid-edit" (widget))
+(declare-function widget-move "wid-edit" (arg &optional suppress-echo))
+(declare-function widget-type "wid-edit" (widget))
(cl-defstruct erc-input
string insertp sendp)
@@ -120,6 +126,16 @@ erc--normalize-module-symbol
(setq symbol (intern (downcase (symbol-name symbol))))
(or (cdr (assq symbol erc--module-name-migrations)) symbol))
+(defvar erc--inside-mode-toggle-p nil
+ "Non-nil when a module's mode toggle is updating module membership.
+This serves as a flag to inhibit the mutual recursion that would
+otherwise occur between an ERC-defined minor-mode function, such
+as `erc-services-mode', and the custom-set function for
+`erc-modules'. For historical reasons, the latter calls
+`erc-update-modules', which, in turn, enables the minor-mode
+functions for all member modules. Also non-nil when a mode's
+widget runs its set function.")
+
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
@@ -137,12 +153,23 @@ erc--assemble-toggle
(,ablsym))
(setq ,mode ,val)
,@body)))
- `(,(if val
- `(cl-pushnew ',(erc--normalize-module-symbol name)
- erc-modules)
- `(setq erc-modules (delq ',(erc--normalize-module-symbol name)
- erc-modules)))
+ ;; No need for `default-value', etc. because a buffer-local
+ ;; `erc-modules' only influences the next session and
+ ;; doesn't survive the major-mode reset that soon follows.
+ `((unless
+ (or erc--inside-mode-toggle-p
+ ,@(let ((v `(memq ',(erc--normalize-module-symbol name)
+ erc-modules)))
+ `(,(if val v `(not ,v)))))
+ (let ((erc--inside-mode-toggle-p t))
+ (custom-set-variables
+ `(erc-modules ',(,(if val 'cons 'delq)
+ ',(erc--normalize-module-symbol name)
+ erc-modules)))))
(setq ,mode ,val)
+ ;; Avoid "changed" state from `erc-update-modules'
+ (unless (called-interactively-p 'any)
+ (put ',mode 'standard-value (list ,val)))
,@body)))))
;; This is a migration helper that determines a module's `:group'
@@ -176,6 +203,55 @@ erc--find-group
(throw 'found found)))
'erc))
+(defun erc--custom-set-minor-mode (variable value)
+ (let ((name (get variable 'erc-module))
+ (erc--inside-mode-toggle-p t))
+ (custom-set-variables
+ `(erc-modules
+ ',(if value (cl-pushnew name erc-modules) (delq name erc-modules))))
+ (custom-set-minor-mode variable value)))
+
+;; This exists as a separate, top-level function to prevent the byte
+;; compiler from warning about widget-related dependencies not being
+;; loaded at runtime.
+
+(defun erc--tick-module-checkbox (name &rest _) ; `name' must be normalized
+ (customize-variable-other-window 'erc-modules)
+ ;; Move to `erc-modules' section.
+ (while (not (eq (widget-type (widget-at)) 'checkbox))
+ (widget-move 1 t))
+ ;; This search for a checkbox can fail when `name' refers to a
+ ;; third-party module that modifies `erc-modules' (improperly) on
+ ;; load.
+ (let (w)
+ (while (and (eq (widget-type (widget-at)) 'checkbox)
+ (not (and (setq w (widget-get-sibling (widget-at)))
+ (eq (widget-value w) name))))
+ (setq w nil)
+ (widget-move 1 t)) ; the `suppress-echo' arg exists in 27.2
+ (if w
+ (progn (widget-apply-action (widget-at))
+ (message "Hit %s to apply or %s to apply and save."
+ (substitute-command-keys "\\[Custom-set]")
+ (substitute-command-keys "\\[Custom-save]")))
+ (error "Failed to find %s in `erc-modules' checklist" name))))
+
+(defun erc--prepare-custom-module-type (name)
+ `(let* ((name (erc--normalize-module-symbol ',name))
+ (fmtd (format " `%s' " name)))
+ `(boolean
+ :button-face '(error custom-button)
+ :format "%{%t%}: %[Deprecated Toggle%] \n%d\n"
+ :doc ,(concat "Setting a module's minor-mode variable is "
+ (propertize "ineffective" 'face 'error) ".\nPlease add"
+ fmtd "directly to `erc-modules' instead.\nYou 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))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -222,6 +298,8 @@ define-erc-module
%s" name name doc)
:global ,(not local-p)
:group (erc--find-group ',name ,(and alias (list 'quote alias)))
+ ,@(unless local-p '(:set #'erc--custom-set-minor-mode))
+ ,@(unless local-p `(:type ,(erc--prepare-custom-module-type name)))
(if ,mode
(,enable)
(,disable)))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 69bdb5d71b1..59ab1f1eab3 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1846,7 +1846,8 @@ erc-modules
(set sym val)
;; this test is for the case where erc hasn't been loaded yet
(when (fboundp 'erc-update-modules)
- (erc-update-modules)))
+ (unless erc--inside-mode-toggle-p
+ (erc-update-modules))))
:type
'(set
:greedy t
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 13ef99be167..a31c2c530a3 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1313,7 +1313,10 @@ define-erc-module--global
((ignore a) (ignore b))
((ignore c) (ignore d)))))
- (should (equal (macroexpand global-module)
+ (should (equal (cl-letf (((symbol-function
+ 'erc--prepare-custom-module-type)
+ #'symbol-name))
+ (macroexpand global-module))
`(progn
(define-minor-mode erc-mname-mode
@@ -1324,6 +1327,8 @@ define-erc-module--global
Some docstring"
:global t
:group (erc--find-group 'mname 'malias)
+ :set #'erc--custom-set-minor-mode
+ :type "mname"
(if erc-mname-mode
(erc-mname-enable)
(erc-mname-disable)))
@@ -1331,15 +1336,27 @@ define-erc-module--global
(defun erc-mname-enable ()
"Enable ERC mname mode."
(interactive)
- (cl-pushnew 'mname erc-modules)
+ (unless (or erc--inside-mode-toggle-p
+ (memq 'mname erc-modules))
+ (let ((erc--inside-mode-toggle-p t))
+ (custom-set-variables
+ `(erc-modules ',(cons 'mname erc-modules)))))
(setq erc-mname-mode t)
+ (unless (called-interactively-p 'any)
+ (put 'erc-mname-mode 'standard-value (list t)))
(ignore a) (ignore b))
(defun erc-mname-disable ()
"Disable ERC mname mode."
(interactive)
- (setq erc-modules (delq 'mname erc-modules))
+ (unless (or erc--inside-mode-toggle-p
+ (not (memq 'mname erc-modules)))
+ (let ((erc--inside-mode-toggle-p t))
+ (custom-set-variables
+ `(erc-modules ',(delq 'mname erc-modules)))))
(setq erc-mname-mode nil)
+ (unless (called-interactively-p 'any)
+ (put 'erc-mname-mode 'standard-value (list nil)))
(ignore c) (ignore d))
(defalias 'erc-malias-mode #'erc-mname-mode)
--
2.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0003-5.6-Fill-doc-strings-for-ERC-modules.patch --]
[-- Type: text/x-patch, Size: 5781 bytes --]
From a6662c04e156b1dd37661991a045acaf3c5cb9b4 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 16 Jan 2023 20:18:32 -0800
Subject: [PATCH 3/3] [5.6] Fill doc strings for ERC modules.
* lisp/erc/erc-common.el (erc--fill-module-docstring): Add helper to
fill doc strings.
(erc--assemble-toggle, define-erc-module): Use helper to fill doc
string.
* test/lisp/erc/erc-tests.el (define-minor-mode--global,
define-minor-mode--local): Adjust expected output for generated doc
strings. (Bug#60935.)
---
lisp/erc/erc-common.el | 20 +++++++++++++++++---
test/lisp/erc/erc-tests.el | 28 ++++++++++++++++------------
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index cd0fc79e568..522803b91e2 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -139,7 +139,7 @@ erc--inside-mode-toggle-p
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
- ,(concat
+ ,(erc--fill-module-docstring
(if val "Enable" "Disable")
" ERC " (symbol-name name) " mode."
(when localp
@@ -252,6 +252,20 @@ erc--prepare-custom-module-type
(if hasp "from" "to") " `erc-modules'.")))
:action ,(apply-partially #'erc--tick-module-checkbox name))))
+(defun erc--fill-module-docstring (&rest strings)
+ (with-temp-buffer
+ (emacs-lisp-mode)
+ (insert "(defun foo ()\n"
+ (format "%S" (apply #'concat strings))
+ "\n(ignore))")
+ (goto-char (point-min))
+ (forward-line 2)
+ (let ((emacs-lisp-docstring-fill-column 65)
+ (sentence-end-double-space t))
+ (fill-paragraph))
+ (goto-char (point-min))
+ (nth 3 (read (current-buffer)))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -291,11 +305,11 @@ define-erc-module
`(progn
(define-minor-mode
,mode
- ,(format "Toggle ERC %S mode.
+ ,(erc--fill-module-docstring (format "Toggle ERC %s mode.
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)
+\n%s" name name doc))
:global ,(not local-p)
:group (erc--find-group ',name ,(and alias (list 'quote alias)))
,@(unless local-p '(:set #'erc--custom-set-minor-mode))
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index a31c2c530a3..ef742c853d6 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1309,7 +1309,7 @@ erc--merge-local-modes
(ert-deftest define-erc-module--global ()
(let ((global-module '(define-erc-module mname malias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d)))))
@@ -1321,10 +1321,11 @@ define-erc-module--global
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global t
:group (erc--find-group 'mname 'malias)
:set #'erc--custom-set-minor-mode
@@ -1369,7 +1370,7 @@ define-erc-module--global
(ert-deftest define-erc-module--local ()
(let* ((global-module '(define-erc-module mname nil ; no alias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d))
'local))
@@ -1381,10 +1382,11 @@ define-erc-module--local
`(progn
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global nil
:group (erc--find-group 'mname nil)
(if erc-mname-mode
@@ -1393,7 +1395,8 @@ define-erc-module--local
(defun erc-mname-enable (&optional ,arg-en)
"Enable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-en
@@ -1405,7 +1408,8 @@ define-erc-module--local
(defun erc-mname-disable (&optional ,arg-dis)
"Disable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-dis
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups
2023-01-18 14:50 bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups J.P.
` (2 preceding siblings ...)
2023-03-14 13:43 ` J.P.
@ 2023-03-15 14:05 ` J.P.
[not found] ` <87edpqglf1.fsf@neverwas.me>
4 siblings, 0 replies; 7+ messages in thread
From: J.P. @ 2023-03-15 14:05 UTC (permalink / raw)
To: 60935; +Cc: emacs-erc
[-- Attachment #1: Type: text/plain, Size: 166 bytes --]
v5. Be more responsible with custom var states. Allow CHANGED for
`erc-modules' but use getter to spoof STANDARD for deprecated mode vars
so widgets stay collapsed.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v4-v5.diff --]
[-- Type: text/x-patch, Size: 7435 bytes --]
From e8a6cbfaffb91019fa9a4a9aa3cae3c7896a6ff5 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Wed, 15 Mar 2023 06:39:32 -0700
Subject: [PATCH 0/3] *** NOT A PATCH ***
v5. Don't touch `standard-value'. Instead, favor a "CHANGED" state for
`erc-modules'. Use a custom getter to spoof a "STANDARD" state for all module
mode vars so their widgets remain dormant and only protest when disturbed.
F. Jason Park (3):
[5.6] Don't associate ERC modules with undefined groups
[5.6] Warn when setting minor-mode vars for ERC modules
[5.6] Fill doc strings for ERC modules.
lisp/erc/erc-capab.el | 1 +
lisp/erc/erc-common.el | 158 ++++++++++++++++++++++++++++++++++---
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 82 +++++++++++++++----
4 files changed, 213 insertions(+), 31 deletions(-)
Interdiff:
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 522803b91e2..3a148dc1196 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -162,14 +162,20 @@ erc--assemble-toggle
erc-modules)))
`(,(if val v `(not ,v)))))
(let ((erc--inside-mode-toggle-p t))
- (custom-set-variables
- `(erc-modules ',(,(if val 'cons 'delq)
- ',(erc--normalize-module-symbol name)
- erc-modules)))))
+ ;; Make the widget show "CHANGED outside Customize."
+ ;; Ideally, it would show "SET for current session"
+ ;; instead, but using `customize-set-variable' here
+ ;; or calling `customize-mark-as-set' afterward isn't
+ ;; enough if `customized-value' and `standard-value'
+ ;; match but differ from `saved-value' because the
+ ;; widget will see a `standard' instead of a `set'
+ ;; state. And clicking "apply and save" won't update
+ ;; `custom-file'. See bug#12864 "Make state button
+ ;; interaction less confusing".
+ (setopt erc-modules (,(if val 'cons 'delq)
+ ',(erc--normalize-module-symbol name)
+ erc-modules))))
(setq ,mode ,val)
- ;; Avoid "changed" state from `erc-update-modules'
- (unless (called-interactively-p 'any)
- (put ',mode 'standard-value (list ,val)))
,@body)))))
;; This is a migration helper that determines a module's `:group'
@@ -203,13 +209,15 @@ erc--find-group
(throw 'found found)))
'erc))
-(defun erc--custom-set-minor-mode (variable value)
- (let ((name (get variable 'erc-module))
- (erc--inside-mode-toggle-p t))
- (custom-set-variables
- `(erc-modules
- ',(if value (cl-pushnew name erc-modules) (delq name erc-modules))))
- (custom-set-minor-mode variable value)))
+(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
@@ -240,12 +248,16 @@ erc--prepare-custom-module-type
`(let* ((name (erc--normalize-module-symbol ',name))
(fmtd (format " `%s' " name)))
`(boolean
- :button-face '(error custom-button)
- :format "%{%t%}: %[Deprecated Toggle%] \n%d\n"
- :doc ,(concat "Setting a module's minor-mode variable is "
- (propertize "ineffective" 'face 'error) ".\nPlease add"
- fmtd "directly to `erc-modules' instead.\nYou can do so"
- " now by clicking the scary button above.")
+ :button-face '(custom-variable-obsolete custom-button)
+ :format "%{%t%}: %[Deprecated Toggle%] \n%h\n"
+ :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
@@ -312,7 +324,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 '(:set #'erc--custom-set-minor-mode))
+ ,@(unless local-p '(:get #'erc--neuter-custom-variable-state))
,@(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 ef742c853d6..baf6826ff97 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1328,7 +1328,7 @@ define-erc-module--global
Some docstring."
:global t
:group (erc--find-group 'mname 'malias)
- :set #'erc--custom-set-minor-mode
+ :get #'erc--neuter-custom-variable-state
:type "mname"
(if erc-mname-mode
(erc-mname-enable)
@@ -1340,11 +1340,8 @@ define-erc-module--global
(unless (or erc--inside-mode-toggle-p
(memq 'mname erc-modules))
(let ((erc--inside-mode-toggle-p t))
- (custom-set-variables
- `(erc-modules ',(cons 'mname erc-modules)))))
+ (setopt erc-modules (cons 'mname erc-modules))))
(setq erc-mname-mode t)
- (unless (called-interactively-p 'any)
- (put 'erc-mname-mode 'standard-value (list t)))
(ignore a) (ignore b))
(defun erc-mname-disable ()
@@ -1353,11 +1350,8 @@ define-erc-module--global
(unless (or erc--inside-mode-toggle-p
(not (memq 'mname erc-modules)))
(let ((erc--inside-mode-toggle-p t))
- (custom-set-variables
- `(erc-modules ',(delq 'mname erc-modules)))))
+ (setopt erc-modules (delq 'mname erc-modules))))
(setq erc-mname-mode nil)
- (unless (called-interactively-p 'any)
- (put 'erc-mname-mode 'standard-value (list nil)))
(ignore c) (ignore d))
(defalias 'erc-malias-mode #'erc-mname-mode)
--
2.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-5.6-Don-t-associate-ERC-modules-with-undefined-group.patch --]
[-- Type: text/x-patch, Size: 7145 bytes --]
From d2ebcb79a86895ada512ce466174d4c0cd7e27e2 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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 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 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' group 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 @@ erc-capab-identify-unidentified
;;; 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 0279b0a0bc4..0eabd3a2fe9 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -145,6 +145,37 @@ 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 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.
@@ -179,7 +210,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
@@ -190,10 +220,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/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index d6c63934163..13ef99be167 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1209,6 +1209,39 @@ 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) "")
+ ((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
@@ -1290,7 +1323,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 +1369,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.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-5.6-Warn-when-setting-minor-mode-vars-for-ERC-module.patch --]
[-- Type: text/x-patch, Size: 11725 bytes --]
From eaaebc28c33300ec0b7e54b92076b83bc09923eb Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sat, 14 Jan 2023 19:08:11 -0800
Subject: [PATCH 2/3] [5.6] Warn when setting minor-mode vars for ERC modules
(erc--inside-mode-toggle-p): Add global var to inhibit mode toggles
from being run by `erc-update-modules'. It must be non-nil inside
custom-set functions for mode toggles created by `define-erc-module'.
(erc--assemble-toggle): Don't modify `erc-modules' when run from
custom-set function.
(erc--neuter-custom-variable-state): Add new function to serve as a
phony getter that deceives Customize into thinking the variable is
always set to its standard value. The justification for this is that
toggling a module's minor mode in Customize has never worked and has
only sewn confusion. Without this hack, mode widgets show a state of
"CHANGED outside Customize", which alone is probably preferable,
except that they all end up toggled open, bringing them unwanted
attention and distracting the user.
(erc--tick-module-checkbox): Add helper to toggle the appropriate
checkbox in the `erc-modules' widget when a user interactively toggles
a minor-mode state variable.
(erc--prepare-custom-module-type): Create spec for minor-mode custom
`:type', deferring various aspects until module-definition time.
(define-erc-module): Add `:get' and `:type' keywords to be passed to
`defcustom' definition for global modules.
* lisp/erc/erc.el (erc-modules): Inhibit `erc-update-modules' when run
from a minor-mode toggle's custom-set function.
* test/lisp/erc/erc-tests.el
(define-erc-module--global, define-erc-module--local): Update
`erc-modules' mutations with `erc--inside-mode-toggle-p' guard
conditions. (Bug#60935.)
---
lisp/erc/erc-common.el | 100 +++++++++++++++++++++++++++++++++++--
lisp/erc/erc.el | 3 +-
test/lisp/erc/erc-tests.el | 17 +++++--
3 files changed, 111 insertions(+), 9 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index 0eabd3a2fe9..fa5d5eb52bd 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -29,6 +29,7 @@
(defvar erc--casemapping-rfc1459)
(defvar erc--casemapping-rfc1459-strict)
(defvar erc-channel-users)
+(defvar erc-modules)
(defvar erc-dbuf)
(defvar erc-log-p)
(defvar erc-server-users)
@@ -37,6 +38,11 @@ erc-session-server
(declare-function erc--get-isupport-entry "erc-backend" (key &optional single))
(declare-function erc-get-buffer "erc" (target &optional proc))
(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-get-sibling "wid-edit" (widget))
+(declare-function widget-move "wid-edit" (arg &optional suppress-echo))
+(declare-function widget-type "wid-edit" (widget))
(cl-defstruct erc-input
string insertp sendp)
@@ -120,6 +126,16 @@ erc--normalize-module-symbol
(setq symbol (intern (downcase (symbol-name symbol))))
(or (cdr (assq symbol erc--module-name-migrations)) symbol))
+(defvar erc--inside-mode-toggle-p nil
+ "Non-nil when a module's mode toggle is updating module membership.
+This serves as a flag to inhibit the mutual recursion that would
+otherwise occur between an ERC-defined minor-mode function, such
+as `erc-services-mode', and the custom-set function for
+`erc-modules'. For historical reasons, the latter calls
+`erc-update-modules', which, in turn, enables the minor-mode
+functions for all member modules. Also non-nil when a mode's
+widget runs its set function.")
+
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
@@ -137,11 +153,28 @@ erc--assemble-toggle
(,ablsym))
(setq ,mode ,val)
,@body)))
- `(,(if val
- `(cl-pushnew ',(erc--normalize-module-symbol name)
- erc-modules)
- `(setq erc-modules (delq ',(erc--normalize-module-symbol name)
- erc-modules)))
+ ;; No need for `default-value', etc. because a buffer-local
+ ;; `erc-modules' only influences the next session and
+ ;; doesn't survive the major-mode reset that soon follows.
+ `((unless
+ (or erc--inside-mode-toggle-p
+ ,@(let ((v `(memq ',(erc--normalize-module-symbol name)
+ erc-modules)))
+ `(,(if val v `(not ,v)))))
+ (let ((erc--inside-mode-toggle-p t))
+ ;; Make the widget show "CHANGED outside Customize."
+ ;; Ideally, it would show "SET for current session"
+ ;; instead, but using `customize-set-variable' here
+ ;; or calling `customize-mark-as-set' afterward isn't
+ ;; enough if `customized-value' and `standard-value'
+ ;; match but differ from `saved-value' because the
+ ;; widget will see a `standard' instead of a `set'
+ ;; state. And clicking "apply and save" won't update
+ ;; `custom-file'. See bug#12864 "Make state button
+ ;; interaction less confusing".
+ (setopt erc-modules (,(if val 'cons 'delq)
+ ',(erc--normalize-module-symbol name)
+ erc-modules))))
(setq ,mode ,val)
,@body)))))
@@ -176,6 +209,61 @@ 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.
+
+(defun erc--tick-module-checkbox (name &rest _) ; `name' must be normalized
+ (customize-variable-other-window 'erc-modules)
+ ;; Move to `erc-modules' section.
+ (while (not (eq (widget-type (widget-at)) 'checkbox))
+ (widget-move 1 t))
+ ;; This search for a checkbox can fail when `name' refers to a
+ ;; third-party module that modifies `erc-modules' (improperly) on
+ ;; load.
+ (let (w)
+ (while (and (eq (widget-type (widget-at)) 'checkbox)
+ (not (and (setq w (widget-get-sibling (widget-at)))
+ (eq (widget-value w) name))))
+ (setq w nil)
+ (widget-move 1 t)) ; the `suppress-echo' arg exists in 27.2
+ (if w
+ (progn (widget-apply-action (widget-at))
+ (message "Hit %s to apply or %s to apply and save."
+ (substitute-command-keys "\\[Custom-set]")
+ (substitute-command-keys "\\[Custom-save]")))
+ (error "Failed to find %s in `erc-modules' checklist" name))))
+
+(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"
+ :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))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -222,6 +310,8 @@ define-erc-module
%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 `(:type ,(erc--prepare-custom-module-type name)))
(if ,mode
(,enable)
(,disable)))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 69bdb5d71b1..59ab1f1eab3 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1846,7 +1846,8 @@ erc-modules
(set sym val)
;; this test is for the case where erc hasn't been loaded yet
(when (fboundp 'erc-update-modules)
- (erc-update-modules)))
+ (unless erc--inside-mode-toggle-p
+ (erc-update-modules))))
:type
'(set
:greedy t
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 13ef99be167..cb63b04eac5 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1313,7 +1313,10 @@ define-erc-module--global
((ignore a) (ignore b))
((ignore c) (ignore d)))))
- (should (equal (macroexpand global-module)
+ (should (equal (cl-letf (((symbol-function
+ 'erc--prepare-custom-module-type)
+ #'symbol-name))
+ (macroexpand global-module))
`(progn
(define-minor-mode erc-mname-mode
@@ -1324,6 +1327,8 @@ define-erc-module--global
Some docstring"
:global t
:group (erc--find-group 'mname 'malias)
+ :get #'erc--neuter-custom-variable-state
+ :type "mname"
(if erc-mname-mode
(erc-mname-enable)
(erc-mname-disable)))
@@ -1331,14 +1336,20 @@ define-erc-module--global
(defun erc-mname-enable ()
"Enable ERC mname mode."
(interactive)
- (cl-pushnew 'mname erc-modules)
+ (unless (or erc--inside-mode-toggle-p
+ (memq 'mname erc-modules))
+ (let ((erc--inside-mode-toggle-p t))
+ (setopt erc-modules (cons 'mname erc-modules))))
(setq erc-mname-mode t)
(ignore a) (ignore b))
(defun erc-mname-disable ()
"Disable ERC mname mode."
(interactive)
- (setq erc-modules (delq 'mname erc-modules))
+ (unless (or erc--inside-mode-toggle-p
+ (not (memq 'mname erc-modules)))
+ (let ((erc--inside-mode-toggle-p t))
+ (setopt erc-modules (delq 'mname erc-modules))))
(setq erc-mname-mode nil)
(ignore c) (ignore d))
--
2.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0003-5.6-Fill-doc-strings-for-ERC-modules.patch --]
[-- Type: text/x-patch, Size: 5795 bytes --]
From e8a6cbfaffb91019fa9a4a9aa3cae3c7896a6ff5 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 16 Jan 2023 20:18:32 -0800
Subject: [PATCH 3/3] [5.6] Fill doc strings for ERC modules.
* lisp/erc/erc-common.el (erc--fill-module-docstring): Add helper to
fill doc strings.
(erc--assemble-toggle, define-erc-module): Use helper to fill doc
string.
* test/lisp/erc/erc-tests.el (define-minor-mode--global,
define-minor-mode--local): Adjust expected output for generated doc
strings. (Bug#60935.)
---
lisp/erc/erc-common.el | 20 +++++++++++++++++---
test/lisp/erc/erc-tests.el | 28 ++++++++++++++++------------
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/lisp/erc/erc-common.el b/lisp/erc/erc-common.el
index fa5d5eb52bd..3a148dc1196 100644
--- a/lisp/erc/erc-common.el
+++ b/lisp/erc/erc-common.el
@@ -139,7 +139,7 @@ erc--inside-mode-toggle-p
(defun erc--assemble-toggle (localp name ablsym mode val body)
(let ((arg (make-symbol "arg")))
`(defun ,ablsym ,(if localp `(&optional ,arg) '())
- ,(concat
+ ,(erc--fill-module-docstring
(if val "Enable" "Disable")
" ERC " (symbol-name name) " mode."
(when localp
@@ -264,6 +264,20 @@ erc--prepare-custom-module-type
(if hasp "from" "to") " `erc-modules'.")))
:action ,(apply-partially #'erc--tick-module-checkbox name))))
+(defun erc--fill-module-docstring (&rest strings)
+ (with-temp-buffer
+ (emacs-lisp-mode)
+ (insert "(defun foo ()\n"
+ (format "%S" (apply #'concat strings))
+ "\n(ignore))")
+ (goto-char (point-min))
+ (forward-line 2)
+ (let ((emacs-lisp-docstring-fill-column 65)
+ (sentence-end-double-space t))
+ (fill-paragraph))
+ (goto-char (point-min))
+ (nth 3 (read (current-buffer)))))
+
(defmacro define-erc-module (name alias doc enable-body disable-body
&optional local-p)
"Define a new minor mode using ERC conventions.
@@ -303,11 +317,11 @@ define-erc-module
`(progn
(define-minor-mode
,mode
- ,(format "Toggle ERC %S mode.
+ ,(erc--fill-module-docstring (format "Toggle ERC %s mode.
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)
+\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))
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index cb63b04eac5..baf6826ff97 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -1309,7 +1309,7 @@ erc--merge-local-modes
(ert-deftest define-erc-module--global ()
(let ((global-module '(define-erc-module mname malias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d)))))
@@ -1321,10 +1321,11 @@ define-erc-module--global
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global t
:group (erc--find-group 'mname 'malias)
:get #'erc--neuter-custom-variable-state
@@ -1363,7 +1364,7 @@ define-erc-module--global
(ert-deftest define-erc-module--local ()
(let* ((global-module '(define-erc-module mname nil ; no alias
- "Some docstring"
+ "Some docstring."
((ignore a) (ignore b))
((ignore c) (ignore d))
'local))
@@ -1375,10 +1376,11 @@ define-erc-module--local
`(progn
(define-minor-mode erc-mname-mode
"Toggle ERC mname mode.
-With a prefix argument ARG, enable mname if ARG is positive,
-and disable it otherwise. If called from Lisp, enable the mode
-if ARG is omitted or nil.
-Some docstring"
+With a prefix argument ARG, enable mname if ARG is positive, and
+disable it otherwise. If called from Lisp, enable the mode if
+ARG is omitted or nil.
+
+Some docstring."
:global nil
:group (erc--find-group 'mname nil)
(if erc-mname-mode
@@ -1387,7 +1389,8 @@ define-erc-module--local
(defun erc-mname-enable (&optional ,arg-en)
"Enable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-en
@@ -1399,7 +1402,8 @@ define-erc-module--local
(defun erc-mname-disable (&optional ,arg-dis)
"Disable ERC mname mode.
-When called interactively, do so in all buffers for the current connection."
+When called interactively, do so in all buffers for the current
+connection."
(interactive "p")
(when (derived-mode-p 'erc-mode)
(if ,arg-dis
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread[parent not found: <87edpqglf1.fsf@neverwas.me>]
* bug#60935: 30.0.50; ERC >5.5: Improve ERC's treatment of customization groups
[not found] ` <87edpqglf1.fsf@neverwas.me>
@ 2023-04-14 14:03 ` J.P.
[not found] ` <87a5zay31h.fsf@neverwas.me>
1 sibling, 0 replies; 7+ messages in thread
From: J.P. @ 2023-04-14 14:03 UTC (permalink / raw)
To: 60935; +Cc: emacs-erc
[-- 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
^ permalink raw reply related [flat|nested] 7+ messages in thread[parent not found: <87a5zay31h.fsf@neverwas.me>]