Subject: Re: bug#70589: [PATCH] Refine the Custom type of generated '*-modes' options

From: Eshel Yaron <[email-masked]>
Date: Fri, 26 Apr 2024 14:24:42 +0200

Eli Zaretskii <[email-masked]> writes:

>> From: Eshel Yaron <[email-masked]>
>> Cc: [email-masked],  Stefan Monnier <[email-masked]>
>> Date: Fri, 26 Apr 2024 13:13:37 +0200
>> 
>> Eli Zaretskii <[email-masked]> writes:
>> 
>> > Why does it make sense to have the "all" and "none" alternatives
>> > twice?
>> 
>> These are slightly different: a value of t ("Enable in all major modes")
>> says to enable everywhere, while a t inside a list ("All major modes")
>> says to enable in modes that were not mentioned earlier in the list (so
>> putting t in the end of the list means "otherwise, enable").  Similarly
>> for the "none" alternatives: a value of nil ("Don't enable...") means we
>> never enable the minor mode, while the effect of a nil inside the list
>> is relative to the previous elements, and means "otherwise, don't".
>> Does that make sense?
>
> It might make sense, if we change the labels to say what you tell
> above.
>
> Thanks.

OK, here's an updated patch with revised labels:

>From 88ad06894cba8cfaf7b7c1916449f9f10e1cb6e7 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <[email-masked]>
Date: Fri, 26 Apr 2024 11:43:29 +0200
Subject: [PATCH v2] Refine the Custom type of generated '*-modes' options

* lisp/emacs-lisp/easy-mmode.el (define-globalized-minor-mode):
Refine the Custom type of the '*-modes' option, generated when
this macro is given a ':predicate' argument.  (Bug#70589)
---
 lisp/emacs-lisp/easy-mmode.el | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index eaad9646985..081619002c8 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -580,7 +580,19 @@ define-globalized-minor-mode
 and nil means \"don't use\".  There's an implicit nil at the end of the
 list."
                       mode)
-             :type '(repeat sexp)
+             :type '(choice (const :tag "Enable in all major modes" t)
+                            (const :tag "Don't enable in any major mode" nil)
+                            (repeat :tag "Rules (earlier takes precedence)..."
+                                    (choice
+                                     (const :tag "By default, enable" t)
+                                     (const :tag "By default, don't enable" nil)
+                                     (symbol :value fundamental-mode
+                                             :tag "Enable in major mode")
+                                     (cons :tag "Don't enable in major modes"
+                                           (const :tag "Don't enable in..." not)
+                                           (repeat
+                                            (symbol :value fundamental-mode
+                                                    :tag "Major mode"))))))
              ,@group))
 
        ;; Autoloading define-globalized-minor-mode autoloads everything
-- 
2.44.0