feat(detectIdentifiers): re-introduce boundary check#332
Merged
davideicardi merged 1 commit intodynamicexpresso:masterfrom Nov 18, 2024
Merged
feat(detectIdentifiers): re-introduce boundary check#332davideicardi merged 1 commit intodynamicexpresso:masterfrom
davideicardi merged 1 commit intodynamicexpresso:masterfrom
Conversation
50b17df to
b0be9df
Compare
davideicardi
approved these changes
Nov 18, 2024
b0be9df to
3732d6f
Compare
This improves the regex evaluation back to when we still had `\b` in front, while (hopefully) doing as it should
3732d6f to
f731e2d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This improves the regex evaluation back to when we still had
\bin front, while (hopefully) doing as it should.For the test referenced in #331, this change drops us back to 90ms, which is still twice as slow as originally, but there's also more features supported now, so I find this acceptable.
@metoule Please check that this is doing what you meant. If I understood your comment about
\bright, you had to remove it because that also matched@, so even though@?is included in the identifier part it was never captured. If so, then this lookbehind should model the same as\bbut it respects@in front. Demo: https://regex101.com/r/WhA5GM/2Close #291