On Mon, Aug 26, 2013 at 12:23 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 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.
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.
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.
Have I got that right?
If so, then I would like to point out to you the convenience I
accidentally encountered using this tool. Perhaps you didn't realize
how helpful it was when you chose to use a colon there.
On the other hand, perhaps a colon is an unfortunate syntactic
collision which should be corrected in the future.
Phil
--
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