Re: [PATCH v3 09/12] get_short_oid / peel_onion: ^{tree} should be tree, not treeish
- From
Jeff King <peff@peff.net>
- Date
- May 8, 2018, 14:34 UTC
- Message-ID
- <20180508143408.GA30183@sigill.intra.peff.net>
- In-Reply-To
- <xmqq6040qf8x.fsf@gitster-ct.c.googlers.com>
On Mon, May 07, 2018 at 01:08:46PM +0900, Junio C Hamano wrote:
Show 20 quoted lines
> Æ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.FWIW, I had the same feeling as you when reading this, that this commit (and the one after) are doing the wrong thing. And these paragraphs sum it up. The "^{tree}" is about asking us to peel to a tree, not about resolving X in the first place. We can use it as a _hint_ when resolving X, but the correct hint is "something that can be peeled to a tree", not "is definitely a tree".
-Peff