Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Horst von Brand <hidden>
Date: 2016-06-15 22:42:28

Junio C Hamano [off-list ref] wrote:
Horst von Brand [off-list ref] writes:
quoted
quoted
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.
I think you meant to say:
quoted
        my $domain_regexp = '[^.<>"\s@]+(\.[^.<>"\s@]+)*';
(i.e. exclude dot from the latter character class),
Right, my bad.
                                                    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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help