Re: [PATCH 4/4] describe/name-rev: tell name-rev to peel the incoming object to commit first

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 9, 2013, 05:33 UTC
Message-ID: <7va9lwxpp5.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130709050615.GF27903@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

>   1. Behave more or less the same between "git name-rev $sha1" and "echo
>      $sha1 | git name-rev --stdin". Your patch improves that. Though I
>      note that --peel-to-commit does not affect --stdin at all. Should
>      it? And of course the two differ in that the command line will take
>      any rev-parse expression, and --stdin only looks for full sha1s.

To "Should it?", I do not deeply care. "--peel-to-commit" is an exception that only is needed to support "describe".

I could instead have tucked "^0" at the end of each argument when "describe" calls out to "name-rev" without adding this new option, which is much much closer to what is really going on.

And that will alleviate your #2 below.

Show 15 quoted lines

>   2. If name-rev prints "$X $Y", I would expect "git rev-parse $X" to
>      equal "git rev-parse $Y". With peeling, that is not the case, and
>      you get the misleading example that Ram showed:
>
>        $ git name-rev 8af0605
>        8af0605 tags/v1.8.3^0
>
>     or more obviously weird:
>
>        $ git name-rev v1.8.3
>        v1.8.3 tags/v1.8.3^0
>
> So I think your series moves in a good direction, but I would just worry
> that it is breaking backwards compatibility (but like I said, I am not
> clear on who is affected and what it means for them).