valid_email_address: wrong return type

patches are here ;)

should we note that it can also return FALSE when an error occurred? http://php.net/preg_match

also, i've added the comment to the 4.6 and 4.7 documentation.

the comment can be removed from the D6 documentation after it has been fixed ;)

Both of these patches look OK to me. The return values could have been wrapped in longer lines, but the format here does line them up nicely, so I won't complain.

Just as a note, this is a Drupal 6 and earlier issue only. If applied to Drupal 6, the version should be set to D5 so it can be applied to Drupal 5.

As noted, FALSE is still a possible return value. So the patch is not ready.

Hmmm...

http://us3.php.net/preg_match says that preg_match will return 1 if there is 1 match, 0 if there are 0 matches, and FALSE if there is an error.

One way to get an error would be if the regular expression has an error in it. But the regex doesn't have an error in it (we know it works and no way for the caller of valid_email_address to change it).

I suppose that probably if $mail is an array, object, or NULL, that might make an error? Should test...

Anyway, agreed: if we think it is possible to get an error output, we should add a note explaining what types of conditions on $mail would generate an error.

I ran some tests on my test box, running PHP 5.x

If you pass in a NULL value for the string in preg_match, you get a 0 return, just as you do if you pass in a string that doesn't match the regex.

If you pass in an array, you get a FALSE return (and a warning on the screen, if you have PHP/Apache set up to display PHP warnings).

I also tested the valid_email_address function itself, As currently written, it returns 0 if the input is an empty string.

So, revised doc patch is attached, which I think documents the actual behavior of the current function in D6.

+++ includes/common.inc	6 Oct 2009 15:44:54 -0000
@@ -939,14 +939,15 @@
+ *   1 if the email address is valid, 0 if it is invalid or empty, and FALSE if
+ *   there is an input error (such as passing in an array instead of a string).

no need to define what's an input error, as we've defined the type already in the @param

This review is powered by Dreditor.

@JoepH
The code is frozen already, we can't change it as it might break some stuff out there, also your code contains some bugs.

@alexanderpas: What wording would you suggest? I thought just saying "FALSE if there is an input error" was not enough, as it led me to wonder what would actually cause such an error.

bump. Someone just reported this again:
#769330: Documentation problem with valid_email_address

Can someone please review again and/or suggest better wording?

And again:
#821626: Documentation problem with valid_email_address in D5,6

RE #12: The patch without the mention of FALSE was rejected by the committer in #4 above. So it needs to have FALSE in there.

Patch in #6 looks good to me.

Looks like does not apply to Drupal 7 as it is implemented differently (http://api.drupal.org/api/drupal/includes--common.inc/function/valid_ema...). Committed to Drupal 6.