* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
@ 2022-12-09 7:42 Juri Linkov
2022-12-10 17:20 ` Juri Linkov
2022-12-11 11:40 ` Augusto Stoffel
0 siblings, 2 replies; 11+ messages in thread
From: Juri Linkov @ 2022-12-09 7:42 UTC (permalink / raw)
To: 59918; +Cc: augusto stoffel
X-Debbugs-Cc: Augusto Stoffel <arstoffel@gmail.com>
0. emacs -Q
1. (setq enable-recursive-minibuffers t)
1. M-! is C-a
2. M-% is
Matches are lazy-highlighted in the original buffer,
but the replacement is going to be used to replace
matches in the minibuffer.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-09 7:42 bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer Juri Linkov
@ 2022-12-10 17:20 ` Juri Linkov
2022-12-10 18:36 ` Eli Zaretskii
2022-12-11 11:40 ` Augusto Stoffel
1 sibling, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2022-12-10 17:20 UTC (permalink / raw)
To: 59918
> 1. (setq enable-recursive-minibuffers t)
> 2. M-! is C-a
> 3. M-% is
>
> Matches are lazy-highlighted in the original buffer,
> but the replacement is going to be used to replace
> matches in the minibuffer.
Unfortunately, this is how minibuffer-selected-window behaves:
1. M-!
2. M-x
3. M-: (minibuffer-selected-window)
returns *scratch*, but not the minibuffer where it was invoked.
So lazy-highlighting could be disabled if (minibuffer-depth) > 1,
if we are sure that not such highlighting would be preferable.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-10 17:20 ` Juri Linkov
@ 2022-12-10 18:36 ` Eli Zaretskii
0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2022-12-10 18:36 UTC (permalink / raw)
To: Juri Linkov; +Cc: 59918
> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 10 Dec 2022 19:20:14 +0200
>
> > 1. (setq enable-recursive-minibuffers t)
> > 2. M-! is C-a
> > 3. M-% is
> >
> > Matches are lazy-highlighted in the original buffer,
> > but the replacement is going to be used to replace
> > matches in the minibuffer.
>
> Unfortunately, this is how minibuffer-selected-window behaves:
>
> 1. M-!
> 2. M-x
> 3. M-: (minibuffer-selected-window)
>
> returns *scratch*, but not the minibuffer where it was invoked.
>
> So lazy-highlighting could be disabled if (minibuffer-depth) > 1,
> if we are sure that not such highlighting would be preferable.
If you know that you are in the minibuffer window, you can get that
window by calling minibuffer-window, right?
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-09 7:42 bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer Juri Linkov
2022-12-10 17:20 ` Juri Linkov
@ 2022-12-11 11:40 ` Augusto Stoffel
2022-12-12 17:43 ` Juri Linkov
1 sibling, 1 reply; 11+ messages in thread
From: Augusto Stoffel @ 2022-12-11 11:40 UTC (permalink / raw)
To: Juri Linkov; +Cc: 59918
On Fri, 9 Dec 2022 at 09:42, Juri Linkov wrote:
> X-Debbugs-Cc: Augusto Stoffel <arstoffel@gmail.com>
>
> 0. emacs -Q
> 1. (setq enable-recursive-minibuffers t)
> 1. M-! is C-a
> 2. M-% is
>
> Matches are lazy-highlighted in the original buffer,
> but the replacement is going to be used to replace
> matches in the minibuffer.
I guess this happens because minibuffer-selected-window returns the
original buffer. I think this patch does the trick?
--8<---------------cut here---------------start------------->8---
modified lisp/isearch.el
@@ -4435,7 +4435,9 @@ minibuffer-lazy-highlight-setup
highlighting.
LAX-WHITESPACE: The value of `isearch-lax-whitespace' and
`isearch-regexp-lax-whitespace' to use for lazy highlighting."
- (if (not highlight)
+ (if (not (and highlight
+ (eq (current-buffer) (window-buffer
+ (minibuffer-selected-window)))))
#'ignore
(let ((unwind (make-symbol "minibuffer-lazy-highlight--unwind"))
(after-change (make-symbol "minibuffer-lazy-highlight--after-change"))
--8<---------------cut here---------------end--------------->8---
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-11 11:40 ` Augusto Stoffel
@ 2022-12-12 17:43 ` Juri Linkov
2022-12-12 18:07 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2022-12-12 17:43 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 59918
>> 1. (setq enable-recursive-minibuffers t)
>> 1. M-! is C-a
>> 2. M-% is
>>
>> Matches are lazy-highlighted in the original buffer,
>> but the replacement is going to be used to replace
>> matches in the minibuffer.
>
> I guess this happens because minibuffer-selected-window returns the
> original buffer. I think this patch does the trick?
We need to wait until Eli decides whether to install this
to emacs-29 or master.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-12 17:43 ` Juri Linkov
@ 2022-12-12 18:07 ` Eli Zaretskii
2022-12-12 19:05 ` Augusto Stoffel
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2022-12-12 18:07 UTC (permalink / raw)
To: Juri Linkov; +Cc: arstoffel, 59918
> Cc: 59918@debbugs.gnu.org
> From: Juri Linkov <juri@linkov.net>
> Date: Mon, 12 Dec 2022 19:43:25 +0200
>
> >> 1. (setq enable-recursive-minibuffers t)
> >> 1. M-! is C-a
> >> 2. M-% is
> >>
> >> Matches are lazy-highlighted in the original buffer,
> >> but the replacement is going to be used to replace
> >> matches in the minibuffer.
> >
> > I guess this happens because minibuffer-selected-window returns the
> > original buffer. I think this patch does the trick?
>
> We need to wait until Eli decides whether to install this
> to emacs-29 or master.
I admit that I don't understand the patch. minibuffer-selected-window
returns a window, not a buffer, and it returns the window that you
didn't want, AFAIU.
Or maybe I don't understand the root cause -- could one of you please
elaborate on what happens and why this is the right patch?
Also, was this code introduced in Emacs 28/29 or earlier?
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-12 18:07 ` Eli Zaretskii
@ 2022-12-12 19:05 ` Augusto Stoffel
2022-12-12 19:28 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Augusto Stoffel @ 2022-12-12 19:05 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 59918, Juri Linkov
On Mon, 12 Dec 2022 at 20:07, Eli Zaretskii wrote:
>> Cc: 59918@debbugs.gnu.org
>> From: Juri Linkov <juri@linkov.net>
>> Date: Mon, 12 Dec 2022 19:43:25 +0200
>>
>> >> 1. (setq enable-recursive-minibuffers t)
>> >> 1. M-! is C-a
>> >> 2. M-% is
>> >>
>> >> Matches are lazy-highlighted in the original buffer,
>> >> but the replacement is going to be used to replace
>> >> matches in the minibuffer.
>> >
>> > I guess this happens because minibuffer-selected-window returns the
>> > original buffer. I think this patch does the trick?
>>
>> We need to wait until Eli decides whether to install this
>> to emacs-29 or master.
>
> I admit that I don't understand the patch. minibuffer-selected-window
> returns a window, not a buffer, and it returns the window that you
> didn't want, AFAIU.
The check in the patch is whether the buffer of the
minibuffer-selected-window is the current buffer. It's meant to fail
when you are about to start a recursive minibuffer.
> Or maybe I don't understand the root cause -- could one of you please
> elaborate on what happens and why this is the right patch?
The original code assumed that when you are about to activate a
minibuffer, the current buffer will become the (window-buffer
(minibuffer-selected-window)). This is not true if you are entering a
recursive minibuffer, hence the check.
It may or may not be the best patch, but I would say it's the safe one
if were to fix this in Emacs 29. I would also say the issue rare and
basically cosmetic.
> Also, was this code introduced in Emacs 28/29 or earlier?
New in Emacs 29.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-12 19:05 ` Augusto Stoffel
@ 2022-12-12 19:28 ` Eli Zaretskii
2022-12-12 22:42 ` Augusto Stoffel
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2022-12-12 19:28 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 59918, juri
> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>, 59918@debbugs.gnu.org
> Date: Mon, 12 Dec 2022 20:05:52 +0100
>
> On Mon, 12 Dec 2022 at 20:07, Eli Zaretskii wrote:
>
> > I admit that I don't understand the patch. minibuffer-selected-window
> > returns a window, not a buffer, and it returns the window that you
> > didn't want, AFAIU.
>
> The check in the patch is whether the buffer of the
> minibuffer-selected-window is the current buffer. It's meant to fail
> when you are about to start a recursive minibuffer.
Isn't that test fragile as well? Which buffer is considered "current"
and which window is considered "selected" when the minibuffer is
active is always a tricky question. Wouldn't it be better to test
instead that the we are in the minibuffer, and if so to use
minibuffer-window instead of minibuffer-selected-window?
> It may or may not be the best patch, but I would say it's the safe one
> if were to fix this in Emacs 29.
So is the method I suggest -- assuming that it works.
> > Also, was this code introduced in Emacs 28/29 or earlier?
>
> New in Emacs 29.
OK, so we must fix it on the emacs-29 branch. What remains is to
decide what fix to use.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-12 19:28 ` Eli Zaretskii
@ 2022-12-12 22:42 ` Augusto Stoffel
2022-12-13 12:05 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Augusto Stoffel @ 2022-12-12 22:42 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 59918, juri
[-- Attachment #1: Type: text/plain, Size: 161 bytes --]
On Mon, 12 Dec 2022 at 21:28, Eli Zaretskii wrote:
> OK, so we must fix it on the emacs-29 branch. What remains is to
> decide what fix to use.
This one :-)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Don-t-allow-lazy-highlight-from-recursive-minibuffer.patch --]
[-- Type: text/x-patch, Size: 989 bytes --]
From bcebf9622bc11cb36e0a56bc1874f24cfa634210 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Mon, 12 Dec 2022 23:07:28 +0100
Subject: [PATCH] Don't allow lazy highlight from recursive minibuffers
See bug#59918.
* lisp/isearch.el (minibuffer-lazy-highlight-setup): Don't activate
when starting a recursive minibuffer.
---
lisp/isearch.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/isearch.el b/lisp/isearch.el
index bc3697deb0a..6a17d18c45e 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -4435,7 +4435,7 @@ minibuffer-lazy-highlight-setup
highlighting.
LAX-WHITESPACE: The value of `isearch-lax-whitespace' and
`isearch-regexp-lax-whitespace' to use for lazy highlighting."
- (if (not highlight)
+ (if (or (not highlight) (minibufferp))
#'ignore
(let ((unwind (make-symbol "minibuffer-lazy-highlight--unwind"))
(after-change (make-symbol "minibuffer-lazy-highlight--after-change"))
--
2.38.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-12 22:42 ` Augusto Stoffel
@ 2022-12-13 12:05 ` Eli Zaretskii
2022-12-13 17:39 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2022-12-13 12:05 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 59918, juri
> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: juri@linkov.net, 59918@debbugs.gnu.org
> Date: Mon, 12 Dec 2022 23:42:46 +0100
>
> On Mon, 12 Dec 2022 at 21:28, Eli Zaretskii wrote:
>
> > OK, so we must fix it on the emacs-29 branch. What remains is to
> > decide what fix to use.
>
> This one :-)
>
> >From bcebf9622bc11cb36e0a56bc1874f24cfa634210 Mon Sep 17 00:00:00 2001
> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Mon, 12 Dec 2022 23:07:28 +0100
> Subject: [PATCH] Don't allow lazy highlight from recursive minibuffers
>
> See bug#59918.
>
> * lisp/isearch.el (minibuffer-lazy-highlight-setup): Don't activate
> when starting a recursive minibuffer.
> ---
> lisp/isearch.el | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/isearch.el b/lisp/isearch.el
> index bc3697deb0a..6a17d18c45e 100644
> --- a/lisp/isearch.el
> +++ b/lisp/isearch.el
> @@ -4435,7 +4435,7 @@ minibuffer-lazy-highlight-setup
> highlighting.
> LAX-WHITESPACE: The value of `isearch-lax-whitespace' and
> `isearch-regexp-lax-whitespace' to use for lazy highlighting."
> - (if (not highlight)
> + (if (or (not highlight) (minibufferp))
> #'ignore
> (let ((unwind (make-symbol "minibuffer-lazy-highlight--unwind"))
> (after-change (make-symbol "minibuffer-lazy-highlight--after-change"))
This is fine with me, but AFAIU it means that replacements in the
minibuffer will never be highlighted, ever. Is that what we want? I
thought Juri wanted to see highlighting there?
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer
2022-12-13 12:05 ` Eli Zaretskii
@ 2022-12-13 17:39 ` Juri Linkov
0 siblings, 0 replies; 11+ messages in thread
From: Juri Linkov @ 2022-12-13 17:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Augusto Stoffel, 59918
close 59918 29.0.60
thanks
>> @@ -4435,7 +4435,7 @@ minibuffer-lazy-highlight-setup
>> highlighting.
>> LAX-WHITESPACE: The value of `isearch-lax-whitespace' and
>> `isearch-regexp-lax-whitespace' to use for lazy highlighting."
>> - (if (not highlight)
>> + (if (or (not highlight) (minibufferp))
>> #'ignore
>> (let ((unwind (make-symbol "minibuffer-lazy-highlight--unwind"))
>> (after-change (make-symbol "minibuffer-lazy-highlight--after-change"))
Thanks for the patch.
> This is fine with me, but AFAIU it means that replacements in the
> minibuffer will never be highlighted, ever. Is that what we want? I
> thought Juri wanted to see highlighting there?
Highlighting is not visible here because the query-replace prompt
replaces the original minibuffer. So I pushed the patch to emacs-29.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-12-13 17:39 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-09 7:42 bug#59918: 29.0.60; query-replace in the minibuffer lazy-highlights original buffer Juri Linkov
2022-12-10 17:20 ` Juri Linkov
2022-12-10 18:36 ` Eli Zaretskii
2022-12-11 11:40 ` Augusto Stoffel
2022-12-12 17:43 ` Juri Linkov
2022-12-12 18:07 ` Eli Zaretskii
2022-12-12 19:05 ` Augusto Stoffel
2022-12-12 19:28 ` Eli Zaretskii
2022-12-12 22:42 ` Augusto Stoffel
2022-12-13 12:05 ` Eli Zaretskii
2022-12-13 17:39 ` 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.