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

feat(Utils): Split string use non capturing group #129

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

bsian03
Copy link
Contributor

@bsian03 bsian03 commented Jan 26, 2021

PULL REQUEST

Overview

Changes the regex to use a non capturing group instead of a capturing group for better performance and prevents possible arbitrary EOL capturing

Status

  • Typings have been updated or don't need to be.
  • This PR have been tested and is ready to be merged.

Semantic versioning classification

  • MAJOR: This PR introduces BREAKING changes (direct API change).
  • MINOR: This PR adds new features, improve the code and/or implies minimal API changes.
  • PATCH: This PR fixes a bug, if needed it also references the relevant issue or documentation.
  • PATCH: This PR improve performance or code refactor without API changes.
  • PATCH: This PR only includes non-code changes (documentation, style, CI, tools...).

@Khaaz
Copy link
Owner

Khaaz commented Jan 26, 2021

Can you show the results for this regex and the old one so I can compare?

@bsian03
Copy link
Contributor Author

bsian03 commented Mar 8, 2021

With capturing group:
image

With non-capturing group:
image

Bear in mind that sometimes they might take the same amount of time but on average, non capturing seems to perform better. I can get a benchmark as well if it's good

(no i didn't forget about this)

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