GNU bug report logs - #21563
24.5; discourage load-hook variables

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21563 in the body.
You can then email your comments to 21563 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Roland Winkler" <winkler <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.5; discourage load-hook variables
Date: Fri, 25 Sep 2015 13:57:21 -0500

Somewhere in my emacs init file I was setting the variable
dired-load-hook.  (I guess I wrote this code years ago when I did
not know much about emacs.)  Yet suddenly this did not work for me
anymore, as I had rearranged my init file so that dired got loaded
before I was setting dired-load-hook.  I suggest to discourage the
usage of any such load-hook variables: I believe eval-after-load is
considered to be the cleaner alternative: it does not give rise to
the type of problems I ran into.  Also it does not require user
variables for each package.

(Apropos gives me the variables align-load-hook, cal-menu-load-hook,
calendar-load-hook, dired-load-hook, ediff-load-hook, and
table-load-hook.  There might be more.)



In GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2015-05-29 on lukas
Windowing system distributor `The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04.3 LTS

Message #8 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: "Roland Winkler" <winkler <at> gnu.org>
Cc: 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Wed, 15 Jan 2020 20:32:12 +0100

"Roland Winkler" <winkler <at> gnu.org> writes:

> Somewhere in my emacs init file I was setting the variable
> dired-load-hook.  (I guess I wrote this code years ago when I did
> not know much about emacs.)  Yet suddenly this did not work for me
> anymore, as I had rearranged my init file so that dired got loaded
> before I was setting dired-load-hook.  I suggest to discourage the
> usage of any such load-hook variables: I believe eval-after-load is
> considered to be the cleaner alternative: it does not give rise to
> the type of problems I ran into.  Also it does not require user
> variables for each package.
>
> (Apropos gives me the variables align-load-hook, cal-menu-load-hook,
> calendar-load-hook, dired-load-hook, ediff-load-hook, and
> table-load-hook.  There might be more.)

This suggests that all such variables should be considered obsolete,
since eval-after-load is cleaner.  I think I agree, but I have only
ever used eval-after-load so I might be missing something.

I would suggest to declare the above variables obsolete and point
users to eval-after-load instead.  Does anyone disagree with that?

Best regards,
Stefan Kangas

Message #11 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>, Roland Winkler <winkler <at> gnu.org>
Cc: 21563 <at> debbugs.gnu.org
Subject: RE: bug#21563: 24.5; discourage load-hook variables
Date: Wed, 15 Jan 2020 12:21:41 -0800 (PST)

>> I had rearranged my init file so that dired got loaded
>> before I was setting dired-load-hook.

IOW, simple pilot error.

> I would suggest to declare the above variables obsolete
> and point users to eval-after-load instead.

Why?  If the only reason (only one given so far) is
that a user set a hook after loading and expected
the hook setting to be effective, then I'd say that's
not a great reason.

Pilot error can also happen for `(with-)eval-after-load'.

And there was talk at one time of discouraging that as
well, I believe.  It's still discouraged for code that's
to be included in Emacs - see (emacs) `Coding Standards'.
(But see also (emacs) `Foldout'.)

And see (elisp) `Hooks for Loading':
 "Normally, well-designed Lisp programs should not
  use 'with-eval-after-load'."

Granted, that's about "Lisp programs" and not init
files.  Still...

> Does anyone disagree with that?

I think I do.  I see something like `dired-load-hook'
as a different tool than `(with-)eval-after-load'.
Not worse or better, either generally or always.

Setting or changing the explicit hook value has
no effect if the library was already loaded, whereas
`(with-)eval-after-load' has an immediate effect in 
that case.  (Sure, the latter could test in its body
whether it's loaded and act conditionally...)

I see no good reason why, because there is more than
one way to do something, and some users might shoot
themselves in the foot with some of those ways, we
should remove those that let you do that.

Emacs and Emacs Lisp provide tons of ways to do
things, and many ways to shoot yourself in the foot.

We can instead make clear in the doc anything that
might need pointing out.  In this case, I doubt that
Roland was just lacking a statement that the load
hook would have no effect if the library had already
been loaded.  But we could certainly make that fact
explicit, if it helps in general.

Note too that `C-h f dired-mode' explicitly lists
the hooks:

 Hooks (use C-h v to see their documentation):

   'dired-before-readin-hook'
   'dired-after-readin-hook'
   'dired-mode-hook'
   'dired-load-hook'

When you use apropos or similar to find a Dired hook
you find one for after-loading.  Removing that hook
means you won't find it.  This doc would then need
to be changed to list the other hooks but also say
that if you want a function to be invoked after
loading then use `(with-)eval-after-load' and invoke
the function in the body.

IOW, no parallelism or discoverability.  Many users
won't know about `(with-)eval-after-load'.

In addition, users can easily get confused between
'dired-after-readin-hook' and 'dired-load-hook'.
Having both, with their documentation, helps users
DTRT.

What does it really hurt, to keep such hooks?

Message #14 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Roland Winkler <winkler <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Wed, 15 Jan 2020 21:42:39 +0100

Drew Adams <drew.adams <at> oracle.com> writes:

>> I would suggest to declare the above variables obsolete
>> and point users to eval-after-load instead.
>
> Why?  If the only reason (only one given so far) is
> that a user set a hook after loading and expected
> the hook setting to be effective, then I'd say that's
> not a great reason.

I think the idea here was mainly to get rid of redundancy.  But see
below.

> And there was talk at one time of discouraging that as
> well, I believe.  It's still discouraged for code that's
> to be included in Emacs - see (emacs) `Coding Standards'.
> (But see also (emacs) `Foldout'.)
>
> And see (elisp) `Hooks for Loading':
>  "Normally, well-designed Lisp programs should not
>   use 'with-eval-after-load'."
>
> Granted, that's about "Lisp programs" and not init
> files.  Still...
>
>> Does anyone disagree with that?
>
> I think I do.  I see something like `dired-load-hook'
> as a different tool than `(with-)eval-after-load'.
> Not worse or better, either generally or always.
>
> Setting or changing the explicit hook value has
> no effect if the library was already loaded, whereas
> `(with-)eval-after-load' has an immediate effect in 
> that case.  (Sure, the latter could test in its body
> whether it's loaded and act conditionally...)

Good point.  So it seems like load-hooks and eval-after-load are
slightly different and could both be useful.  In that case, I guess
this should be closed as a wontfix?

Best regards,
Stefan Kangas

Message #17 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Roland Winkler <winkler <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Wed, 15 Jan 2020 17:06:31 -0500

Stefan Kangas wrote:

> This suggests that all such variables should be considered obsolete,
> since eval-after-load is cleaner.  I think I agree, but I have only
> ever used eval-after-load so I might be missing something.
>
> I would suggest to declare the above variables obsolete and point
> users to eval-after-load instead.

I did this years ago for a few modes; eg dd210a6, 041e909, but never got
round to doing the rest. As I said in https://debbugs.gnu.org/24491#8, I
see no need to keep any *-load-hooks.

Message #20 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Roland Winkler <winkler <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 01:03:37 +0100

Glenn Morris <rgm <at> gnu.org> writes:

> Stefan Kangas wrote:
>
>> This suggests that all such variables should be considered obsolete,
>> since eval-after-load is cleaner.  I think I agree, but I have only
>> ever used eval-after-load so I might be missing something.
>>
>> I would suggest to declare the above variables obsolete and point
>> users to eval-after-load instead.
>
> I did this years ago for a few modes; eg dd210a6, 041e909, but never got
> round to doing the rest. As I said in https://debbugs.gnu.org/24491#8, I
> see no need to keep any *-load-hooks.

Interesting.  Since they also seem to be prone to cause problems like
in Bug#24491, perhaps it is indeed better to get rid of them.  They do
look crufty and redundant once you start reviewing them in context --
with no clear benefit.

I believe Drew already pointed out that any use of them could be
easily replaced by (with-)?eval-after-load.

I'll be happy to do (at least parts of) this, but should such changes
be announced in NEWS?  I noticed that you didn't do that in dd210a6 or
041e909.

Best regards,
Stefan Kangas

Message #23 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>, Glenn Morris <rgm <at> gnu.org>
Cc: Roland Winkler <winkler <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: RE: bug#21563: 24.5; discourage load-hook variables
Date: Wed, 15 Jan 2020 16:24:10 -0800 (PST)

> Interesting.  Since they also seem to be prone to cause problems like
> in Bug#24491, perhaps it is indeed better to get rid of them.  They do
> look crufty and redundant once you start reviewing them in context --
> with no clear benefit.
> 
> I believe Drew already pointed out that any use of them could be
> easily replaced by (with-)?eval-after-load.

No.  I didn't say anything about "any use of them".

I said only that the behavior that a load hook isn't invoked
if the library has already been loaded can be realized by
using conditional code inside `(with-)?eval-after-load'.

A load hook is a function.  Code can invoke it anytime, in
any context.  It has no predefined behavior, on its own -
in particular, nothing like `(with-)?eval-after-load'
behavior.  The only similarity is that by convention a load
hook is invoked at the end of a Lisp file.  But nothing
prevents using a (funcall dired-load-hook) anywhere.

This is not to say that we really need to be able to do
that.  It's just to say that there's no way to claim that
`(with-)?eval-after-load' can be made to do what a load
hook does, in general.

I don't have a giant objection to doing what you're talking
about doing.  But I think it's unfortunate, and little, if
anything, is really gained.  Who knows what 3rd-party code
out there makes use of such hooks?  And again, they're easy
for users to discover.  And I think they're likely to be
used by code, and not just in init files.  That's not so
true of `(with-)?eval-after-load' (explicitly discouraged).

Message #26 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: "Roland Winkler" <winkler <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 21563 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Wed, 15 Jan 2020 18:27:57 -0600

On Wed Jan 15 2020 Stefan Kangas wrote:
> Drew Adams <drew.adams <at> oracle.com> writes:
> > Setting or changing the explicit hook value has
> > no effect if the library was already loaded, whereas
> > `(with-)eval-after-load' has an immediate effect in 
> > that case.  (Sure, the latter could test in its body
> > whether it's loaded and act conditionally...)
> 
> Good point.  So it seems like load-hooks and eval-after-load are
> slightly different and could both be useful.

What is the benefit of being able to put something into a load-hook
that will be ignored because the feature was already loaded?
Is there a real-world example that can illustrate how this is
useful?

Message #29 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Roland Winkler <winkler <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 01:31:15 +0100

I now noticed that there is a bunch of hooks in eshell which seems to
never be called with run-hooks?  For example, eshell-alias-load-hook
or eshell-glob-load-hook.

Am I missing something here?

Best regards,
Stefan Kangas

Message #32 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Glenn Morris <rgm <at> gnu.org>, Roland Winkler <winkler <at> gnu.org>,
 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 01:54:45 +0100

Drew Adams <drew.adams <at> oracle.com> writes:

> I said only that the behavior that a load hook isn't invoked
> if the library has already been loaded can be realized by
> using conditional code inside `(with-)?eval-after-load'.

Thanks, you are correct.  I didn't mean to misquote you.

> A load hook is a function.  Code can invoke it anytime, in
> any context.  It has no predefined behavior, on its own -
> in particular, nothing like `(with-)?eval-after-load'
> behavior.  The only similarity is that by convention a load
> hook is invoked at the end of a Lisp file.  But nothing
> prevents using a (funcall dired-load-hook) anywhere.
>
> This is not to say that we really need to be able to do
> that.  It's just to say that there's no way to claim that
> `(with-)?eval-after-load' can be made to do what a load
> hook does, in general.

The question now though is more limited: is it useful to keep them or
not?  Glenn says that it is not useful, and I think I agree with him.

> I don't have a giant objection to doing what you're talking
> about doing.  But I think it's unfortunate, and little, if
> anything, is really gained.

Well, Emacs is old.  Like any old software, it has a certain amount of
cruft and/or historical accidents.  Getting rid of such things when we
can makes Emacs easier to maintain in the long run.

> Who knows what 3rd-party code out there makes use of such hooks?

It should migrate to use (with-)eval-after-load.  That should be
backwards compatible through "Emacs version 19.29" according to C-h f.

Note that the way these hooks have been obsoleted by Glenn is to still
run them if they exist, but add an obsoletion warning.  I think this
is the correct approach.

Given how long it takes for us to finally delete obsolete stuff, that
should give ten years, give or take, before any third party code
depending on these hooks would stop working.

> And again, they're easy for users to discover.

I think a general facility is even more discoverable and user
friendly.  In particular, it shows users that this could be used for
any package, and not just packages which has defined a particular
hook.

> And I think they're likely to be used by code, and not just in init
> files.  That's not so true of `(with-)?eval-after-load' (explicitly
> discouraged).

Shouldn't use of these hooks in code be considered bad practice for
the same reasons that eval-after-load is?

Best regards,
Stefan Kangas

Message #35 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Glenn Morris <rgm <at> gnu.org>, Roland Winkler <winkler <at> gnu.org>,
 21563 <at> debbugs.gnu.org
Subject: RE: bug#21563: 24.5; discourage load-hook variables
Date: Wed, 15 Jan 2020 19:56:09 -0800 (PST)

> > I don't have a giant objection to doing what you're talking
> > about doing.  But I think it's unfortunate, and little, if
> > anything, is really gained.
> 
> Well, Emacs is old.  Like any old software, it has a certain amount of
> cruft and/or historical accidents.  Getting rid of such things when we
> can makes Emacs easier to maintain in the long run.

So is the argument now that the existence of load
hooks makes maintenance too complicated?  What will
be the next argument?

`eval-after-load' is at least as old as load hooks.
Probably both have been there since about Day One.
Maybe ask RMS why.  Maybe load hooks are just cruft,
or maybe not.

> > Who knows what 3rd-party code out there makes use of such hooks?
> 
> It should migrate to use (with-)eval-after-load.  That should be
> backwards compatible through "Emacs version 19.29" according to C-h f.

Why should it migrate?  That's my question.  Why is
it good now to toss load hooks, when that hasn't been
needed before?  40 years of Emacs, and now there's a
reason to toss them.  What's so important about this
now?  A bug filed because someone set a hook too late
in his init file?  If that were a big problem, don't
you think load hooks would have been removed long ago?

> Note that the way these hooks have been obsoleted by Glenn is to still
> run them if they exist, but add an obsoletion warning.  I think this
> is the correct approach.
> 
> Given how long it takes for us to finally delete obsolete stuff, that
> should give ten years, give or take, before any third party code
> depending on these hooks would stop working.

Why delete them at all, finally or immediately?  What's
the real point?  The only thing new (and that's several
years old now) was the addition of `with-eval-after-load'
(because some users were forgetting that `eval-after-load'
is a function, and so evaluates its args).

> > And again, they're easy for users to discover.
> 
> I think a general facility is even more discoverable and user
> friendly.  In particular, it shows users that this could be used for
> any package, and not just packages which has defined a particular
> hook.

The two aren't the same, as I pointed out.  And see
what I said earlier about discoverability.

A hook named after a feature draws your attention to
the possibility of hooking in some code there - at
that spot, for that feature.  A general facility to
eval some code after loading a file, any file, isn't
quite the same, in particular in terms of notice
(discoverability).

> > And I think they're likely to be used by code, and not just in init
> > files.  That's not so true of `(with-)?eval-after-load' (explicitly
> > discouraged).
> 
> Shouldn't use of these hooks in code be considered bad practice for
> the same reasons that eval-after-load is?

Dunno.  Why so?  They haven't been introduced at the
end of every single library.  Someone presumably had
a reason for introducing each one, or at least most.

A hook is an explicit signal that you kinda expect
someone (some code) to want to hook in some other code
at that spot.  A hook is an invitation: "Here's a
place you might want to do something."  There are no
doubt specific hooks that no one has ever used and
that could perhaps be removed.  But why do so?

Many load hooks are even user options (I don't argue
they should be).  Users who've customized them will
eventually find that they've become no-ops.
`dired-load-hook', for example, has this added to its
doc string, to tell you about possible use cases:
"You can customize key bindings or load extensions with this."

Why do we still have _any_ hooks?  Why not argue
(maybe someone will - Stefan?) that nadvice makes all
other hooks obsolete.

Anyway, as I say, I don't care much, if no one else
cares about this.

Please be sure to take care of all places that might
take such hooks into consideration.  This comment in
dired-x.el, for example (no idea whether it's important):

;; This is a no-op if dired-x is being loaded via `dired-load-hook',
;; but maybe not if a dired-x function is being autoloaded.
(require 'dired)

And there are some hooks with `-load-hook' in the name
that don't seem to be hooks run after loading a file.
`ff-pre-load-hook' and `ff-post-load-hook', for example.
For them, "loading" a file apparently means something
quite different (if so, that's a naming bug).  And they
are buffer-local variables.

Message #38 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 08:49:50 -0300

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefan <at> marxist.se> writes:

> I now noticed that there is a bunch of hooks in eshell which seems to
> never be called with run-hooks?  For example, eshell-alias-load-hook
> or eshell-glob-load-hook.
>
> Am I missing something here?

eshell-mode runs every necessary eshell-*-load-hook.  Those that belong
to eshell modules, like the ones you mentioned, run only if the module
is activated (when it is a member of the customizable option
eshell-modules-list).

[Message part 2 (text/html, inline)]

Message #41 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Glenn Morris <rgm <at> gnu.org>, Roland Winkler <winkler <at> gnu.org>,
 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 14:33:19 +0100

Drew Adams <drew.adams <at> oracle.com> writes:

> So is the argument now that the existence of load
> hooks makes maintenance too complicated?  What will
> be the next argument?

To summarize what has come up in this thread so far, the load-hook
convention has the following problems:

1. They are never called if set by a user after a library is loaded.

2. They can lead to issues like Bug#24491.

3. They require every package (that wants to use them) to add boiler
   plate code.

They are also redundant since we have eval-after-load and
with-eval-after-load, which do not have any of the above problems.

Best regards,
Stefan Kangas

Message #44 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Glenn Morris <rgm <at> gnu.org>, Roland Winkler <winkler <at> gnu.org>,
 21563 <at> debbugs.gnu.org
Subject: RE: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 08:15:58 -0800 (PST)

> To summarize what has come up in this thread so far, the load-hook
> convention has the following problems:
> 
> 1. They are never called if set by a user after a library is loaded.
> 2. They can lead to issues like Bug#24491.
> 3. They require every package (that wants to use them) to add boiler
>    plate code.
> 
> They are also redundant since we have eval-after-load and
> with-eval-after-load, which do not have any of the above problems.

Here's what I'd suggest, if you are bent on removing
all load hooks and deprecating them:

1. Try removing all of them from the vanilla
   (distributed) Elisp code.

2. Run with that for a major release or two.  If no
   problems, then deprecate (declare obsolete).

Doing the second first (at the outset) makes no sense
to me.  Try it first.  Those hooks were added, over
time, at specific spots in the Emacs code for a reason,
presumably - at least some of them were deliberate, we
can suppose.

(Load hooks were never added to "every package".  And
I doubt that, at least in some cases, they were just
added as "boiler plate code".  Please try to find out
why given a package "wants to use them" before removing.)

I don't see why it would make sense to just suppose
that this is all unnecessary cruft, and tell users
to start removing their own such, without any trial
of removing it from the Emacs code first.

Message #47 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Glenn Morris <rgm <at> gnu.org>, Roland Winkler <winkler <at> gnu.org>,
 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 21:30:44 +0100

[Message part 1 (text/plain, inline)]

Drew Adams <drew.adams <at> oracle.com> writes:

>> To summarize what has come up in this thread so far, the load-hook
>> convention has the following problems:
>> 
>> 1. They are never called if set by a user after a library is loaded.
>> 2. They can lead to issues like Bug#24491.
>> 3. They require every package (that wants to use them) to add boiler
>>    plate code.
>> 
>> They are also redundant since we have eval-after-load and
>> with-eval-after-load, which do not have any of the above problems.
>
> Here's what I'd suggest, if you are bent on removing
> all load hooks and deprecating them:
>
> 1. Try removing all of them from the vanilla
>    (distributed) Elisp code.
>
> 2. Run with that for a major release or two.  If no
>    problems, then deprecate (declare obsolete).

I'm not sure I understand the proposal.  What is the "vanilla
(distributed) Elisp code"?  What does "removing all of them" mean, and
how is removing them more cautious than adding a deprecation warning
to the variables?

Or, are we perhaps talking about different things?

IIUC, load-hooks is just a convention to:

a) (defvar foo-load-hook nil)  ;; or defcustom spanning several lines, or
                               ;; indeed no declaration at all
b) (run-hooks 'foo-load-hook)  ;; added just before the provide statement

Third party packages are free to continue doing that.  AFAICT, we have
no way to stop them -- and I wouldn't advocate for that.

Am I missing something here?

I'm not sure if this was clear, but the course of action suggested by
Glenn was to add deprecation warnings to the load-hook variables in
GNU Emacs.  Please see the attached patches for an example.

Best regards,
Stefan Kangas

[0001-Obsolete-viper-load-hook-in-favor-of-eval-after-load.patch (text/x-diff, attachment)]

[0002-Obsolete-reftex-load-hook-in-favor-of-eval-after-loa.patch (text/x-diff, attachment)]

Message #50 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Glenn Morris <rgm <at> gnu.org>, Roland Winkler <winkler <at> gnu.org>,
 21563 <at> debbugs.gnu.org
Subject: RE: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 13:08:56 -0800 (PST)

> > Here's what I'd suggest, if you are bent on removing
> > all load hooks and deprecating them:
> >
> > 1. Try removing all of them from the vanilla
> >    (distributed) Elisp code.
> >
> > 2. Run with that for a major release or two.  If no
> >    problems, then deprecate (declare obsolete).
> 
> I'm not sure I understand the proposal.  What is the "vanilla
> (distributed) Elisp code"?

The code that GNU Emacs distributes.

> What does "removing all of them" mean, and
> how is removing them more cautious than adding a deprecation warning
> to the variables?

It doesn't matter to me whether you remove all or only
some.  I meant remove them - any or all.  The more you
remove, the more sure you are that deprecation might
make sense.  Don't deprecate (step 2) before removing
them all, though, to be sure your replacement handles
all of the existing GNU Emacs cases, at least.

> Third party packages are free to continue doing that.  AFAICT, we have
> no way to stop them -- and I wouldn't advocate for that.

For step 1, I'm suggesting that you _not_ deprecate
(declare obsolete) or recommend against.  Otherwise,
you are prescribing something for 3rd-party code.

And you're doing that before you've actually tried
it thoroughly for your own code.

> Am I missing something here?
> 
> I'm not sure if this was clear, but the course of action suggested by
> Glenn was to add deprecation warnings to the load-hook variables in
> GNU Emacs.  Please see the attached patches for an example.

And that is just what I'm suggesting not to do.  First
remove them from wherever you want from the GNU Emacs
code.  Then wait, and see how that goes.  Then, after
a release or two, provide your deprecation warnings.

There's no hurry for this, AFAIK.  And these things were
added on purpose - they didn't fall from the sky.  Wait
and see how things go.  That's my suggestion.

It makes little sense (to me) to deprecate something
before you've even tried doing without it for a while.
Go on the diet yourself (not you, personally!) before
you start telling everyone outside core Emacs to go
on it.  That's all.

Message #53 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 21563 <at> debbugs.gnu.org, Glenn Morris <rgm <at> gnu.org>,
 Roland Winkler <winkler <at> gnu.org>, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 16:35:17 -0500

Stefan Kangas <stefan <at> marxist.se> writes:

> Drew Adams <drew.adams <at> oracle.com> writes:
>
>> Here's what I'd suggest, if you are bent on removing
>> all load hooks and deprecating them:
>>
>> 1. Try removing all of them from the vanilla
>>    (distributed) Elisp code.
>>
>> 2. Run with that for a major release or two.  If no
>>    problems, then deprecate (declare obsolete).
>
> I'm not sure I understand the proposal.  What is the "vanilla
> (distributed) Elisp code"?  What does "removing all of them" mean, and
> how is removing them more cautious than adding a deprecation warning
> to the variables?

I think Drew meant to remove the *usage* of the hooks.  That is, remove
cases of (add-hook 'foo-load-hook ...), before deprecating foo-load-hook
itself.

Message #56 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Noam Postavsky <npostavs <at> gmail.com>, Stefan Kangas <stefan <at> marxist.se>
Cc: Glenn Morris <rgm <at> gnu.org>, Roland Winkler <winkler <at> gnu.org>,
 21563 <at> debbugs.gnu.org
Subject: RE: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Jan 2020 14:19:57 -0800 (PST)

> >> Here's what I'd suggest, if you are bent on removing
> >> all load hooks and deprecating them:
> >> 1. Try removing all of them from the vanilla
> >>    (distributed) Elisp code.
> >> 2. Run with that for a major release or two.  If no
> >>    problems, then deprecate (declare obsolete).
> >
> > I'm not sure I understand the proposal.  What is the "vanilla
> > (distributed) Elisp code"?  What does "removing all of them" mean,
> > and how is removing them more cautious than adding a deprecation
> > warning to the variables?
> 
> I think Drew meant to remove the *usage* of the hooks.  That is, remove
> cases of (add-hook 'foo-load-hook ...), before deprecating foo-load-
> hook itself.

Yes.  Sorry if that wasn't clear.  That should be sufficient for
testing the (later) removal of the variables, as far as their
usage by vanilla Emacs code is concerned.

(It won't test effects on user code, of course.  But the point is
to not (possibly) bother users for the duration of step 1.)

Message #59 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Glenn Morris <rgm <at> gnu.org>, Noam Postavsky <npostavs <at> gmail.com>,
 21563 <at> debbugs.gnu.org, Roland Winkler <winkler <at> gnu.org>
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Fri, 17 Jan 2020 05:44:18 +0100

[Message part 1 (text/plain, inline)]

Drew Adams <drew.adams <at> oracle.com> writes:

>> I think Drew meant to remove the *usage* of the hooks.  That is, remove
>> cases of (add-hook 'foo-load-hook ...), before deprecating foo-load-
>> hook itself.
>
> Yes.  Sorry if that wasn't clear.  That should be sufficient for
> testing the (later) removal of the variables, as far as their
> usage by vanilla Emacs code is concerned.

Thanks for clearing that up.

I've attached a patch removing all uses of load-hooks that I could
find in the tree.

In my opinion, the patch demonstrates clearly that, in every case
where we use load-hooks, eval-after-load makes the code both clearer
and more succinct.

I don't mind if we have a certain grace period between installing this
and declaring the variables obsolete.  I don't see that it's necessary
to wait until Emacs 29.1 to do it, though.

Best regards,
Stefan Kangas

[0001-Prefer-eval-after-load-over-load-hooks.patch (text/x-diff, inline)]

From 247bd801163ba2e11f09d827150e35ab6b8e7cab Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas <at> gmail.com>
Date: Fri, 17 Jan 2020 05:11:58 +0100
Subject: [PATCH] Prefer eval-after-load over load-hooks

* lisp/cedet/ede.el:
* lisp/cedet/semantic/imenu.el:
* lisp/emacs-lisp/edebug.el:
* lisp/emacs-lisp/eieio-opt.el:
* lisp/emacs-lisp/eieio-speedbar.el:
* lisp/erc/erc-speedbar.el:
* lisp/info.el:
* lisp/mail/rmail.el:
* lisp/progmodes/gud.el:
* lisp/progmodes/vhdl-mode.el: Replace usage of load-hooks with
eval-after-load.  (Bug#21563)
---
 lisp/cedet/ede.el                 |  4 +---
 lisp/cedet/semantic/imenu.el      |  4 +---
 lisp/emacs-lisp/edebug.el         | 10 ++--------
 lisp/emacs-lisp/eieio-opt.el      | 20 +++++++-------------
 lisp/emacs-lisp/eieio-speedbar.el |  8 ++------
 lisp/erc/erc-speedbar.el          |  4 +---
 lisp/info.el                      |  4 +---
 lisp/mail/rmail.el                |  4 +---
 lisp/progmodes/gud.el             |  4 +---
 lisp/progmodes/vhdl-mode.el       |  7 +++----
 10 files changed, 20 insertions(+), 49 deletions(-)

diff --git a/lisp/cedet/ede.el b/lisp/cedet/ede.el
index 1418ad9539..8d46dfb72f 100644
--- a/lisp/cedet/ede.el
+++ b/lisp/cedet/ede.el
@@ -1527,8 +1527,6 @@ project-roots
 
 ;; If this does not occur after the provide, we can get a recursive
 ;; load.  Yuck!
-(if (featurep 'speedbar)
-    (ede-speedbar-file-setup)
-  (add-hook 'speedbar-load-hook 'ede-speedbar-file-setup))
+(eval-after-load 'speedbar '(ede-speedbar-file-setup))
 
 ;;; ede.el ends here
diff --git a/lisp/cedet/semantic/imenu.el b/lisp/cedet/semantic/imenu.el
index 19e0515ac6..0609e2bea8 100644
--- a/lisp/cedet/semantic/imenu.el
+++ b/lisp/cedet/semantic/imenu.el
@@ -44,9 +44,7 @@
 
 ;; Because semantic imenu tags will hose the current imenu handling
 ;; code in speedbar, force semantic/sb in.
-(if (featurep 'speedbar)
-    (require 'semantic/sb)
-  (add-hook 'speedbar-load-hook (lambda () (require 'semantic/sb))))
+(eval-after-load 'speedbar '(require 'semantic/sb))
 
 (defgroup semantic-imenu nil
   "Semantic interface to Imenu."
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index b8d2fb5beb..f600299da2 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -4522,13 +4522,8 @@ edebug-menu
 ;;; Autoloading of Edebug accessories
 
 ;; edebug-cl-read and cl-read are available from liberte <at> cs.uiuc.edu
-(defun edebug--require-cl-read ()
-  (require 'edebug-cl-read))
-
-(if (featurep 'cl-read)
-    (add-hook 'edebug-setup-hook #'edebug--require-cl-read)
-  ;; The following causes edebug-cl-read to be loaded when you load cl-read.el.
-  (add-hook 'cl-read-load-hooks #'edebug--require-cl-read))
+;; FIXME: Is this still relevant, or should it be removed?
+(eval-after-load 'cl-read '(require 'edebug-cl-read))
 
 
 ;;; Finalize Loading
@@ -4564,7 +4559,6 @@ edebug-unload-function
       (run-with-idle-timer 0 nil #'(lambda () (unload-feature 'edebug)))))
   (remove-hook 'called-interactively-p-functions
                #'edebug--called-interactively-skip)
-  (remove-hook 'cl-read-load-hooks #'edebug--require-cl-read)
   (edebug-uninstall-read-eval-functions)
   ;; Continue standard unloading.
   nil)
diff --git a/lisp/emacs-lisp/eieio-opt.el b/lisp/emacs-lisp/eieio-opt.el
index dda9037306..e0095796ae 100644
--- a/lisp/emacs-lisp/eieio-opt.el
+++ b/lisp/emacs-lisp/eieio-opt.el
@@ -278,19 +278,13 @@ eieio-class-speedbar-make-map
 
 (if eieio-class-speedbar-key-map
     nil
-  (if (not (featurep 'speedbar))
-      (add-hook 'speedbar-load-hook (lambda ()
-				      (eieio-class-speedbar-make-map)
-				      (speedbar-add-expansion-list
-				       '("EIEIO"
-					 eieio-class-speedbar-menu
-					 eieio-class-speedbar-key-map
-					 eieio-class-speedbar))))
-    (eieio-class-speedbar-make-map)
-    (speedbar-add-expansion-list '("EIEIO"
-				   eieio-class-speedbar-menu
-				   eieio-class-speedbar-key-map
-				   eieio-class-speedbar))))
+  (eval-after-load 'speedbar (progn
+                               (eieio-class-speedbar-make-map)
+                               (speedbar-add-expansion-list
+                                '("EIEIO"
+                                  eieio-class-speedbar-menu
+                                  eieio-class-speedbar-key-map
+                                  eieio-class-speedbar)))))
 
 (defvar eieio-class-speedbar-menu
   ()
diff --git a/lisp/emacs-lisp/eieio-speedbar.el b/lisp/emacs-lisp/eieio-speedbar.el
index c11608da5d..3236a08e89 100644
--- a/lisp/emacs-lisp/eieio-speedbar.el
+++ b/lisp/emacs-lisp/eieio-speedbar.el
@@ -140,12 +140,8 @@ eieio-speedbar-create
 MODENAME is a string used to identify this browser mode.
 FETCHER is a generic function used to fetch the base object list used when
 creating the speedbar display."
-  (if (not (featurep 'speedbar))
-      (add-hook 'speedbar-load-hook
-		(list 'lambda nil
-		      (list 'eieio-speedbar-create-engine
-			    map-fn map-var menu-var modename fetcher)))
-    (eieio-speedbar-create-engine map-fn map-var menu-var modename fetcher)))
+  (eval-after-load 'speedbar
+    '(eieio-speedbar-create-engine map-fn map-var menu-var modename fetcher)))
 
 (defun eieio-speedbar-create-engine (map-fn map-var menu-var modename fetcher)
   "Create a speedbar mode for displaying an object hierarchy.
diff --git a/lisp/erc/erc-speedbar.el b/lisp/erc/erc-speedbar.el
index 1ff44afe46..365510bd81 100644
--- a/lisp/erc/erc-speedbar.el
+++ b/lisp/erc/erc-speedbar.el
@@ -89,9 +89,7 @@ erc-speedbar-menu-items
   "Additional menu-items to add to speedbar frame.")
 
 ;; Make sure our special speedbar major mode is loaded
-(if (featurep 'speedbar)
-    (erc-install-speedbar-variables)
-  (add-hook 'speedbar-load-hook 'erc-install-speedbar-variables))
+(eval-after-load 'speedbar '(erc-install-speedbar-variables))
 
 ;;; ERC hierarchy display method
 ;;;###autoload
diff --git a/lisp/info.el b/lisp/info.el
index 7a11bb3ff9..cbec97e99f 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -5135,9 +5135,7 @@ Info-speedbar-menu-items
   "Additional menu-items to add to speedbar frame.")
 
 ;; Make sure our special speedbar major mode is loaded
-(if (featurep 'speedbar)
-    (Info-install-speedbar-variables)
-  (add-hook 'speedbar-load-hook 'Info-install-speedbar-variables))
+(eval-after-load 'speedbar '(Info-install-speedbar-variables))
 
 ;;; Info hierarchy display method
 ;;;###autoload
diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
index d798ffa051..895f2a1243 100644
--- a/lisp/mail/rmail.el
+++ b/lisp/mail/rmail.el
@@ -4392,9 +4392,7 @@ rmail-speedbar-menu-items
 		  (text face mouse function &optional token prevline))
 
 ;; Make sure our special speedbar major mode is loaded
-(if (featurep 'speedbar)
-    (rmail-install-speedbar-variables)
-  (add-hook 'speedbar-load-hook 'rmail-install-speedbar-variables))
+(eval-after-load 'speedbar '(rmail-install-speedbar-variables))
 
 (defun rmail-speedbar-buttons (buffer)
   "Create buttons for BUFFER containing rmail messages.
diff --git a/lisp/progmodes/gud.el b/lisp/progmodes/gud.el
index d5fd1dce6f..96a617826f 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -486,9 +486,7 @@ gud-speedbar-menu-items
   "Additional menu items to add to the speedbar frame.")
 
 ;; Make sure our special speedbar mode is loaded
-(if (featurep 'speedbar)
-    (gud-install-speedbar-variables)
-  (add-hook 'speedbar-load-hook 'gud-install-speedbar-variables))
+(eval-after-load 'speedbar '(gud-install-speedbar-variables))
 
 (defun gud-expansion-speedbar-buttons (_directory _zero)
   "Wrapper for call to `speedbar-add-expansion-list'.
diff --git a/lisp/progmodes/vhdl-mode.el b/lisp/progmodes/vhdl-mode.el
index b225a9b1d9..9e11a17e60 100644
--- a/lisp/progmodes/vhdl-mode.el
+++ b/lisp/progmodes/vhdl-mode.el
@@ -16147,10 +16147,9 @@ vhdl-speedbar-instantiation-selected-face
     (select-frame current-frame)))
 
 ;; initialize speedbar
-(if (not (boundp 'speedbar-frame))
-    (add-hook 'speedbar-load-hook 'vhdl-speedbar-initialize)
-  (vhdl-speedbar-initialize)
-  (when speedbar-frame (vhdl-speedbar-refresh)))
+(eval-after-load 'speedbar '(vhdl-speedbar-initialize))
+(when (and (boundp 'speedbar-frame) speedbar-frame)
+  (vhdl-speedbar-refresh))
 
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-- 
2.20.1

Message #62 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Roland Winkler <winkler <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Apr 2020 06:14:18 +0200

Glenn Morris <rgm <at> gnu.org> writes:

> I did this years ago for a few modes; eg dd210a6, 041e909, but never got
> round to doing the rest. As I said in https://debbugs.gnu.org/24491#8, I
> see no need to keep any *-load-hooks.

I see you have now done the rest on master.  Does that mean that this
bug can also be closed?  Thanks.

Best regards,
Stefan Kangas

Message #65 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: "Roland Winkler" <winkler <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Glenn Morris <rgm <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Thu, 16 Apr 2020 11:07:40 +0200

On Thu Apr 16 2020 Stefan Kangas wrote:
> Glenn Morris <rgm <at> gnu.org> writes:
> 
> > I did this years ago for a few modes; eg dd210a6, 041e909, but never got
> > round to doing the rest. As I said in https://debbugs.gnu.org/24491#8, I
> > see no need to keep any *-load-hooks.
> 
> I see you have now done the rest on master.  Does that mean that this
> bug can also be closed?  Thanks.

Thanks for taking care of this.  I am glad the load hooks are gone.
From my perspective the bug can be closed.

Message #68 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: "Roland Winkler" <winkler <at> gnu.org>
Cc: Glenn Morris <rgm <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Sun, 26 Apr 2020 16:33:16 +0200

close 21563 28.1
thanks

"Roland Winkler" <winkler <at> gnu.org> writes:

> On Thu Apr 16 2020 Stefan Kangas wrote:
>> Glenn Morris <rgm <at> gnu.org> writes:
>> 
>> > I did this years ago for a few modes; eg dd210a6, 041e909, but never got
>> > round to doing the rest. As I said in https://debbugs.gnu.org/24491#8, I
>> > see no need to keep any *-load-hooks.
>> 
>> I see you have now done the rest on master.  Does that mean that this
>> bug can also be closed?  Thanks.
>
> Thanks for taking care of this.  I am glad the load hooks are gone.
>>From my perspective the bug can be closed.

No further comments within a week, so I'm closing the bug now.
Thanks.

Best regards,
Stefan Kangas

Message #77 received at 21563 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Roland Winkler <winkler <at> gnu.org>
Cc: Glenn Morris <rgm <at> gnu.org>, 21563 <at> debbugs.gnu.org
Subject: Re: bug#21563: 24.5; discourage load-hook variables
Date: Tue, 20 Oct 2020 17:16:50 +0000

Stefan Kangas <stefan <at> marxist.se> writes:

>>> > I did this years ago for a few modes; eg dd210a6, 041e909, but never got
>>> > round to doing the rest. As I said in https://debbugs.gnu.org/24491#8, I
>>> > see no need to keep any *-load-hooks.
>>>
>>> I see you have now done the rest on master.  Does that mean that this
>>> bug can also be closed?  Thanks.
>>
>> Thanks for taking care of this.  I am glad the load hooks are gone.
>>>>From my perspective the bug can be closed.
>
> No further comments within a week, so I'm closing the bug now.

Some load-hooks were never obsoleted here.  I've now marked them
obsolete on the master branch in commit 6c58d90042.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.