-
Notifications
You must be signed in to change notification settings - Fork 460
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 missing ltrim in regex parse #3495
Fix missing ltrim in regex parse #3495
Conversation
Needs cs fix |
what cs fix? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test does no fail even if I revert changes from src/
@ondrejmirtes as explained above, it cannot be currently tested |
I don't get it what's the purpose of this PR then. |
9210422
to
756ec63
Compare
756ec63
to
3eb2e46
Compare
3eb2e46
to
dd8e963
Compare
@ondrejmirtes test added |
Now we have a failing unit test but I still don't understand what kind of issue this PR fixes. |
I understand that it does not currently fix a functional usecase, but it fixes internal hidden bug. It fixes a method behaviour that is needed for future implementations of regex shape narrowing as the initial delimiter needs to be properly removed per PCRE grammar. |
Alright, I get it. |
Thank you. |
No description provided.