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

> Phil Hord <phil.hord@gmail.com> writes:
>
>> On Mon, Aug 26, 2013 at 3:14 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>>>
>>>>> I think Phil meant that when "git grep" is asked to search within
>>>>> "HEAD:some/path", filenames tacked on at the end should be appended
>>>>> with a '/' separator instead of the usual ':' (e.g.,
>>>>> "HEAD:some/path/inner/path.c", not "HEAD:some/path:inner/path.c").
>>>>
>>>> Ah, OK.
>>>>
>>>> I am not sure if we have a proper hint in the revision parser
>>>> machinery, but it may not be too hard to teach rev-cmdline interface
>>>> to keep that kind of information (i.e. "This tree object name is a
>>>> result of parsing '<tree-ish>:path' syntax").
>>>
>>> Actually, having thought about this a bit more, I am no longer sure
>>> if this is even a good idea to begin with.
>>>
>>> An output line that begins with HEAD:some/path:inner/path.c from
>>> "git grep" is an answer to this question: find the given pattern in
>>> a tree-ish specified with "HEAD:some/path".
>>>
>>> On the other hand, HEAD:some/path/inner/path.c is an answer to a
>>> different question: find the pattern in a tree-ish specified with
>>> "HEAD".  The question may optionally be further qualified with "but
>>> limit the search to some/path".  Both the question and its answer
>>> are more intuitive than the former one.
>>
>> I disagree.  The man page says that git grep lists the filenames of
>> matching files.
>
> But you need to realize that the manual page gives a white lie in
> order to stay short enough to be readable.  It does give filenames
> when reading from the working tree, the index or a top-level tree
> object.  When given arbitrary tree object that is not top-level, it
> does give filenames relative to the given tree object, which is the
> answer HEAD:some/path:inner/path.c to the question "where in the
> tree HEAD:some/path do we have hits?".
>
>>> If the user asked the question of the former form, i.e.
>>>
>>>     $ git grep -e pattern HEAD:some/path
>>>
>>> there may be a reason why the user wanted to ask it in that
>>> (somewhat strange) way.  I am not 100% sure if it is a good idea to
>>> give an answer to a question different from what the user asked by
>>> internally rewriting the question to
>>>
>>>     $ git grep -e pattern HEAD -- some/path
>>
>> We are not rewriting the question at all.
>
> That statement is trapped in a narrow "implementor" mind. I know you
> are not rewriting the question in your implementation, but what do
> the end users see? It gives an answer to a question different from
> they asked; the observed behaviour is as if the question was
> rewritten before the machinery went to work.
>
> 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.