Re: [PATCH] Cleanup git-send-email.perl:extract_valid_email

From: Eric Wong <normalperson@yhbt.net>
Date: Jun 3, 2006, 22:49 UTC
Message-ID: <20060603224935.GA10324@hand.yhbt.net>
In-Reply-To: <11493547080-git-send-email-vonbrand@inf.utfsm.cl>

"Horst H. von Brand" <vonbrand@inf.utfsm.cl> wrote:

Show 18 quoted lines

> - Fix the regular expressions for local addresses
> - Fix the fallback regexp for non-local addresses, simplify the logic
> 
> Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
> ---
>  git-send-email.perl |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index ed1d89b..a7a7797 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -314,18 +314,15 @@ sub extract_valid_address {
>  	my $address = shift;
>  
>  	# check for a local address:
> -	return $address if ($address =~ /^([\w\-]+)$/);
> +	return $address if ($address =~ /^([\w\-.]+)$/);

I keep forgetting this, '+' is a valid (and useful) setup, too.

Show 13 quoted lines

>  
>  	if ($have_email_valid) {
>  		return Email::Valid->address($address);
>  	} else {
>  		# less robust/correct than the monster regexp in Email::Valid,
>  		# but still does a 99% job, and one less dependency
> -		my $cleaned_address;
> -		if ($address =~ /([^\"<>\s]+@[^<>\s]+)/) {
> -			$cleaned_address = $1;
> -		}
> -		return $cleaned_address;
> +		$address =~ /([\w\-.]+@[\w\-.]+)/;
> +		return $1;

Actually, I'm retracting my earlier ack on this. This is way too restrictive. I'd rather allow an occasional invalid email address than to reject valid ones. I generally trust git users to know what they're doing when entering email addresses[1].

*, $, ^, +, = are all valid characters in the username portion (not sure about local accounts, though), and I'm sure there are more that I don't know about.

[1] - of course, without address book support in git-send-email, I think I would've left the 'k' out of Junio's email address by now. Of course there's also the issue of where I work and having several people using variations of rob/robert in their email address. I'm likely to make errors like these far more than entering addresses that email systems consider invalid, and I suspect I'm not the only one.

-- 
Eric Wong