unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#21563: 24.5; discourage load-hook variables
@ 2015-09-25 18:57 Roland Winkler
  2020-01-15 19:32 ` Stefan Kangas
  2020-01-16 11:49 ` Mauro Aranda
  0 siblings, 2 replies; 20+ messages in thread
From: Roland Winkler @ 2015-09-25 18:57 UTC (permalink / raw)
  To: 21563


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





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

* bug#21563: 24.5; discourage load-hook variables
  2015-09-25 18:57 bug#21563: 24.5; discourage load-hook variables Roland Winkler
@ 2020-01-15 19:32 ` Stefan Kangas
  2020-01-15 20:21   ` Drew Adams
  2020-01-15 22:06   ` Glenn Morris
  2020-01-16 11:49 ` Mauro Aranda
  1 sibling, 2 replies; 20+ messages in thread
From: Stefan Kangas @ 2020-01-15 19:32 UTC (permalink / raw)
  To: Roland Winkler; +Cc: 21563

"Roland Winkler" <winkler@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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-15 19:32 ` Stefan Kangas
@ 2020-01-15 20:21   ` Drew Adams
  2020-01-15 20:42     ` Stefan Kangas
  2020-01-15 22:06   ` Glenn Morris
  1 sibling, 1 reply; 20+ messages in thread
From: Drew Adams @ 2020-01-15 20:21 UTC (permalink / raw)
  To: Stefan Kangas, Roland Winkler; +Cc: 21563

>> 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?





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-15 20:21   ` Drew Adams
@ 2020-01-15 20:42     ` Stefan Kangas
  2020-01-16  0:27       ` Roland Winkler
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Kangas @ 2020-01-15 20:42 UTC (permalink / raw)
  To: Drew Adams; +Cc: Roland Winkler, 21563

Drew Adams <drew.adams@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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-15 19:32 ` Stefan Kangas
  2020-01-15 20:21   ` Drew Adams
@ 2020-01-15 22:06   ` Glenn Morris
  2020-01-16  0:03     ` Stefan Kangas
  2020-04-16  4:14     ` Stefan Kangas
  1 sibling, 2 replies; 20+ messages in thread
From: Glenn Morris @ 2020-01-15 22:06 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Roland Winkler, 21563

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.





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-15 22:06   ` Glenn Morris
@ 2020-01-16  0:03     ` Stefan Kangas
  2020-01-16  0:24       ` Drew Adams
  2020-01-16  0:31       ` Stefan Kangas
  2020-04-16  4:14     ` Stefan Kangas
  1 sibling, 2 replies; 20+ messages in thread
From: Stefan Kangas @ 2020-01-16  0:03 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Roland Winkler, 21563

Glenn Morris <rgm@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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16  0:03     ` Stefan Kangas
@ 2020-01-16  0:24       ` Drew Adams
  2020-01-16  0:54         ` Stefan Kangas
  2020-01-16  0:31       ` Stefan Kangas
  1 sibling, 1 reply; 20+ messages in thread
From: Drew Adams @ 2020-01-16  0:24 UTC (permalink / raw)
  To: Stefan Kangas, Glenn Morris; +Cc: Roland Winkler, 21563

> 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).





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-15 20:42     ` Stefan Kangas
@ 2020-01-16  0:27       ` Roland Winkler
  0 siblings, 0 replies; 20+ messages in thread
From: Roland Winkler @ 2020-01-16  0:27 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 21563

On Wed Jan 15 2020 Stefan Kangas wrote:
> Drew Adams <drew.adams@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?





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16  0:03     ` Stefan Kangas
  2020-01-16  0:24       ` Drew Adams
@ 2020-01-16  0:31       ` Stefan Kangas
  1 sibling, 0 replies; 20+ messages in thread
From: Stefan Kangas @ 2020-01-16  0:31 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Roland Winkler, 21563

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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16  0:24       ` Drew Adams
@ 2020-01-16  0:54         ` Stefan Kangas
  2020-01-16  3:56           ` Drew Adams
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Kangas @ 2020-01-16  0:54 UTC (permalink / raw)
  To: Drew Adams; +Cc: Roland Winkler, 21563

Drew Adams <drew.adams@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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16  0:54         ` Stefan Kangas
@ 2020-01-16  3:56           ` Drew Adams
  2020-01-16 13:33             ` Stefan Kangas
  0 siblings, 1 reply; 20+ messages in thread
From: Drew Adams @ 2020-01-16  3:56 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Roland Winkler, 21563

> > 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.





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

* bug#21563: 24.5; discourage load-hook variables
  2015-09-25 18:57 bug#21563: 24.5; discourage load-hook variables Roland Winkler
  2020-01-15 19:32 ` Stefan Kangas
@ 2020-01-16 11:49 ` Mauro Aranda
  1 sibling, 0 replies; 20+ messages in thread
From: Mauro Aranda @ 2020-01-16 11:49 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 21563

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

Stefan Kangas <stefan@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).

[-- Attachment #2: Type: text/html, Size: 591 bytes --]

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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16  3:56           ` Drew Adams
@ 2020-01-16 13:33             ` Stefan Kangas
  2020-01-16 16:15               ` Drew Adams
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Kangas @ 2020-01-16 13:33 UTC (permalink / raw)
  To: Drew Adams; +Cc: Roland Winkler, 21563

Drew Adams <drew.adams@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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16 13:33             ` Stefan Kangas
@ 2020-01-16 16:15               ` Drew Adams
  2020-01-16 20:30                 ` Stefan Kangas
  0 siblings, 1 reply; 20+ messages in thread
From: Drew Adams @ 2020-01-16 16:15 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Roland Winkler, 21563

> 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.





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16 16:15               ` Drew Adams
@ 2020-01-16 20:30                 ` Stefan Kangas
  2020-01-16 21:08                   ` Drew Adams
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Kangas @ 2020-01-16 20:30 UTC (permalink / raw)
  To: Drew Adams; +Cc: Roland Winkler, 21563

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

Drew Adams <drew.adams@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Obsolete-viper-load-hook-in-favor-of-eval-after-load.patch --]
[-- Type: text/x-diff, Size: 2037 bytes --]

From a47b2cb213af71978e8a16d596934fa318847971 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 16 Jan 2020 01:07:20 +0100
Subject: [PATCH 1/3] Obsolete viper-load-hook in favor of eval-after-load

* lisp/emulation/viper-init.el (viper-load-hook): Make obsolete.
* doc/misc/viper.texi (Rudimentary Changes): Doc fix - no longer
mention viper-load-hook.
---
 doc/misc/viper.texi          | 5 +----
 lisp/emulation/viper-init.el | 3 +++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/misc/viper.texi b/doc/misc/viper.texi
index 9ce809e7d4..087eb2701d 100644
--- a/doc/misc/viper.texi
+++ b/doc/misc/viper.texi
@@ -1755,7 +1755,7 @@ Rudimentary Changes
 Insert state, or Emacs state.  This heuristics works well in virtually all
 cases.  @code{nil} means you either has to invoke @code{viper-mode} manually
 for each buffer (or you can add @code{viper-mode} to the appropriate major mode
-hooks using @code{viper-load-hook}).
+hooks).
 
 This option must be set in your Viper customization file.
 @item viper-custom-file-name "~/.emacs.d/viper"
@@ -1903,9 +1903,6 @@ Rudimentary Changes
 @item viper-emacs-state-hook nil
 List of (parameterless) functions called just after switching from Vi state
 to Emacs state.
-@item viper-load-hook nil
-List of (parameterless) functions called just after loading Viper.  This is
-the last chance to do customization before Viper is up and running.
 @end table
 @noindent
 You can reset some of these constants in Viper with the Ex command @kbd{:set}
diff --git a/lisp/emulation/viper-init.el b/lisp/emulation/viper-init.el
index 511c68f24a..09fa0ba7f6 100644
--- a/lisp/emulation/viper-init.el
+++ b/lisp/emulation/viper-init.el
@@ -923,6 +923,9 @@ viper-load-hook
   :type 'hook
   :group 'viper-hooks)
 
+(make-obsolete-variable 'viper-load-hook
+                        "use `with-eval-after-load' instead." "28.1")
+
 (defun viper-restore-cursor-type ()
   (condition-case nil
       (setq cursor-type (default-value 'cursor-type))
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Obsolete-reftex-load-hook-in-favor-of-eval-after-loa.patch --]
[-- Type: text/x-diff, Size: 2051 bytes --]

From c225cd8059607eeef0d65b57db31122e36b0471c Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 16 Jan 2020 01:23:46 +0100
Subject: [PATCH 2/3] Obsolete reftex-load-hook in favor of eval-after-load

* lisp/textmodes/reftex-vars.el (reftex-load-hook): Make obsolete.
* doc/misc/reftex.texi (Key Bindings, Keymaps and Hooks): No
longer mention reftex-load-hook.  (Bug#21563)
---
 doc/misc/reftex.texi          | 9 ++-------
 lisp/textmodes/reftex-vars.el | 3 +++
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/doc/misc/reftex.texi b/doc/misc/reftex.texi
index 013c5639a1..0dab524151 100644
--- a/doc/misc/reftex.texi
+++ b/doc/misc/reftex.texi
@@ -2896,9 +2896,8 @@ Key Bindings
 Note that this variable has to be set before @RefTeX{} is loaded to
 have an effect.
 
-@vindex reftex-load-hook
-Changing and adding to @RefTeX{}'s key bindings is best done in the hook
-@code{reftex-load-hook}.  For information on the keymaps
+Changing and adding to @RefTeX{}'s key bindings is best done using
+@code{with-eval-after-load}.  For information on the keymaps
 which should be used to add keys, see @ref{Keymaps and Hooks}.
 
 @node Faces
@@ -5320,10 +5319,6 @@ Keymaps and Hooks
 The keymap for @RefTeX{} mode.
 @end deffn
 
-@deffn {Normal Hook} reftex-load-hook
-Normal hook which is being run when loading @file{reftex.el}.
-@end deffn
-
 @deffn {Normal Hook} reftex-mode-hook
 Normal hook which is being run when turning on @RefTeX{} mode.
 @end deffn
diff --git a/lisp/textmodes/reftex-vars.el b/lisp/textmodes/reftex-vars.el
index ca92541331..d2324dd4ae 100644
--- a/lisp/textmodes/reftex-vars.el
+++ b/lisp/textmodes/reftex-vars.el
@@ -2101,6 +2101,9 @@ reftex-load-hook
   :group 'reftex-miscellaneous-configurations
   :type 'hook)
 
+(make-obsolete-variable 'reftex-load-hook
+                        "use `with-eval-after-load' instead." "28.1")
+
 (defcustom reftex-mode-hook nil
   "Hook which is being run when turning on RefTeX mode."
   :group 'reftex-miscellaneous-configurations
-- 
2.20.1


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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-16 20:30                 ` Stefan Kangas
@ 2020-01-16 21:08                   ` Drew Adams
  0 siblings, 0 replies; 20+ messages in thread
From: Drew Adams @ 2020-01-16 21:08 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Roland Winkler, 21563

> > 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.





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

* bug#21563: 24.5; discourage load-hook variables
  2020-01-15 22:06   ` Glenn Morris
  2020-01-16  0:03     ` Stefan Kangas
@ 2020-04-16  4:14     ` Stefan Kangas
  2020-04-16  9:07       ` Roland Winkler
  1 sibling, 1 reply; 20+ messages in thread
From: Stefan Kangas @ 2020-04-16  4:14 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Roland Winkler, 21563

Glenn Morris <rgm@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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-04-16  4:14     ` Stefan Kangas
@ 2020-04-16  9:07       ` Roland Winkler
  2020-04-26 14:33         ` Stefan Kangas
  0 siblings, 1 reply; 20+ messages in thread
From: Roland Winkler @ 2020-04-16  9:07 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 21563

On Thu Apr 16 2020 Stefan Kangas wrote:
> Glenn Morris <rgm@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.





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

* bug#21563: 24.5; discourage load-hook variables
  2020-04-16  9:07       ` Roland Winkler
@ 2020-04-26 14:33         ` Stefan Kangas
  2020-10-20 17:16           ` Stefan Kangas
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Kangas @ 2020-04-26 14:33 UTC (permalink / raw)
  To: Roland Winkler; +Cc: 21563

close 21563 28.1
thanks

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

> On Thu Apr 16 2020 Stefan Kangas wrote:
>> Glenn Morris <rgm@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





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

* bug#21563: 24.5; discourage load-hook variables
  2020-04-26 14:33         ` Stefan Kangas
@ 2020-10-20 17:16           ` Stefan Kangas
  0 siblings, 0 replies; 20+ messages in thread
From: Stefan Kangas @ 2020-10-20 17:16 UTC (permalink / raw)
  To: Roland Winkler; +Cc: Glenn Morris, 21563

Stefan Kangas <stefan@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.





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

end of thread, other threads:[~2020-10-20 17:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-25 18:57 bug#21563: 24.5; discourage load-hook variables Roland Winkler
2020-01-15 19:32 ` Stefan Kangas
2020-01-15 20:21   ` Drew Adams
2020-01-15 20:42     ` Stefan Kangas
2020-01-16  0:27       ` Roland Winkler
2020-01-15 22:06   ` Glenn Morris
2020-01-16  0:03     ` Stefan Kangas
2020-01-16  0:24       ` Drew Adams
2020-01-16  0:54         ` Stefan Kangas
2020-01-16  3:56           ` Drew Adams
2020-01-16 13:33             ` Stefan Kangas
2020-01-16 16:15               ` Drew Adams
2020-01-16 20:30                 ` Stefan Kangas
2020-01-16 21:08                   ` Drew Adams
2020-01-16  0:31       ` Stefan Kangas
2020-04-16  4:14     ` Stefan Kangas
2020-04-16  9:07       ` Roland Winkler
2020-04-26 14:33         ` Stefan Kangas
2020-10-20 17:16           ` Stefan Kangas
2020-01-16 11:49 ` Mauro Aranda

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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).