GNU bug report logs - #55110
29.0.50; Regression in query-replace prompt

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55110 in the body.
You can then email your comments to 55110 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; Regression in query-replace prompt
Date: Mon, 25 Apr 2022 18:37:22 +0300

X-Debbugs-Cc: Augusto Stoffel <arstoffel <at> 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?

Message #8 received at 55110 <at> debbugs.gnu.org (full text, mbox):

From: Augusto Stoffel <arstoffel <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 55110 <at> debbugs.gnu.org
Subject: Re: bug#55110: 29.0.50; Regression in query-replace prompt
Date: Tue, 26 Apr 2022 21:42:28 +0200

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 <at> 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?

Message #11 received at 55110 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: 55110 <at> debbugs.gnu.org
Subject: Re: bug#55110: 29.0.50; Regression in query-replace prompt
Date: Wed, 27 Apr 2022 10:44:56 +0300

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

Message #14 received at 55110 <at> debbugs.gnu.org (full text, mbox):

From: Augusto Stoffel <arstoffel <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 55110 <at> debbugs.gnu.org
Subject: Re: bug#55110: 29.0.50; Regression in query-replace prompt
Date: Sat, 14 May 2022 18:02:09 +0200

[Message part 1 (text/plain, inline)]

On Wed, 27 Apr 2022 at 10:44, Juri Linkov <juri <at> 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)'.

[0001-Make-minibuffer-lazy-highlight-setup-buffer-local-wh.patch (text/x-patch, attachment)]

Message #17 received at 55110 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: 55110 <at> debbugs.gnu.org
Subject: Re: bug#55110: 29.0.50; Regression in query-replace prompt
Date: Sun, 15 May 2022 21:45:51 +0300

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

Previous Next

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