bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Drew Adams]

I just started to look at this, so forgive me if I am missing something.
 
`define-derived-mode' picks up the :group keyword and uses it, among
other things, to do this: (put ',child 'custom-mode-group ,group), that
is, to declare the group as the custom-mode-group for the new mode.
 
My suggestion is for `define-minor-mode' - and perhaps other
mode-defining macros - to do likewise.  `define-minor-mode' apparently
uses the :group keyword only for defining the user options it defines.
 
As it stands today, you can define a minor mode `foo' using
`define-minor-mode', and then try `C-u customize-mode, but `foo' will
not appear as a possible completion, because neither property
`custom-mode-group' nor property `custom-group' has been applied to the
symbol `foo'.  This is too bad.
 
Grepping for `custom-mode-group' shows that it is applied _only_ in
`define-derived-mode', and it is used _only_ in `custom-group-of-mode',
which in turn is used _only_ in `customize-mode'.
 
Grepping for `custom-group' and filtering a bit, my impression is that
that property is put on symbols only by `custom-add-to-group'.  And
grepping for `custom-add-to-group', it is not too clear where
`custom-group' is put on symbols.  Things like `custom-declare-variable'
do so, but is that used by `defcustom' or does `defcustom' do likewise?
 
In any case, I do not see a `custom-group' property on any of the user
options or faces I have defined using `defcustom' and `defface'.
Likewise, for the options defined by `define-minor-mode'.
 
So what's the story here?  Does `customize-mode' work only for derived
modes?  If so, why?  Can we please DTRT so that it works (e.g. with a prefix
arg) for any major or minor mode with a clearly identified :group?  See related
bug #11299.
 
Please do what it takes to have major and minor modes - at least those clearly
recognizable as such via `define-minor-mode' etc. and that clearly have a
defined :group - be assigned the property `custom-mode-group' (or
`custom-group'), so that `custom-group-of-mode' recognizes them.
 

In GNU Emacs 24.1.50.1 (i386-mingw-nt5.1.2600)
 of 2012-04-19 on MARVIN
Bzr revision: 107968 monnier@iro.umontreal.ca-20120419220225-gijdcbfxuiqy5dhb
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.6) --no-opt --enable-checking --cflags
 -ID:/devel/emacs/libs/libXpm-3.5.8/include
 -ID:/devel/emacs/libs/libXpm-3.5.8/src
 -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
 -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
 -ID:/devel/emacs/libs/giflib-4.1.4-1/include
 -ID:/devel/emacs/libs/jpeg-6b-4/include
 -ID:/devel/emacs/libs/tiff-3.8.2-1/include
 -ID:/devel/emacs/libs/gnutls-3.0.9/include
 -ID:/devel/emacs/libs/libiconv-1.13.1-1-dev/include
 -ID:/devel/emacs/libs/libxml2-2.7.8/include/libxml2'
 

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Drew Adams]

OK, so I looked a bit further.  For major or minor mode `foo-mode':

`custom-group-of-mode' checks for property `custom-mode-group' on the symbol
`foo-mode'.

Lacking that, it checks for property `custom-group' on the same symbol.

Lacking that, it checks for `custom-group' on symbol `foo'.

So if we look at `icomplete-mode', which is defined using
`define-minor-mode' specifying :group as `icomplete':
(custom-group-of-mode 'icomplete-mode) returns non-nil,
because symbol `icomplete' gets property `custom-group'.
Where does that come from?  

It seems to come from the :group.  But the :group value is not applied as
property `custom-group' to the _mode_ symbol.  It is applied only to the :group
value itself.  E.g., with :group 'Foo, property `custom-group' is applied to the
symbol `Foo' but not (also) to the mode name `foo-mode' or even `foo'.

So if you use as the :group value something that is not _identical_ to the
mode-function symbol (or that minus `-mode'), then `custom-group-of-mode' does
not work.  Its attempt to find the mode's group is too limited.  If you use
:group 'Foo or :group 'Foos, you are out of luck.  You must use :group 'foo for
`foo-mode' to be recognized by `custom-group-of-mode'.  (And no such limitations
are documented.)

Please DTRT: pick up whatever :group specifies as the group and include it in
the value of property `custom-group' for the mode name.

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Lars Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> My suggestion is for `define-minor-mode' - and perhaps other
> mode-defining macros - to do likewise.  `define-minor-mode' apparently
> uses the :group keyword only for defining the user options it defines.

I've tried following the logic and history of the :group keyword in
easy-mmode.el, and I'm not sure how that's supposed to work any more.

`define-minor-mode' used to document taking a :group keyword, but that's
been removed in:

commit 6e7736ac5f42e2f1b17aacdfb2a60d8bb951d038
Author:     Stefan Monnier <monnier@iro.umontreal.ca>
AuthorDate: Wed Sep 9 13:29:59 2020 -0400

The only remaining mention is in this example:

For example, you could write
  (define-minor-mode foo-mode \"If enabled, foo on you!\"
    :lighter \" Foo\" :require \\='foo :global t :group \\='hassle :version \"27.5\"
    ...BODY CODE...)

`define-globalized-minor-mode' does mention it, though, and passes it on
to `define-minor-mode'...  which doesn't use it?

I've added Stefan to the CCs; I'm sure he knows what's going on.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I've tried following the logic and history of the :group keyword in
> easy-mmode.el, and I'm not sure how that's supposed to work any more.
>
> `define-minor-mode' used to document taking a :group keyword, but that's
> been removed in:

Yes, it tried to be clever and guess the `:group` arg when none was
provided, but that guess was worse than the defaulting used by
`defcustom`.

Since the `:group` was only used for the `defcustom`, I just removed
this special handling.  It doesn't mean it doesn't work any more, but
now it's just another one of the keyword args that are simply passed
as-is to `defcustom`.

So this has no direct impact on whether or not `define-minor-mode`
should register the mode for use in `customize-mode` (which sounds like
a good idea).


        Stefan

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Since the `:group` was only used for the `defcustom`, I just removed
> this special handling.  It doesn't mean it doesn't work any more, but
> now it's just another one of the keyword args that are simply passed
> as-is to `defcustom`.

Ah, I see.  But it seems like `define-minor-mode' just discards :group
now.  Here's a test case:

(define-minor-mode foo-mode "doc" :group 'mouse)

Since this isn't a global minor mode, we end up in this branch:

         ((not globalp)
          `(progn
             :autoload-end
             (defvar-local ,mode ,init-value
               ,(concat (format "Non-nil if %s is enabled.\n" pretty-name)
                        (internal--format-docstring-line
                         "Use the command `%s' to change this variable." mode)))))

and :group (which ends up in `extra-keywords') isn't used.  Should this
instead use `defcustom' with :local t?  I.e., basically merge with the
global case in that cond?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2021-06-03 09:45:34] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Since the `:group` was only used for the `defcustom`, I just removed
>> this special handling.  It doesn't mean it doesn't work any more, but
>> now it's just another one of the keyword args that are simply passed
>> as-is to `defcustom`.
>
> Ah, I see.  But it seems like `define-minor-mode' just discards :group
> now.  Here's a test case:

The "extra args for decustom" are only used when define-minor-mode uses
a defcustom, i.e. only for global minor modes.


        Stefan

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The "extra args for decustom" are only used when define-minor-mode uses
> a defcustom, i.e. only for global minor modes.

Yes.  But why?  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2021-06-04 10:45:42] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> The "extra args for decustom" are only used when define-minor-mode uses
>> a defcustom, i.e. only for global minor modes.
> Yes.  But why?  :-)

Because we haven't been able to provide Custom support for buffer-local
variables in the many years since `defcustom` was introduced.  ;-)


        Stefan

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Because we haven't been able to provide Custom support for buffer-local
> variables in the many years since `defcustom` was introduced.  ;-)

Hm...  defcustom does have a :local keyword now (since 2019), and...
er...  It probably works?  But I'm not sure what `M-x customize' does
with that.  Probably nothing?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#11301: 24.1.50; `define-minor-mode' should set `custom-mode-group' according to :group

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2021-06-06 11:29:00] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Because we haven't been able to provide Custom support for buffer-local
>> variables in the many years since `defcustom` was introduced.  ;-)
> Hm...  defcustom does have a :local keyword now (since 2019), and...
> er...  It probably works?  But I'm not sure what `M-x customize' does
> with that.  Probably nothing?

That sounds about right.


        Stefan