Fix issues with emailRegexPattern. #1640
Merged
Conversation
This file contains 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
The suggested solution by Diego was to make the limit 10, but making it unbounded probably makes more sense. It lets people use custom TLDs in local delivery / email testing without having to worry about length.
| @@ -32,7 +32,7 @@ object ProtoRules extends Factory with LazyLoggable { | |||
| * The regular expression pattern for matching email addresses. This | |||
| * assumes that the email address has been converted to lower case. | |||
| */ | |||
| val emailRegexPattern = new FactoryMaker(Pattern.compile("^[a-z0-9._%\\-+]+@(?:[a-z0-9\\-]+\\.)+[a-z]{2,4}$")) {} | |||
| val emailRegexPattern = new FactoryMaker(Pattern.compile("(?i)^[a-z0-9._%\\-+]+@(?:[a-z0-9\\-]+\\.)+[a-z]{2,}$")) {} | |||
There was a problem hiding this comment.
I think it'd be clearer if we want the insensitivity to apply throughout to apply it as the second flags parameter to compile rather than in the regex.
There was a problem hiding this comment.
Also need to fix the comment that indicates the email should be lowercase.
This should make the case insensitivity of the pattern more visible to someone else who is looking at the code.
|
All comments addressed. |
|
👍 |
|
@fmpwizard Why'd you move this to the M3 milestone? It looks like it's ready to go yeah? |
|
Oh 3.0-M2 already shipped. lol |
farmdawgnation
added a commit
that referenced
this pull request
Nov 27, 2014
This resolve issues with emailRegexPattern not being up to date w.r.t. new TLDs longer than four characters and being case sensitive.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 resolve issues with
emailRegexPatternnot being up to date w.r.t. new TLDs longer than four characters. While I was in there I also made the matcher case insensitive.fixes #1634