Re: [RFC/PATCH] Fix path prefixing in grep_object
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:31
Phil Hord [off-list ref] writes:
On Mon, Aug 26, 2013 at 3:14 AM, Junio C Hamano [off-list ref] wrote:quoted
Junio C Hamano [off-list ref] writes:quoted
Jonathan Nieder [off-list ref] writes:quoted
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?".
quoted
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/pathWe 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.