From 622bc9309155b05af1a0d85bfb643bf6280eba35 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Sun, 31 Mar 2013 18:40:40 -0700
Subject: [PATCH 1/3] send-email: use "return;" not "return undef;" on error
codepaths
All the callers of "ask", "extract_valid_address", and "validate_patch"
subroutines assign the return values from them to a single scalar:
$var = subr(...);
and "return undef;" in these subroutine can safely be turned into a
simpler "return;". Doing so will also future-proof a new caller that
mistakenly does this:
@foo = ask(...);
if (@foo) { ... we got an answer ... } else { ... we did not ... }
Note that we leave "return undef;" in validate_address on purpose,
even though Perlcritic may complain. The primary "return" site of
the function returns whatever is in the scalar variable $address, so
it is pointless to change only the other "return undef;" to "return".
The caller must be prepared to see an array with a single undef as
the return value from this subroutine anyway.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-send-email.perl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5b59..79cc5bee97 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -711,7 +711,7 @@ sub ask {
}
}
}
- return undef;
+ return;
}
my %broken_encoding;
@@ -833,7 +833,7 @@ sub extract_valid_address {
# less robust/correct than the monster regexp in Email::Valid,
# but still does a 99% job, and one less dependency
return $1 if $address =~ /($local_part_regexp\@$domain_regexp)/;
- return undef;
+ return;
}
sub extract_valid_address_or_die {
@@ -1484,7 +1484,7 @@ sub validate_patch {
return "$.: patch contains a line longer than 998 characters";
}
}
- return undef;
+ return;
}
sub file_has_nonascii {
From 9b39703920b2d64985abcc1348b169d8fa658c24 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Sun, 31 Mar 2013 18:40:41 -0700
Subject: [PATCH 2/3] send-email: drop misleading function prototype
The subroutine check_file_rev_conflict() is called from two places,
both of which expects to pass a single scalar variable and see if
that can be interpreted as a pathname or a revision name. It is
defined with a function prototype ($) to force a scalar context
while evaluating the arguments at the calling site but it does not
help the current calling sites. The only effect it has is to hurt
future calling sites that may want to build an argument list in an
array variable and call it as check_file_rev_confict(@args).
Drop the misleading prototype, as Perlcritic suggests.
While at it, rename the function to avoid new call sites unaware of
this change arising and add a comment clarifying what this function
is for.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-send-email.perl | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 79cc5bee97..fd8bfff3b2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -512,8 +512,9 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
($sender) = expand_aliases($sender) if defined $sender;
-# returns 1 if the conflict must be solved using it as a format-patch argument
-sub check_file_rev_conflict($) {
+# is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
+# $f is a revision list specification to be passed to format-patch.
+sub is_format_patch_arg {
return unless $repo;
my $f = shift;
try {
@@ -529,6 +530,7 @@ to produce patches for. Please disambiguate by...
* Giving --format-patch option if you mean a range.
EOF
} catch Git::Error::Command with {
+ # Not a valid revision. Treat it as a filename.
return 0;
}
}
@@ -540,14 +542,14 @@ while (defined(my $f = shift @ARGV)) {
if ($f eq "--") {
push @rev_list_opts, "--", @ARGV;
@ARGV = ();
- } elsif (-d $f and !check_file_rev_conflict($f)) {
+ } elsif (-d $f and !is_format_patch_arg($f)) {
opendir my $dh, $f
or die "Failed to opendir $f: $!";
push @files, grep { -f $_ } map { catfile($f, $_) }
sort readdir $dh;
closedir $dh;
- } elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) {
+ } elsif ((-f $f or -p $f) and !is_format_patch_arg($f)) {
push @files, $f;
} else {
push @rev_list_opts, $f;
From a47eab03f613fa55b9e690d5354e95bc165dceee Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Sun, 31 Mar 2013 18:40:42 -0700
Subject: [PATCH 3/3] send-email: use the three-arg form of open in
recipients_cmd
Perlcritic does not want to see the trailing pipe in the two-args
form of open(), i.e.
open my $fh, "$cmd \Q$file\E |";
If $cmd were a single-token command name, it would make a lot more
sense to use four-or-more-args form "open FILEHANDLE,MODE,CMD,ARGS"
to avoid shell from expanding metacharacters in $file, but we do
expect multi-word string in $to_cmd and $cc_cmd to be expanded by
the shell, so we cannot rewrite it to
open my $fh, "-|", $cmd, $file;
for extra safety. At least, by using this in the three-arg form:
open my $fh, "-|", "$cmd \Q$file\E";
we can silence Perlcritic, even though we do not gain much safety by
doing so.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-send-email.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index fd8bfff3b2..70cad15ec4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1440,7 +1440,7 @@ sub recipients_cmd {
my $sanitized_sender = sanitize_address($sender);
my @addresses = ();
- open my $fh, "$cmd \Q$file\E |"
+ open my $fh, "-|", "$cmd \Q$file\E"
or die "($prefix) Could not execute '$cmd'";
while (my $address = <$fh>) {
$address =~ s/^\s*//g;