* bug#55110: 29.0.50; Regression in query-replace prompt
@ 2022-04-25 15:37 Juri Linkov
2022-04-26 19:42 ` Augusto Stoffel
0 siblings, 1 reply; 5+ messages in thread
From: Juri Linkov @ 2022-04-25 15:37 UTC (permalink / raw)
To: 55110; +Cc: augusto stoffel
X-Debbugs-Cc: Augusto Stoffel <arstoffel@gmail.com>
I noticed a recent regression after minibuffer-lazy-highlight changes:
0. emacs -Q
1. perform one query-replace to add some FROM and TO to its history:
M-% a RET b RET
2. check that isearch still works in the minibuffer history:
M-% C-r a
3. activate the region, e.g. C-SPC and move point
4. try to isearch in the history again:
M-% C-r a
It fails with: [Failing I-search backward: a]
Maybe this is caused by minibuffer-lazy-highlight-setup
that sets filter to replace--region-filter in the minibuffer
instead of the original buffer?
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt
2022-04-25 15:37 bug#55110: 29.0.50; Regression in query-replace prompt Juri Linkov
@ 2022-04-26 19:42 ` Augusto Stoffel
2022-04-27 7:44 ` Juri Linkov
0 siblings, 1 reply; 5+ messages in thread
From: Augusto Stoffel @ 2022-04-26 19:42 UTC (permalink / raw)
To: Juri Linkov; +Cc: 55110
Thanks for the heads up, I'll take a look at this some time soon.
On Mon, 25 Apr 2022 at 18:37, Juri Linkov <juri@linkov.net> wrote:
> Maybe this is caused by minibuffer-lazy-highlight-setup
> that sets filter to replace--region-filter in the minibuffer
> instead of the original buffer?
Most likely, yes. `replace--region-filter' is modified globally, so a
similar problem should happen if you temporarily leave the minibuffer
and do Isearch in any other buffer.
If that's the case, I think we would have two options:
1) Add a quick fix for the minibuffer Isearch only.
2) A more complicated change that solves the issue generally by saving
the region filter in the fashion of isearch-lazy-highlight-regexp et
alii.
WDYT?
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt
2022-04-26 19:42 ` Augusto Stoffel
@ 2022-04-27 7:44 ` Juri Linkov
2022-05-14 16:02 ` Augusto Stoffel
0 siblings, 1 reply; 5+ messages in thread
From: Juri Linkov @ 2022-04-27 7:44 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 55110
>> Maybe this is caused by minibuffer-lazy-highlight-setup
>> that sets filter to replace--region-filter in the minibuffer
>> instead of the original buffer?
>
> Most likely, yes. `replace--region-filter' is modified globally, so a
> similar problem should happen if you temporarily leave the minibuffer
> and do Isearch in any other buffer.
>
> If that's the case, I think we would have two options:
>
> 1) Add a quick fix for the minibuffer Isearch only.
>
> 2) A more complicated change that solves the issue generally by saving
> the region filter in the fashion of isearch-lazy-highlight-regexp et
> alii.
>
> WDYT?
Recently we fixed a similar problem in `perform-replace'
by creating a dynamically bound value in `let':
(let ((opos (point-marker))
;; Restore original isearch filter to allow
;; using isearch in a recursive edit even
;; when perform-replace was started from
;; `xref--query-replace-1' that let-binds
;; `isearch-filter-predicate' (bug#53758).
(isearch-filter-predicate #'isearch-filter-visible))
So maybe a buffer-local value of `isearch-filter-predicate'
in the minibuffer would help.
Also I recommend to make all hooks in `minibuffer-lazy-highlight-setup'
local by adding the argument LOCAL to add-hook/remove-hook.
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt
2022-04-27 7:44 ` Juri Linkov
@ 2022-05-14 16:02 ` Augusto Stoffel
2022-05-15 18:45 ` Juri Linkov
0 siblings, 1 reply; 5+ messages in thread
From: Augusto Stoffel @ 2022-05-14 16:02 UTC (permalink / raw)
To: Juri Linkov; +Cc: 55110
[-- Attachment #1: Type: text/plain, Size: 1944 bytes --]
On Wed, 27 Apr 2022 at 10:44, Juri Linkov <juri@linkov.net> wrote:
>>> Maybe this is caused by minibuffer-lazy-highlight-setup
>>> that sets filter to replace--region-filter in the minibuffer
>>> instead of the original buffer?
>>
>> Most likely, yes. `replace--region-filter' is modified globally, so a
>> similar problem should happen if you temporarily leave the minibuffer
>> and do Isearch in any other buffer.
>>
>> If that's the case, I think we would have two options:
>>
>> 1) Add a quick fix for the minibuffer Isearch only.
>>
>> 2) A more complicated change that solves the issue generally by saving
>> the region filter in the fashion of isearch-lazy-highlight-regexp et
>> alii.
>>
>> WDYT?
>
> Recently we fixed a similar problem in `perform-replace'
> by creating a dynamically bound value in `let':
>
> (let ((opos (point-marker))
> ;; Restore original isearch filter to allow
> ;; using isearch in a recursive edit even
> ;; when perform-replace was started from
> ;; `xref--query-replace-1' that let-binds
> ;; `isearch-filter-predicate' (bug#53758).
> (isearch-filter-predicate #'isearch-filter-visible))
>
> So maybe a buffer-local value of `isearch-filter-predicate'
> in the minibuffer would help.
Yes, that indeed solves the problem. See attached patch.
> Also I recommend to make all hooks in `minibuffer-lazy-highlight-setup'
> local by adding the argument LOCAL to add-hook/remove-hook.
Indeed, the minibuffer lazy highlight feature is currently incompatible
with recursive minibuffers. The patch fixes that as well.
There's a caveat, though: isearch in a recursive minibuffer is again
affected by the presence of an inappropriate filter function. Fixing
that in a robust way might require a bigger refactoring of the lazy
highlight feature, I think. Another option might be to make
`replace--region-filter' also check for the value of `(current-buffer)'.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-minibuffer-lazy-highlight-setup-buffer-local-wh.patch --]
[-- Type: text/x-patch, Size: 1599 bytes --]
From 37de4e53fce6adbf5e52f3184c163574e3129def Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 14 May 2022 17:21:27 +0200
Subject: [PATCH] Make minibuffer lazy highlight setup buffer-local where
appropriate
* lisp/isearch.el (minibuffer-lazy-highlight-setup): Modify hooks
buffer-locally, so that recursive minibuffers are not affected by the
special behavior of lazy-highlight. Also make
'isearch-filter-predicate' buffer-local, so that isearch in the
minibuffer is not affected by the region filter (bug#55110).
---
lisp/isearch.el | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 96168f94bd..3e1dab4d15 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -4441,12 +4441,13 @@ minibuffer-lazy-highlight-setup
(format minibuffer-lazy-count-format
isearch-lazy-count-total)))))
(lambda ()
- (add-hook 'minibuffer-exit-hook unwind)
- (add-hook 'after-change-functions after-change)
+ (add-hook 'minibuffer-exit-hook unwind nil t)
+ (add-hook 'after-change-functions after-change nil t)
(when minibuffer-lazy-count-format
(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))
(funcall after-change nil nil nil)))))
--
2.36.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* bug#55110: 29.0.50; Regression in query-replace prompt
2022-05-14 16:02 ` Augusto Stoffel
@ 2022-05-15 18:45 ` Juri Linkov
0 siblings, 0 replies; 5+ messages in thread
From: Juri Linkov @ 2022-05-15 18:45 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 55110
>> So maybe a buffer-local value of `isearch-filter-predicate'
>> in the minibuffer would help.
>
> Yes, that indeed solves the problem. See attached patch.
Thanks, pushed.
>> Also I recommend to make all hooks in `minibuffer-lazy-highlight-setup'
>> local by adding the argument LOCAL to add-hook/remove-hook.
>
> Indeed, the minibuffer lazy highlight feature is currently incompatible
> with recursive minibuffers. The patch fixes that as well.
>
> There's a caveat, though: isearch in a recursive minibuffer is again
> affected by the presence of an inappropriate filter function. Fixing
> that in a robust way might require a bigger refactoring of the lazy
> highlight feature, I think. Another option might be to make
> `replace--region-filter' also check for the value of `(current-buffer)'.
I see no problem to fix this because every minibuffer uses own buffer,
so they have separate buffer-local variables.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-15 18:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-25 15:37 bug#55110: 29.0.50; Regression in query-replace prompt Juri Linkov
2022-04-26 19:42 ` Augusto Stoffel
2022-04-27 7:44 ` Juri Linkov
2022-05-14 16:02 ` Augusto Stoffel
2022-05-15 18:45 ` Juri Linkov
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.