Subject: Re: [PATCH 2/3] log: add default decoration filter

From: Derrick Stolee <[email-masked]>
Date: Wed, 27 Jul 2022 15:07:08 -0400

On 7/27/2022 10:50 AM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <[email-masked]> writes:
> 
>> From: Derrick Stolee <[email-masked]>
>>
>> When a user runs 'git log', they expect a certain set of helpful
>> decorations. This includes:
>>
>> * The HEAD ref
> 
> OK.
> 
>> * Branches (refs/heads/)
> 
> OK.
> 
>> * Notes (refs/notes/)
> 
> Questionable.  None of the notes commits are on the primary history,
> so it is unlikely to be even used for decoration, so it would not
> hurt to have them as part of possible decoration sources, but on the
> other hand, the value of using them is dubious.

I had originally left this out of the filter, but then noticed that test
4013.102 (git log --decorate --all # magic is (not used)) includes an
instance of refs/notes/. Perhaps this is just old test cruft and could be
changed.

I could remove this from the filter and change the old test to match the
new behavior.

>> * Stashes (refs/stash/)
> 
> Questionable, for the same reason as 'notes'.

I had also left this out initially, but t4207-log-decoration-colors.sh
fails, and it's because 'stash' is one of the supported "slots" in the
color.decorate.<slot> config option. This elevates the importance of the
stash refs. (Notes are not a slot in this config option, for instance.)

The other slot that isn't included in my list is "grafted". This includes
grafts _and_ replace-objects.  Grafts appear to add decorations in a
different path that side-steps this filter, but replace-objects are
subject to this filter. Things are a bit more complicated here because of
the GIT_REPLACE_REF_BASE environment variable.

I'll add replace-objects in the next re-roll using the value of
GIT_REPLACE_REF_BASE to construct the filter. I'll add some tests to keep
the existing behavior, too, especially around color.decorate.grafted.

>> * Tags (refs/tags/)
> 
> Abolutely useful.
> 
>> * Remote branches (refs/remotes/)
> 
> Ditto.

Thanks,
-Stolee