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

From: Jeff King <peff@peff.net>
Date: Feb 7, 2017, 19:50 UTC
Message-ID: <20170207195011.2yvq6hutssl4htol@sigill.intra.peff.net>
In-Reply-To: <20170207150414.GD8583@stefanha-x1.localdomain>

On Tue, Feb 07, 2017 at 03:04:14PM +0000, Stefan Hajnoczi wrote:

Show 9 quoted lines

> > I assume Stefan just grabbed my naive suggestion hence why it checks
> > equality with a commit.  So that's my fault :)  Either of these may
> > not be enough though, since if you do 'git grep malloc v2.9.3^{tree}'
> > with this change the output prefix is 'v2.9.3^{tree}/' instead of the
> > correct prefix 'v2.9.3^{tree}:'
> 
> I revisited this series again today and am coming to the conclusion that
> forming output based on the user's rev is really hard to get right in
> all cases.  I don't have a good solution to the v2.9.3^{tree} problem.

I think the rule you need is not "are we at a tree", but rather "did we traverse a path while resolving the name?". Only the get_sha1() parser can tell you that. I think:

  char delim = ':';
  struct object_context oc;
  if (get_sha1_with_context(name, 0, sha1, &oc))
          die("...");
  if (oc.path[0])
          delim = '/'; /* name had a partial path */

would work. Root trees via "v2.9.3^{tree}" or "v2.9.3:" would have no path, but "v2.9.3:Documentation" would. I think you'd still need to avoid duplicating a trailing delimiter, but I can't think of a case where it is wrong to do that based purely on the name.

-Peff