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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Phil Hord <phil.hord@xxxxxxxxx> writes:

> On Mon, Aug 26, 2013 at 3:14 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>>
>>> Jonathan Nieder <jrnieder@xxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]