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

normalizeEmail improvements #583

Merged
merged 5 commits into from
Sep 27, 2016
Merged

normalizeEmail improvements #583

merged 5 commits into from
Sep 27, 2016

Conversation

ItalyPaleAle
Copy link
Contributor

This patch contains multiple improvements to the normalizeEmail function. In particular:

  • Includes support for normalizing other common email providers beside GMail: iCloud (incl. MobileMe), Outlook.com (including Windows Live and Hotmail) and Yahoo Mail. For example, allows stripping sub-addresses ("[email protected]" -> "[email protected]"), transformation of the local part to lowercase (as they're known for being case insensitive), etc
  • Offers more granular options, for example letting users disable transformations that were before always applied

Previous options passed to "normalizeEmail" have been removed:

  • "lowercase", which was only considered for non-GMail addresses, has been replaced with "all_lowercase" (applies to all addresses when "true") and "*_lowercase" (for each provider), offering more granular control
  • "remove_dots", which before applied only to GMail, was renamed to "gmail_remove_dots" so it's clearer it's limited to GMail (no other provider does to same, to my knowledge)
  • "remove_extension" which before applied only to GMail was replaced with "*_remove_subaddress" for all providers that support it (GMail, iCloud and Outlook.com use the + symbol; Yahoo uses -). The word "subaddress" is also more appropriate to describe them - that's how they're called, for example, on Wikipedia
  • A new option has been added to enable/disable the @googlemail.com to @gmail.com conversion

When running without options, the behaviour of this function is almost identical to before (except that now subaddresses are removed for more providers and not just GMail). However, I was not able to make this patch fully backwards-compatible, meaning that I was not able to ensure that the same results would come out with supporting the same legacy switches (I tried!). Because of that, "legacy" switches (such as "remove_dots") have simply been removed, and this patch is partly backwards-incompatible. However, I do strongly feel that it's a necessary step in the right direction, to allow adding more providers in the future in a coherent way.

PS: It's been a while since I submitted you PR's... Nice to be back :)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 98.882% when pulling c047210 on EgoAleSum:email-patch into 3266767 on chriso:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 98.882% when pulling 309a530 on EgoAleSum:email-patch into 3266767 on chriso:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 99.14% when pulling e12cd97 on EgoAleSum:email-patch into 3266767 on chriso:master.

@chriso
Copy link
Collaborator

chriso commented Sep 25, 2016

Thanks, looks good! I won't merge this one just yet; I'll push it through as v6 with some other breaking changes.

@ItalyPaleAle
Copy link
Contributor Author

Thanks, what is the timeframe for v6?

@chriso
Copy link
Collaborator

chriso commented Sep 26, 2016

@EgoAleSum hoping to have it out this week.

@chriso chriso merged commit 9c2a506 into validatorjs:master Sep 27, 2016
chriso added a commit that referenced this pull request Sep 27, 2016
builtinnya added a commit to builtinnya/DefinitelyTyped that referenced this pull request Feb 20, 2018
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.

4 participants