Re: [PATCH 2/4] http-fetch: allow custom index-pack args

From: Jonathan Tan <jonathantanmy@google.com>
Date: Feb 17, 2021, 19:46 UTC
Message-ID: <20210217194604.871194-1-jonathantanmy@google.com>
In-Reply-To: <xmqqpn0zoc2l.fsf@gitster.c.googlers.com>

Show 17 quoted lines

> Josh Steadmon <steadmon@google.com> writes:
> 
> >> +--index-pack-args=<args>::
> >> +	For internal use only. The command to run on the contents of the
> >> +	downloaded pack. Arguments are URL-encoded separated by spaces.
> >
> > I'm a bit skeptical of using URL encoding to work around embedded
> > spaces. I believe in Emily's config-based hooks series, she wrote an
> > argument parser to pull repeated arguments into a strvec, could you do
> > something like that here?
> >
> > I'm sympathetic to the idea that since this is an internal-only flag, we
> > can be a bit weird with the argument format, though.
> 
> We tend to prefer quote.c::sq_quote*() suite of quoting; does this
> codepath have very different constraints that require different
> encoding?

My main issue was that I needed to join arbitrary strings and then split them, which is why I URL-encoded them (so that they would no longer contain spaces) and then used spaces as the "join" separator. With Josh's suggestion, I wouldn't need any sort of encoding or quoting, so I think I'll use that.