Review nits

[anba]

https://tc39.github.io/proposal-regexp-named-groups/#sec-backreference-matcher

Let len be s's length.

should be:

Let len be the number of elements in s.

because s is a List and the length of a List is typically accessed through the term "the number of elements in ...".

https://tc39.github.io/proposal-regexp-named-groups/#sec-atomescape

• Typo: "an RegExpIdentifierName" -> s/an/a/
• The "for an RegExpIdentifierName" in "Search the enclosing RegExp for an instance of a GroupSpecifier for an RegExpIdentifierName which has ..." doesn't sound grammatically correct to me.

https://tc39.github.io/proposal-regexp-named-groups/#sec-regexpbuiltinexec
https://tc39.github.io/proposal-regexp-named-groups/#sec-getsubstitution
https://tc39.github.io/proposal-regexp-named-groups/#sec-regexp.prototype-@@replace

• If-steps with sub-steps need a trailing, ", then".

https://tc39.github.io/proposal-regexp-named-groups/#annex-b-grammar

• AtomEscape :: [+N] k GroupName is missing
• I don't understand why SourceCharacterIdentityEscape[N] :: [+N] SourceCharacter but not one of c or k was added. Annex B only has IdentityEscape :: [~U] SourceCharacter but not c to ensure /\c/ is parsed as /\\c/ instead of /c/, but I don't see why that monstrosity is also needed for k. Am I overlooking some detail? Edit: see comment below

[mathiasbynens]

I noticed a few (but not all) of these during a review today and fixed them locally while offline. I’ll submit a PR once I get back to my laptop. The remaining nits should be easy to fix, too. Thanks!

…

On Nov 2, 2017 17:09, "André Bargull" ***@***.***> wrote: https://tc39.github.io/proposal-regexp-named-groups/# sec-backreference-matcher Let len be s's length. should be: Let len be the number of elements in s. because s is a List and the length of a List is typically accessed through the term "the number of elements in ...". https://tc39.github.io/proposal-regexp-named-groups/#sec-atomescape - Typo: "an RegExpIdentifierName" -> s/an/a/ - The "for an RegExpIdentifierName" in "Search the enclosing RegExp for an instance of a GroupSpecifier for an RegExpIdentifierName which has ..." doesn't sound grammatically correct to me. https://tc39.github.io/proposal-regexp-named-groups/#sec-regexpbuiltinexec https://tc39.github.io/proposal-regexp-named-groups/#sec-getsubstitution https://tc39.github.io/proposal-regexp-named-groups/# sec-regexp.prototype-@@replace - If-steps with sub-steps need a trailing, ", then". https://tc39.github.io/proposal-regexp-named-groups/#annex-b-grammar - AtomEscape :: [+N] k GroupName is missing - I don't understand why SourceCharacterIdentityEscape[N] :: [+N] SourceCharacter but not one of c or k was added. Annex B only has IdentityEscape :: [~U] SourceCharacter but not c to ensure /\c/ is parsed as /\\c/ instead of /c/, but I don't see why that monstrosity is also needed for k. Am I overlooking some detail? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#35>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFAFnTHE9L1AtBPvOj9hQU2czQH4Vjlks5syj2YgaJpZM4QQagd> .

[mathiasbynens]

My PR is at #36. Note that it’s not a strict subset of what @anba found. (E.g., isn‘t ClassAtomNoDash[U] missing too?)

[anba]

I don't understand why SourceCharacterIdentityEscape[N] :: [+N] SourceCharacter but not one of c or k was added. Annex B only has IdentityEscape :: [~U] SourceCharacter but not c to ensure /\c/ is parsed as /\c/ instead of /c/, but I don't see why that monstrosity is also needed for k. Am I overlooking some detail?

Ah never mind, I found out why this is needed. It's to ensure /\k(?<a>)/ is a SyntaxError.

[anba]

Not all issues mentioned in #35 (comment)
270823718 have been resolved in #38. Maybe reopen?

[mathiasbynens]

Which issues have not been resolved?

[anba]

Which issues have not been resolved?

https://tc39.github.io/proposal-regexp-named-groups/#sec-backreference-matcher

Let len be s's length.

should be:

Let len be the number of elements in s.

because s is a List and the length of a List is typically accessed through the term "the number of elements in ...".

https://tc39.github.io/proposal-regexp-named-groups/#sec-atomescape

Typo: "an RegExpIdentifierName" -> s/an/a/
The "for an RegExpIdentifierName" in "Search the enclosing RegExp for an instance of a GroupSpecifier for an RegExpIdentifierName which has ..." doesn't sound grammatically correct to me.

[mathiasbynens]

[littledan]

Sorry about this! I updated gh-pages.

[anba]

I included those changes in 6387879, having noticed some of them myself. It’s just that the gh-pages branch hasn’t been updated since that got merged.

Err, nope. :-)

The three issues mentioned above are still present in current master.

proposal-regexp-named-groups/spec.html

Line 114 in b7940ce

1. Let _len_ be _s_'s length.

proposal-regexp-named-groups/spec.html

Line 129 in b7940ce

1. Search the enclosing RegExp for an instance of a |GroupSpecifier| for an |RegExpIdentifierName| which has a StringValue equal to the StringValue of the |RegExpIdentifierName| contained in |GroupName|.

[littledan]

@anba Sorry about that; should be better now.

[mathiasbynens]

Err, nope. :-)

Whoops, I might’ve been thinking about the upstream spec patch, which has your corrections: https://github.com/tc39/ecma262/pull/1027/files Sorry I forgot to “backport” these :)

[anba]

@mathiasbynens Ah, I see. So confusing having to track the same proposal in two places. :-)