bug#14708: 24.2; query-replace-regexp when match and replacement are the same

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Ed Avis]

This bug report will be sent to the Bug-GNU-Emacs mailing list
and the GNU bug tracker at debbugs.gnu.org.  Please check that
the From: line contains a valid email address.  After a delay of up
to one day, you should receive an acknowledgement at that address.

Please write in English if possible, as the Emacs maintainers
usually do not have translators for other languages.

Please describe exactly what actions triggered the bug, and
the precise symptoms of the bug.  If you can, give a recipe
starting from `emacs -Q':

Enter some text in a buffer with some space characters and some
double  or   triple   spaced  like  this.

Do M-x query-replace-regexp and enter ' +' as the regexp and ' '
as the replacement text.  Emacs matches each sequence of one or
more space characters, as specified in the regexp, and asks whether
to replace it.

However, for many of the cases the replacement text will be the same
as the match, so it makes no difference whether you replace or not.
Since the aim of query-replace-regexp is primarily to do a search
and replace, not to find all places in the buffer where a regexp
matches, it should instead skip over those cases and only ask about
places where replacing will make a difference.

So in this example, only the double and triple spaces would be
asked about for replacement.

If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/usr/share/emacs/24.2/etc/DEBUG.


In GNU Emacs 24.2.1 (x86_64-redhat-linux-gnu, GTK+ Version 3.6.4)
 of 2013-04-12 on buildvm-05.phx2.fedoraproject.org
Configured using:
 `configure '--build=x86_64-redhat-linux-gnu'
 '--host=x86_64-redhat-linux-gnu' '--program-prefix='
 '--disable-dependency-tracking' '--prefix=/usr' '--exec-prefix=/usr'
 '--bindir=/usr/bin' '--sbindir=/usr/sbin' '--sysconfdir=/etc'
 '--datadir=/usr/share' '--includedir=/usr/include'
 '--libdir=/usr/lib64' '--libexecdir=/usr/libexec'
 '--localstatedir=/var' '--sharedstatedir=/var/lib'
 '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--with-dbus'
 '--with-gif' '--with-jpeg' '--with-png' '--with-rsvg' '--with-tiff'
 '--with-xft' '--with-xpm' '--with-x-toolkit=gtk3' '--with-gpm=no'
 '--with-wide-int' 'build_alias=x86_64-redhat-linux-gnu'
 'host_alias=x86_64-redhat-linux-gnu' 'CFLAGS=-DMAIL_USE_LOCKF -O2 -g
 -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
 --param=ssp-buffer-size=4 -m64 -mtune=generic' 'LDFLAGS=-Wl,-z,relro ''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: C
  value of $LC_CTYPE: en_GB.UTF-8
  value of $LC_MESSAGES: en_GB.UTF-8
  value of $LC_MONETARY: en_GB.UTF-8
  value of $LC_NUMERIC: en_GB.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_GB.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: HTML

Minor modes in effect:
  diff-auto-refine-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
a t SPC w a c s l = " C-x o ESC O C ESC O C ESC O C
ESC O C ESC O C ESC O C ESC O C ESC O C ESC O C ESC
O C ESC O C ESC O C ESC O C ESC O C ESC O C ESC O C
ESC O C ESC O C ESC O C ESC O C ESC O C ESC O C ESC
O C ESC O C ESC O C ESC O C C-@ ESC O B ESC O B ESC
O B ESC O B ESC O D ESC O B ESC O D ESC w C-x o C-y
ESC O A ESC O A ESC O A ESC O A ESC O A C-e C-d C-e
C-d C-e C-d C-e C-d C-e C-d C-e " SPC f o r m a t =
" 0 . 0 DEL # " SPC r e p o r t - c h a n g e s = "
1 " SPC / > < / T D > C-a ESC d C-_ ESC f ESC f ESC
x r e DEL DEL q u e r SPC r e SPC - r e g e SPC RET
SPC c C-h + C-g ESC x ESC O A RET SPC + RET SPC RET
y y y y y y y y y y y y y y y C-a ESC x r e p o r t
SPC e m SPC b SPC SPC SPC SPC SPC SPC SPC SPC SPC SPC
SPC SPC SPC SPC SPC SPC SPC SPC SPC DEL DEL DEL DEL
DEL DEL DEL DEL DEL DEL DEL DEL DEL DEL DEL DEL DEL
DEL DEL DEL RET

Recent messages:
Keyboard macro defined
(Type e to repeat macro) [17 times]
Making completion list...
Mark set
Mark saved where search started [2 times]
Auto-saving...done
Mark set [2 times]
Undo!
Quit [2 times]
Mark set

Load-path shadows:


______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Josh]

On Mon, Jun 24, 2013 at 4:44 AM, Ed Avis <eda@waniasset.com> wrote:
> Do M-x query-replace-regexp and enter ' +' as the regexp and ' '
> as the replacement text.  Emacs matches each sequence of one or
> more space characters, as specified in the regexp, and asks whether
> to replace it.

Why not target two or more spaces in the search expression, i.e. "  +"?

> However, for many of the cases the replacement text will be the same
> as the match, so it makes no difference whether you replace or not.
> Since the aim of query-replace-regexp is primarily to do a search
> and replace, not to find all places in the buffer where a regexp
> matches, it should instead skip over those cases and only ask about
> places where replacing will make a difference.

I wouldn't like it if Emacs were to behave that way, because when
confirming a replacement would result in no change to the text it often
indicates a broken regexp (as in this case) that I want to know about.

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Ed Avis]

Yes, I could have specified the regexp as '  +' instead of just ' +'.
That would not have given any cases where the match and replacement are the same.
I typed the regexp without too much thought, but it is not incorrect;
replacing one or more spaces with one space does what I want, just
with more replacement operations than are strictly necessary.

I think it would make Emacs more usable if it only prompted for replacements
which make a difference, after all, if the buffer contents will not be affected
one way or the other why waste the user's time asking for yes or no?

I do take your point that a no-op replacement can indicate a bug in
the regexp.  Perhaps query-replace-regexp could print 'Skipping some
cases where matched text and replacement text are the same'.

-- 
Ed Avis <eda@waniasset.com>

______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Andreas Schwab]

Should this no-op replacement increment the replacement counter (\#)?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Stefan Monnier]

> Since the aim of query-replace-regexp is primarily to do a search
> and replace, not to find all places in the buffer where a regexp
> matches, it should instead skip over those cases and only ask about
> places where replacing will make a difference.

I don't think the difference is very important, but I wouldn't oppose
such a change.


        Stefan

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Josh Feinstein]

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

On Mon, Jun 24, 2013 at 6:49 PM, Stefan Monnier <monnier@iro.umontreal.ca>wrote:

> > Since the aim of query-replace-regexp is primarily to do a search
> > and replace, not to find all places in the buffer where a regexp
> > matches, it should instead skip over those cases and only ask about
> > places where replacing will make a difference.
>
> I don't think the difference is very important, but I wouldn't oppose
> such a change.
>

Did you read the point I made above and which the reporter conceded, namely
that such no-op replacements often indicate broken regexps?  Changing the
behavior to either silently ignore such cases or issue a vague "skipped
some replacements" message would make it more difficult to detect such
breakage and the affected buffer locations, with the only benefit stated so
far being to save someone typing "  +" instead of " +".

[-- Attachment #2: Type: text/html, Size: 1333 bytes --]

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Ed Avis]

I wouldn't say that the regexp in this case is broken - only suboptimal. The user entered something correct, to replace all sequences of spaces by a single space. If done as a global replace without confirmation, you wouldn't even notice. The user asked for confirmation of each change to end up with the right text in the buffer, not as an academic exercise in finding the best possible regexp.

So prompting for each no-op match is not really helping the user. I can see that sometimes you might use query-replace-regexp as a debugging aid (though surely a simple search without replacement would be better). But even then it would be enough just to flag up one case where match=replacement, not request y or n for each one, since they are all identical. (Here I am assuming no capturing groups.)

So maybe the answer is to flag the first no-op match but then skip the rest. 

______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Stefan Monnier]

>> I don't think the difference is very important, but I wouldn't oppose
>> such a change.
> Did you read the point I made above and which the reporter conceded, namely
> that such no-op replacements often indicate broken regexps?

I did.  And just like the improvement he proposes, this downside
is minor.  I don't think either of the arguments justifies spending
time on a patch,


        Stefan

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Drew Adams]

> I wouldn't say that the regexp in this case is broken - only suboptimal. The
> user entered something correct, to replace all sequences of spaces by a
> single space. If done as a global replace without confirmation, you wouldn't
> even notice. The user asked for confirmation of each change to end up with
> the right text in the buffer, not as an academic exercise in finding the
> best possible regexp.
> 
> So prompting for each no-op match is not really helping the user. I can see
> that sometimes you might use query-replace-regexp as a debugging aid (though
> surely a simple search without replacement would be better). But even then
> it would be enough just to flag up one case where match=replacement, not
> request y or n for each one, since they are all identical. (Here I am
> assuming no capturing groups.)
> 
> So maybe the answer is to flag the first no-op match but then skip the rest.

Haven't been following this thread, so excuse if I misunderstand.

It would be wrong, IMHO, to simply skip ANY matches, e.g., because the occurrence precisely matches the replacement string.

This could be an optional behavior, but it certainly should not simply replace the longstanding behavior.

Why?  Because query replacing is not just about replacing.  It can be about checking occurrences (all of them).  It can involve stopping and doing something (e.g. editing the occurrence in a different way from the replacement text, or editing surrounding text).  And that "stopping" can be either via recursive edit (allowing q-r resuming) or simply stopping altogether (and perhaps restarting, at the same spot or elsewhere).

In sum, there is a lot more to a q-r interaction than simply y/n replacement.  Do not mess up what has already been available.  If you like, provide an option (on the fly via a key or via a user option) to do what you request.  But please do not just replace the existing, rich behavior.

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Josh]

On Tue, Jun 25, 2013 at 6:39 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>>> I don't think the difference is very important, but I wouldn't oppose
>>> such a change.
>> Did you read the point I made above and which the reporter conceded, namely
>> that such no-op replacements often indicate broken regexps?
>
> I did.  And just like the improvement he proposes, this downside
> is minor.  I don't think either of the arguments justifies spending
> time on a patch,

I don't understand what you mean here.  I see one argument (the reporter's)
in favor of some form of patch and one argument (mine) in favor of no change
whatsoever to the current behavior.

bug#14708: 24.2; query-replace-regexp when match and replacement are the same

[Josh]

On Tue, Jun 25, 2013 at 7:10 AM, Drew Adams <drew.adams@oracle.com> wrote:
>> I wouldn't say that the regexp in this case is broken - only suboptimal. The
>> user entered something correct, to replace all sequences of spaces by a
>> single space. If done as a global replace without confirmation, you wouldn't
>> even notice. The user asked for confirmation of each change to end up with
>> the right text in the buffer, not as an academic exercise in finding the
>> best possible regexp.
>>
>> So prompting for each no-op match is not really helping the user. I can see
>> that sometimes you might use query-replace-regexp as a debugging aid (though
>> surely a simple search without replacement would be better). But even then
>> it would be enough just to flag up one case where match=replacement, not
>> request y or n for each one, since they are all identical. (Here I am
>> assuming no capturing groups.)
>>
>> So maybe the answer is to flag the first no-op match but then skip the rest.
>
> Haven't been following this thread, so excuse if I misunderstand.
>
> It would be wrong, IMHO, to simply skip ANY matches, e.g., because the occurrence precisely matches the replacement string.
>
> This could be an optional behavior, but it certainly should not simply replace the longstanding behavior.
>
> Why?  Because query replacing is not just about replacing.  It can be about checking occurrences (all of them).  It can involve stopping and doing something (e.g. editing the occurrence in a different way from the replacement text, or editing surrounding text).  And that "stopping" can be either via recursive edit (allowing q-r resuming) or simply stopping altogether (and perhaps restarting, at the same spot or elsewhere).
>
> In sum, there is a lot more to a q-r interaction than simply y/n replacement.  Do not mess up what has already been available.  If you like, provide an option (on the fly via a key or via a user option) to do what you request.  But please do not just replace the existing, rich behavior.

Yes, exactly.