Re: [PATCH] tag: do not show ambiguous tag names as "tags/foo"

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 24, 2016, 22:19 UTC
Message-ID: <xmqqvb6iboxj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160124071815.GB24084@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 21 quoted lines

> On Sun, Jan 24, 2016 at 02:12:35AM -0500, Jeff King wrote:
>
>> In theory, the ref-filter code could figure this out by us
>> passing FILTER_REFS_TAGS. But there are two complications
>> there:
>> 
>>   1. The handling of refname:short is deep in formatting
>>      code that does not even have our ref_filter struct, let
>>      alone the arguments to the filter_ref struct.
>> 
>>   2. In git v2.7.0, we expose the formatting language to the
>>      user. If we follow this path, it will mean that
>>      "%(refname:short)" behaves differently for "tag" versus
>>      "for-each-ref" (including "for-each-ref refs/tags/"),
>>      which can lead to confusion.
>> 
>> Instead, let's extend the "short" modifier in the formatting
>> language to handle a specific prefix. This fixes "git tag",
>> and lets users invoke the same behavior from their own
>> custom formats (for "tag" or "for-each-ref") while leaving
>> ":short" with its same consistent meaning in all places.

Yeah, I do agree with the analysis.

I however wonder if short=$prefix is a good end-user interface, though, as strip=$prefix may be more intuitive to them, I suspect.

Show 22 quoted lines

> I think the patch I posted is a reasonable way to go. But I also don't
> think that having "%(refname:short)" behave specially for "git-tag" is
> all that unreasonable, either. But I'm open to argument.
>
> Here are a few more considerations I had.
>
>  - I'm not sure if the "special" behavior works as well for git-branch,
>    which may want to shorten both "refs/heads/" and "refs/remotes/",
>    depending on the type of ref.
>
>    My solution may not extend there naturally, either, depending on how
>    it is implemented.
>
>  - To let users get the same behavior out of for-each-ref, we could
>    perhaps auto-infer that looking at "refs/tags/" means shortening and
>    disambiguation should only happen with respect to the "refs/tags/"
>    hierarchy.
>
>    But I'm uncomfortable changing the meaning of ":short" without at
>    least a new option. And what would it mean for "git for-each-ref
>    refs/heads/foo/"? Would it shorten "refs/heads/foo/bar" to just
>    "bar", or would it still be "foo/bar"?

Also there is "what happens if the expected prefix is not there?" question. Perhaps strip=2 can be defined to "strip 2 levels of hierarchy prefix no matter what that is", and strip refs/tags/foo, refs/heads/foo and refs/remotes/origin/foo to foo, foo, origin/foo, respectively? The filtering natively done by the listing mode of "branch" and "tags" would ensure the prefixes are always what we implicitly expect, so the case we need to worry about how we should signal errors becomes "what if there are not enough levels". That may be simpler to handle.