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

Normative: Update GetSubstitution to match reality #3157

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

gibson042
Copy link
Contributor

@gibson042 gibson042 commented Aug 29, 2023

$nn patterns fall back to $n when there aren't at least nn captures

Fixes gh-1426

test262 PR: tc39/test262#3931

@gibson042 gibson042 added needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 web reality normative change Affects behavior required to correctly evaluate some ECMAScript source text labels Aug 29, 2023
spec.html Outdated Show resolved Hide resolved
Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

LGTM otherwise.

@michaelficarra
Copy link
Member

@michaelficarra michaelficarra added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Aug 29, 2023
@michaelficarra
Copy link
Member

I guess we should also have a test for when the first digit alone still exceeds the number of captures, like $20 when there's only one capture. Re-adding the "needs tests" label.

@michaelficarra michaelficarra added the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Aug 29, 2023
@ljharb

This comment was marked as resolved.

@michaelficarra

This comment was marked as resolved.

@ljharb

This comment was marked as resolved.

@michaelficarra michaelficarra added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Sep 27, 2023
@ljharb ljharb removed the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Sep 27, 2023
@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Oct 12, 2023
$nn patterns fall back to $n when there aren't at least nn captures

Fixes tc39#1426
@ljharb ljharb force-pushed the gh-1426-getsubstitution-dollar-nn-2 branch from 1e0e4c8 to 5eaee2f Compare October 12, 2023 23:25
@ljharb ljharb merged commit 5eaee2f into tc39:main Oct 12, 2023
7 checks passed
zhangenming pushed a commit to zhangenming/ecma262 that referenced this pull request Dec 22, 2023
$nn patterns fall back to $n when there aren't at least nn captures

Fixes tc39#1426
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text ready to merge Editors believe this PR needs no further reviews, and is ready to land. web reality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible bug in GetSubstitution or test262
4 participants