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

Add smartcase to the regex engine. #5208

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jamiedsmith95
Copy link

Implementation of smart case for regex, using (?c) as a modifier flag.
I believe completes feature request: #4856

implement smart case using (?c), this activates ignore case until an
upper char is present (not escape or modifier etc..)
  I dedicate any and all copyright interest in this software to the
  public domain.  I make this dedication for the benefit of the public at
  large and to the detriment of my heirs and successors.  I intend this
  dedication to be an overt act of relinquishment in perpetuity of all
  present and future rights to this software under copyright law.
src/regex_impl.cc Outdated Show resolved Hide resolved
src/regex_impl.cc Outdated Show resolved Hide resolved
src/regex_impl.cc Outdated Show resolved Hide resolved
Copy link
Owner

@mawww mawww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments on the implementation, would be nice to add a few test cases and add this to the regex documentation.

src/regex_impl.cc Outdated Show resolved Hide resolved
@jamiedsmith95
Copy link
Author

A few comments on the implementation, would be nice to add a few test cases and add this to the regex documentation.

I have addressed your comments. I have also modified it to also make an uppercase char alter existing character classes (otherwise the behaviour of character classes would depend on whether they were before or after the uppercase). A couple of tests and a line in the modifier part of the docs. The only thing to emphasise is that character classes will depend on smart case but will not themselves trigger it. e.g. [A-Z] will not count as an uppercase.

Copy link
Owner

@mawww mawww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, could you move those tests to the unit tests at the end of regex_impl.cc ? I dont think there is much value in using the script based test framework for this.

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