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

2 files changed, 36 insertions, 40 deletions

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