Skip to content
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

Add support for additional characters in group names #649

Closed
wants to merge 1 commit into from

Conversation

@bruceg
Copy link
Contributor

@bruceg bruceg commented Mar 1, 2020

We have an application that requires the ability to use field access
notation, which includes periods and square braces in group names. This
commit adds support and tests for these additional characters.

We have an application that requires the ability to use field access
notation, which includes periods and square braces in group names. This
commit adds support and tests for these additional characters.
@BurntSushi
Copy link
Member

@BurntSushi BurntSushi commented Oct 11, 2020

So I think there are two problems with this PR.

The first one is that this only changes what's allowed in the syntax, but doesn't modify replacement expansion to account for the new characters.

The second one is that if we do modify expansion to account for the new characters, then sadly, this becomes a breaking change. Consider this case:

regex: \[(?P<Z>[a-z]+)
haystack: [abc
replacement: $Z[

The current (and correct) result is abc[. But if [ became a valid capture letter, then the expansion parser would inclue [ as part of the name, so it would look for the group named Z[ instead of Z.

The only way I can think of to do this without making it a breaking change would be to require the use of braces when using characters in a capture group outside of [_0-9A-Za-z]. So that the behavior above would remain the same, but folks could use, e.g., ${foo[bar].baz} if they wanted to reference groups with [, ] or . in their names. Doing this will require some delicate surgery on the expansion routine.

I'll look into doing this.

BurntSushi added a commit that referenced this pull request Oct 11, 2020
This slightly expands the set of characters allowed in capture group
names to be `[][_0-9A-Za-z.]` from `[_0-9A-Za-z]`.

This required some delicacy in order to avoid replacement strings like
`$Z[` from referring to invalid capture group names where the intent was
to refer to the capture group named `Z`. That is, in order to use `[`,
`]` or `.` in a capture group name, one must use the explicit brace
syntax: `${Z[}`. We clarify the docs around this issue.

Regretably, we are not much closer to handling #595. In order to
support, say, all Unicode word characters, our replacement parser would
need to become UTF-8 aware on `&[u8]`. But std makes this difficult and
I would prefer not to add another dependency on ad hoc UTF-8 decoding or
a dependency on another crate.

Closes #649
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants