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

From: Derrick Stolee <derrickstolee@github.com>
Date: Jul 27, 2022, 19:07 UTC
Message-ID: <e437576e-537c-ecfb-9143-6e440f3bb893@github.com>
In-Reply-To: <xmqqczdqtxro.fsf@gitster.g>

On 7/27/2022 10:50 AM, Junio C Hamano wrote:

Show 21 quoted lines

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>>
>> 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.

Show 7 quoted lines

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

Thanks, -Stolee