Re: [PATCH v2 2/3] transport.c: introduce core.alternateRefsCommand

[Taylor Blau <me@ttaylorr.com>]

On Wed, Sep 26, 2018 at 02:38:53PM -0400, Jeff King wrote:
> On Wed, Sep 26, 2018 at 06:39:56AM -0700, Taylor Blau wrote:
>
> > > A perl tangent if you're interested:
> > [...]
> >
> > To be clear, we ought to leave this function as:
> >
> >   extract_haves () {
> >     depacketize | perl -lne '/^(\S+) \.have/ and print $1'
> >   }
>
> Yes, I agree. You cannot do the "$@" there because it relies on
> depacketize, which only handles stdin.
>
> > Or are you suggesting that we change it to:
> >
> >   extract_haves () {
> >     perl -lne '/^(\S+) \.have/ and print $1'
> >   }
>
> No, sorry. I just used the ".have" snippet as filler text, but I see
> that muddied my meaning considerably. This really was just a tangent for
> the future. What you've written above is the best thing for this case.

I see, and I had assumed that you meant the later, not that including
" .have" was a good way to go forward. So I think that we're in
agreement here.

> > And call it as:
> >
> >   printf "0000" | git receive-pack fork >actual &&
> >   depacketize <actual >actual.packets
> >   extract_haves <actual.packets >actual.haves &&
> >
> > Frankly, (and I think that this is what you're getting at in your reply
> > above), I think that the former (e.g., calling 'depacketize()' in
> > 'extract_haves()') is cleaner. This approach leaves us with "actual" and
> > "actual.haves", and obviates the need for another intermediary,
> > "actual.packets".
>
> Yeah. I have no problem with the three-liner you wrote above, but I do
> not see any particular reason for it.

Good. That's the version that I'll send shortly, then.

Thanks,
Taylor