Skip to content

Commit d21616c

Browse files
avargitster
authored andcommitted
git-send-email: refactor duplicate $? checks into a function
Refactor the duplicate checking of $? into a function. There's an outstanding series[1] wanting to add a third use of system() in this file, let's not copy this boilerplate anymore when that happens. 1. http://lore.kernel.org/git/[email protected] Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent e585210 commit d21616c

File tree

1 file changed

+28
-17
lines changed

1 file changed

+28
-17
lines changed

git-send-email.perl

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -212,22 +212,31 @@ sub format_2822_time {
212212
my $multiedit;
213213
my $editor;
214214

215+
sub system_or_msg {
216+
my ($args, $msg) = @_;
217+
system(@$args);
218+
my $signalled = $? & 127;
219+
my $exit_code = $? >> 8;
220+
return unless $signalled or $exit_code;
221+
222+
return sprintf(__("failed to run command %s, died with code %d"),
223+
"@$args", $exit_code);
224+
}
225+
226+
sub system_or_die {
227+
my $msg = system_or_msg(@_);
228+
die $msg if $msg;
229+
}
230+
215231
sub do_edit {
216232
if (!defined($editor)) {
217233
$editor = Git::command_oneline('var', 'GIT_EDITOR');
218234
}
235+
my $die_msg = __("the editor exited uncleanly, aborting everything");
219236
if (defined($multiedit) && !$multiedit) {
220-
for (@_) {
221-
system('sh', '-c', $editor.' "$@"', $editor, $_);
222-
if (($? & 127) || ($? >> 8)) {
223-
die(__("the editor exited uncleanly, aborting everything"));
224-
}
225-
}
237+
system_or_die(['sh', '-c', $editor.' "$@"', $editor, $_], $die_msg) for @_;
226238
} else {
227-
system('sh', '-c', $editor.' "$@"', $editor, @_);
228-
if (($? & 127) || ($? >> 8)) {
229-
die(__("the editor exited uncleanly, aborting everything"));
230-
}
239+
system_or_die(['sh', '-c', $editor.' "$@"', $editor, @_], $die_msg);
231240
}
232241
}
233242

@@ -698,9 +707,7 @@ sub is_format_patch_arg {
698707
if ($validate) {
699708
foreach my $f (@files) {
700709
unless (-p $f) {
701-
my $error = validate_patch($f, $target_xfer_encoding);
702-
$error and die sprintf(__("fatal: %s: %s\nwarning: no patches were sent\n"),
703-
$f, $error);
710+
validate_patch($f, $target_xfer_encoding);
704711
}
705712
}
706713
}
@@ -1952,11 +1959,13 @@ sub validate_patch {
19521959
chdir($repo->wc_path() or $repo->repo_path())
19531960
or die("chdir: $!");
19541961
local $ENV{"GIT_DIR"} = $repo->repo_path();
1955-
$hook_error = "rejected by sendemail-validate hook"
1956-
if system($validate_hook, $target);
1962+
$hook_error = system_or_msg([$validate_hook, $target]);
19571963
chdir($cwd_save) or die("chdir: $!");
19581964
}
1959-
return $hook_error if $hook_error;
1965+
if ($hook_error) {
1966+
die sprintf(__("fatal: %s: rejected by sendemail-validate hook\n" .
1967+
"warning: no patches were sent\n"), $fn);
1968+
}
19601969
}
19611970

19621971
# Any long lines will be automatically fixed if we use a suitable transfer
@@ -1966,7 +1975,9 @@ sub validate_patch {
19661975
or die sprintf(__("unable to open %s: %s\n"), $fn, $!);
19671976
while (my $line = <$fh>) {
19681977
if (length($line) > 998) {
1969-
return sprintf(__("%s: patch contains a line longer than 998 characters"), $.);
1978+
die sprintf(__("fatal: %s: %d: patch contains a line longer than 998 characters\n" .
1979+
"warning: no patches were sent\n"),
1980+
$fn, $.);
19701981
}
19711982
}
19721983
}

0 commit comments

Comments
 (0)