-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
regexp: Expand rules are confusing #40329
Comments
Thanks. This isn't something we can change today, so it has to be a documentation fix. |
Change https://golang.org/cl/244625 mentions this issue: |
I think the simplest change is to nudge people to use the more explicit syntax. A lot of the time, people don't look at the examples in the docs, they just look at the text that pops up in their development environment, so how about this?
The problem with skipping the braces when referring to submatches is hinted at in the example code, but it's a bit too subtle, maybe adding a comment there would bring it to the attention. I don't think any more than that is needed. // $1W is not a named submatch, so the replacement is with an empty slice.
fmt.Printf("%s\n", re.ReplaceAll([]byte("-ab-axxb-"), []byte("$1W"))) |
Rebased PR to the current master, can we review one more time and land it ? |
Change https://go.dev/cl/446836 mentions this issue: |
…Expand part For #40329 Change-Id: Ie0cb337545ce39cd169129227c45f7d2eaebc898 GitHub-Last-Rev: c017d4c GitHub-Pull-Request: #56507 Reviewed-on: https://go-review.googlesource.com/c/go/+/446836 Reviewed-by: Michael Knyszek <mknyszek@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
https://play.golang.org/p/WYmtk1Gi-T_4
https://play.golang.org/p/XAdw3-cgx0z
What did you expect to see?
Expected that the first capture submatch
(.)
resulted in the characterb
, so the result should have beencba
, according to the docs at https://golang.org/pkg/regexp/#Regexp.ReplaceAllStringWhat did you see instead?
I just got the character
c
, I expected to getcba
, because the replacement had been interpreted differently.When I dug into it to raise this report, I found this in the documentation on the
Expand
function:I didn't realise I was using the name form, I thought I was using the number form. This made it clearer that the regexp group was being interpreted as
${1a}
, not$1
, followed bya
.That means, I would need to use "c${1}a" as my replacement, which is quite unusual because I haven't seen another programming language that works this way, e.g. Node.js will return
cba
.sed
also works as I expected (although it uses\1
as the reference instead of$1
).It was surprising behaviour to me, and I spent a while debugging the program I was working on before I realised there was something else going on.
Ideally, the Go implementation would change to match other programming languages and text editors, i.e. after a dollar, if the first character is
[0-9]
, then it's a numeric reference up until the next non-numeric character, e.g.However, if that's not possible, then a documentation change might help.
$1
is brittle, it only works if your reference to the capture group is the whole replacement ($1
), or is followed by a space character ($1 a
), at the end of the replacement (a$1
) etc. So the documentation should use the long form by default.The text was updated successfully, but these errors were encountered: