Minor tweaks / cleanup #370
Merged
Conversation
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
* Use re2 instead of re in more places * Simplify config * Simplify `isinstance` calls with tuples * Use comprehensions when reasonable
rodaine
approved these changes
Sep 12, 2025
timostamm
reviewed
Sep 12, 2025
|
|
||
| # See https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address | ||
| _email_regex = re.compile( | ||
| _email_regex = re2.compile( |
There was a problem hiding this comment.
This regular expression from the WHATWG group is "JavaScript- and Perl-compatible". Are we sure that switching to re2 doesn't change behavior?
There was a problem hiding this comment.
we're running the protovalidate conformance tests and they still passed (relevant test cases). I'm not sure if we view those as exhaustive or more of a basic test of functionality.
In general, re2 is meant to be a drop-in replacement for re, and we aren't any of the documented PCRE-incompatible bits.
If there is a change in behavior, I'd still rather opt us towards using re2 since CEL expects that behavior. Happy to note it in release notes, though, thoughts?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
isinstancecalls with tuples