Re: [PATCH] Cleanup git-send-email.perl:extract_valid_email
- From
Horst von Brand <vonbrand@inf.utfsm.cl>
- Date
- Jun 6, 2006, 16:05 UTC
- Message-ID
- <200606061605.k56G5gHo006581@laptop11.inf.utfsm.cl>
- In-Reply-To
- <junkio@cox.net>
Junio C Hamano <junkio@cox.net> wrote:
Show 24 quoted lines
> Horst von Brand <vonbrand@inf.utfsm.cl> writes:
>
> >> diff --git a/git-send-email.perl b/git-send-email.perl
> >> index a7a7797..700d0c3 100755
> >> --- a/git-send-email.perl
> >> +++ b/git-send-email.perl
> >> @@ -312,16 +312,18 @@ our ($message_id, $cc, %mail, $subject,
> >>
> >> sub extract_valid_address {
> >> my $address = shift;
> >> + my $local_part_regexp = '[^<>"\s@]+';
> >> + my $domain_regexp = '[^.<>"\s@]+\.[^<>"\s@]+';
> >
> > This forces a '.' in the domain, while vonbrand@localhost is perfectly
> > reasonable. Plus it doesn't disallow adyacent '.'s. What about:
> >
> > my $domain_regexp = '[^.<>"\s@]+(\.[^<>"\s@]+)*';
> >
> > (but this is probably nitpicking...)
>
> I do not have preference either way about allowing an address
> like tld-administrator@net myself, but Email::Valid->address
> does not seem to allow it, and I just copied that behaviour for
> consistency between two alternative implementations.Reasonable.
Show 5 quoted lines
> I think you meant to say: > > > my $domain_regexp = '[^.<>"\s@]+(\.[^.<>"\s@]+)*'; > > (i.e. exclude dot from the latter character class),
Right, my bad.
Show 6 quoted lines
> but I am > inclined to do this instead: > > my $domain_regexp = '[^.<>"\s@]+(?:\.[^.<>"\s@]+)+'; > > (i.e. still require at least two levels).
OK, but be careful as this (?:...) is an extended regexp (needs /x on match). I'd just leave it plain (the performance impact shouldn't be noticeable). I don't see any use except for $1, so the extra parenthesis should be safe.
-- Dr. Horst H. von Brand User #22616 counter.li.org Departamento de Informatica Fono: +56 32 654431 Universidad Tecnica Federico Santa Maria +56 32 654239 Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513