query-replace-regexp-eval is quite nice, but...

query-replace-regexp-eval is quite nice, but...

[David Kastrup]

The description tells us

[...]

    TO-EXPR is a Lisp expression evaluated to compute each
    replacement.  It may reference `replace-count' to get the number
    of replacements already made.  If the result of TO-EXPR is not a
    string, it is converted to one using `prin1-to-string' with the
    NOESCAPE argument (which see).

    For convenience, when entering TO-EXPR interactively, you can use
    `\&' or `\0' to stand for whatever matched the whole of REGEXP,
    and `\N' (where N is a digit) to stand for whatever matched the
    Nth `\(...\)' in REGEXP.  Use `\#&' or `\#N' if you want a number
    instead of a string.

So the idea is to build your replacement string with Lisp, and this
is quite an excellent thing.  It is completely defeated because the
replacement is then done non-literally.  Which means that if
\0
would have matched \footnote, replacing the string just with \0 (which
one would expect to do nothing in effect) will barf because the
regexp replacer will not know what \f is supposed to be.

Is there anybody that would make a case for a non-literal
replacement?  If not, is there anybody with enough of a clue to find
out how to fix this?  I have taken a look at perform-replace, but it
does not seem to offer an option for literal replacement for
regexps.  Should we add one?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: query-replace-regexp-eval is quite nice, but...

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> The description tells us
>
> [...]
>
>     TO-EXPR is a Lisp expression evaluated to compute each
>     replacement.  It may reference `replace-count' to get the number
>     of replacements already made.  If the result of TO-EXPR is not a
>     string, it is converted to one using `prin1-to-string' with the
>     NOESCAPE argument (which see).
>
>     For convenience, when entering TO-EXPR interactively, you can use
>     `\&' or `\0' to stand for whatever matched the whole of REGEXP,
>     and `\N' (where N is a digit) to stand for whatever matched the
>     Nth `\(...\)' in REGEXP.  Use `\#&' or `\#N' if you want a number
>     instead of a string.
>
> So the idea is to build your replacement string with Lisp, and this
> is quite an excellent thing.  It is completely defeated because the
> replacement is then done non-literally.  Which means that if
> \0
> would have matched \footnote, replacing the string just with \0 (which
> one would expect to do nothing in effect) will barf because the
> regexp replacer will not know what \f is supposed to be.

I agree this is a bug.

> Is there anybody that would make a case for a non-literal
> replacement?  If not, is there anybody with enough of a clue to find
> out how to fix this?  I have taken a look at perform-replace, but it
> does not seem to offer an option for literal replacement for
> regexps.  Should we add one?

Currently, perform-replace uses literal replacement for all non-regexp
searches, and non-literal otherwise.  What we could do is offer a special
version of match-string that quotes backslashes and use that in
replace-match-string-symbols.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: query-replace-regexp-eval is quite nice, but...

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
> 
> > The description tells us
> >
> > [...]
> >
> >     TO-EXPR is a Lisp expression evaluated to compute each
> >     replacement.  It may reference `replace-count' to get the number
> >     of replacements already made.  If the result of TO-EXPR is not a
> >     string, it is converted to one using `prin1-to-string' with the
> >     NOESCAPE argument (which see).
> >
> >     For convenience, when entering TO-EXPR interactively, you can use
> >     `\&' or `\0' to stand for whatever matched the whole of REGEXP,
> >     and `\N' (where N is a digit) to stand for whatever matched the
> >     Nth `\(...\)' in REGEXP.  Use `\#&' or `\#N' if you want a number
> >     instead of a string.
> >
> > So the idea is to build your replacement string with Lisp, and this
> > is quite an excellent thing.  It is completely defeated because the
> > replacement is then done non-literally.  Which means that if
> > \0
> > would have matched \footnote, replacing the string just with \0 (which
> > one would expect to do nothing in effect) will barf because the
> > regexp replacer will not know what \f is supposed to be.
> 
> I agree this is a bug.
> 
> > Is there anybody that would make a case for a non-literal
> > replacement?  If not, is there anybody with enough of a clue to find
> > out how to fix this?  I have taken a look at perform-replace, but it
> > does not seem to offer an option for literal replacement for
> > regexps.  Should we add one?
> 
> Currently, perform-replace uses literal replacement for all non-regexp
> searches, and non-literal otherwise.  What we could do is offer a special
> version of match-string that quotes backslashes and use that in
> replace-match-string-symbols.

But that means only that you get \0 and its ilk quoted.  The idea of
query-replace-regexp-eval is to build your replacement string
yourself.  And I won't get anything else quoted then.

I'll be perfectly able to enter
(concat "buzz" &1 &2)
instead of "buzz\\1\\2".

Really, I don't see any purpose whatsoever to do a non-literal
replacement here: all building blocks that a non-literal string would
be able to access are equally available in a form convenient for
replacements.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: query-replace-regexp-eval is quite nice, but...

[Ehud Karni]

on
	Mon, 02 Feb 2004 11:24:34 +0100)
Organization: Mivtach-Simon Insurance agencies
Reply-to: ehud@unix.mvs.co.il
References: <x5fzdtooaz.fsf@lola.goethe.zz> <jevfmpeq3h.fsf@sykes.suse.de=
>
X-Mailer: Emacs 21.3.1 rmail (send-msg 1.108)
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-8-i
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 02 Feb 2004 11:24:34 +0100, Andreas Schwab <schwab@suse.de> wrote:
>
> David Kastrup <dak@gnu.org> writes:
>
> [snip]
> >
> > So the idea is to build your replacement string with Lisp, and this
> > is quite an excellent thing.  It is completely defeated because the
> > replacement is then done non-literally.  Which means that if \0
> > would have matched \footnote, replacing the string just with \0 (which
> > one would expect to do nothing in effect) will barf because the
> > regexp replacer will not know what \f is supposed to be.
>
> I agree this is a bug.
>
> > Is there anybody that would make a case for a non-literal
> > replacement?  If not, is there anybody with enough of a clue to find
> > out how to fix this?  I have taken a look at perform-replace, but it
> > does not seem to offer an option for literal replacement for
> > regexps.  Should we add one?
>
> Currently, perform-replace uses literal replacement for all non-regexp
> searches, and non-literal otherwise.  What we could do is offer a special
> version of match-string that quotes backslashes and use that in
> replace-match-string-symbols.

How about checking regexp-flag (which is bound by `perform-replace') ?

Assuming your "literal" result string (containing \n) is STR you can
add something like this to your lisp:

       (if regexp-flag
           (regexp-quote STR)
           STR)

Ehud.


- --
 Ehud Karni           Tel: +972-3-7966-561  /"\
 Mivtach - Simon      Fax: +972-3-7966-667  \ /  ASCII Ribbon Campaign
 Insurance agencies   (USA) voice mail and   X   Against   HTML   Mail
 http://www.mvs.co.il  FAX:  1-815-5509341  / \
 GnuPG: 98EA398D <http://www.keyserver.net/>    Better Safe Than Sorry
-----BEGIN PGP SIGNATURE-----
Comment: use http://www.keyserver.net/ to get my key (and others)

iD8DBQFAHj+mLFvTvpjqOY0RAjh3AJ4nvVP+9BbwmMCaA0GDAYIpJ16wowCeNfbV
FPg2/jokXKyj9mUdIXD6T68=
=IyxP
-----END PGP SIGNATURE-----

Re: query-replace-regexp-eval is quite nice, but...

[David Kastrup]

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

"Ehud Karni" <ehud@unix.mvs.co.il> writes:

> On Mon, 02 Feb 2004 11:24:34 +0100, Andreas Schwab <schwab@suse.de> wrote:
> >
> > David Kastrup <dak@gnu.org> writes:
> >
> > [snip]
> > >
> > > So the idea is to build your replacement string with Lisp, and this
> > > is quite an excellent thing.  It is completely defeated because the
> > > replacement is then done non-literally.  Which means that if \0
> > > would have matched \footnote, replacing the string just with \0 (which
> > > one would expect to do nothing in effect) will barf because the
> > > regexp replacer will not know what \f is supposed to be.
> >
> > I agree this is a bug.
> >
> > > Is there anybody that would make a case for a non-literal
> > > replacement?  If not, is there anybody with enough of a clue to find
> > > out how to fix this?  I have taken a look at perform-replace, but it
> > > does not seem to offer an option for literal replacement for
> > > regexps.  Should we add one?
> >
> > Currently, perform-replace uses literal replacement for all non-regexp
> > searches, and non-literal otherwise.  What we could do is offer a special
> > version of match-string that quotes backslashes and use that in
> > replace-match-string-symbols.
> 
> How about checking regexp-flag (which is bound by `perform-replace') ?
> 
> Assuming your "literal" result string (containing \n) is STR you can
> add something like this to your lisp:
> 
>        (if regexp-flag
>            (regexp-quote STR)
>            STR)

"add something like this to my lisp"?  query-replace-regexp-eval is
an interactive function!  Even if it weren't, there is no point in
having two substitution mechanisms employed after another.

I have cobbled together the following minimal-invasive patch.  The
new argument-option of regexp-eval is not documented but obvious.
Since no other args are documented, either, with perform-replace,
this does not seem to do much harm and it won't affect existing use
of it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1563 bytes --]

*** replace.el.~1.166.~	2003-09-08 17:57:00.000000000 +0200
--- replace.el	2004-02-02 13:07:58.000000000 +0100
***************
*** 223,229 ****
  	   (if (and transient-mark-mode mark-active)
  	       (region-end)))))
    (perform-replace regexp (cons 'replace-eval-replacement to-expr)
! 		   t t delimited nil nil start end))
  
  (defun map-query-replace-regexp (regexp to-strings &optional n start end)
    "Replace some matches for REGEXP with various strings, in rotation.
--- 223,229 ----
  	   (if (and transient-mark-mode mark-active)
  	       (region-end)))))
    (perform-replace regexp (cons 'replace-eval-replacement to-expr)
! 		   t 'literal delimited nil nil start end))
  
  (defun map-query-replace-regexp (regexp to-strings &optional n start end)
    "Replace some matches for REGEXP with various strings, in rotation.
***************
*** 1057,1063 ****
  	(case-fold-search (and case-fold-search
  			       (string-equal from-string
  					     (downcase from-string))))
! 	(literal (not regexp-flag))
  	(search-function (if regexp-flag 're-search-forward 'search-forward))
  	(search-string from-string)
  	(real-match-data nil)		; the match data for the current match
--- 1057,1063 ----
  	(case-fold-search (and case-fold-search
  			       (string-equal from-string
  					     (downcase from-string))))
! 	(literal (or (not regexp-flag) (eq regexp-flag 'literal)))
  	(search-function (if regexp-flag 're-search-forward 'search-forward))
  	(search-string from-string)
  	(real-match-data nil)		; the match data for the current match

[-- Attachment #3: Type: text/plain, Size: 410 bytes --]


Since the current behavior is clearly wrong, the fix does not cause
any incompatibilities elsewhere, and the semantics are the obvious
ones as already documented, I'll just check it in.

If one wants to force more complicated things onto people, one can
still change stuff (including the documentation), but for the current
docs this behavior seems correct.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: query-replace-regexp-eval is quite nice, but...

[Andreas Schwab]

"Ehud Karni" <ehud@unix.mvs.co.il> writes:

> How about checking regexp-flag (which is bound by `perform-replace') ?
>
> Assuming your "literal" result string (containing \n) is STR you can
> add something like this to your lisp:
>
>        (if regexp-flag
>            (regexp-quote STR)
>            STR)

regexp-flag is always non-nil when called from query-replace-regexp-eval,
and regexp-quote quotes too much for replace-match.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: query-replace-regexp-eval is quite nice, but...

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Since the current behavior is clearly wrong, the fix does not cause
> any incompatibilities elsewhere, and the semantics are the obvious
> ones as already documented, I'll just check it in.

IMHO it should be documented in NEWS, since it's a user visible change in
behaviour.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: query-replace-regexp-eval is quite nice, but...

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
> 
> > Since the current behavior is clearly wrong, the fix does not
> > cause any incompatibilities elsewhere, and the semantics are the
> > obvious ones as already documented, I'll just check it in.
> 
> IMHO it should be documented in NEWS, since it's a user visible
> change in behaviour.

Actually, I'd say it is a user visible bug fix: I find the new
behavior more in consistence with the documentation.

But if people say so, I'll be willing to add an entry.  What is the
procedure for adding to the NEWS file?  Presumably it is news.texi or
something, and does one add just at the top or at a better place?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: query-replace-regexp-eval is quite nice, but...

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> "Ehud Karni" <ehud@unix.mvs.co.il> writes:
> 
> > How about checking regexp-flag (which is bound by `perform-replace') ?
> >
> > Assuming your "literal" result string (containing \n) is STR you can
> > add something like this to your lisp:
> >
> >        (if regexp-flag
> >            (regexp-quote STR)
> >            STR)
> 
> regexp-flag is always non-nil when called from query-replace-regexp-eval,
> and regexp-quote quotes too much for replace-match.

Not to mention that "e" (edit the replacement string) would be really
annoying.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: query-replace-regexp-eval is quite nice, but...

[Stefan Monnier]

>> Since the current behavior is clearly wrong, the fix does not cause
>> any incompatibilities elsewhere, and the semantics are the obvious
>> ones as already documented, I'll just check it in.

> IMHO it should be documented in NEWS, since it's a user visible change in
> behaviour.

I would qualify it as "bug fix".
The NEWS file is already so bloated with details that very few users
read it.


        Stefan

Re: query-replace-regexp-eval is quite nice, but...

[Andreas Schwab]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Since the current behavior is clearly wrong, the fix does not cause
>>> any incompatibilities elsewhere, and the semantics are the obvious
>>> ones as already documented, I'll just check it in.
>
>> IMHO it should be documented in NEWS, since it's a user visible change in
>> behaviour.
>
> I would qualify it as "bug fix".
> The NEWS file is already so bloated with details that very few users
> read it.

Ok, I agree.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."