Re: [PATCH] log: add log.excludeDecoration config option

From

Derrick Stolee <stolee@gmail.com>

Date

Apr 15, 2020, 14:14 UTC

Message-ID

<707d61f5-02fe-73a9-0719-d7eece4669b4@gmail.com>

In-Reply-To

<xmqqtv1m7xc5.fsf@gitster.c.googlers.com>

On 4/14/2020 2:10 PM, Junio C Hamano wrote:

> Derrick Stolee <stolee@gmail.com> writes:
> 
>>>  * Given that we have command line options to specify what patterns
>>>    to include as well as to exclude, it feels somewhat asymmetric to
>>>    have only the configuration to exclude.  Should we also have a
>>>    configuration for including?
>>
>> I left the other side out for simplicity and because I didn't know
>> the use case. It seems all refs are included by default.
> 
> It is a bit more subtle than that.  
> 
> Once you have an inclusion pattern, nothing is included by default.
> Only the ones that match an inclusion pattern would be considered.
> When there is no inclusion pattern, we behave as if there is a
> match-all inclusion pattern.

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index b5de449e51..e9c9e59461 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -742,8 +742,24 @@ test_expect_success 'decorate-refs with glob' '
        octopus-a (octopus-a)
        reach
        EOF
+       cat >expect.no-decorate <<-\EOF &&
+       Merge-tag-reach
+       Merge-tags-octopus-a-and-octopus-b
+       seventh
+       octopus-b
+       octopus-a
+       reach
+       EOF
        git log -n6 --decorate=short --pretty="tformat:%f%d" \
                --decorate-refs="heads/octopus*" >actual &&
+       test_cmp expect.decorate actual &&
+       git log -n6 --decorate=short --pretty="tformat:%f%d" \
+               --decorate-refs-exclude="heads/octopus*" \
+               --decorate-refs="heads/octopus*" >actual &&
+       test_cmp expect.no-decorate actual &&
+       git -c log.excludeDecoration="heads/octopus*" log \
+               -n6 --decorate=short --pretty="tformat:%f%d" \
+               --decorate-refs="heads/octopus*" >actual &&
        test_cmp expect.decorate actual
 '

This test fails at the last test_cmp with the current
patch.

Note that if we have both --decorate-refs-exclude=X and
--decorate-refs=X, then the exclusion wins. This means that
we will need to split the "configured" exclusions from the
"command-line" exclusions and give them different priority.

But, I believe that if we can get this test to pass, then
we will have the correct inclusion/exclusion logic. I will
get started on this right now.

Thanks,
-Stolee