Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 7, 2017, 00:18 UTC
Message-ID: <xmqqbmkfhrf3.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <B24042DB-BB27-41DE-82B7-5F3ED502D7D0@gmail.com>

Jacob Keller <jacob.keller@gmail.com> writes:

Show 11 quoted lines

> On November 3, 2017 8:49:15 PM PDT, Junio C Hamano <gitster@pobox.com> wrote:
>>Rafael Ascensão <rafa.almas@gmail.com> writes:
>>
>>Why should this be a special case that burdens users to remember one
>>more rule?  Wouldn't users find "--decorate-refs=refs/tags" useful
>>and it woulld be shorter and nicer than having to say "refs/tags/*"?
>
> Actually, I would expect these to behave more like git describes
> match and exclude which don't have an extra /*. It seems natural
> to me that glob would always add an extra glob, but.. I don't
> recall if match and exlude do so.

I would have to say that the describe's one is wrong if it does not match what for_each_glob_ref() does for the log family of commands' "--branches=<pattern>" etc. describe.c::get_name() uses positive and negative patterns, just like log-tree.c::add_ref_decoration() would with the patch we are discussing, so perhaps the items in these lists should get the same "normalize" treatment the patch 1/2 of this series brings in to make things consistent?

> That being said, if we think the extra glob would not cause
> problems and generally do what people mean... I guess consistent
> with --glob would be good... But it's definitely not what I'd
> expect at first glance.

FWIW, what describe --match/--exclude do is not what I'd have expected ;-) In any case, we spotted an existing inconsistency that we would want to resolve (the resolution could be "leave it as-is"; I do not think we have thought this through enough yet), which is good.

Thanks.