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

URLPattern: Emit {} around custom names followed by a regexp group. #31882

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Dec 3, 2021

This CL fixes another problem case from:

whatwg/urlpattern#145

In this case we need to preserve { ... } braces around a named group
followed by a group that may be emitted in regexp ( ... ) group. We
don't want {:foo}(.*) to be changed to :foo(.*) since that changes
the meaning from two groups to a single group.

Bug: 1263673
Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3315419
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#949705}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-3315419 branch 3 times, most recently from c030451 to bc53b22 Compare December 8, 2021 15:21
This CL fixes another problem case from:

whatwg/urlpattern#145

In this case we need to preserve `{ ... }` braces around a named group
followed by a group that may be emitted in regexp `( ... )` group.  We
don't want `{:foo}(.*)` to be changed to `:foo(.*)` since that changes
the meaning from two groups to a single group.

Bug: 1263673
Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3315419
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#949705}
@KyleJu KyleJu merged commit 938a88f into master Dec 8, 2021
@KyleJu KyleJu deleted the chromium-export-cl-3315419 branch December 8, 2021 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants