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

Stage 3 Specification Review: Waldemar Horwat #4

Open
rbuckton opened this issue Dec 16, 2021 · 4 comments
Open

Stage 3 Specification Review: Waldemar Horwat #4

rbuckton opened this issue Dec 16, 2021 · 4 comments
Assignees

Comments

@rbuckton
Copy link
Collaborator

This is a placeholder task for the Stage 3 Specification Review feedback from Waldemar Horwat (@waldemarhorwat).

@rbuckton
Copy link
Collaborator Author

@waldemarhorwat have you had an opportunity to review the specification text?

@rbuckton
Copy link
Collaborator Author

FYI, I am considering bringing this to plenary in June for Stage 3, pending reviews.

@waldemarhorwat
Copy link

Here's my review:

The Pattern Semantics section defines the following aliases:

DotAll is true if the RegExp object's [[OriginalFlags]] internal slot contains "s" and otherwise is false.
IgnoreCase is true if the RegExp object's [[OriginalFlags]] internal slot contains "i" and otherwise is false.
Multiline is true if the RegExp object's [[OriginalFlags]] internal slot contains "m" and otherwise is false.

These aliases shouldn't be needed any more because they're all replaced with accesses of modifiers objects being passed around. If any remain, that's probably a bug.

Extra "the" before "any" in

It is a Syntax Error if the any code point in the source text matched by the first RegularExpressionFlags is also contained in the source text matched by the second RegularExpressionFlags.

There is a dangling free variable modifiers in the definition of CompileToCharSet:

CharacterClassEscape :: w

  1. Return GetWordCharacters(modifiers).

This one may require a significant amount of refactoring to fix.

There may also be tricky interactions between this proposal's spec text and the new character classes proposal's spec text in how they handle case-insensitive character sets.

@rbuckton
Copy link
Collaborator Author

I believe all of this feedback is addressed in #14, which updates the proposal specification text to align with the latest version of ECMA-262, which includes the RegExp v flag. Unicode character sets always leverage the value of rer.[[IgnoreCase]], which this proposal now updates when the i modifier is set or cleared, so it should be consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants