bug#21563: 24.5; discourage load-hook variables

[Stefan Kangas <stefan@marxist.se>]

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