unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#20395: 24.3; Documentation for `replace-regexp-in-string'
@ 2015-04-21  9:59 Eli Barzilay
  2015-04-21 15:00 ` Eli Zaretskii
  2015-04-25  9:40 ` Eli Zaretskii
  0 siblings, 2 replies; 3+ messages in thread
From: Eli Barzilay @ 2015-04-21  9:59 UTC (permalink / raw)
  To: 20395

(In GNU Emacs 24.3.1, but same in git)

I tried to use `replace-regexp-in-string' like this:

    (let ((text "foo\nbar\nbaz\n") (r "*"))
      (replace-regexp-in-string
       "\n\\(.\\)"
       (lambda (_) (concat "\n" r (match-string 1 text)))
       text))

and it surprised me that this didn't work.  Looking at the docstring,
I found this

    When REP is called, the match data are the result of matching
    REGEXP against a substring of STRING.

and IMO that "a substring" is very subtle and easy to miss.  I then
looked at the code, and at least in its current form, I saw that I
could do this instead:

    (let ((text "foo\nbar\nbaz\n") (r "*"))
      (replace-regexp-in-string
       "\n\\(.\\)"
       (lambda (s) (concat "\n" r (match-string 1 s)))
       text))

So I think that it would be really good if this was made explicit in
the documentation, better with an example.  Something like

    When REP is called, the match data are the result of matching
    REGEXP against only the currently matched substring of STRING.
    For example, (lambda (s) (concat "<" (match-string 1 s) ">")) as
    REP is equivalent to "<\\1>".

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!





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

* bug#20395: 24.3; Documentation for `replace-regexp-in-string'
  2015-04-21  9:59 bug#20395: 24.3; Documentation for `replace-regexp-in-string' Eli Barzilay
@ 2015-04-21 15:00 ` Eli Zaretskii
  2015-04-25  9:40 ` Eli Zaretskii
  1 sibling, 0 replies; 3+ messages in thread
From: Eli Zaretskii @ 2015-04-21 15:00 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: 20395

> From: Eli Barzilay <eli@barzilay.org>
> Date: Tue, 21 Apr 2015 05:59:01 -0400
> 
>     When REP is called, the match data are the result of matching
>     REGEXP against a substring of STRING.
> 
> and IMO that "a substring" is very subtle and easy to miss.  I then
> looked at the code, and at least in its current form, I saw that I
> could do this instead:
> 
>     (let ((text "foo\nbar\nbaz\n") (r "*"))
>       (replace-regexp-in-string
>        "\n\\(.\\)"
>        (lambda (s) (concat "\n" r (match-string 1 s)))
>        text))
> 
> So I think that it would be really good if this was made explicit in
> the documentation, better with an example.  Something like
> 
>     When REP is called, the match data are the result of matching
>     REGEXP against only the currently matched substring of STRING.
>     For example, (lambda (s) (concat "<" (match-string 1 s) ">")) as
>     REP is equivalent to "<\\1>".

Thanks.

However, I wonder whether your suggested change is really enough of an
improvement.  AFAIU, the main problem with the existing doc string is
that it doesn't explain _which_ substring is being alluded to here, it
only hints on the answer.  And your suggested change also leaves that
part unexplained, leaving it to the reader to glean that from the fact
that 'concat' is being used.

So I think it would be good to come up with text that actually makes
that part clear.  WDYT?





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

* bug#20395: 24.3; Documentation for `replace-regexp-in-string'
  2015-04-21  9:59 bug#20395: 24.3; Documentation for `replace-regexp-in-string' Eli Barzilay
  2015-04-21 15:00 ` Eli Zaretskii
@ 2015-04-25  9:40 ` Eli Zaretskii
  1 sibling, 0 replies; 3+ messages in thread
From: Eli Zaretskii @ 2015-04-25  9:40 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: 20395-done

> From: Eli Barzilay <eli@barzilay.org>
> Date: Tue, 21 Apr 2015 05:59:01 -0400
> 
> (In GNU Emacs 24.3.1, but same in git)
> 
> I tried to use `replace-regexp-in-string' like this:
> 
>     (let ((text "foo\nbar\nbaz\n") (r "*"))
>       (replace-regexp-in-string
>        "\n\\(.\\)"
>        (lambda (_) (concat "\n" r (match-string 1 text)))
>        text))
> 
> and it surprised me that this didn't work.  Looking at the docstring,
> I found this
> 
>     When REP is called, the match data are the result of matching
>     REGEXP against a substring of STRING.
> 
> and IMO that "a substring" is very subtle and easy to miss.  I then
> looked at the code, and at least in its current form, I saw that I
> could do this instead:
> 
>     (let ((text "foo\nbar\nbaz\n") (r "*"))
>       (replace-regexp-in-string
>        "\n\\(.\\)"
>        (lambda (s) (concat "\n" r (match-string 1 s)))
>        text))
> 
> So I think that it would be really good if this was made explicit in
> the documentation, better with an example.  Something like
> 
>     When REP is called, the match data are the result of matching
>     REGEXP against only the currently matched substring of STRING.
>     For example, (lambda (s) (concat "<" (match-string 1 s) ">")) as
>     REP is equivalent to "<\\1>".

I tried to clarify this issue in a slightly different way.





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

end of thread, other threads:[~2015-04-25  9:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-21  9:59 bug#20395: 24.3; Documentation for `replace-regexp-in-string' Eli Barzilay
2015-04-21 15:00 ` Eli Zaretskii
2015-04-25  9:40 ` 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).