Re: [PATCH v2 06/10] log: add default decoration filter

From: Derrick Stolee <derrickstolee@github.com>
Date: Aug 5, 2022, 14:27 UTC
Message-ID: <ac7b3a81-7ca6-05d3-6633-4204066aa933@github.com>
In-Reply-To: <220804.86iln8e9hl.gmgdl@evledraar.gmail.com>

On 8/4/2022 3:08 AM, Ævar Arnfjörð Bjarmason wrote:

Show 7 quoted lines

> There's no lack of "stability", because the ref hiding only act on
> what's known to be something we can ignore, because our git version
> knows about it.
> 
> If git v2.40 knows about refs/magical-1/* but not refs/magical-2/*, but
> git v2.50 knows about both it's not a lack of stability that v2.40 shows
> one decorated by default, but v2.50 shows neither.

You are describing how the behavior changes between these versions on the same repository. That's what I mean by lack of stability.

> But it's not just that I disagree, I genuinely don't get your POV
> here.

I'm optimizing for non-experts who never need any refs outside of the standard set. Now that this version removed the notes ref from the decoration, the stance for inclusion is simple:

  If Git offers to color the namespace with color.decoration.<slot>,
  then Git decorates with that namespace by default.

Show 22 quoted lines

>> +test_expect_success 'log --decorate does not include things outside filter' '
>> +	reflist="refs/prefetch refs/rebase-merge refs/bundle" &&
>> +
>> +	for ref in $reflist
>> +	do
>> +		git update-ref $ref/fake HEAD || return 1
>> +	done &&
>> +
>> +	git log --decorate=full --oneline >actual &&
>> +
>> +	for ref in $reflist
>> +	do
>> +		! grep $ref/fake actual || return 1
>> +	done
> 
> I haven't tested, but isn't that last for-loop replacable with:
> 
> 	! grep /fake actual
> 
> ?
> 
> Or do we have other "/fake" refs we want to include?

This is a nice efficient replacement. Thanks.

-Stolee