Re: [PATCHv4 4/6] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
- From
Jeff King <peff@peff.net>
- Date
- Feb 12, 2013, 21:17 UTC
- Message-ID
- <20130212211759.GA30329@sigill.intra.peff.net>
- In-Reply-To
- <7va9r9fd4e.fsf@alter.siamese.dyndns.org>
On Tue, Feb 12, 2013 at 01:14:57PM -0800, Junio C Hamano wrote:
Show 24 quoted lines
> Jeff King <peff@peff.net> writes:
>
> > On Tue, Feb 12, 2013 at 03:02:31PM +0100, Michal Nazarewicz wrote:
> >
> >> sub command_close_bidi_pipe {
> >> local $?;
> >> my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
> >> - _cmd_close($ctx, $in, $out);
> >> + _cmd_close($ctx, grep defined, $in, $out);
> >
> > Maybe it is just me, but I find the "grep EXPR" form a little subtle
> > inside an argument list. Either:
> >
> > _cmd_close($ctx, grep { defined } $in, $out);
> >
> > or
> >
> > _cmd_close($ctx, grep(defined, $in, $out));
> >
> > is a little more obvious to me.
>
> I would actually vote for the most explicit:
>
> _cmd_close($ctx, (grep { defined } ($in, $out)));Gross. My perl spider-sense tingles at seeing that many optional punctuation characters, but it should at least be obvious to a casual or new perl programmer what is going on. I'm fine with it.
-Peff