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

Regex literal syntax #6776

Merged
merged 7 commits into from
Jul 13, 2024
Merged

Conversation

glennsl
Copy link
Contributor

@glennsl glennsl commented May 25, 2024

This adds syntax for regex literals equivalent to the literal syntax in JavaScript. It is effectively just syntax sugar for the %re syntax extension, but opens up the additional possibility of checking the regex syntax during compilation.

The syntax looks like:

let re = /a.*c/mg

which is equivalent to:

let re = %re(`/a.*c/mg`)

and compiles to:

var re = /a.*c/mg;

Closes #6287

@glennsl glennsl force-pushed the feat/syntax/regex-literals branch from ad00e77 to c42ce97 Compare May 25, 2024 13:47
@bloodyowl
Copy link
Collaborator

I think it might be worth adding an attribute to the expression not to incorrectly print escaped regexp as an unescaped one

@glennsl glennsl force-pushed the feat/syntax/regex-literals branch from c42ce97 to 69a3237 Compare May 26, 2024 07:45
@glennsl
Copy link
Contributor Author

glennsl commented May 26, 2024

I think it might be worth adding an attribute to the expression not to incorrectly print escaped regexp as an unescaped one

I`m not sure I follow. Can you give an example?

@glennsl
Copy link
Contributor Author

glennsl commented May 26, 2024

Tests passing! 🥳

I'm sure the code quality could be improved, and we could add validation of the internal regex syntax either before or after merging this.

@cknitt
Copy link
Member

cknitt commented May 26, 2024

@glennsl Could you add a CHANGELOG entry?

@IwanKaramazow Good to go?

@IwanKaramazow
Copy link

@cknitt I'll do one last pass over the next few days to see if there aren't any edge cases.

@cknitt
Copy link
Member

cknitt commented Jun 14, 2024

@cknitt I'll do one last pass over the next few days to see if there aren't any edge cases.

Ping @IwanKaramazow 🙂

@IwanKaramazow
Copy link

Ah, yes, will do a review this weekend!

@cknitt
Copy link
Member

cknitt commented Jun 27, 2024

@IwanKaramazow Ping again. 🙂
Would be great to get this merged!

@IwanKaramazow
Copy link

Yes, sorry, I have some time off next week. Will get to this.

@glennsl glennsl force-pushed the feat/syntax/regex-literals branch from 69a3237 to f4823dc Compare July 7, 2024 14:05
@glennsl glennsl force-pushed the feat/syntax/regex-literals branch from f4823dc to 1051f47 Compare July 7, 2024 14:18
@glennsl
Copy link
Contributor Author

glennsl commented Jul 7, 2024

Rebased and added changelog

@cknitt
Copy link
Member

cknitt commented Jul 12, 2024

@glennsl As @IwanKaramazow does not seem to be available for review: Good to go from your point of view?

Then I'll merge and we can always refine things in follow-up PRs if necessary.

@IwanKaramazow
Copy link

Yea, sorry about this. The work looks good in general 👍

@cknitt cknitt merged commit 18a727a into rescript-lang:master Jul 13, 2024
19 checks passed
@glennsl glennsl deleted the feat/syntax/regex-literals branch July 13, 2024 07:55
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

Successfully merging this pull request may close these issues.

Syntax for RegExp literals
4 participants