unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
@ 2023-08-25 13:16 Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-25 14:18 ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-25 13:16 UTC (permalink / raw)
  To: 65536

Not sure about this one, maybe I have overlooked something ...

The Emacs Lisp manual says:

   Notice that all functions are allowed to overwrite the match data
   unless they’re explicitly documented not to do so.

And in particular function `replace-regexp-in-string' could be
assumed to modify the match data, but it uses an explicit call to
`save-match-data' to not do so.

Maybe this should be explicitly documented in its doc string and/or
the Emacs Lisp manual ((elisp) Search and Replace) so that users of
the function can rely on that fact?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
  2023-08-25 13:16 bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-25 14:18 ` Eli Zaretskii
  2023-08-25 15:26   ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-08-25 14:18 UTC (permalink / raw)
  To: Jens Schmidt; +Cc: 65536

> Date: Fri, 25 Aug 2023 15:16:29 +0200
> From:  Jens Schmidt via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Not sure about this one, maybe I have overlooked something ...
> 
> The Emacs Lisp manual says:
> 
>    Notice that all functions are allowed to overwrite the match data
>    unless they’re explicitly documented not to do so.
> 
> And in particular function `replace-regexp-in-string' could be
> assumed to modify the match data, but it uses an explicit call to
> `save-match-data' to not do so.
> 
> Maybe this should be explicitly documented in its doc string and/or
> the Emacs Lisp manual ((elisp) Search and Replace) so that users of
> the function can rely on that fact?

Is it important to promise never to clobber match-data in this
function?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
  2023-08-25 14:18 ` Eli Zaretskii
@ 2023-08-25 15:26   ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-25 16:56     ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 8+ messages in thread
From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-25 15:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 65536

On 2023-08-25  16:18, Eli Zaretskii wrote:
>> Date: Fri, 25 Aug 2023 15:16:29 +0200
>> From:  Jens Schmidt via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>

>> Maybe this should be explicitly documented in its doc string and/or
>> the Emacs Lisp manual ((elisp) Search and Replace) so that users of
>> the function can rely on that fact?
> 
> Is it important to promise never to clobber match-data in this
> function?

Not sure whether the following is convincing enough ...

- I came across this question in the following scenario:

  (match-fat-regexp with a lot of subres, one of which matches quoted
   chars: "\\(?5:\\(?:[[:alnum:]_]+\\|\\\\.\\)+\\)")
  (let ((a (match-string 1 input))
        ;; unquote quoted chars
        (b (replace-regexp-in-string
            "\\\\\\(.\\)" "\\1"
            (match-string 5 input)
            t nil))
        (c (match-string 7 input)))
    ...)

  So for that and similar scenarios it would be helpful.

- And since `replace-regexp-in-string' contains that `save-match-data'
  for a long time already (at least since Emacs 23) I guess that a lot
  of authors have been relying on that fact, either consciously, after
  having peeked into the function, or unconsciously.

- If it helps, I could provide a patch ...






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
  2023-08-25 15:26   ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-25 16:56     ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-08-26  6:09       ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-08-25 16:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 65536

On 2023-08-25  17:26, Jens Schmidt wrote:
> On 2023-08-25  16:18, Eli Zaretskii wrote:
>>> Date: Fri, 25 Aug 2023 15:16:29 +0200
>>> From:  Jens Schmidt via "Bug reports for GNU Emacs,
>>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
>>> Maybe this should be explicitly documented in its doc string and/or
>>> the Emacs Lisp manual ((elisp) Search and Replace) so that users of
>>> the function can rely on that fact?
>>
>> Is it important to promise never to clobber match-data in this
>> function?
> 
> Not sure whether the following is convincing enough ...

OTOH, I just found the following in the doc string of
`save-match-data', which means that `replace-regexp-in-string'
is actually not "convention-conforming":

  NOTE: The convention in Elisp is that any function, except for a few
  exceptions like car/assoc/+/goto-char, can clobber the match data,
  so ‘save-match-data’ should normally be used to save *your* match data
  rather than your caller’s match data.

The final decision is yours ...






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
  2023-08-25 16:56     ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-08-26  6:09       ` Eli Zaretskii
  2023-08-26  6:24         ` Ihor Radchenko
  2023-08-26  8:40         ` Stefan Kangas
  0 siblings, 2 replies; 8+ messages in thread
From: Eli Zaretskii @ 2023-08-26  6:09 UTC (permalink / raw)
  To: Jens Schmidt, Stefan Kangas; +Cc: 65536

> Date: Fri, 25 Aug 2023 18:56:15 +0200
> From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
> Cc: 65536@debbugs.gnu.org
> 
> On 2023-08-25  17:26, Jens Schmidt wrote:
> > On 2023-08-25  16:18, Eli Zaretskii wrote:
> >>> Date: Fri, 25 Aug 2023 15:16:29 +0200
> >>> From:  Jens Schmidt via "Bug reports for GNU Emacs,
> >>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> > 
> >>> Maybe this should be explicitly documented in its doc string and/or
> >>> the Emacs Lisp manual ((elisp) Search and Replace) so that users of
> >>> the function can rely on that fact?
> >>
> >> Is it important to promise never to clobber match-data in this
> >> function?
> > 
> > Not sure whether the following is convincing enough ...
> 
> OTOH, I just found the following in the doc string of
> `save-match-data', which means that `replace-regexp-in-string'
> is actually not "convention-conforming":
> 
>   NOTE: The convention in Elisp is that any function, except for a few
>   exceptions like car/assoc/+/goto-char, can clobber the match data,
>   so ‘save-match-data’ should normally be used to save *your* match data
>   rather than your caller’s match data.
> 
> The final decision is yours ...

I think it's okay to leave things as they are.

Stefan, WDYT?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
  2023-08-26  6:09       ` Eli Zaretskii
@ 2023-08-26  6:24         ` Ihor Radchenko
  2023-08-26  8:40         ` Stefan Kangas
  1 sibling, 0 replies; 8+ messages in thread
From: Ihor Radchenko @ 2023-08-26  6:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 65536, Jens Schmidt, Stefan Kangas

Eli Zaretskii <eliz@gnu.org> writes:

>>   NOTE: The convention in Elisp is that any function, except for a few
>>   exceptions like car/assoc/+/goto-char, can clobber the match data,
>>   so ‘save-match-data’ should normally be used to save *your* match data
>>   rather than your caller’s match data.
>> 
>> The final decision is yours ...
>
> I think it's okay to leave things as they are.

I can share some experience from me dropping some `save-match-data' from
Org parser - I had to fight the consequences, as a lot of Org's own
code implicitly assumed that match data is saved, which caused huge
number of tests to fail. Fixing this was not fun, although I was able to
do it because that particular function had a history modifying match
data in the past and only relatively recent code additions were
affected.

If something like `replace-regexp-in-string' suddenly changes its
behaviour (even undocumented), I expect issues in third-party code.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
  2023-08-26  6:09       ` Eli Zaretskii
  2023-08-26  6:24         ` Ihor Radchenko
@ 2023-08-26  8:40         ` Stefan Kangas
  2023-08-26  9:09           ` Eli Zaretskii
  1 sibling, 1 reply; 8+ messages in thread
From: Stefan Kangas @ 2023-08-26  8:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 65536, Jens Schmidt

Eli Zaretskii <eliz@gnu.org> writes:

> I think it's okay to leave things as they are.
>
> Stefan, WDYT?

We do not want to change replace-regexp-in-string's behavior.  This
goes without saying IMO, and needs no documentation changes.

I also see nothing highly surprising or unusual about it saving match
data.  Interested users can easily read the code to see if it does,
just like Jens did.

So I also think it's fine not to change anything here, on balance.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data
  2023-08-26  8:40         ` Stefan Kangas
@ 2023-08-26  9:09           ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2023-08-26  9:09 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 65536, jschmidt4gnu

tags 65536 notabug wontfix
close 65536
thanks

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 26 Aug 2023 10:40:18 +0200
> Cc: Jens Schmidt <jschmidt4gnu@vodafonemail.de>, 65536@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I think it's okay to leave things as they are.
> >
> > Stefan, WDYT?
> 
> We do not want to change replace-regexp-in-string's behavior.  This
> goes without saying IMO, and needs no documentation changes.
> 
> I also see nothing highly surprising or unusual about it saving match
> data.  Interested users can easily read the code to see if it does,
> just like Jens did.
> 
> So I also think it's fine not to change anything here, on balance.

Thanks, so I'm closing this bug.





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-08-26  9:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-25 13:16 bug#65536: 30.0.50; replace-regexp-in-string documentation does not mention it saves match data Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-25 14:18 ` Eli Zaretskii
2023-08-25 15:26   ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-25 16:56     ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-26  6:09       ` Eli Zaretskii
2023-08-26  6:24         ` Ihor Radchenko
2023-08-26  8:40         ` Stefan Kangas
2023-08-26  9:09           ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).