Phil Hord [off-list ref] writes:
quoted
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.
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.
On Mon, Aug 26, 2013 at 1:26 PM, Junio C Hamano [off-list ref] wrote:
Phil Hord [off-list ref] writes:
quoted
quoted
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.
Anyone who is interested in the matched trees from the original
command-line only needs to compare the matched paths' prefixes with
the original args to see which one is responsible for each hit. But
this is not convincing to me because they may not know the original
args to the grep command.
I do not know if this a good reason to keep supporting this mode,
though. I can just as easily see some script being confused by four
colons in
origin/master:some/:path/file.c:1:int main()
instead of only three that he is used to because someone passed in a
longer tree-path than expected.
I don't know if I can make that argument.
I think I _can_ argue that the colon separator here is historically
just a part of the filename because it is not affected by "--null".
quoted
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.
That would also solve this problem, usually. But I don't like it
nearly as much as my patch, and I agree it seems extreme for this one
corner-case.
Phil