Re: [RFC/PATCH] Fix path prefixing in grep_object

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2013, 17:26 UTC
Message-ID: <xmqqbo4kicsm.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CABURp0odEGgZO1yWFgXS+akPb4wJHiTLoQcmqBd00oYnPZ77vA@mail.gmail.com>

Phil Hord <phil.hord@gmail.com> writes:

Show 18 quoted lines

>> If your justification were "above says 'there may be a readon why
>> the user wanted to ask it in that way', i.e. 'find in this tree
>> object HEAD:some/path and report where hits appear', but the reason
>> can only be from laziness and/or broken script and the user always
>> wants the answer from within the top-level tree-ish", then that
>> argument may make some sense. You need to justify why it is OK to
>> lose information in the answer by rewriting the colon that separates
>> the question ("in this tree object") and the answer ("at this path
>> relative to the tree object given").
>>
>> Whether you rewrite the input or the output is not important; you
>> are trying to give an answer to a different question, which is what
>> I find questionable.
>
> Ok, so if I can summarize what I am inferring from your objection:
>
>  1. The (tree-path, found-path) pair is useful information to get back
> from git-grep.

At least that was the intent. I can be persuaded that your change will not break anybody if you successfully argue that it is not a useful information, though.

Show 7 quoted lines

>  2. A colon is used to delimit these pieces of information, just as a
> colon is used to delimit the filename from the matched-line results.
>
>  3. The fact that the colon is also the separator used in object refs
> is mere coincidence; the colon was _not_ chosen because it
> conveniently turns the results list into valid object references.  A
> comma could have been instead, or even a \t.

Not necessarily. If the user is asking the question in a more natural way (I want to see where in 'next' branch's tip commit hits appear, by the way, I know I am only interested in builtin/ so I'd give pathspec as well when I am asking this question), the output does give <commit> <colon> <path>, so it is more than coincidence.

I do not think it is worth doing only for this particular use case, but it might be a good change to allow A:B:C to be parsed as a proper extended SHA-1 expression and make it yield

	git rev-parse $(git rev-parse $(git rev-parse A):B):C

Right now, "B:C" is used as a path inside tree-ish "A", but I think we can safely fall back, when path B:C does not appear in tree-ish A, to see if path B appears in it and is a tree, and then turn it into a look-up of path C in that tree A:B.