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

chore(regular_expression): Update example to support RegExp constructor #5106

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

leaysgur
Copy link
Contributor

@leaysgur leaysgur commented Aug 23, 2024

  • Fix example to handle new RegExp() too
  • Update NOTE comments

Until I tried interacting with the actual AST parsed by oxc_parser, I thought that the current oxc_regular_expression lacked support for the RegExp constructor due to escape sequences.

This was because "\"" remained "\"" after reading the source text from .js files.

However, once it was parsed by oxc_parser, I found that everything was resolved! (Wonderful work as usual. 👏🏻 )

Now there is nothing to worry about. 😌

Copy link

codspeed-hq bot commented Aug 23, 2024

CodSpeed Performance Report

Merging #5106 will not alter performance

Comparing regexp-exp (c7b81f5) with main (aa7718a)

Summary

✅ 29 untouched benchmarks

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Aug 23, 2024
Copy link

graphite-app bot commented Aug 23, 2024

Merge activity

  • Aug 23, 12:56 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Aug 23, 12:56 AM EDT: Boshen added this pull request to the Graphite merge queue.
  • Aug 23, 1:00 AM EDT: Boshen merged this pull request with the Graphite merge queue.

…or (#5106)

- Fix example to handle `new RegExp()` too
- Update NOTE comments

- - -

Until I tried interacting with the actual AST parsed by `oxc_parser`, I thought that the current `oxc_regular_expression` lacked support for the `RegExp` constructor due to escape sequences.

This was because `"\""` remained `"\""` after reading the source text from `.js` files.

However, once it was parsed by `oxc_parser`, I found that everything was [resolved](https://github.com/oxc-project/oxc/blob/8ef85a43c019a1ce9aa50b61ec4dbb5dbaeb3b7b/crates/oxc_parser/src/lexer/string.rs)! (Wonderful work as usual. 👏🏻 )

Now there is nothing to worry about. 😌
@graphite-app graphite-app bot merged commit c7b81f5 into main Aug 23, 2024
24 checks passed
@graphite-app graphite-app bot deleted the regexp-exp branch August 23, 2024 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-merge Merge with Graphite Merge Queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants