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

Fix capturing group numbering bug and a few more regex-related issues #1614

Merged
merged 1 commit into from
Aug 7, 2023
Merged

Fix capturing group numbering bug and a few more regex-related issues #1614

merged 1 commit into from
Aug 7, 2023

Conversation

adams85
Copy link
Contributor

@adams85 adams85 commented Aug 5, 2023

Counterpart of sebastienros/esprima-dotnet#392

Besides the issues mentioned in the linked PR, this one also fixes the fast path implementation of RegExpPrototype.Split, which is a bit broken.

Unfortunately, the remaining 2 test cases excluded here cannot be fixed with our current regex handling approach due to the differences/limitations of the .NET regex engine.

Oh, and one more thing. I'm not sure about this: "https://github.com/adams85/jint/blob/95029c6cbbfc09e81749e39916841296a2e52f4d/Jint/Runtime/Interop/DefaultObjectConverter.cs#L31-L34" I'm afraid that passing a non-adapted, external Regex instance to Jint may lead to some pretty confusing behavior.

@adams85
Copy link
Contributor Author

adams85 commented Aug 7, 2023

Just tested with the newly released Esprima version, fixes work as expected. So, as for me, PR is ready for merge.

@lahma lahma merged commit e9ace1a into sebastienros:main Aug 7, 2023
3 checks passed
@lahma
Copy link
Collaborator

lahma commented Aug 7, 2023

Looking good, thanks for the help 👍🏻

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.

2 participants