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

Replace email regex with EmailValidator gem dependency #255

Closed
croaky opened this issue Feb 13, 2013 · 5 comments
Closed

Replace email regex with EmailValidator gem dependency #255

croaky opened this issue Feb 13, 2013 · 5 comments

Comments

@croaky
Copy link
Contributor

croaky commented Feb 13, 2013

I dislike the long, unreadable regular expression we use to validate the user's email. There are some nice EmailValidator gems out in the community. I did some research and I think this one is the nicest-written one:

https://rubygems.org/gems/validate_as_email

It uses the Mail gem as a dependency, which I like. I'd wager a guess that it's the most popular way to send email so I think we can feel good about depending on the Mail gem's parsing abilities.

One thing I'm not sure about is whether it's actually validating just the email part of the RFC spec or whether the Mail gem allows things like "Dan Croak dan@thoughtbot.com". I'm also not sure whether that's a bad thing. Need to play around a bit in a test app.

Would love some opinions from others.

@joshuaclayton
Copy link
Contributor

Dan,

This looks pretty nice. Initial concerns were that it calls a private method to interact with Treetop (not horribly concerned, just hoping that the mail gem follows SemVer since his req in the gemspec is ~> 2) and that it hasn't been updated in more than six months (which is also somewhat bogus because it may be working just fine - there are no PRs or issues).

Out of curiosity, what other gems were you evaluating and what made them sub-par?

@jferris
Copy link
Contributor

jferris commented Feb 13, 2013

The validate_as_email approach does seem brittle. It calls a private method and then digs around internal structures to figure out if the address is valid. It also has a very loose version requirement. I'm pretty sure that will allow ANY version of mail, so the risk that you install a version of mail that doesn't work with validate_as_email seems high.

I wonder if there's a better way to interact with Mail::Address; I couldn't even find that class in the mail source.

@croaky
Copy link
Contributor Author

croaky commented Feb 15, 2013

Sounds like Treetop and using a maybe-not-public interface from Mail gem makes validates_as_email not as clean as I was thinking.

My second runner-up in my research was https://github.com/balexand/email_validator. It uses it a regex but has decent specs and is exactly the name I would expect this gem and related class (EmailValidator) to be.

@jferris
Copy link
Contributor

jferris commented Feb 15, 2013

email_validator looks good to me.

@croaky
Copy link
Contributor Author

croaky commented Feb 24, 2013

Opened PR #266 for this.

@croaky croaky closed this as completed Feb 24, 2013
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

No branches or pull requests

3 participants