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

parser: add regex AST node #5060

Closed
Boshen opened this issue Aug 22, 2024 · 5 comments · Fixed by #5256
Closed

parser: add regex AST node #5060

Boshen opened this issue Aug 22, 2024 · 5 comments · Fixed by #5256
Assignees
Labels
C-enhancement Category - New feature or request

Comments

@Boshen
Copy link
Member

Boshen commented Aug 22, 2024

let _pattern = self
.options
.parse_regular_expression
.then(|| self.parse_regex_pattern(pattern_start, pattern, flags));

This pattern node cannot be added to the AST due to not having CloneIn implemented. cc @rzvxa

@rzvxa
Copy link
Contributor

rzvxa commented Aug 27, 2024

@Boshen Do we want these types to be feature-gated? If so It seems this would take more time than I initially expected. It requires me to go in and refactor stuff related to our AST layout to support conditional layouts.
It would also mean that we have to distinguish between these layouts when doing AST transfer but it isn't too hard to support, It just takes more time as we need to represent conditional fields and type definitions.

@Boshen
Copy link
Member Author

Boshen commented Aug 28, 2024

@Boshen Do we want these types to be feature-gated? If so It seems this would take more time than I initially expected. It requires me to go in and refactor stuff related to our AST layout to support conditional layouts. It would also mean that we have to distinguish between these layouts when doing AST transfer but it isn't too hard to support, It just takes more time as we need to represent conditional fields and type definitions.

Welcome back ❤️

No feature gate is needed, all the regular expression parsing code is really lightweight.

@rzvxa
Copy link
Contributor

rzvxa commented Aug 28, 2024

Welcome back ❤️

Thanks Captain!⚓

No feature gate is needed, all the regular expression parsing code is really lightweight.

That's great to hear, It Simplifies the process and doesn't prevent us from adding it later on as a follow-up PR.
If you would like to feature-gate it in the future, Let me know so I can create an issue for it in the backlog.

@rzvxa
Copy link
Contributor

rzvxa commented Sep 3, 2024

Should we close this issue?

@rzvxa
Copy link
Contributor

rzvxa commented Sep 3, 2024

I'll close it since #5256 addresses this. Feel free to reopen if you think the requirements for this issue aren't met yet.

@rzvxa rzvxa closed this as completed Sep 3, 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants