Re: [PATCH v2 2/2] grep: use '/' delimiter for paths

From: Jeff King <peff@peff.net>
Date: Feb 9, 2017, 05:20 UTC
Message-ID: <20170209052034.ksoupjcj4qs7x4hz@sigill.intra.peff.net>
In-Reply-To: <xmqqtw84rlna.fsf@gitster.mtv.corp.google.com>

On Wed, Feb 08, 2017 at 09:14:17PM -0800, Junio C Hamano wrote:

Show 22 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> >   master:a:a:a:a:a:a:a:a:a:a:a
> >
> > I think there are 2^(n-1) possible paths (each colon can be a real colon
> > or a slash). Though I guess if you walk the trees as you go, you only
> > have to examine at most "n" paths to find the first-level tree, and then
> > at most "n-1" paths at the second level, and so on.
> >
> > Unless you really do have ambiguous trees, in which case you have to
> > walk down multiple paths.
> >
> > It certainly would not be the first combinatoric explosion you can
> > convince Git to perform. But it does seem like a lot of complication for
> > something as simple as path lookups.
> 
> That is true, and we may want to avoid the implementation complexity
> of the backtracking name resolution.  If you are on the other hand
> worried about the runtime cost, it will be an issue to begin with
> only for those who do "git grep -e pattern HEAD:t/perf", which is an
> unnatural way to do "git grep -e pattern HEAD -- t/perf", and the
> output from the latter won't have such an issue, so...

I thought your point was to move it into the get_sha1() parser (so that while the form is only generated by "git grep", it can be accepted by any git command). That exposes it in a lot of places, including ones which are network accessible to things like gitweb (or GitHub, of course, which is my concern).

Even without the runtime cost, though, I think the general complexity makes it an ugly path to go down (e.g., handling ambiguous cases). I wouldn't want to have to write the documentation for it. :)

(I _do_ think Stefan's proposed direction is worth it simply because the result is easier to read, but I agree the whole thing can be avoided by using pathspecs, as you've noted).

-Peff