Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
3 years, 8 months ago by jgruber Modified:
3 years, 8 months ago Reviewers:
Dan Ehrenberg, Yang CC:
v8-reviews_googlegroups.com Target Ref:
refs/heads/master Project:
v8 Visibility:
Public. |
Description[regexp] Updates for unicode escapes in capture names
Update docs and tests for recent changes in the spec for unicode escapes
in capture group names.
https://github.com/tc39/proposal-regexp-named-groups/issues/23
BUG=v8:5437
Review-Url: https://codereview.chromium.org/2788423003
Cr-Commit-Position: refs/heads/master@{#44474}
Committed: https://chromium.googlesource.com/v8/v8/+/f3b848fe5d1dcca6a2e490e5b5674d551f7d27db
Patch Set 1 #
Total comments: 3
Patch Set 2 : Address comments and rebase #
Created: 3 years, 8 months ago
Messages
Total messages: 18 (12 generated)
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
jgruber@chromium.org changed reviewers: + littledan@chromium.org, yangguo@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2788423003/diff/1/test/mjsunit/harmony/regexp... File test/mjsunit/harmony/regexp-named-captures.js (right): https://codereview.chromium.org/2788423003/diff/1/test/mjsunit/harmony/regexp... test/mjsunit/harmony/regexp-named-captures.js:188: assertThrows("/(?<a\\uD801>.)/u", SyntaxError); // \u Lead Also test the case where you don't use the \u escape in the regexp pattern? I.e. no double \\, like this "/(?<a\uD801>.)/u"
https://codereview.chromium.org/2788423003/diff/1/test/mjsunit/harmony/regexp... File test/mjsunit/harmony/regexp-named-captures.js (right): https://codereview.chromium.org/2788423003/diff/1/test/mjsunit/harmony/regexp... test/mjsunit/harmony/regexp-named-captures.js:188: assertThrows("/(?<a\\uD801>.)/u", SyntaxError); // \u Lead On 2017/04/05 08:03:35, Yang wrote: > Also test the case where you don't use the \u escape in the regexp pattern? I.e. > no double \\, like this "/(?<a\uD801>.)/u" You mean just a standard group name? "/(?<a\uD801>.)/u" is equivalent to /(?<auD801>.)/u.
https://codereview.chromium.org/2788423003/diff/1/test/mjsunit/harmony/regexp... File test/mjsunit/harmony/regexp-named-captures.js (right): https://codereview.chromium.org/2788423003/diff/1/test/mjsunit/harmony/regexp... test/mjsunit/harmony/regexp-named-captures.js:188: assertThrows("/(?<a\\uD801>.)/u", SyntaxError); // \u Lead On 2017/04/07 08:01:23, jgruber wrote: > On 2017/04/05 08:03:35, Yang wrote: > > Also test the case where you don't use the \u escape in the regexp pattern? > I.e. > > no double \\, like this "/(?<a\uD801>.)/u" > > You mean just a standard group name? "/(?<a\uD801>.)/u" is equivalent to > /(?<auD801>.)/u. Never mind, it's not. Will add a test.
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by jgruber@chromium.org
The CQ bit was checked by jgruber@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org Link to the patchset: https://codereview.chromium.org/2788423003/#ps20001 (title: "Address comments and rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491553686054310,
"parent_rev": "2e889e4da2afb204727ec12bbaa104c8ea1c2db5", "commit_rev":
"f3b848fe5d1dcca6a2e490e5b5674d551f7d27db"}
Message was sent while issue was closed.
Description was changed from ========== [regexp] Updates for unicode escapes in capture names Update docs and tests for recent changes in the spec for unicode escapes in capture group names. https://github.com/tc39/proposal-regexp-named-groups/issues/23 BUG=v8:5437 ========== to ========== [regexp] Updates for unicode escapes in capture names Update docs and tests for recent changes in the spec for unicode escapes in capture group names. https://github.com/tc39/proposal-regexp-named-groups/issues/23 BUG=v8:5437 Review-Url: https://codereview.chromium.org/2788423003 Cr-Commit-Position: refs/heads/master@{#44474} Committed: https://chromium.googlesource.com/v8/v8/+/f3b848fe5d1dcca6a2e490e5b5674d551f7... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/v8/v8/+/f3b848fe5d1dcca6a2e490e5b5674d551f7... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
