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

Added test for unused capturing groups #1996

Merged

Conversation

RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Jul 23, 2019

This adds a test which checks for whether there are unused capturing groups in a pattern.
Edit: I added another test which ensures that lookbehind groups are always the first thing being matched in a pattern.

The new test does this by parsing every pattern using regexpp. This is a quite costly operation but it only doubles the test time from 0.5s to 1s one my machine so it's not even close to the 7s for the >1.3k language tests.

@Golmote once went on a killing spree to eliminate every last unused capturing group and this test makes sure that no new ones get added.

@mAAdhaTTah
Copy link
Member

Would we be able to use the new dependency loader for testing the peer dependencies?

@RunDevelopment
Copy link
Member Author

What do you mean by new dependency loader? Do you mean #1998?
If so, then yes of course! #1998 is able to do everything and more what our current logic is able to do.

@mAAdhaTTah
Copy link
Member

Yes, #1998, which I want to land because I'm getting bugs on the babel plugin that I want to verify is fixed by this.

@RunDevelopment RunDevelopment merged commit c187e22 into PrismJS:master Oct 16, 2019
@RunDevelopment RunDevelopment deleted the test-unused-capturing-groups branch October 16, 2019 10:06
cedporter pushed a commit to cedporter/prism that referenced this pull request Oct 17, 2019
This adds a test that checks for unused capturing groups in patterns and another test which ensures that lookbehind groups are always the first thing being matched in a pattern.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants