Merge branch 'ab/send-email-perl'

* ab/send-email-perl:
  send-email: extract_valid_address use qr// regexes
  send-email: is_rfc2047_quoted use qr// regexes
  send-email: use Perl idioms in while loop
  send-email: make_message_id use "require" instead of "use"
  send-email: send_message die on $!, not $?
  send-email: use (?:) instead of () if no match variables are needed
  send-email: sanitize_address use qq["foo"], not "\"foo\""
  send-email: sanitize_address use $foo, not "$foo"
  send-email: use \E***\Q instead of \*\*\*
  send-email: cleanup_compose_files doesn't need a prototype
  send-email: unique_email_list doesn't need a prototype
  send-email: file_declares_8bit_cte doesn't need a prototype
  send-email: get_patch_subject doesn't need a prototype
  send-email: use lexical filehandles during sending
  send-email: use lexical filehandles for $compose
  send-email: use lexical filehandle for opendir

Conflicts:
	git-send-email.perl
This commit is contained in:
Junio C Hamano 2010-10-26 22:02:52 -07:00
commit 7ebee44167
2 changed files with 36 additions and 40 deletions

View File

@ -139,9 +139,6 @@ my $have_mail_address = eval { require Mail::Address; 1 };
my $smtp; my $smtp;
my $auth; my $auth;
sub unique_email_list(@);
sub cleanup_compose_files();
# Variables we fill in automatically, or via prompting: # Variables we fill in automatically, or via prompting:
my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh, my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,
$initial_reply_to,$initial_subject,@files, $initial_reply_to,$initial_subject,@files,
@ -377,7 +374,7 @@ my(%suppress_cc);
if (@suppress_cc) { if (@suppress_cc) {
foreach my $entry (@suppress_cc) { foreach my $entry (@suppress_cc) {
die "Unknown --suppress-cc field: '$entry'\n" die "Unknown --suppress-cc field: '$entry'\n"
unless $entry =~ /^(all|cccmd|cc|author|self|sob|body|bodycc)$/; unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;
$suppress_cc{$entry} = 1; $suppress_cc{$entry} = 1;
} }
} }
@ -521,12 +518,12 @@ while (defined(my $f = shift @ARGV)) {
push @rev_list_opts, "--", @ARGV; push @rev_list_opts, "--", @ARGV;
@ARGV = (); @ARGV = ();
} elsif (-d $f and !check_file_rev_conflict($f)) { } elsif (-d $f and !check_file_rev_conflict($f)) {
opendir(DH,$f) opendir my $dh, $f
or die "Failed to opendir $f: $!"; or die "Failed to opendir $f: $!";
push @files, grep { -f $_ } map { catfile($f, $_) } push @files, grep { -f $_ } map { catfile($f, $_) }
sort readdir(DH); sort readdir $dh;
closedir(DH); closedir $dh;
} elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) { } elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) {
push @files, $f; push @files, $f;
} else { } else {
@ -558,7 +555,7 @@ if (@files) {
usage(); usage();
} }
sub get_patch_subject($) { sub get_patch_subject {
my $fn = shift; my $fn = shift;
open (my $fh, '<', $fn); open (my $fh, '<', $fn);
while (my $line = <$fh>) { while (my $line = <$fh>) {
@ -576,7 +573,7 @@ if ($compose) {
$compose_filename = ($repo ? $compose_filename = ($repo ?
tempfile(".gitsendemail.msg.XXXXXX", DIR => $repo->repo_path()) : tempfile(".gitsendemail.msg.XXXXXX", DIR => $repo->repo_path()) :
tempfile(".gitsendemail.msg.XXXXXX", DIR => "."))[1]; tempfile(".gitsendemail.msg.XXXXXX", DIR => "."))[1];
open(C,">",$compose_filename) open my $c, ">", $compose_filename
or die "Failed to open for writing $compose_filename: $!"; or die "Failed to open for writing $compose_filename: $!";
@ -584,7 +581,7 @@ if ($compose) {
my $tpl_subject = $initial_subject || ''; my $tpl_subject = $initial_subject || '';
my $tpl_reply_to = $initial_reply_to || ''; my $tpl_reply_to = $initial_reply_to || '';
print C <<EOT; print $c <<EOT;
From $tpl_sender # This line is ignored. From $tpl_sender # This line is ignored.
GIT: Lines beginning in "GIT:" will be removed. GIT: Lines beginning in "GIT:" will be removed.
GIT: Consider including an overall diffstat or table of contents GIT: Consider including an overall diffstat or table of contents
@ -597,9 +594,9 @@ In-Reply-To: $tpl_reply_to
EOT EOT
for my $f (@files) { for my $f (@files) {
print C get_patch_subject($f); print $c get_patch_subject($f);
} }
close(C); close $c;
if ($annotate) { if ($annotate) {
do_edit($compose_filename, @files); do_edit($compose_filename, @files);
@ -607,23 +604,23 @@ EOT
do_edit($compose_filename); do_edit($compose_filename);
} }
open(C2,">",$compose_filename . ".final") open my $c2, ">", $compose_filename . ".final"
or die "Failed to open $compose_filename.final : " . $!; or die "Failed to open $compose_filename.final : " . $!;
open(C,"<",$compose_filename) open $c, "<", $compose_filename
or die "Failed to open $compose_filename : " . $!; or die "Failed to open $compose_filename : " . $!;
my $need_8bit_cte = file_has_nonascii($compose_filename); my $need_8bit_cte = file_has_nonascii($compose_filename);
my $in_body = 0; my $in_body = 0;
my $summary_empty = 1; my $summary_empty = 1;
while(<C>) { while(<$c>) {
next if m/^GIT:/; next if m/^GIT:/;
if ($in_body) { if ($in_body) {
$summary_empty = 0 unless (/^\n$/); $summary_empty = 0 unless (/^\n$/);
} elsif (/^\n$/) { } elsif (/^\n$/) {
$in_body = 1; $in_body = 1;
if ($need_8bit_cte) { if ($need_8bit_cte) {
print C2 "MIME-Version: 1.0\n", print $c2 "MIME-Version: 1.0\n",
"Content-Type: text/plain; ", "Content-Type: text/plain; ",
"charset=UTF-8\n", "charset=UTF-8\n",
"Content-Transfer-Encoding: 8bit\n"; "Content-Transfer-Encoding: 8bit\n";
@ -648,10 +645,10 @@ EOT
print "To/Cc/Bcc fields are not interpreted yet, they have been ignored\n"; print "To/Cc/Bcc fields are not interpreted yet, they have been ignored\n";
next; next;
} }
print C2 $_; print $c2 $_;
} }
close(C); close $c;
close(C2); close $c2;
if ($summary_empty) { if ($summary_empty) {
print "Summary email is empty, skipping it\n"; print "Summary email is empty, skipping it\n";
@ -688,7 +685,7 @@ sub ask {
my %broken_encoding; my %broken_encoding;
sub file_declares_8bit_cte($) { sub file_declares_8bit_cte {
my $fn = shift; my $fn = shift;
open (my $fh, '<', $fn); open (my $fh, '<', $fn);
while (my $line = <$fh>) { while (my $line = <$fh>) {
@ -717,7 +714,7 @@ if (!defined $auto_8bit_encoding && scalar %broken_encoding) {
if (!$force) { if (!$force) {
for my $f (@files) { for my $f (@files) {
if (get_patch_subject($f) =~ /\*\*\* SUBJECT HERE \*\*\*/) { if (get_patch_subject($f) =~ /\Q*** SUBJECT HERE ***\E/) {
die "Refusing to send because the patch\n\t$f\n" die "Refusing to send because the patch\n\t$f\n"
. "has the template subject '*** SUBJECT HERE ***'. " . "has the template subject '*** SUBJECT HERE ***'. "
. "Pass --force if you really want to send.\n"; . "Pass --force if you really want to send.\n";
@ -789,8 +786,8 @@ our ($message_id, %mail, $subject, $reply_to, $references, $message,
sub extract_valid_address { sub extract_valid_address {
my $address = shift; my $address = shift;
my $local_part_regexp = '[^<>"\s@]+'; my $local_part_regexp = qr/[^<>"\s@]+/;
my $domain_regexp = '[^.<>"\s@]+(?:\.[^.<>"\s@]+)+'; my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
# check for a local address: # check for a local address:
return $address if ($address =~ /^($local_part_regexp)$/); return $address if ($address =~ /^($local_part_regexp)$/);
@ -831,7 +828,7 @@ sub make_message_id {
last if (defined $du_part and $du_part ne ''); last if (defined $du_part and $du_part ne '');
} }
if (not defined $du_part or $du_part eq '') { if (not defined $du_part or $du_part eq '') {
use Sys::Hostname qw(); require Sys::Hostname;
$du_part = 'user@' . Sys::Hostname::hostname(); $du_part = 'user@' . Sys::Hostname::hostname();
} }
my $message_id_template = "<%s-git-send-email-%s>"; my $message_id_template = "<%s-git-send-email-%s>";
@ -864,8 +861,8 @@ sub quote_rfc2047 {
sub is_rfc2047_quoted { sub is_rfc2047_quoted {
my $s = shift; my $s = shift;
my $token = '[^][()<>@,;:"\/?.= \000-\037\177-\377]+'; my $token = qr/[^][()<>@,;:"\/?.= \000-\037\177-\377]+/;
my $encoded_text = '[!->@-~]+'; my $encoded_text = qr/[!->@-~]+/;
length($s) <= 75 && length($s) <= 75 &&
$s =~ m/^(?:"[[:ascii:]]*"|=\?$token\?$token\?$encoded_text\?=)$/o; $s =~ m/^(?:"[[:ascii:]]*"|=\?$token\?$token\?$encoded_text\?=)$/o;
} }
@ -876,7 +873,7 @@ sub sanitize_address {
my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/); my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
if (not $recipient_name) { if (not $recipient_name) {
return "$recipient"; return $recipient;
} }
# if recipient_name is already quoted, do nothing # if recipient_name is already quoted, do nothing
@ -893,7 +890,7 @@ sub sanitize_address {
# double quotes are needed if specials or CTLs are included # double quotes are needed if specials or CTLs are included
elsif ($recipient_name =~ /[][()<>@,;:\\".\000-\037\177]/) { elsif ($recipient_name =~ /[][()<>@,;:\\".\000-\037\177]/) {
$recipient_name =~ s/(["\\\r])/\\$1/g; $recipient_name =~ s/(["\\\r])/\\$1/g;
$recipient_name = "\"$recipient_name\""; $recipient_name = qq["$recipient_name"];
} }
return "$recipient_name $recipient_addr"; return "$recipient_name $recipient_addr";
@ -1049,7 +1046,7 @@ X-Mailer: git-send-email $gitversion
exec($smtp_server, @sendmail_parameters) or die $!; exec($smtp_server, @sendmail_parameters) or die $!;
} }
print $sm "$header\n$message"; print $sm "$header\n$message";
close $sm or die $?; close $sm or die $!;
} else { } else {
if (!defined $smtp_server) { if (!defined $smtp_server) {
@ -1155,7 +1152,7 @@ $subject = $initial_subject;
$message_num = 0; $message_num = 0;
foreach my $t (@files) { foreach my $t (@files) {
open(F,"<",$t) or die "can't open file $t"; open my $fh, "<", $t or die "can't open file $t";
my $author = undef; my $author = undef;
my $author_encoding; my $author_encoding;
@ -1169,7 +1166,7 @@ foreach my $t (@files) {
$message = ""; $message = "";
$message_num++; $message_num++;
# First unfold multiline header fields # First unfold multiline header fields
while(<F>) { while(<$fh>) {
last if /^\s*$/; last if /^\s*$/;
if (/^\s+\S/ and @header) { if (/^\s+\S/ and @header) {
chomp($header[$#header]); chomp($header[$#header]);
@ -1252,7 +1249,7 @@ foreach my $t (@files) {
} }
} }
# Now parse the message body # Now parse the message body
while(<F>) { while(<$fh>) {
$message .= $_; $message .= $_;
if (/^(Signed-off-by|Cc): (.*)$/i) { if (/^(Signed-off-by|Cc): (.*)$/i) {
chomp; chomp;
@ -1269,7 +1266,7 @@ foreach my $t (@files) {
$c, $_) unless $quiet; $c, $_) unless $quiet;
} }
} }
close F; close $fh;
push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t) push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
if defined $to_cmd; if defined $to_cmd;
@ -1340,10 +1337,9 @@ sub recipients_cmd {
my $sanitized_sender = sanitize_address($sender); my $sanitized_sender = sanitize_address($sender);
my @addresses = (); my @addresses = ();
open(F, "$cmd \Q$file\E |") open my $fh, "$cmd \Q$file\E |"
or die "($prefix) Could not execute '$cmd'"; or die "($prefix) Could not execute '$cmd'";
while(<F>) { while (my $address = <$fh>) {
my $address = $_;
$address =~ s/^\s*//g; $address =~ s/^\s*//g;
$address =~ s/\s*$//g; $address =~ s/\s*$//g;
$address = sanitize_address($address); $address = sanitize_address($address);
@ -1352,20 +1348,20 @@ sub recipients_cmd {
printf("($prefix) Adding %s: %s from: '%s'\n", printf("($prefix) Adding %s: %s from: '%s'\n",
$what, $address, $cmd) unless $quiet; $what, $address, $cmd) unless $quiet;
} }
close F close $fh
or die "($prefix) failed to close pipe to '$cmd'"; or die "($prefix) failed to close pipe to '$cmd'";
return @addresses; return @addresses;
} }
cleanup_compose_files(); cleanup_compose_files();
sub cleanup_compose_files() { sub cleanup_compose_files {
unlink($compose_filename, $compose_filename . ".final") if $compose; unlink($compose_filename, $compose_filename . ".final") if $compose;
} }
$smtp->quit if $smtp; $smtp->quit if $smtp;
sub unique_email_list(@) { sub unique_email_list {
my %seen; my %seen;
my @emails; my @emails;

View File

@ -222,7 +222,7 @@ test_expect_success $PREREQ 'tocmd works' '
test_expect_success $PREREQ 'cccmd works' ' test_expect_success $PREREQ 'cccmd works' '
clean_fake_sendmail && clean_fake_sendmail &&
cp $patches cccmd.patch && cp $patches cccmd.patch &&
echo cccmd--cccmd@example.com >>cccmd.patch && echo "cccmd-- cccmd@example.com" >>cccmd.patch &&
{ {
echo "#!$SHELL_PATH" echo "#!$SHELL_PATH"
echo sed -n -e s/^cccmd--//p \"\$1\" echo sed -n -e s/^cccmd--//p \"\$1\"