GNU bug report logs - #72147
30.0.60; 30.0.60; Misleading regexp highlighting in 'Q' dired command

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 72147 in the body.
You can then email your comments to 72147 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: Visuwesh <visuweshm <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.60; 30.0.60; Misleading regexp highlighting in 'Q' dired command
Date: Wed, 17 Jul 2024 09:13:07 +0530

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

When using the Q command in dired to query-replace-regexp across marked
files in dired, the regexp typed in the first prompt highlights text in
the _dired_ buffer.

To reproduce,

        1. emacs -Q
        2. C-x d ~
        3. Q -

Notice how the dashes in the permission fields are highlighted.  This is
misleading and even confusing.  It would be better to turn off regexp
highlighting for this command.  The attached patch does the job on my
end.  Let binding the variable this way does not turn off
lazy-highlighting during the replacement.

[0001-Don-t-highlight-regexp-matches-when-using-dired-Q-co.patch (text/x-diff, attachment)]

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

In GNU Emacs 30.0.60 (build 5, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.18.0, Xaw scroll bars) of 2024-07-10 built on astatine
Repository revision: 5cf8d60e0dec5606a94749d17a5d678455b7a437
Repository branch: emacs-30
Windowing system distributor 'The X.Org Foundation', version 11.0.12101011
System Description: Debian GNU/Linux trixie/sid

Configured using:
 'configure --with-sound=alsa --with-x-toolkit=lucid --without-xaw3d
 --without-gconf --without-libsystemd --with-cairo'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM
XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LC_MONETARY: ta_IN.UTF-8
  value of $LC_NUMERIC: ta_IN.UTF-8
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
xinput2 x multi-tty move-toolbar make-network-process emacs)

Memory information:
((conses 16 41209 13476) (symbols 48 5821 0) (strings 32 14271 1663)
 (string-bytes 1 317312) (vectors 16 9302)
 (vector-slots 8 110789 8365) (floats 8 23 3) (intervals 56 256 1)
 (buffers 984 10))

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

From: Juri Linkov <juri <at> linkov.net>
To: Visuwesh <visuweshm <at> gmail.com>
Cc: 72147 <at> debbugs.gnu.org
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Fri, 19 Jul 2024 09:50:23 +0300

> When using the Q command in dired to query-replace-regexp across marked
> files in dired, the regexp typed in the first prompt highlights text in
> the _dired_ buffer.
>
> To reproduce,
>
>         1. emacs -Q
>         2. C-x d ~
>         3. Q -
>
> Notice how the dashes in the permission fields are highlighted.  This is
> misleading and even confusing.  It would be better to turn off regexp
> highlighting for this command.  The attached patch does the job on my
> end.  Let binding the variable this way does not turn off
> lazy-highlighting during the replacement.
>
> @@ -3801,9 +3801,10 @@ dired-do-query-replace-regexp
>    (interactive
> -   (let ((common
> -	  (query-replace-read-args
> -	   "Query replace regexp in marked files" t t)))
> +   (let* ((query-replace-lazy-highlight)
> +          (common
> +	   (query-replace-read-args
> +	    "Query replace regexp in marked files" t t)))

Thanks, I tried your patch, but it has no effect since
by default Q is bound to dired-do-find-regexp-and-replace.
Shouldn't query-replace-lazy-highlight be added to all
functions in dired that use query-replace-read-args?

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

From: Visuwesh <visuweshm <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 72147 <at> debbugs.gnu.org
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Fri, 19 Jul 2024 13:55:24 +0530

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

[வெள்ளி ஜூலை 19, 2024] Juri Linkov wrote:

>> When using the Q command in dired to query-replace-regexp across marked
>> files in dired, the regexp typed in the first prompt highlights text in
>> the _dired_ buffer.
>>
>> To reproduce,
>>
>>         1. emacs -Q
>>         2. C-x d ~
>>         3. Q -
>>
>> Notice how the dashes in the permission fields are highlighted.  This is
>> misleading and even confusing.  It would be better to turn off regexp
>> highlighting for this command.  The attached patch does the job on my
>> end.  Let binding the variable this way does not turn off
>> lazy-highlighting during the replacement.
>>
>> @@ -3801,9 +3801,10 @@ dired-do-query-replace-regexp
>>    (interactive
>> -   (let ((common
>> -	  (query-replace-read-args
>> -	   "Query replace regexp in marked files" t t)))
>> +   (let* ((query-replace-lazy-highlight)
>> +          (common
>> +	   (query-replace-read-args
>> +	    "Query replace regexp in marked files" t t)))
>
> Thanks, I tried your patch, but it has no effect since
> by default Q is bound to dired-do-find-regexp-and-replace.

Ah, I guess I didn't test properly in emacs -Q.  I ended up mixing my
own config and emacs -Q.  My bad.

> Shouldn't query-replace-lazy-highlight be added to all
> functions in dired that use query-replace-read-args?

Yes, you're right.  I have attached a patch that does it.

BTW, is there a reason why xref--query-replace-1 let-binds
query-replace-lazy-highlight to nil?

[0001-Don-t-highlight-matches-in-dired-query-replace-comma.patch (text/x-diff, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Visuwesh <visuweshm <at> gmail.com>
Cc: 72147 <at> debbugs.gnu.org
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Tue, 23 Jul 2024 09:37:54 +0300

close 72147 31.0.50
thanks

>> Shouldn't query-replace-lazy-highlight be added to all
>> functions in dired that use query-replace-read-args?
>
> Yes, you're right.  I have attached a patch that does it.

Thanks for the patch, now pushed to master.

> BTW, is there a reason why xref--query-replace-1 let-binds
> query-replace-lazy-highlight to nil?

I guess for the same reason why you let-bind it to nil in Dired,
i.e. in Xref it's called in an irrelevant buffer too.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> linkov.net>
Cc: 72147 <at> debbugs.gnu.org, Visuwesh <visuweshm <at> gmail.com>
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Tue, 23 Jul 2024 11:27:30 +0200

Juri Linkov <juri <at> linkov.net> writes:

> Thanks for the patch, now pushed to master.

Wouldn't it be better to add a new optional argument NO-HIGHLIGHT to
`query-replace-read-args', for stylistic reasons?


Michael.

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

From: Juri Linkov <juri <at> linkov.net>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 72147 <at> debbugs.gnu.org, Visuwesh <visuweshm <at> gmail.com>
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Tue, 23 Jul 2024 20:40:47 +0300

>> Thanks for the patch, now pushed to master.
>
> Wouldn't it be better to add a new optional argument NO-HIGHLIGHT to
> `query-replace-read-args', for stylistic reasons?

It's not clear how such arg should be combined with
`query-replace-lazy-highlight' in `query-replace-read-args':

(defun query-replace-read-args (prompt regexp-flag &optional noerror no-highlight)
  ...
  (from (minibuffer-with-setup-hook
            (minibuffer-lazy-highlight-setup
             :highlight query-replace-lazy-highlight

with `and'?  With `or'?  With `xor'?
`(and query-replace-lazy-highlight (not no-highlight))'?

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> linkov.net>
Cc: 72147 <at> debbugs.gnu.org, Visuwesh <visuweshm <at> gmail.com>
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Wed, 24 Jul 2024 19:09:48 +0200

Juri Linkov <juri <at> linkov.net> writes:

> >> Thanks for the patch, now pushed to master.
> >
> > Wouldn't it be better to add a new optional argument NO-HIGHLIGHT to
> > `query-replace-read-args', for stylistic reasons?
>
> It's not clear how such arg should be combined with
> `query-replace-lazy-highlight' in `query-replace-read-args':
>
> (defun query-replace-read-args (prompt regexp-flag &optional noerror no-highlight)
>   ...
>   (from (minibuffer-with-setup-hook
>             (minibuffer-lazy-highlight-setup
>              :highlight query-replace-lazy-highlight
>
> with `and'?  With `or'?  With `xor'?
> `(and query-replace-lazy-highlight (not no-highlight))'?

The installed patch binds query-replace-lazy-highlight to nil and
shadows the old binding.  This would correspond to

  (and query-replace-lazy-highlight (not no-highlight))

which looks reasonable in this case: we just an argument to inhibit
highlighting.  Because we never want highlighting in some cases (like
here).


Michael.

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

From: Juri Linkov <juri <at> linkov.net>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 72147 <at> debbugs.gnu.org, Visuwesh <visuweshm <at> gmail.com>
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Thu, 25 Jul 2024 21:08:42 +0300

>> >> Thanks for the patch, now pushed to master.
>> >
>> > Wouldn't it be better to add a new optional argument NO-HIGHLIGHT to
>> > `query-replace-read-args', for stylistic reasons?
>>
>> It's not clear how such arg should be combined with
>> `query-replace-lazy-highlight' in `query-replace-read-args':
>>
>> (defun query-replace-read-args (prompt regexp-flag &optional noerror no-highlight)
>>   ...
>>   (from (minibuffer-with-setup-hook
>>             (minibuffer-lazy-highlight-setup
>>              :highlight query-replace-lazy-highlight
>>
>> with `and'?  With `or'?  With `xor'?
>> `(and query-replace-lazy-highlight (not no-highlight))'?
>
> The installed patch binds query-replace-lazy-highlight to nil and
> shadows the old binding.  This would correspond to
>
>   (and query-replace-lazy-highlight (not no-highlight))
>
> which looks reasonable in this case: we just an argument to inhibit
> highlighting.  Because we never want highlighting in some cases (like
> here).

Agreed, let's add a new argument.

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

From: Visuwesh <visuweshm <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 72147 <at> debbugs.gnu.org
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Sun, 28 Jul 2024 10:29:05 +0530

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

[வியாழன் ஜூலை 25, 2024] Juri Linkov wrote:

>>> > Wouldn't it be better to add a new optional argument NO-HIGHLIGHT to
>>> > `query-replace-read-args', for stylistic reasons?
>>>
>>> It's not clear how such arg should be combined with
>>> `query-replace-lazy-highlight' in `query-replace-read-args':
>>>
>>> (defun query-replace-read-args (prompt regexp-flag &optional noerror no-highlight)
>>>   ...
>>>   (from (minibuffer-with-setup-hook
>>>             (minibuffer-lazy-highlight-setup
>>>              :highlight query-replace-lazy-highlight
>>>
>>> with `and'?  With `or'?  With `xor'?
>>> `(and query-replace-lazy-highlight (not no-highlight))'?
>>
>> The installed patch binds query-replace-lazy-highlight to nil and
>> shadows the old binding.  This would correspond to
>>
>>   (and query-replace-lazy-highlight (not no-highlight))
>>
>> which looks reasonable in this case: we just an argument to inhibit
>> highlighting.  Because we never want highlighting in some cases (like
>> here).
>
> Agreed, let's add a new argument.

So how about the attached patch?

[0001-Add-new-optional-argument-NO-HIGHLIGHT-to-query-repl.patch (text/x-diff, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Visuwesh <visuweshm <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 72147 <at> debbugs.gnu.org
Subject: Re: bug#72147: 30.0.60; 30.0.60; Misleading regexp highlighting in
 'Q' dired command
Date: Sun, 28 Jul 2024 09:54:29 +0300

>>> The installed patch binds query-replace-lazy-highlight to nil and
>>> shadows the old binding.  This would correspond to
>>>
>>>   (and query-replace-lazy-highlight (not no-highlight))
>>>
>>> which looks reasonable in this case: we just an argument to inhibit
>>> highlighting.  Because we never want highlighting in some cases (like
>>> here).
>>
>> Agreed, let's add a new argument.
>
> So how about the attached patch?

Thanks, now pushed to master.

PS: It seems adding a new argument is not possible in xref.el
because it's used in older versions via an ELPA package.

Previous Next

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