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

From: Junio C Hamano <gitster@pobox.com>
Date: May 7, 2018, 04:08 UTC
Message-ID: <xmqq6040qf8x.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <87lgczyfq6.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Right, and I'm with you so far, this makes sense to me for all existing
> uses of the peel syntax, otherwise v2.17.0^{tree} wouldn't be the same
> as rev-parse v2.17.0^{tree}^{tree}...

More importantly, you could spell v2.17.0 part of the above with a short hexadecimal string. And that string should be naming some tree-ish, the most important thing being that it is *NOT* required to be a tree (and practically, it is likely that the user has a tree-ish that is *NOT* a tree).

I guess I have a reaction to the title

    "get_short_oid/peel_onion: ^{tree} should be tree"

"X^{tree}" should *RESULT* in a tree, but it should *REQUIRE* X to be a tree-ish. It is unclear "should be tree" is about the former and I read (perhaps mis-read) it as saying "it should require X to be a tree"---that statement is utterly incorrect as we agreed above.

> case-by-case[1]:
>
>     ^{tag}:
>     7452b4b5786778d5d87f5c90a94fab8936502e20

I take it as "git rev-parse 7452^{tag}" output (similarly ^{$type} for the rest)? That probably is desirable, as blobs, trees and commits cannot be peeled down to a tag.

>     ^{commit}:
>     hint:   74521eee4c commit 2007-12-01 - git-gui: install-sh from automake does not like -m755
>     hint:   745224e04a commit 2014-06-18 - refs.c: SSE2 optimizations for check_refname_component

If 7452 points at a commit, that tag itself should also be given as a possible object the user may have meant in the "hint" thing. I agree it is a good idea to exclude trees and blobs from the hint, for the same reason why I think it makes sense to exclude blobs, trees and commits from hints for a X in "X^{tag}" above.

Show 7 quoted lines

>     ^{tree}:
>     hint:   7452336aa3 tree
>     hint:   74524f384d tree
>     hint:   7452813bcd tree
>     hint:   7452b1a701 tree
>     hint:   7452b73c42 tree
>     hint:   7452ca1557 tree

Again, if there is a commit or a tag (that points at a commit or a tree) whose name begins with 7452, it should be included in the hint above. Not having blobs in the hint of course makes sense, as a blob cannot be X in "X^{tree}".

Show 6 quoted lines

> And[2]:
>
>     core.disambiguate=tag:
>     [same as ^{tag]
>     core.disambiguate=commit:
>     [same as ^{commit}]

When core.disambiguate tells us to "interprete hexadecimal literals to name commit objects only", giving only commits in hints: section makes sense, because we are explicitly saying that "when I say 7452, I do not mean any tag whose name begins with 7452", so "sorry, your request is not explicit enough---there are two commits and a tag that begin with that prefix" is not helpful---it should stop at "you may have meant one of these two commits" and not mention any tag.