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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 5, 2017, 02:00 UTC
Message-ID: <xmqq1sldmqms.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <b0e3856b-e627-0d22-90da-3da1781f98b3@gmail.com>

Rafael Ascensão <rafa.almas@gmail.com> writes:

Show 14 quoted lines

>>> The pattern follows similar rules as `--glob` except it doesn't assume a
>>> trailing '/*' if glob characters are missing.
>>
>> 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/*"?
>
> I wanted to allow exact patterns like:
> "--decorate-refs=refs/heads/master" and for that I disabled the flag
> that adds the trailing '/*' if no globs are found. As a side effect, I
> lost the shortcut.
>
> Is adding a yet another flag that appends '/*' only if the pattern
> equals "refs/{heads,remotes,tags}" a good idea?

No.

Show 6 quoted lines

> Because changing the default behavior of that function has
> implications on multiple commands which I think shouldn't change. But
> at the same time, would be nice to have the logic that deals with
> glob-ref patterns all in one place.
>
> What's the sane way to do this?

Learn to type "--decorate-refs="refs/heads/[m]aster", and not twewak the code at all, perhaps. The users of existing "with no globbing, /* is appended" interface are already used to that way and they do not have to learn a new and inconsistent interface.

After all, "I only want to see 'git log' output with 'master' decorated" (i.e. not specifying "this class of refs I can glob by using the naming convention I am using" and instead enumerating the ones you care about) does not sound like a sensible thing people often want to do, so making it follow the other codepath so that people can say "refs/tags" to get "refs/tags/*", while still allowing such a rare but specific and exact one possible, may not sound too bad to me.