Path to Stage 4!

[ljharb]

Stage 4

• [ ] committee approval
• [ ] two implementations
  • [ ] chrome / node
  • [ ] firefox
  • [ ] safari
  • [x] es-shims polyfill
• [ ] significant in-the-field experience
• [ ] ecma262 PR approved
• [x] prepare ecma262 PR

~Stage 3~

• [x] committee approval
• [x] merge test262 tests
• [x] write test262 tests (issue, PR)

~Stage 2.7~

• [x] committee approval
• [x] spec editor signoff
  • [ ] @syg
  • [x] @bakkot
• [x] spec reviewer signoff
  • [x] @jridgewell (#60)
  • [x] @michaelficarra
  • [x] @gibson042
• [x] resolve #71
• [x] resolve #69
• [x] resolve #65
• [x] resolve #66
• [x] resolve hex escapes vs character escapes
• [x] update spec to make escapes legal noops in u/v mode

  • 55

  • 52

~Stage 2~

• [x] committee approval
• [x] spec reviewers selected
• [x] spec text written

~Stage 1~

• [x] committee approval

[oliverfoster]

Congratulations on getting to Stage 2. :+1:

[benjamingr]

Hey, need help on any of these? (I'm happy to try and contribute the test262 tests or amend the spec according to the consensus)

[ljharb]

I'm comfortable handling the spec, but the test262 tests would be most helpful. Be warned, though, they'll need to be extremely rigorous.

[jridgewell]

My review is at https://github.com/tc39/proposal-regex-escaping/issues/60

[michaelficarra]

My feedback:

1. I don't like how this is introducing identity escapes in Unicode mode for these new punctuators. This wasn't a goal of the RegExp escaping proposal. You can accomplish RegExp escaping with hex escapes and without introducing new features to the RegExp grammar.
2. There's no reason to define the phrase "the ASCII punctuators that need escaping" if it's just used once, in the algorithm that follows. Just inline the string. Also I don't like how the string is denoted since it contains a ". Can you figure out some other way to denote it?

Maybe a table (code point, hex escape string) would solve both of my issues.

[ljharb]

For the second, I have no strong opinions about how to denote the characters; I can certainly inline the string.

For the first, isn't that required, otherwise this proposal won't be able to produce output that works in both u and v mode?

[michaelficarra]

No, I believe hex escapes are sufficient for this purpose. Do you have a specific counterexample?

[ljharb]

cc @bakkot since this was a result of their research

[bakkot]

Hex encoding works, it just makes the output completely unreadable. I can't imagine we're going to make \& mean anything other than & in u-mode regexps, so I don't think there's much cost in making those legal, and I think we ought to pay that cost for the benefit of more readable output.

[michaelficarra]

I do not value the readability of the output. This function is meant only for composing and then compiling RegExps.

[bakkot]

Sometimes you have to debug compiled regexps.

[michaelficarra]

You can decompile them and represent them however you like. Paste them into a visualiser.

[bakkot]

That's a bunch more work than doing console.log(regex). Why should anyone have to do that work? That's a very concrete cost to your suggestion and I see approximately no offsetting benefit. What benefit do you see to your suggestion?

[ljharb]

@michaelficarra as far as the spec review goes, I've addressed your editorial comment; can I check you off?

The normative one we can certainly discuss further if needed.

[michaelficarra]

The spec text is fine for what you intended. I still have issue with the escaping design though.

[ljharb]

Thanks! I'll check you off, but can you file a new issue to further discuss the escaping design? That seems like the only obstacle to seeking stage 3.

[sophiebits]

Escaping using hex escapes also has the advantage that it's straightforwardly polyfillable in practice, whereas changing the UnicodeMode grammar would seem to not really be.

[gibson042]

My review:

• [x] In |IdentityEscape|, / should only appear once in the new expansion to non-|SyntaxCharacter| punctuators and ` should be included (but I am in favor of introducing the new expansions that improve parity with non-Unicode literals as anticipated by https://github.com/tc39/proposal-regexp-v-flag/issues/71#issuecomment-1286637976 — it cuts off any future special use of those sequences, but that doesn't seem like a problem to me).
• [x] In the RegExp.escape introduction, the phrase "control characters" should be replaced with something less confusable with the similarly-named ASCII concept (e.g., "This method takes a string and returns a similar string in which each character that is potentially special in a regular expression |Pattern| has been replaced by an escape sequence representing that character.").
• [x] [#66] In the RegExp.escape algorithm, I think it is necessary to escape not just a leading decimal digit but also a leading ASCII letter (consider e.g. new RegExp("\\c" + RegExp.escape("J")) in a web browser implementing |ExtendedAtom|, where the result should match "\\cJ" and [unlike /\cJ/] fail to match "\n"), and possibly just to always escape the first character. I also think comprehensibility would be improved by adding a note explaining that logic branch and by making it more complete (rather than separating \x3 from the second hexadecimal digit by an intervening Else step). For example, assuming it's acceptable to always emit a \u… rather than something more dynamic like QuoteJSONString:

  1. For each code point _c_ in _cpList_, do
   1. If _escapedList_ is empty [and _c_ needs escaping], then
     1. NOTE: Escaping is required to ensure that the output may be suffixed onto an arbitrary prefix without being misinterpreted as part of an escape sequence that starts within that prefix.
     1. Let _codeUnits_ be UTF16EncodeCodePoint(_c_).
     1. Let _len_ be the length of _codeUnits_.
     1. Assert: _len_ = 1 or _len_ = 2.
     1. Let _codeUnit_ be the code unit at index 0 within _codeUnits_.
     1. Let _escapedCodeUnit_ be UnicodeEscape(_codeUnit_).
     1. Append to _escapedList_ the elements of StringToCodePoints(_escapedCodeUnit_).
     1. If len = 2, then
       1. Set _codeUnit_ to the code unit at index 1 within _codeUnits_.
       1. Set _escapedCodeUnit_ to UnicodeEscape(_codeUnit_).
       1. Append to _escapedList_ the elements of StringToCodePoints(_escapedCodeUnit_).
   1. Else,
     1. If _toEscape_ contains _c_ or _c_ is matched by |WhiteSpace|, then
       1. Append code unit U+005C (REVERSE SOLIDUS) to _escapedList_.
     1. Append _c_ to _escapedList_.
  1. Return CodePointsToString(_escapedList_).

The first issue is trivial to fix and the second is purely editorial, leaving only the third as something to actually resolve (potentially even by accepting the risk and making no normative change). In my opinion, this is ready for stage 3.

[ljharb]

First two are fixed; we can discuss the third in #66 but iirc the intention was simply to not support using the output of RegExp.escape in contexts like that.

[syg]

Spec draft is fairly short and I didn't see any glaring editorial issues.

I find the alias definition for "the ASCII punctuators that need escaping" as a separate paragraph the precede the algorithmic steps strange. Why not have a step that says "Let the ASCII punctuators [...] be the String [...]"?

[ljharb]

Sure, I could do that - happy to go with whatever yall want there.

[ljharb]

ok, this has been reworked with #67 - @jridgewell @michaelficarra @gibson042 @syg @bakkot, can you confirm that you're signed off, assuming you are?

[jridgewell]

LGTM

[bakkot]

LGTM

[michaelficarra]

@bakkot Does the string coercion in step 1 align with our new coercion strategy?

[bakkot]

Oh, good point. This should throw a TypeError on any non-string inputs.

[ljharb]

ah, good catch. updated in 29b08c3f5c8e450430bf8e5fcfea28a4e0d683e2

[michaelficarra]

LGTM

[gibson042]

I see one normative issue and a handful of editorial issues:

• [x] [#69] EncodeForRegExpEscape as specified can return output that would be misinterpreted in non-Unicode patterns.
• [x] [#74] [mea culpa] In RegExp.escape, "This method takes a string" is a novel phrasing that should probably be replaced (suggestion: convert to a structured header with description like "_It returns a copy of S in which characters that are potentially special in a regular expression |Pattern| have been replaced by equivalent escape sequences._").
• [x] [#75] A NOTE step explaining the reason for escaping a leading decimal digit would be nice.
• [x] EncodeForRegExpEscape makes a couple references to "punctuator", but there is no good definition for this term (and the plausible |Punctuator| includes multi-character sequences). Further, the value of punctuators is syntactically invalid (containing an unescaped embedded ") and potentially confusing (because we're not clear about whether or not rendered backslashes in \ strings like the "…\/…" here are raw characters). Tackling both, I would recommend instead defining something like "the ASCII punctuator characters" in the same paragraph as "the ASCII word characters", and possibly also "the ASCII control characters" (the union of U+0000 through U+001F with U+007F) to establish a complete partitioning of the Unicode Basic Latin block.
• [x] [#73] The new algorithms would be a bit shorter if they used string-concatentation rather than List appending, and in particular having EncodeForRegExpEscape return a string rather than a List of code points.

[ljharb]

Regarding the last item, I'm not sure optimizing for brevity helps readability here.

Regarding item 4, I had "the ASCII punctuators that need escaping" as a dfn, but removed it after https://github.com/tc39/proposal-regex-escaping/issues/60#issue-1916402272 and some editor feedback that a dfn wasn't needed.

Will take a look at the item 1 issue (thanks!) and will fix item 2 soon.

[michaelficarra]

@ljharb The point about an unescaped delimiter (") and a possibly-confusing backslash still stands. We can just construct punctuators by concatenating the non-controversial characters as a string with the individual problematic code units. Thankfully order doesn't matter so it's easy to just stick them on the end.

[michaelficarra]

Let punctuators be the string-concatenation of "(){}[]|,.?*+-^$=<>/#&!%:;@~'`", the code unit 0x0022 (QUOTATION MARK), and the code unit 0x005C (REVERSE SOLIDUS).

[ljharb]

ah ok, gotcha, thanks.

[ljharb]

k, that's landed in c95db790abb5182897a73c0270adac4ffecc61b5.

[ljharb]

Filxed #71 to handle the surrogate pair stuff.

[ljharb]

@gibson042 @syg latest changes are landed; can you confirm if/that you've signed off?

[gibson042]

Yep, that resolves the normative issues so works for me. I'll open PRs to demonstrate my editorial preferences.

[ljharb]

@syg given your stamp on #73, does that mean you've signed off?

[syg]

@syg given your stamp on https://github.com/tc39/proposal-regex-escaping/pull/73, does that mean you've signed off?

Yep, editorially lgtm.

[ljharb]

No advancement this meeting:

• 66 has been reopened

• unpaired surrogates need to be escaped
• things like newlines should be escaped as \n, so it's more readable, as long as it's valid syntax in unicode mode. @waldemarhorwat, it would be super helpful if you could file an issue listing the items you had in mind.

Once these are resolved, I'll return and re-ask for 2.7.

[michaelficarra]

@ljharb I imagine it means anything in ControlEscape, which doesn't have any alternative conditional on the UnicodeMode flag.

[bakkot]

Presumably also SyntaxCharacter and /, i.e. those characters which can be used in IdentityEscape even in u-mode RegExps.

[bakkot]

The other thing in CharacterEscape is \0. \0 is an interesting case, since you can't use it if the next character is an ASCII digit. I would favor not doing \0; \x00 is one of the few hex escapes you don't need to memorize.

[waldemarhorwat]

You can't do \0 for the NUL character because bad things could happen if the escaped string ended with a NUL and got concatenated with something that started with a digit. \x00 is fine for NUL.

[mevanlc]

I've been watching the progress on this repo for a long time now. I just want to say I appreciate the hard work and careful thought everyone has put into this (including the upcoming work as well.) It's been fascinating observing the concerns and challenges that the web environment raises for ECMAScript that were not much of a concern for other languages that have long had the same feature. I am wondering if the repo owner(s) would consider opening the GitHub Discussions tab in order to allow more back-and-forth discussion with fewer concerns about constantly pinging the people watching the Issues? I am sure if anything interesting distills out of Discussions, people will be more than happy to filter the info back into Issues (I don't think there is an expectation that the owners will monitor / reply to discussions, besides the unlikely chance that moderation is needed.) Just a thought.

[ljharb]

I'm not sure there's any way to disable issue notifications while enabling discussion notifications, and I think the same expectations exist for both about owner responsiveness.

[ljharb]

@jridgewell @michaelficarra @gibson042 @syg the proposal has been updated, and a fresh review would be appreciated :-)

[jridgewell]

LGTM

[gibson042]

Comments:

• I don't think 𝔽(_c_) for code point c is valid, because 𝔽 is defined as "conversion from a mathematical value or extended mathematical value", while a code point is neither of those (a code point has a numeric value which is an integer, but is not equal to it, cf. Source Text and Identity [among other sections]). Either the "Let hex be Number::toString(𝔽(c), 16)" step should be split to address this, or 𝔽 should be updated to explicitly accommodate it.
• As noted at https://github.com/tc39/proposal-regex-escaping/pull/77#discussion_r1561989827 , I consider it unwise to use character escapes rather than hexadecimal escapes for |SyntaxCharacter| because it's inviting future issues. I can live with this behavior, but would still like to discuss it in plenary.
• The population of |WhiteSpace| can change over time, therefore the behavior of RegExp.escape is not guaranteed to be stable (e.g., the addition of a code point to general category Space_Separator in some version of the Unicode Standard means that implementations incorporating that version will escape it while implementations incorporating an earlier version will not). This seems reasonable, but probably merits an editorial note.

[michaelficarra]

All the same points as @gibson042, plus there's some wording things that I would change, but they wouldn't hold up stages 2.7/3. LGTM otherwise.