views:

79

answers:

4

So, I have this JavaScript function:

ME.Utils = {
    RxEmail: new RegExp(/^(("[\w-\s]+")|([\w-]+(?:\.[\w-]+)*)|("[\w-\s]+")([\w-]+(?:\.[\w-]+)*))(@((?:[\w-]+\.)*\w[\w-]{0,66})\.([a-z]{2,6}(?:\.[a-z]{2})?)$)|(@\[?((25[0-5]\.|2[0-4][0-9]\.|1[0-9]{2}\.|[0-9]{1,2}\.))((25[0-5]|2[0-4][0-9]|1[0-9]{2}|[0-9]{1,2})\.){2}(25[0-5]|2[0-4][0-9]|1[0-9]{2}|[0-9]{1,2})\]?$)/i),

    ValidateEmail: function(email) {
        return ME.Utils.RxEmail.test(email);
    },

    GetEmailAddresses: function(text) {
        return text.match(ME.Utils.RxEmail);
    },

    HasEmail: function(text) {
        return ME.Utils.GetEmailAddresses != null;
    }
};

ValidateEmail works very well. However, HasEmail and GetEmailAddresses is not working properly.

GetEmailAdresses always returns null, except for when the string only contains an email address. In this case, GetEmailAdresses returns an array not only containing the email address, but the email address ([email protected]), just the id (test) plus some unidentified etc. etc...

Can you help me figure out what's wrong in my expression?

A: 
if (emailMatch = ME.Utils.GetEmailAddresses(myEmail))
  // do stuff with emailMatch[1]
Delan Azabani
not very helpful. I need to get an array with all email addresses in the string, like ["[email protected]", "[email protected]", [email protected]"]. Right now GetEmailAddresses returns an array full of junk.
Mickel
+1  A: 

The reason you're getting multiple entries in the array when presenting it with an email address is that you have capture groups in your expression. In Javascript, the result of match is an array where index 0 is the total string matched, and then there are (optionally) additional indexes for each capture group. You can make your groups non-capturing by changing them from (...) to (?:...).

As for why you're not getting the results you expect when using GetEmailAddresses with a string containing an email address, try this instead:

GetEmailAddresses: function(text) {
    var rv = [];
    var match;

    while (match = ME.Utils.RxEmail.exec(text)) {
        rv.push(match[0]);
    }
    return rv.length == 0 ? null : rv;
},

See this question and answer, I can't say I know why String#match isn't quite the same as the RegExp#exec loop above, but it isn't.

Edit And you'll need to fix the issue that oedo pointed out as well; the RegExp needs to be allowed to match substrings.

T.J. Crowder
+1  A: 

your regexp specifically matches an entire string

RxEmail: new RegExp(/^ ... $)/i),

the ^ and $ match the start and end of the input respectively. try removing those characters and see how you get on?

oedo
+1 Doh! Missed that part of it.
T.J. Crowder
thanks. apart from that your answer is more complete so +1 for you too buddy :)
oedo
+2  A: 

There are a few problems.

  1. Your regex is anchored at the start and end of the string. You should remove the ^ and $ characters from it.

  2. If you want to return only the email addresses, use non-capturing groups.

  3. In HasEmail(), you're not calling GetEmailAddresses(). You're actually checking if the value of that property is defined.

All in all, a fixed version might look like:

ME.Utils = {
    RxEmail: /(?:(?:"[\w-\s]+")|(?:[\w-]+(?:\.[\w-]+)*)|(?:"[\w-\s]+")(?:[\w-]+(?:\.[\w-]+)*))(?:@(?:(?:[\w-]+\.)*\w[\w-]{0,66})\.(?:[a-z]{2,6}(?:?:\.[a-z]{2})?))|(?:@\[?(?:(?:25[0-5]\.|2[0-4][0-9]\.|1[0-9]{2}\.|[0-9]{1,2}\.))(?:(?:25[0-5]|2[0-4][0-9]|1[0-9]{2}|[0-9]{1,2})\.){2}(?:25[0-5]|2[0-4][0-9]|1[0-9]{2}|[0-9]{1,2})\]?)/gi,

    ValidateEmail: function(email) {
        // We can't do a simple test() since we're using an unanchored regex now.
        var match = ME.Utils.RxEmail.match(email);
        return match.length == 1 && match[0] == email;
    },

    GetEmailAddresses: function(text) {
        return text.match(ME.Utils.RxEmail);
    },

    HasEmail: function(text) {
        return ME.Utils.GetEmailAddresses(text) != null;
    }
};
Max Shawabkeh