bug#5293: 23.1; unload-feature on buffer-local hooks

[Štěpán Němec]

On Wed, 30 Sep 2020 20:44:04 +0200
Lars Ingebrigtsen wrote:

> The patch no longer applied to the trunk,

That's just the usual etc/NEWS conflict, I think.

> and it referred to a variable called `removables' that I couldn't
> find.

It's a 3-patch series; the variable is introduced by the previous patch.

For convenience, I've attached the whole series rebased on top of
current master.

Given the lack of further feedback, I haven't pushed (for pushing) this,
as I kinda liked Stefan's other suggestion, too ("If we need it to go
faster maybe we could also arrange for (add-hook V F ..) to do
(cl-pushnew V (get F 'hooks-where-it-has-been-put)).", from

https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-04/msg00163.html
jwvsghgcxi5.fsf-monnier+emacs@gnu.org
)

But this version seems fast enough, so maybe we can push this and be
done with it, at least for the time being.

-- 
Štěpán

>From 0d41a85c1881cd24a384e89227b3079bb62d5e30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= <stepnem@gmail.com>
Date: Mon, 6 Apr 2020 13:25:41 +0200
Subject: [PATCH 1/3] unload-feature: Improve logic (don't repeat computation)

* lisp/loadhist.el (unload-feature): Don't do the same computation twice.
---
 lisp/loadhist.el | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/lisp/loadhist.el b/lisp/loadhist.el
index a1ff2f6270..60da00cceb 100644
--- a/lisp/loadhist.el
+++ b/lisp/loadhist.el
@@ -287,22 +287,23 @@ unload-feature
        ;; functions which the package might just have installed, and
        ;; there might be other important state, but this tactic
        ;; normally works.
-       (mapatoms
-        (lambda (x)
-          (when (and (boundp x)
-                     (or (and (consp (symbol-value x)) ; Random hooks.
-                              (string-match "-hooks?\\'" (symbol-name x)))
-                         (memq x unload-feature-special-hooks)))       ; Known 
abnormal hooks etc.
-            (dolist (y unload-function-defs-list)
-              (when (and (eq (car-safe y) 'defun)
-                         (not (get (cdr y) 'autoload)))
-                (remove-hook x (cdr y)))))))
-       ;; Remove any feature-symbols from auto-mode-alist as well.
-       (dolist (y unload-function-defs-list)
-         (when (and (eq (car-safe y) 'defun)
-                    (not (get (cdr y) 'autoload)))
-           (setq auto-mode-alist
-                 (rassq-delete-all (cdr y) auto-mode-alist)))))
+        (let ((removables (cl-loop for def in unload-function-defs-list
+                                   when (and (eq (car-safe def) 'defun)
+                                             (not (get (cdr def) 'autoload)))
+                                   collect (cdr def))))
+          (mapatoms
+          (lambda (x)
+            (when (and (boundp x)
+                       (or (and (consp (symbol-value x)) ; Random hooks.
+                                (string-match "-hooks?\\'" (symbol-name x)))
+                            ;; Known abnormal hooks etc.
+                           (memq x unload-feature-special-hooks)))
+              (dolist (func removables)
+                (remove-hook x func)))))
+          ;; Remove any feature-symbols from auto-mode-alist as well.
+          (dolist (func removables)
+            (setq auto-mode-alist
+                  (rassq-delete-all func auto-mode-alist)))))
 
       ;; Change major mode in all buffers using one defined in the feature 
being unloaded.
       (unload--set-major-mode)
-- 
2.28.0

>From c50b6dec73739e5c3e1f18774b98adae213cf76c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= <stepnem@gmail.com>
Date: Mon, 6 Apr 2020 13:30:11 +0200
Subject: [PATCH 2/3] unload-feature: Handle local hooks

Buffer-local hooks were introduced in

1994-09-30T20:47:13+00:00!rms@gnu.org
0e4d378b32 (add-hook): Initialize default value and local value.

but `unload-feature' has not been updated to handle them.

* lisp/loadhist.el (unload-feature): Handle local hooks.
---
 etc/NEWS         | 3 +++
 lisp/loadhist.el | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index c571fa95d1..3739b1c67e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1743,6 +1743,9 @@ to lexical binding, where dynamic (special) variables 
bound in one
 file can affect code in another.  For details, see the manual section
 '(Elisp) Converting to Lexical Binding'.
 
+---
+** 'unload-feature' now also tries to undo additions to buffer-local hooks.
+
 
 * Changes in Emacs 28.1 on Non-Free Operating Systems
 
diff --git a/lisp/loadhist.el b/lisp/loadhist.el
index 60da00cceb..81576679c3 100644
--- a/lisp/loadhist.el
+++ b/lisp/loadhist.el
@@ -300,6 +300,15 @@ unload-feature
                            (memq x unload-feature-special-hooks)))
               (dolist (func removables)
                 (remove-hook x func)))))
+          (save-current-buffer
+            (dolist (buffer (buffer-list))
+              (pcase-dolist (`(,sym . ,val) (buffer-local-variables buffer))
+                (when (or (and (consp val)
+                               (string-match "-hooks?\\'" (symbol-name sym)))
+                          (memq sym unload-feature-special-hooks))
+                  (set-buffer buffer)
+                  (dolist (func removables)
+                    (remove-hook sym func t))))))
           ;; Remove any feature-symbols from auto-mode-alist as well.
           (dolist (func removables)
             (setq auto-mode-alist
-- 
2.28.0

>From eb9a34d1e1e5527686a41561db5fd2797edb5074 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= <stepnem@gmail.com>
Date: Mon, 6 Apr 2020 17:05:33 +0200
Subject: [PATCH 3/3] unload-feature: Correct doc string to match info manual
 and reality

'unload-feature' doesn't try to "undo any additions the library has
made" to hooks, it tries to remove functions defined by the library
from hooks, no matter how they got there.

* lisp/loadhist.el (unload-feature): Correct the doc string.
* doc/lispref/loading.texi (Unloading): Clarify, fix typo.
---
 doc/lispref/loading.texi | 4 ++--
 lisp/loadhist.el         | 9 ++++-----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/doc/lispref/loading.texi b/doc/lispref/loading.texi
index aa6ef307b1..e5364152d5 100644
--- a/doc/lispref/loading.texi
+++ b/doc/lispref/loading.texi
@@ -1063,7 +1063,7 @@ Unloading
 (Loading saves these in the @code{autoload} property of the symbol.)
 
 Before restoring the previous definitions, @code{unload-feature} runs
-@code{remove-hook} to remove functions in the library from certain
+@code{remove-hook} to remove functions defined by the library from certain
 hooks.  These hooks include variables whose names end in @samp{-hook}
 (or the deprecated suffix @samp{-hooks}), plus those listed in
 @code{unload-feature-special-hooks}, as well as
@@ -1071,7 +1071,7 @@ Unloading
 function because important hooks refer to functions that are no longer
 defined.
 
-Standard unloading activities also undoes ELP profiling of functions
+Standard unloading activities also undo ELP profiling of functions
 in that library, unprovides any features provided by the library, and
 cancels timers held in variables defined by the library.
 
diff --git a/lisp/loadhist.el b/lisp/loadhist.el
index 81576679c3..8ac575e8e3 100644
--- a/lisp/loadhist.el
+++ b/lisp/loadhist.el
@@ -234,11 +234,10 @@ unload-feature
 is nil, raise an error.
 
 Standard unloading activities include restoring old autoloads for
-functions defined by the library, undoing any additions that the
-library has made to hook variables or to `auto-mode-alist', undoing
-ELP profiling of functions in that library, unproviding any features
-provided by the library, and canceling timers held in variables
-defined by the library.
+functions defined by the library, removing such functions from
+hooks and `auto-mode-alist', undoing their ELP profiling,
+unproviding any features provided by the library, and canceling
+timers held in variables defined by the library.
 
 If a function `FEATURE-unload-function' is defined, this function
 calls it with no arguments, before doing anything else.  That function
-- 
2.28.0