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

Previous Next

Full log

View this message in rfc822 format

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

Previous Next

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