Re: master 2b3f3d421a: Make minibuffer lazy highlight setup buffer-local where appropriate

[Augusto Stoffel <arstoffel@gmail.com>]

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

On Sun, 15 May 2022 at 16:16, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>>          (when filter
>> +          (make-local-variable 'isearch-filter-predicate)
>>            (add-function :after-while isearch-filter-predicate filter))
>
> `add-function` is modeled after `add-hook`, so the above
> `make-local-variable` would be better replaced with
>
>     (add-function :after-while (local 'isearch-filter-predicate) filter)
>
> [ Of course, that's pure theory.  You'd better test it first.  ]

Yes, I guess that's the right way to approach this.  The attached patch
implements it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-last-change-in-minibuffer-lazy-highlight-setup.patch --]
[-- Type: text/x-patch, Size: 2173 bytes --]

From 6699f0b1313f475e83757dc954ee59c8171e9500 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sun, 15 May 2022 22:48:50 +0200
Subject: [PATCH] Fix last change in minibuffer-lazy-highlight-setup

* lisp/isearch.el (minibuffer-lazy-highlight-setup): Apply advices
buffer-locally.
---
 lisp/isearch.el | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 3e1dab4d15..31fbdf01bf 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -4410,14 +4410,17 @@ minibuffer-lazy-highlight-setup
     (let ((unwind (make-symbol "minibuffer-lazy-highlight--unwind"))
           (after-change (make-symbol "minibuffer-lazy-highlight--after-change"))
           (display-count (make-symbol "minibuffer-lazy-highlight--display-count"))
+          (buffer (current-buffer))
           overlay)
       (fset unwind
             (lambda ()
-              (remove-function isearch-filter-predicate filter)
+              (when filter
+                (with-current-buffer buffer
+                  (remove-function (local 'isearch-filter-predicate) filter)))
               (remove-hook 'lazy-count-update-hook display-count)
               (when overlay (delete-overlay overlay))
-              (remove-hook 'after-change-functions after-change)
-              (remove-hook 'minibuffer-exit-hook unwind)
+              (remove-hook 'after-change-functions after-change t)
+              (remove-hook 'minibuffer-exit-hook unwind t)
               (let ((lazy-highlight-cleanup cleanup))
                 (lazy-highlight-cleanup))))
       (fset after-change
@@ -4447,8 +4450,8 @@ minibuffer-lazy-highlight-setup
           (setq overlay (make-overlay (point-min) (point-min) (current-buffer) t))
           (add-hook 'lazy-count-update-hook display-count))
         (when filter
-          (make-local-variable 'isearch-filter-predicate)
-          (add-function :after-while isearch-filter-predicate filter))
+          (with-current-buffer buffer
+            (add-function :after-while (local 'isearch-filter-predicate) filter)))
         (funcall after-change nil nil nil)))))
 
 \f
-- 
2.36.1


[-- Attachment #3: Type: text/plain, Size: 435 bytes --]


Now, I had tried it earlier and noticed something looked fishy.  Now I
can reproduce the problem:

1. With an active region, call C-M-%
2. Switch from the minibuffer back to the original buffer
3. Call keyboard-escape-quit (maybe twice) to quit the minibuffer
   prompt.

Then the buffer-local value of `isearch-filter-predicate' is not cleaned
up.  Is it by any chance intentional that minibuffer-exit-hook doesn't
run in this case?