Re: [PATCH v3 09/12] get_short_oid / peel_onion: ^{tree} should be tree, not treeish

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

 



Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:

>> Before we had any disambiguation code, resolving X^{tree} really was two
>> independent steps: resolve X, and then peel it to a tree. When we added
>> the disambiguation code, the goal was to provide a hint to the first
>> step in such a way that we could never eliminate any resolutions that
>> the user _might_ have meant. But it's OK to take a situation where every
>> case but one would result in an error, and assume the user meant that
>> case. Sort of a "do no harm" rule.
>>
>> By disambiguating with just a tree and not a tree-ish, that hint is now
>> eliminating possibilities that would have worked in the second step,
>> which violates the rule.
>>
>> Does thinking about it that way make more sense?
>
> Okey, so to rephrase that to make sure I understand it. It would be
> documented as something like this:
>
>     When the short SHA-1 X is ambiguous X^{<type>} doesn't mean do the
>     peel itself in X any way, rather it means list all those objects
>     matching X where a subsequent X^{<type>} wouldn't be an error.

With the understanding that this paragraph is written primarily for
your own enlightenment, I wouldn't complain too much, but if you
meant this to become part of end-user documentation, I have a strong
issue with the verb "list" used here.

X^{<type>} never means to "list" anything (FWIW just X does not mean
to "list" anything, either).  It just means that the user wants to
specify a single object whose object name is X^{<type>}, i.e. the
user expects that X names a single object, that can be peeled to
<type>.

Now, it is an error when (1) X does not specify a single object in
the first place, or (2) the single object cannot be peeled to <type>.

When diagnosing such an error, we would give hints.  The hint would
show possible objects that the user could have meant with X.  The
^{<type>} suffix given to it may be used to limit the hints to
subset of the objects that the user could have meant with X;
e.g. when there is an object of each of type blob, tree, commit, and
tag, whose name begins with 7777, the short and ambiguous prefix
7777 could mean any of these four objects, but when given with
suffix, e.g. 7777^{tree}, it makes useless for the hint to include
the blob object, as it can never peel down to a tree object.

If the tag whose name begins with 7777 in this example points
directly to a blob, excluding that tag from the hint would make the
hint more useful.  I do not offhand know what the code does right
now.  I wouldn't call it a bug if such a tag is included in the
hint, but if a change stops such a tag from getting included, I
would call such a change an improvement.

>     I.e. X^{commit} will list tags and commits, since both can be peeled
>     to reveal a commit, X^{tree} will similarly list tags, commits and
>     trees, and X^{blob} will list tags and blobs[1], and X^{tag} will
>     only list tags.

Modulo the use of "list", which I have trouble is as it makes it
sound as if listing something is the purpose of the notation, I
think we are on the same page up to this point

I think the best way to explain core.disambiguate to readers is to
make them rethink what I meant with "the user expects that X names a
single object" in the early part of the above response.

Without constraint, Git understood that the user used 7777 to name
any one of the objects of all four types.  With core.disambiguate,
the user can tell Git "when I give potentially ambiguous object name
with a short prefix, assume that only a commit or a tag whose name
begins with the prefix is what I expected the short prefix to name
uniquely", so Git understood that the user wanted to name either a
commit or a tag.  It would still trigger an error as it does not
uniquely name an object (for which an attempt to apply the ^{tree}
peeling would further be made).




[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]

  Powered by Linux