Message ID | pull.610.git.1586791720114.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | log: add log.excludeDecoration config option | expand |
Hi Stolee, On Mon, Apr 13, 2020 at 03:28:39PM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@microsoft.com> > > In 'git log', the --decorate-refs-exclude option appends a pattern > to a string_list. This list is used to prevent showing some refs > in the decoration output, or even by --simplify-by-decoration. > > Users may want to use their refs space to store utility refs that > should not appear in the decoration output. For example, Scalar [1] > runs a background fetch but places the "new" refs inside the > refs/scalar/hidden/<remote>/* refspace instead of refs/<remote>/* > to avoid updating remote refs when the user is not looking. However, > these "hidden" refs appear during regular 'git log' queries. > > A similar idea to use "hidden" refs is under consideration for core > Git [2]. > > Add the 'log.excludeDecoration' config option so users can exclude > some refs from decorations by default instead of needing to use > --decorate-refs-exclude manually. The config value is multi-valued > much like the command-line option. > > There are several tests in t4202-log.sh that test the > --decorate-refs-(include|exclude) options, so these are extended. > Since the expected output is already stored as a file, simply add > new calls that replace a "--decorate-refs-exclude" option with an > in-line config setting. > > [1] https://github.com/microsoft/scalar > [2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/ > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > log: add log.excludeDecoration config option > > This was something hinted at in the "fetch" step of the background > maintenance RFC. Should be a relatively minor addition to our config > options. > > We definitely want this feature for microsoft/git (we would set > log.excludeDecoration=refs/scalar/* in all Scalar repos), but we will > wait for feedback from the community. > > Thanks, -Stolee > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/610 > > Documentation/config/log.txt | 5 +++++ > builtin/log.c | 14 ++++++++++++++ > t/t4202-log.sh | 22 ++++++++++++++++++++++ > 3 files changed, 41 insertions(+) > > diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt > index e9e1e397f3f..1a158324f79 100644 > --- a/Documentation/config/log.txt > +++ b/Documentation/config/log.txt > @@ -18,6 +18,11 @@ log.decorate:: > names are shown. This is the same as the `--decorate` option > of the `git log`. > > +log.excludeDecoration:: > + Exclude the specified patterns from the log decorations. This multi- > + valued config option is the same as the `--decorate-refs-exclude` > + option of `git log`. > + Thanks for this documentation update. Do you think that it would be worth updating the entry for '--decorate-refs-exclude', too? I figure that it would be good, since scripters who look at 'git log's man page before 'git config's would have a trail to follow in case they want a persistent '--decorate-refs-exclude' option. > log.follow:: > If `true`, `git log` will act as if the `--follow` option was used when > a single <path> is given. This has the same limitations as `--follow`, > diff --git a/builtin/log.c b/builtin/log.c > index 83a4a6188e2..d7d1d5b7143 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, > } > > if (decoration_style) { > + const struct string_list *config_exclude = > + repo_config_get_value_multi(the_repository, > + "log.excludeDecoration"); > + > + if (config_exclude) { > + struct string_list_item *item; > + for (item = config_exclude->items; > + item && item < config_exclude->items + config_exclude->nr; > + item++) Any reason not to use the 'for_each_string_list_item' macro in 'string-list.h' for this? > + string_list_append(&decorate_refs_exclude, > + item->string); > + } > + On my first read, I thought that there might be some value in using a hash set here, since I (incorrectly) thought that only literal reference names were added here, in which case we'd benefit from a constant time lookup. But, since we store patterns here, it's not helpful for us to have a constant time lookup, since we don't have anything to look up in the first place! I guess you could expand out all of these patterns ahead of time, but I don't think that this is universally a good thing to do. We may have a pattern like 'refs/heads/*', but we're only doing a 'git log' over a part of history that doesn't have many/any refs pointing at it, in which case expanding ahead of time was a bad thing to do. Of course, 'string_list_append' is going to potentially introduce duplicate patterns, but I don't think that should be a huge problem, since 'ref_filter_match' stops as soon as it finds an exclude/include. I guess you could use 'string_list_insert', but it makes insertions more expensive without a clear benefit. > rev->show_decorations = 1; > + > load_ref_decorations(&decoration_filter, decoration_style); > } > > diff --git a/t/t4202-log.sh b/t/t4202-log.sh > index 0f766ba65f5..b5de449e510 100755 > --- a/t/t4202-log.sh > +++ b/t/t4202-log.sh > @@ -787,6 +787,9 @@ test_expect_success 'decorate-refs-exclude with glob' ' > EOF > git log -n6 --decorate=short --pretty="tformat:%f%d" \ > --decorate-refs-exclude="heads/octopus*" >actual && > + test_cmp expect.decorate actual && > + git -c log.excludeDecoration="heads/octopus*" log \ > + -n6 --decorate=short --pretty="tformat:%f%d" >actual && > test_cmp expect.decorate actual > ' > > @@ -801,6 +804,9 @@ test_expect_success 'decorate-refs-exclude without globs' ' > EOF > git log -n6 --decorate=short --pretty="tformat:%f%d" \ > --decorate-refs-exclude="tags/reach" >actual && > + test_cmp expect.decorate actual && > + git -c log.excludeDecoration="tags/reach" log \ > + -n6 --decorate=short --pretty="tformat:%f%d" >actual && > test_cmp expect.decorate actual > ' > > @@ -816,6 +822,14 @@ test_expect_success 'multiple decorate-refs-exclude' ' > git log -n6 --decorate=short --pretty="tformat:%f%d" \ > --decorate-refs-exclude="heads/octopus*" \ > --decorate-refs-exclude="tags/reach" >actual && > + test_cmp expect.decorate actual && > + git -c log.excludeDecoration="heads/octopus*" \ > + -c log.excludeDecoration="tags/reach" log \ > + -n6 --decorate=short --pretty="tformat:%f%d" >actual && > + test_cmp expect.decorate actual && > + git -c log.excludeDecoration="heads/octopus*" log \ > + --decorate-refs-exclude="tags/reach" \ > + -n6 --decorate=short --pretty="tformat:%f%d" >actual && > test_cmp expect.decorate actual > ' > > @@ -831,6 +845,10 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' ' > git log -n6 --decorate=short --pretty="tformat:%f%d" \ > --decorate-refs="heads/*" \ > --decorate-refs-exclude="heads/oc*" >actual && > + test_cmp expect.decorate actual && > + git -c log.excludeDecoration="heads/oc*" log \ > + --decorate-refs="heads/*" \ > + -n6 --decorate=short --pretty="tformat:%f%d" >actual && > test_cmp expect.decorate actual > ' > > @@ -846,6 +864,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' ' > git log -n6 --decorate=short --pretty="tformat:%f%d" \ > --decorate-refs-exclude="*octopus*" \ > --simplify-by-decoration >actual && > + test_cmp expect.decorate actual && > + git -c log.excludeDecoration="*octopus*" log \ > + -n6 --decorate=short --pretty="tformat:%f%d" \ > + --simplify-by-decoration >actual && > test_cmp expect.decorate actual > ' > Thanks for the tests, they look good. I like your strategy of repeating the test with and without the new config settings. > > base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925 > -- > gitgitgadget Thanks, Taylor
On 4/13/2020 11:49 AM, Taylor Blau wrote: > Hi Stolee, > > On Mon, Apr 13, 2020 at 03:28:39PM +0000, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee <dstolee@microsoft.com> >> >> In 'git log', the --decorate-refs-exclude option appends a pattern >> to a string_list. This list is used to prevent showing some refs >> in the decoration output, or even by --simplify-by-decoration. >> >> Users may want to use their refs space to store utility refs that >> should not appear in the decoration output. For example, Scalar [1] >> runs a background fetch but places the "new" refs inside the >> refs/scalar/hidden/<remote>/* refspace instead of refs/<remote>/* >> to avoid updating remote refs when the user is not looking. However, >> these "hidden" refs appear during regular 'git log' queries. >> >> A similar idea to use "hidden" refs is under consideration for core >> Git [2]. >> >> Add the 'log.excludeDecoration' config option so users can exclude >> some refs from decorations by default instead of needing to use >> --decorate-refs-exclude manually. The config value is multi-valued >> much like the command-line option. >> >> There are several tests in t4202-log.sh that test the >> --decorate-refs-(include|exclude) options, so these are extended. >> Since the expected output is already stored as a file, simply add >> new calls that replace a "--decorate-refs-exclude" option with an >> in-line config setting. >> >> [1] https://github.com/microsoft/scalar >> [2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/ >> >> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> >> --- >> log: add log.excludeDecoration config option >> >> This was something hinted at in the "fetch" step of the background >> maintenance RFC. Should be a relatively minor addition to our config >> options. >> >> We definitely want this feature for microsoft/git (we would set >> log.excludeDecoration=refs/scalar/* in all Scalar repos), but we will >> wait for feedback from the community. >> >> Thanks, -Stolee >> >> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v1 >> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v1 >> Pull-Request: https://github.com/gitgitgadget/git/pull/610 >> >> Documentation/config/log.txt | 5 +++++ >> builtin/log.c | 14 ++++++++++++++ >> t/t4202-log.sh | 22 ++++++++++++++++++++++ >> 3 files changed, 41 insertions(+) >> >> diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt >> index e9e1e397f3f..1a158324f79 100644 >> --- a/Documentation/config/log.txt >> +++ b/Documentation/config/log.txt >> @@ -18,6 +18,11 @@ log.decorate:: >> names are shown. This is the same as the `--decorate` option >> of the `git log`. >> >> +log.excludeDecoration:: >> + Exclude the specified patterns from the log decorations. This multi- >> + valued config option is the same as the `--decorate-refs-exclude` >> + option of `git log`. >> + > > Thanks for this documentation update. Do you think that it would be > worth updating the entry for '--decorate-refs-exclude', too? I figure > that it would be good, since scripters who look at 'git log's man page > before 'git config's would have a trail to follow in case they want a > persistent '--decorate-refs-exclude' option. That seems like a good way to help users. >> log.follow:: >> If `true`, `git log` will act as if the `--follow` option was used when >> a single <path> is given. This has the same limitations as `--follow`, >> diff --git a/builtin/log.c b/builtin/log.c >> index 83a4a6188e2..d7d1d5b7143 100644 >> --- a/builtin/log.c >> +++ b/builtin/log.c >> @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, >> } >> >> if (decoration_style) { >> + const struct string_list *config_exclude = >> + repo_config_get_value_multi(the_repository, >> + "log.excludeDecoration"); >> + >> + if (config_exclude) { >> + struct string_list_item *item; >> + for (item = config_exclude->items; >> + item && item < config_exclude->items + config_exclude->nr; >> + item++) > > Any reason not to use the 'for_each_string_list_item' macro in > 'string-list.h' for this? The reason is I forgot about it. Thanks, -Stolee
On Tue, Apr 14, 2020 at 11:10:33AM -0400, Derrick Stolee wrote: > >> log.follow:: > >> If `true`, `git log` will act as if the `--follow` option was used when > >> a single <path> is given. This has the same limitations as `--follow`, > >> diff --git a/builtin/log.c b/builtin/log.c > >> index 83a4a6188e2..d7d1d5b7143 100644 > >> --- a/builtin/log.c > >> +++ b/builtin/log.c > >> @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, > >> } > >> > >> if (decoration_style) { > >> + const struct string_list *config_exclude = > >> + repo_config_get_value_multi(the_repository, > >> + "log.excludeDecoration"); > >> + > >> + if (config_exclude) { > >> + struct string_list_item *item; > >> + for (item = config_exclude->items; > >> + item && item < config_exclude->items + config_exclude->nr; > >> + item++) > > > > Any reason not to use the 'for_each_string_list_item' macro in > > 'string-list.h' for this? > > The reason is I forgot about it. Heh, in fairness I forgot about it, too :). I thought that this code looked familiar, but it was only luck that I had 'string-list.h' open at the time I was reading this. I don't think that it really matters much each way, but if you're already re-rolling... > Thanks, > -Stolee Thanks, Taylor
On 4/14/2020 11:45 AM, Taylor Blau wrote: > On Tue, Apr 14, 2020 at 11:10:33AM -0400, Derrick Stolee wrote: >>>> log.follow:: >>>> If `true`, `git log` will act as if the `--follow` option was used when >>>> a single <path> is given. This has the same limitations as `--follow`, >>>> diff --git a/builtin/log.c b/builtin/log.c >>>> index 83a4a6188e2..d7d1d5b7143 100644 >>>> --- a/builtin/log.c >>>> +++ b/builtin/log.c >>>> @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, >>>> } >>>> >>>> if (decoration_style) { >>>> + const struct string_list *config_exclude = >>>> + repo_config_get_value_multi(the_repository, >>>> + "log.excludeDecoration"); >>>> + >>>> + if (config_exclude) { >>>> + struct string_list_item *item; >>>> + for (item = config_exclude->items; >>>> + item && item < config_exclude->items + config_exclude->nr; >>>> + item++) >>> >>> Any reason not to use the 'for_each_string_list_item' macro in >>> 'string-list.h' for this? >> >> The reason is I forgot about it. > > Heh, in fairness I forgot about it, too :). I thought that this code > looked familiar, but it was only luck that I had 'string-list.h' open at > the time I was reading this. > > I don't think that it really matters much each way, but if you're > already re-rolling... This is probably worth a re-roll on its own. I'll give the patch some extra time to simmer before pushing a v2, though. -Stolee
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > if (decoration_style) { > + const struct string_list *config_exclude = > + repo_config_get_value_multi(the_repository, > + "log.excludeDecoration"); > + > + if (config_exclude) { > + struct string_list_item *item; > + for (item = config_exclude->items; > + item && item < config_exclude->items + config_exclude->nr; > + item++) > + string_list_append(&decorate_refs_exclude, > + item->string); > + } > + > rev->show_decorations = 1; > + > load_ref_decorations(&decoration_filter, decoration_style); > } A few random thoughts. Unlike my other usual reviews, please do not take "should we do X" as a suggestion (these are purely me wondering and nothing more at this point): * 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? * The new code only adds to decorate_refs_exclude, which has the patterns that were given with the "--decorate-refs-exclude" command line option. As refs.c:ref_filter_match() rejects anything that match an exclude pattern first before looking at the include patterns, there is no way to countermand what is configured to be excluded with the configuration from the command line, even with --decorate-refs" option. Should we have a new command line option to "clear" the exclude list read from the configuration? And if we add configuration for including for symmetry, should that be cleared as well? * As this is a multi-valued configuration, there probably are cases where you have configured three patterns, and for this single invocation you would want to override only one of them. It might not be usable if the only way to override were to "clear" with a new option and then add two that you want from the command line. What if we had (configured) exclusion for X, Y and Z, and then allowed the command line to say "include Y", that would result in the combination to specify exclusion of X and Z only? Can we get away by not having "include these" configuration at all, perhaps, because "if there is no inclusion pattern, anything that does not match exclusion patterns is included" is how the matcher works? I guess the last one, despite what I said upfront, is the beginning of my suggestion. If we take the quoted change as-is, and then before load_ref_decorations() uses the decoration_filter, perhaps we can see for each pattern in the "exclude" list, if there is the same entry in the "include" list, and remove it from both lists. That way, when the users wonder why their "git log" does not use certain refs to decorate (let's say, you configured "refs/heads/*" in the exclusion list), they can countermand by giving "--decorate-refs" from the command line, perhaps? It is still unclear to me how well such a scheme works, e.g. how should patterns "refs/tags/*" and "refs/tags/*-rc*" interact when they are given as configs and options to include/exclude in various permutations, though. Thanks.
On 4/14/2020 1:19 PM, Junio C Hamano wrote: > "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> if (decoration_style) { >> + const struct string_list *config_exclude = >> + repo_config_get_value_multi(the_repository, >> + "log.excludeDecoration"); >> + >> + if (config_exclude) { >> + struct string_list_item *item; >> + for (item = config_exclude->items; >> + item && item < config_exclude->items + config_exclude->nr; >> + item++) >> + string_list_append(&decorate_refs_exclude, >> + item->string); >> + } >> + >> rev->show_decorations = 1; >> + >> load_ref_decorations(&decoration_filter, decoration_style); >> } > > A few random thoughts. Unlike my other usual reviews, please do not > take "should we do X" as a suggestion (these are purely me wondering > and nothing more at this point): > > * 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. > * The new code only adds to decorate_refs_exclude, which has the > patterns that were given with the "--decorate-refs-exclude" > command line option. As refs.c:ref_filter_match() rejects > anything that match an exclude pattern first before looking at > the include patterns, there is no way to countermand what is > configured to be excluded with the configuration from the command > line, even with --decorate-refs" option. Should we have a new > command line option to "clear" the exclude list read from the > configuration? And if we add configuration for including for > symmetry, should that be cleared as well? > > * As this is a multi-valued configuration, there probably are cases > where you have configured three patterns, and for this single > invocation you would want to override only one of them. It might > not be usable if the only way to override were to "clear" with a > new option and then add two that you want from the command line. > > What if we had (configured) exclusion for X, Y and Z, and then > allowed the command line to say "include Y", that would result in > the combination to specify exclusion of X and Z only? Can we get > away by not having "include these" configuration at all, perhaps, > because "if there is no inclusion pattern, anything that does not > match exclusion patterns is included" is how the matcher works? This is a very good point. We should be able to use command-line options to override configured values. Something like this should show decorations for refs/hidden/origin/master: git -c log.excludeDecoration=refs/hidden/* log --decorate-refs=refs/hidden/* But, the current patch does not. > I guess the last one, despite what I said upfront, is the beginning > of my suggestion. If we take the quoted change as-is, and then > before load_ref_decorations() uses the decoration_filter, perhaps we > can see for each pattern in the "exclude" list, if there is the same > entry in the "include" list, and remove it from both lists. That > way, when the users wonder why their "git log" does not use certain > refs to decorate (let's say, you configured "refs/heads/*" in the > exclusion list), they can countermand by giving "--decorate-refs" > from the command line, perhaps? It is still unclear to me how well > such a scheme works, e.g. how should patterns "refs/tags/*" and > "refs/tags/*-rc*" interact when they are given as configs and > options to include/exclude in various permutations, though. My next version will allow this "overwrite" of configured values. It seems like an important use case that I had missed. Without getting into the code immediately, I predict the change will be to include a second pass of "configured patterns" after the command-line patterns. If the explicit command-line patterns have already included the ref, then the configured exclude patterns should not be tested. Thanks! -Stolee
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.
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. I did a quick check to verify how things currently work with this diff: 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
diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt index e9e1e397f3f..1a158324f79 100644 --- a/Documentation/config/log.txt +++ b/Documentation/config/log.txt @@ -18,6 +18,11 @@ log.decorate:: names are shown. This is the same as the `--decorate` option of the `git log`. +log.excludeDecoration:: + Exclude the specified patterns from the log decorations. This multi- + valued config option is the same as the `--decorate-refs-exclude` + option of `git log`. + log.follow:: If `true`, `git log` will act as if the `--follow` option was used when a single <path> is given. This has the same limitations as `--follow`, diff --git a/builtin/log.c b/builtin/log.c index 83a4a6188e2..d7d1d5b7143 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, } if (decoration_style) { + const struct string_list *config_exclude = + repo_config_get_value_multi(the_repository, + "log.excludeDecoration"); + + if (config_exclude) { + struct string_list_item *item; + for (item = config_exclude->items; + item && item < config_exclude->items + config_exclude->nr; + item++) + string_list_append(&decorate_refs_exclude, + item->string); + } + rev->show_decorations = 1; + load_ref_decorations(&decoration_filter, decoration_style); } diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 0f766ba65f5..b5de449e510 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -787,6 +787,9 @@ test_expect_success 'decorate-refs-exclude with glob' ' EOF git log -n6 --decorate=short --pretty="tformat:%f%d" \ --decorate-refs-exclude="heads/octopus*" >actual && + test_cmp expect.decorate actual && + git -c log.excludeDecoration="heads/octopus*" log \ + -n6 --decorate=short --pretty="tformat:%f%d" >actual && test_cmp expect.decorate actual ' @@ -801,6 +804,9 @@ test_expect_success 'decorate-refs-exclude without globs' ' EOF git log -n6 --decorate=short --pretty="tformat:%f%d" \ --decorate-refs-exclude="tags/reach" >actual && + test_cmp expect.decorate actual && + git -c log.excludeDecoration="tags/reach" log \ + -n6 --decorate=short --pretty="tformat:%f%d" >actual && test_cmp expect.decorate actual ' @@ -816,6 +822,14 @@ test_expect_success 'multiple decorate-refs-exclude' ' git log -n6 --decorate=short --pretty="tformat:%f%d" \ --decorate-refs-exclude="heads/octopus*" \ --decorate-refs-exclude="tags/reach" >actual && + test_cmp expect.decorate actual && + git -c log.excludeDecoration="heads/octopus*" \ + -c log.excludeDecoration="tags/reach" log \ + -n6 --decorate=short --pretty="tformat:%f%d" >actual && + test_cmp expect.decorate actual && + git -c log.excludeDecoration="heads/octopus*" log \ + --decorate-refs-exclude="tags/reach" \ + -n6 --decorate=short --pretty="tformat:%f%d" >actual && test_cmp expect.decorate actual ' @@ -831,6 +845,10 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' ' git log -n6 --decorate=short --pretty="tformat:%f%d" \ --decorate-refs="heads/*" \ --decorate-refs-exclude="heads/oc*" >actual && + test_cmp expect.decorate actual && + git -c log.excludeDecoration="heads/oc*" log \ + --decorate-refs="heads/*" \ + -n6 --decorate=short --pretty="tformat:%f%d" >actual && test_cmp expect.decorate actual ' @@ -846,6 +864,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' ' git log -n6 --decorate=short --pretty="tformat:%f%d" \ --decorate-refs-exclude="*octopus*" \ --simplify-by-decoration >actual && + test_cmp expect.decorate actual && + git -c log.excludeDecoration="*octopus*" log \ + -n6 --decorate=short --pretty="tformat:%f%d" \ + --simplify-by-decoration >actual && test_cmp expect.decorate actual '