On Thu, May 03 2018, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> But ^{tree} shows just the trees, but would previously be equivalent
>> to the above:
>>
>> $ git rev-parse e8f2^{tree}
>> error: short SHA1 e8f2 is ambiguous
>> hint: The candidates are:
>> hint: e8f2093055 tree
>> hint: e8f25a3a50 tree
>> hint: e8f28d537c tree
>> hint: e8f2cf6ec0 tree
>> [...]
>
> When a user says "git $cmd e8f2^{tree}", the user is telling Git
> that s/he knows e8f2 *is* a tree-ish, but for whatever reason $cmd
> wants a tree and does not accept an arbitrary tree-ish---that is the
> whole piont of appending ^{tree} as a suffix. A useful hint in such
> a case would be "oh, you said e8f2 is a tree-ish, but there are more
> than one tree-ish, so let me show them to you to help you decide
> which one among them is the one you meant". When $cmd is rev-parse,
> I would even say that the user is saying "I know e8f2 is a tree-ish,
> and I know it not a tree--it merely is a tree-ish. I want the tree
> that e8f2 thing points at".
>
> Limiting that hint to show only real trees does not make any sense
> to me. I do not think we care _too_ deeply, because most of the
> time, command line location that expects a tree-ish can be given a
> tree-ish, so there is not much reason to use ^{tree} suffix these
> days. But in a case where it _does_ matter, I think this change
> makes the "hint" almost useless.
>
> Or am I misleading what you wanted to achieve with this patch?
The reason I'm doing this is because I found it confusing that I can't
do:
for t in tag commit tree blob; do ./git --exec-path=$PWD rev-parse 7452^{$t}; done
And get, respectively, only the SHAs that match the respective type, but
currently (with released git) you can do:
for t in tag commit committish treeish tree blob; do git -c core.disambiguate=$t rev-parse 7452; done
And while =tag doesn't work the others do (inluding =blob), so
core.disambiguate=tree gives you just trees, but ^{tree} gives you
treeish.
Why should ^{tree} be giving me ^{treeish} but =tree be giving me trees,
and =treeish be synonymous with ^{tree}?
There's no other cases I know of where the ^{<type>} peel syntax won't
give you *only* the <type> you asked for. See peel_onion() ->
peel_to_type() and how get_oid_1() will short-circuit if it has an
answer, and then finally fall back to this get_short_oid() codepath.
Looking at the code & git log maybe it'll do that internally, but when
you peel a tag or commit ^{tree} will only ever find one thing, unlike
this disambiguation case where we can match multiple things.
So:
1) Am I missing some subtlety or am I correct that there was no way to
get git to return more than one SHA-1 for ^{commit} or ^{tree} before
this disambiguation feature was added?
2) I think the behavior I've implemented is consistent with how the peel
syntax has been documented in revisions.txt:
'<rev>{caret}{<type>}', e.g. 'v0.99.8{caret}\{commit\}'::
A suffix '{caret}' followed by an object type name enclosed in
brace pair means dereference the object at '<rev>' recursively until
an object of type '<type>' is found or the object cannot be
dereferenced anymore (in which case, barf).
For example, if '<rev>' is a commit-ish, '<rev>{caret}\{commit\}'
describes the corresponding commit object.
Similarly, if '<rev>' is a tree-ish, '<rev>{caret}\{tree\}'
describes the corresponding tree object.
'<rev>{caret}0'
is a short-hand for '<rev>{caret}\{commit\}'.
Note "until an object of type '<type>' is found". I.e. my mental model
of this has been that yes you can *start* the search at a different
object (e.g. tag -> tree), but it'll only ever return the tree. The
disambiguation implementation has been inconsistent with this as
documented, because it hasn't been drilling don to an object of '<type>'
given a request like $short^{<type>}, but rather returning something
matching $short because it could be a container for <type>.
Anyway, I'm not saying I'm right. This is the first time I really look
at sha1-name.c in any detail. But the above describes the thought
process behind this patch.
