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

["Štěpán Němec" <stepnem@gmail.com>]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-unload-feature-Improve-logic-don-t-repeat-computatio.patch --]
[-- Type: text/x-patch, Size: 2412 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-unload-feature-Handle-local-hooks.patch --]
[-- Type: text/x-patch, Size: 1963 bytes --]

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.
+
 \f
 * 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-unload-feature-Correct-doc-string-to-match-info-manu.patch --]
[-- Type: text/x-patch, Size: 2700 bytes --]

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