unofficial mirror of help-gnu-emacs@gnu.org
 help / color / mirror / Atom feed
* Enabling a globalized-minor-mode by default
@ 2020-09-10 19:09 Kévin Le Gouguec
  2020-09-10 21:14 ` Stefan Monnier
  0 siblings, 1 reply; 5+ messages in thread
From: Kévin Le Gouguec @ 2020-09-10 19:09 UTC (permalink / raw)
  To: help-gnu-emacs

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

Hello Emacs,

I'm trying to understand how to define a globalized minor mode that is
enabled by default.  My goal is to make this mode's keymap available
with no configuration on the user's part, unless the user explicitly
disables the minor mode.

I've attached a toy example which defines the testautoload-mode minor
mode, as well as its globalized variant global-testautoload-mode:


[-- Attachment #2: testautoload.el --]
[-- Type: application/emacs-lisp, Size: 747 bytes --]

[-- Attachment #3: Type: text/plain, Size: 616 bytes --]


Now, if I:

- byte-compile this file,
- M-: (package-generate-autoloads "testautoload" default-directory)
- emacs -Q -L .
- M-: (load-file "testautoload-autoloads.el")

then:

- global-testautoload-mode is t,
- the testautoload-mode variable is undefined,
- the testautoload-mode-map keymap is undefined.

From there, (customize-set-variable 'global-testautoload-mode t) enables
the mode in all buffers and the keymap works.  Is there a way to do away
with this extra step?  I've messed with :require and :initialize, but
they don't help AFAICT.

What does help is adding this snippet at the bottom of my library:


[-- Attachment #4: addendum.el --]
[-- Type: application/emacs-lisp, Size: 150 bytes --]

[-- Attachment #5: Type: text/plain, Size: 949 bytes --]


Is that bad form somehow, or is that the way to go?


Admittedly, maybe forcing a globalized minor mode on users by default is
bad form.  For context, I am trying to make magit-file-mode work
out-of-the-box, i.e. without users having to (1) (require 'anything) in
their config or (2) customize global-magit-file-mode to t explicitly,
which should be redundant because this is the default value.

I made a naive attempt to fix this[1] with more or less the same
autoload form I showed above, but that seemed to have unfortunate
side-effects[2].  Jonas suggests moving magit-file-mode and its keymap
to a new library that does not (require 'magit); IIUC the autoload form
would then work with no further complication?


I hope I'm making sense; if not, I hope somebody will tell me where I
got off the rails.

Thank you for your time!


[1] https://github.com/magit/magit/pull/4207
[2] https://github.com/magit/magit/pull/4207#issuecomment-688320025

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Enabling a globalized-minor-mode by default
  2020-09-10 19:09 Enabling a globalized-minor-mode by default Kévin Le Gouguec
@ 2020-09-10 21:14 ` Stefan Monnier
  2020-09-11  9:03   ` Kévin Le Gouguec
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2020-09-10 21:14 UTC (permalink / raw)
  To: help-gnu-emacs

> I'm trying to understand how to define a globalized minor mode that is
> enabled by default.  My goal is to make this mode's keymap available
> with no configuration on the user's part, unless the user explicitly
> disables the minor mode.
[...]
> Now, if I:
>
> - byte-compile this file,
> - M-: (package-generate-autoloads "testautoload" default-directory)
> - emacs -Q -L .
> - M-: (load-file "testautoload-autoloads.el")

Loading an ELisp file should not affect the visible behavior of Emacs,
so according to that principle a global mode that's pre-enabled should
only ever exist if it's bundled with Emacs.

For the same kind of reasons merely installing a package should not
affect the visible behavior of Emacs.

[ And yes "affect the visible behavior" is not well defined, or at least
  it needs to be tempered by some tolerated exceptions.  For example,
  it's considered normal for a package to add itself (via its autoloads)
  to `auto-mode-alist`, which does have a visible impact on Emacs's
  behavior.  ]

> ;;;###autoload
> (progn
>   (require 'cl-macs)
>   (cl-eval-when (load eval)
>     (when global-testautoload-mode
>       (global-testautoload-mode 1))))

[ Please require `cl-lib` rather than `cl-macs`, because we want to be
  free to move definitions between the various internal files of
  `cl-lib`.  ]

> Is that bad form somehow, or is that the way to go?

Yup.

> Admittedly, maybe forcing a globalized minor mode on users by default is
> bad form.  For context, I am trying to make magit-file-mode work
> out-of-the-box, i.e. without users having to (1) (require 'anything) in
> their config or (2) customize global-magit-file-mode to t explicitly,
> which should be redundant because this is the default value.

`require` is definitely not needed here.
Only `(global-magit-file-mode 1)` needs to be added to the init file
(or do the equivalent via Customize) and it shouldn't be redundant
because t shouldn't be the default value ;-)


        Stefan




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Enabling a globalized-minor-mode by default
  2020-09-10 21:14 ` Stefan Monnier
@ 2020-09-11  9:03   ` Kévin Le Gouguec
  2020-09-11 15:03     ` Stefan Monnier
  0 siblings, 1 reply; 5+ messages in thread
From: Kévin Le Gouguec @ 2020-09-11  9:03 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: help-gnu-emacs

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

> Loading an ELisp file should not affect the visible behavior of Emacs,
> so according to that principle a global mode that's pre-enabled should
> only ever exist if it's bundled with Emacs.
>
> For the same kind of reasons merely installing a package should not
> affect the visible behavior of Emacs.

OK.  I see that "(elisp) Defining Minor Modes" indeed says that
define-minor-mode's INIT-VALUE should be nil under "most" circumstances.
Nothing in the docstring suggests that though AFAICT; maybe it would be
worth hinting at this?


> [ And yes "affect the visible behavior" is not well defined, or at least
>   it needs to be tempered by some tolerated exceptions.  For example,
>   it's considered normal for a package to add itself (via its autoloads)
>   to `auto-mode-alist`, which does have a visible impact on Emacs's
>   behavior.  ]

Interesting!  I see "(elisp) Major Mode Conventions" and "(elisp)
Packaging Basics" both mention this use-case; would it make sense to add
this "acceptable use" (and maybe others, if they exist) to "(elisp) When
to Autoload"?


>> Is that bad form somehow, or is that the way to go?
>
> Yup.

M-x spit-take


>> Admittedly, maybe forcing a globalized minor mode on users by default is
>> bad form.  For context, I am trying to make magit-file-mode work
>> out-of-the-box, i.e. without users having to (1) (require 'anything) in
>> their config or (2) customize global-magit-file-mode to t explicitly,
>> which should be redundant because this is the default value.
>
> `require` is definitely not needed here.

(Right; I actually started this whole "crusade" after seeing you argue
that a .emacs should not require anything[1][2][3]).


> Only `(global-magit-file-mode 1)` needs to be added to the init file
> (or do the equivalent via Customize) and it shouldn't be redundant
> because t shouldn't be the default value ;-)

Got it.  I would joke and say that this will all be solved when (1) we
get Magit into GNU ELPA and (2) we implement bundling GNU ELPA packages
into Emacs, but I'm pretty sure that would score way too high on some
twisted bad-taste private-joke form of bingo, and it's not a game I care
to play ;o)


[1] https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg00947.html
    emacs-devel <jwvh7xjchxv.fsf&#45;monnier+emacs@gnu.org>

[2] https://gitter.im/magit/magit?at=5ea154f561a0002f7943209e

[3] https://gitter.im/magit/magit?at=5eac311a9f0c955d7d9a89b9



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Enabling a globalized-minor-mode by default
  2020-09-11  9:03   ` Kévin Le Gouguec
@ 2020-09-11 15:03     ` Stefan Monnier
  2020-09-12 10:01       ` Kévin Le Gouguec
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2020-09-11 15:03 UTC (permalink / raw)
  To: help-gnu-emacs

>> Loading an ELisp file should not affect the visible behavior of Emacs,
>> so according to that principle a global mode that's pre-enabled should
>> only ever exist if it's bundled with Emacs.
>> For the same kind of reasons merely installing a package should not
>> affect the visible behavior of Emacs.
> OK.  I see that "(elisp) Defining Minor Modes" indeed says that
> define-minor-mode's INIT-VALUE should be nil under "most" circumstances.
> Nothing in the docstring suggests that though AFAICT; maybe it would be
> worth hinting at this?

I guess the docstring is so long already that adding a couple more lines
couldn't hurt, but really it's just a general principle, not specific to
minor modes.

Would the patch below help?

    diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
    index e3eb9294ed..1cde808e7a 100644
    --- a/lisp/emacs-lisp/easy-mmode.el
    +++ b/lisp/emacs-lisp/easy-mmode.el
    @@ -137,6 +137,10 @@ define-minor-mode
     usage of the mode argument.
     
     Optional INIT-VALUE is the initial value of the mode's variable.
    +  Note that the minor mode function won't be called by setting
    +  this option, so the value *reflects* the minor mode's natural
    +  initial state, rather than *setting* it.
    +  In the vast majority of cases it should be nil.
     Optional LIGHTER is displayed in the mode line when the mode is on.
     Optional KEYMAP is the default keymap bound to the mode keymap.
       If non-nil, it should be a variable name (whose value is a keymap),

>> [ And yes "affect the visible behavior" is not well defined, or at least
>>   it needs to be tempered by some tolerated exceptions.  For example,
>>   it's considered normal for a package to add itself (via its autoloads)
>>   to `auto-mode-alist`, which does have a visible impact on Emacs's
>>   behavior.  ]
>
> Interesting!  I see "(elisp) Major Mode Conventions" and "(elisp)
> Packaging Basics" both mention this use-case; would it make sense to add
> this "acceptable use" (and maybe others, if they exist) to "(elisp) When
> to Autoload"?

Hmm... I'm not sure it fits in there (this section is currently
perfectly applicable to packages bundled with Emacs, which is
a different situation since those *can* enable a minor mode this way
(after going through the corresponding years of negotiation to get
people to reluctantly agree to the default behavior ;-) )

Maybe we need some reorganisation or a new Node.

>>> Is that bad form somehow, or is that the way to go?
>> Yup.
> M-x spit-take

[No match]

¡Ha!

>> Only `(global-magit-file-mode 1)` needs to be added to the init file
>> (or do the equivalent via Customize) and it shouldn't be redundant
>> because t shouldn't be the default value ;-)
> Got it.  I would joke and say that this will all be solved when (1) we
> get Magit into GNU ELPA and (2) we implement bundling GNU ELPA packages
> into Emacs,

(1) seems a bit difficult for now, but we should get (½) pretty soon
(i.e. Magit seems like one of the main candidates for the new NonGNU ELPA).

(2) is not terribly hard to do (there's some proof of concept code for
it already) and there's a general interest to make it happen, so we may
get lucky and get that in Emacs-28 (after I miserably failed to make it
happen for Emacs-25 :-( ).

Maybe we could then wait for (2½) where Emacs bundles some NonGNU ELPA
packages as well.  This involves a delicate political decision, so don't
hold your breath!


        Stefan




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Enabling a globalized-minor-mode by default
  2020-09-11 15:03     ` Stefan Monnier
@ 2020-09-12 10:01       ` Kévin Le Gouguec
  0 siblings, 0 replies; 5+ messages in thread
From: Kévin Le Gouguec @ 2020-09-12 10:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: help-gnu-emacs

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

> I guess the docstring is so long already that adding a couple more lines
> couldn't hurt, but really it's just a general principle, not specific to
> minor modes.

Fair enough.  I don't know how people come to write minor modes (I was
just trying to "fix" one).  Maybe most of them are diligent enough to
read the manual first?  I'd bet that a few lost souls will just read
define-minor-mode's docstring and go from there, and won't necessarily
be acquainted with this general principle.

For better or worse, Emacs's docstrings look fairly comprehensive, so
it's tempting to assume they cover everything one needs to know.  I, for
one, have yet to acquire the reflex to dig up the manual when something
doesn't match the expectations I got from the docstring.

It's not that C-h S is harder to hit than C-h f, but if nothing in the
docstring hints at things being More Complicated Than They Look™, it
won't occur to me to reach for the manual.

> Would the patch below help?
>
>     diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
>     index e3eb9294ed..1cde808e7a 100644
>     --- a/lisp/emacs-lisp/easy-mmode.el
>     +++ b/lisp/emacs-lisp/easy-mmode.el
>     @@ -137,6 +137,10 @@ define-minor-mode
>      usage of the mode argument.
>      
>      Optional INIT-VALUE is the initial value of the mode's variable.
>     +  Note that the minor mode function won't be called by setting
>     +  this option, so the value *reflects* the minor mode's natural
>     +  initial state, rather than *setting* it.
>     +  In the vast majority of cases it should be nil.
>      Optional LIGHTER is displayed in the mode line when the mode is on.
>      Optional KEYMAP is the default keymap bound to the mode keymap.
>        If non-nil, it should be a variable name (whose value is a keymap),

I think it does.  AFAICT the current description does nothing to warn
the user that there are two sources of truth (the mode variable, and
calling the mode function).  IMO it's easy to assume that INIT-VALUE
controls activation: what would be the point of letting the user set the
variable without calling the mode function?

The added lines hint that activation is more or less independent of
INIT-VALUE; I think it's enough to prompt the user to go fish for more
documentation.

Thank you for coming up with these!


PS: while trying to assess how common the ":init-value t enables the
    mode" fallacy is (if at all), I found this thread:

    https://old.reddit.com/r/emacs/comments/aw84yn/defineminormode_initvalue_t_not_running_its_hook/

    This smonnier person sure sounds like a knowledgeable fellow :D and
    look at /u/tarsius_, with this helpful example code!



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-12 10:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-10 19:09 Enabling a globalized-minor-mode by default Kévin Le Gouguec
2020-09-10 21:14 ` Stefan Monnier
2020-09-11  9:03   ` Kévin Le Gouguec
2020-09-11 15:03     ` Stefan Monnier
2020-09-12 10:01       ` Kévin Le Gouguec

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).