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

regular_expression: Improve ES2025 proposal-duplicate-named-capturing-groups logic #6358

Open
3 of 4 tasks
leaysgur opened this issue Aug 23, 2024 · 2 comments · Fixed by #6847
Open
3 of 4 tasks

regular_expression: Improve ES2025 proposal-duplicate-named-capturing-groups logic #6358

leaysgur opened this issue Aug 23, 2024 · 2 comments · Fixed by #6847
Labels
C-enhancement Category - New feature or request good first issue Experience Level - Good for newcomers

Comments

@leaysgur
Copy link
Collaborator

leaysgur commented Aug 23, 2024

Done in #6847 , but there is room for performance improvement.

We want to keep the code as simple as possible and process the current 2 loops in 1 loop for duplicates detection.


https://github.com/tc39/proposal-duplicate-named-capturing-groups
https://tc39.es/ecma262/#sec-mightbothparticipate

For now, /(?<a>.)(?<a>.)/ is invalid syntax, name a is duplicated.

With this change, we can use the same name if they are splitted by | and in the same Disjunction like /(?<a>.)|(?<a>.)/.

But it still not allowed in some cases like /(?<n>|(?<n>))/, /(?<n>(?<n>)|)/, /((?<n>)|(?<n>))(?<n>)/, etc...

You need to track nesting level and | to mange scope.

TODOs

@Boshen Boshen changed the title regular_expression: Support ES2025 Duplicated named capturing groups regular_expression: Support ES2025 proposal-duplicate-named-capturing-groups Sep 4, 2024
@Boshen Boshen transferred this issue from oxc-project/oxc Sep 11, 2024
@Boshen Boshen transferred this issue from oxc-project/backlog Oct 8, 2024
@Boshen Boshen added the good first issue Experience Level - Good for newcomers label Oct 8, 2024
@preyneyv
Copy link

I'd like to take a stab at this!

@Boshen
Copy link
Member

Boshen commented Oct 24, 2024

@preyneyv You may continue with #6847

Boshen pushed a commit that referenced this issue Oct 25, 2024
…oups (#6847)

Closes #6358

@preyneyv I know you've been working on this problem.

This is an implementation that has been dormant on my local for a while.

- All tests are passing
- However, the approach is simple but not general, so there might be some edge cases that were missed
- There's also room for improvement in terms of performance

For these reasons, it was marked as WIP for me.

I believe the test cases and other parts are usable, so feel free to fork and replace them with your implementation if you'd like.
Orenbek pushed a commit to Orenbek/oxc that referenced this issue Oct 28, 2024
…oups (oxc-project#6847)

Closes oxc-project#6358

@preyneyv I know you've been working on this problem.

This is an implementation that has been dormant on my local for a while.

- All tests are passing
- However, the approach is simple but not general, so there might be some edge cases that were missed
- There's also room for improvement in terms of performance

For these reasons, it was marked as WIP for me.

I believe the test cases and other parts are usable, so feel free to fork and replace them with your implementation if you'd like.
@leaysgur leaysgur added the C-enhancement Category - New feature or request label Oct 28, 2024
@leaysgur leaysgur changed the title regular_expression: Support ES2025 proposal-duplicate-named-capturing-groups regular_expression: Improve ES2025 proposal-duplicate-named-capturing-groups logic Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category - New feature or request good first issue Experience Level - Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants