[PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close
From: Michal Nazarewicz min...@mina86.com
The body of the loop in command_close_bidi_pipe function is identical to
what _cmd_close function does so instead of duplicating, refactor change
_cmd_close so that it accepts list of file handlers to be closed, which
makes it usable with command_close_bidi_pipe.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
perl/Git.pm | 30 +++---
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..6bc9a3c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -267,13 +267,13 @@ sub command {
if (not defined wantarray) {
# Nothing to pepper the possible exception with.
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} elsif (not wantarray) {
local $/;
my $text = $fh;
try {
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -286,7 +286,7 @@ sub command {
my @lines = $fh;
defined and chomp for @lines;
try {
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
my $E = shift;
$E-{'-outputref'} = \@lines;
@@ -313,7 +313,7 @@ sub command_oneline {
my $line = $fh;
defined $line and chomp $line;
try {
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
} catch Git::Error::Command with {
# Pepper with the output:
my $E = shift;
@@ -381,7 +381,7 @@ have more complicated structure.
sub command_close_pipe {
my ($self, $fh, $ctx) = _maybe_self(@_);
$ctx ||= 'unknown';
- _cmd_close($fh, $ctx);
+ _cmd_close($ctx, $fh);
}
=item command_bidi_pipe ( COMMAND [, ARGUMENTS... ] )
@@ -431,18 +431,8 @@ have more complicated structure.
sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
- foreach my $fh ($in, $out) {
- unless (close $fh) {
- if ($!) {
- carp error closing pipe: $!;
- } elsif ($? 8) {
- throw Git::Error::Command($ctx, $? 8);
- }
- }
- }
-
+ _cmd_close($ctx, $in, $out);
waitpid $pid, 0;
-
if ($? 8) {
throw Git::Error::Command($ctx, $? 8);
}
@@ -1355,9 +1345,11 @@ sub _execv_git_cmd { exec('git', @_); }
# Close pipe to a subprocess.
sub _cmd_close {
- my ($fh, $ctx) = @_;
- if (not close $fh) {
- if ($!) {
+ my $ctx = shift @_;
+ foreach my $fh (@_) {
+ if (close $fh) {
+ # nop;
+ } elsif ($!) {
# It's just close, no point in fatalities
carp error closing pipe: $!;
} elsif ($? 8) {
--
1.8.1.3.572.g32bae1f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close
Michal Nazarewicz m...@google.com writes: From: Michal Nazarewicz min...@mina86.com The body of the loop in command_close_bidi_pipe function is identical to what _cmd_close function does so instead of duplicating, refactor change _cmd_close so that it accepts list of file handlers to be closed, which s/file handlers/file handles/, I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close
On Tue, Feb 12, 2013 at 10:55:05AM -0800, Junio C Hamano wrote: Michal Nazarewicz m...@google.com writes: From: Michal Nazarewicz min...@mina86.com The body of the loop in command_close_bidi_pipe function is identical to what _cmd_close function does so instead of duplicating, refactor change _cmd_close so that it accepts list of file handlers to be closed, which s/file handlers/file handles/, I think. And s/refactor change/refactor/. Other than that, I think the series looks OK. I have one style micro-nit on patch 4 which I'll reply in-line. But it is either fix while applying or ignore, I don't think it will be worth a re-roll. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close
Michal Nazarewicz m...@google.com writes: The body of the loop in command_close_bidi_pipe function is identical to what _cmd_close function does so instead of duplicating, refactor change _cmd_close so that it accepts list of file handlers to be closed, which On Tue, Feb 12, 2013 at 10:55:05AM -0800, Junio C Hamano wrote: s/file handlers/file handles/, I think. On Tue, Feb 12 2013, Jeff King wrote: And s/refactor change/refactor/. Other than that, I think the series looks OK. I have one style micro-nit on patch 4 which I'll reply in-line. But it is either fix while applying or ignore, I don't think it will be worth a re-roll. All fixed. Junio, do you want me to resend or would you be fine with just pulling: git://github.com/mina86/git.git master -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo-- pgpLiQYC6X4hj.pgp Description: PGP signature
Re: [PATCHv4 3/6] Git.pm: refactor command_close_bidi_pipe to use _cmd_close
Michal Nazarewicz min...@mina86.com writes: Michal Nazarewicz m...@google.com writes: The body of the loop in command_close_bidi_pipe function is identical to what _cmd_close function does so instead of duplicating, refactor change _cmd_close so that it accepts list of file handlers to be closed, which On Tue, Feb 12, 2013 at 10:55:05AM -0800, Junio C Hamano wrote: s/file handlers/file handles/, I think. On Tue, Feb 12 2013, Jeff King wrote: And s/refactor change/refactor/. Other than that, I think the series looks OK. I have one style micro-nit on patch 4 which I'll reply in-line. But it is either fix while applying or ignore, I don't think it will be worth a re-roll. All fixed. Junio, do you want me to resend or would you be fine with just pulling: git://github.com/mina86/git.git master Neither. I agree with Peff that these micronits are not enough reason for the trouble of rerolling the series, so I'll just amend them at my end. Please double-check what you see on the 'pu' branch when I push today's integration result out later. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
