{"thread":{"id":"53219","subject":"[PATCH] log: add log.excludeDecoration config option","startedAt":"2020-04-13T15:28:45Z","lastAt":"2020-04-17T02:01:50Z","messageCount":22,"participants":["Derrick Stolee via GitGitGadget","Taylor Blau","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"395346","messageId":"pull.610.git.1586791720114.gitgitgadget@gmail.com","threadId":"53219","inReplyTo":null,"subject":"[PATCH] log: add log.excludeDecoration config option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-13T15:28:39Z","receivedAt":"2020-04-13T15:28:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIn 'git log', the --decorate-refs-exclude option appends a pattern\nto a string_list. This list is used to prevent showing some refs\nin the decoration output, or even by --simplify-by-decoration.\n\nUsers may want to use their refs space to store utility refs that\nshould not appear in the decoration output. For example, Scalar [1]\nruns a background fetch but places the \"new\" refs inside the\nrefs/scalar/hidden//* refspace instead of refs//*\nto avoid updating remote refs when the user is not looking. However,\nthese \"hidden\" refs appear during regular 'git log' queries.\n\nA similar idea to use \"hidden\" refs is under consideration for core\nGit [2].\n\nAdd the 'log.excludeDecoration' config option so users can exclude\nsome refs from decorations by default instead of needing to use\n--decorate-refs-exclude manually. The config value is multi-valued\nmuch like the command-line option.\n\nThere are several tests in t4202-log.sh that test the\n--decorate-refs-(include|exclude) options, so these are extended.\nSince the expected output is already stored as a file, simply add\nnew calls that replace a \"--decorate-refs-exclude\" option with an\nin-line config setting.\n\n[1] https://github.com/microsoft/scalar\n[2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n\nSigned-off-by: Derrick Stolee \n---\n log: add log.excludeDecoration config option\n \n This was something hinted at in the \"fetch\" step of the background\n maintenance RFC. Should be a relatively minor addition to our config\n options.\n \n We definitely want this feature for microsoft/git (we would set\n log.excludeDecoration=refs/scalar/* in all Scalar repos), but we will\n wait for feedback from the community.\n \n Thanks, -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/610\n\n Documentation/config/log.txt | 5 +++++\n builtin/log.c | 14 ++++++++++++++\n t/t4202-log.sh | 22 ++++++++++++++++++++++\n 3 files changed, 41 insertions(+)\n\ndiff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\nindex e9e1e397f3f..1a158324f79 100644\n--- a/Documentation/config/log.txt\n+++ b/Documentation/config/log.txt\n@@ -18,6 +18,11 @@ log.decorate::\n \tnames are shown. This is the same as the `--decorate` option\n \tof the `git log`.\n \n+log.excludeDecoration::\n+\tExclude the specified patterns from the log decorations. This multi-\n+\tvalued config option is the same as the `--decorate-refs-exclude`\n+\toption of `git log`.\n+\n log.follow::\n \tIf `true`, `git log` will act as if the `--follow` option was used when\n \ta single is given. This has the same limitations as `--follow`,\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 83a4a6188e2..d7d1d5b7143 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t}\n \n \tif (decoration_style) {\n+\t\tconst struct string_list *config_exclude =\n+\t\t\trepo_config_get_value_multi(the_repository,\n+\t\t\t\t\t\t \"log.excludeDecoration\");\n+\n+\t\tif (config_exclude) {\n+\t\t\tstruct string_list_item *item;\n+\t\t\tfor (item = config_exclude->items;\n+\t\t\t item && item < config_exclude->items + config_exclude->nr;\n+\t\t\t item++)\n+\t\t\t\tstring_list_append(&decorate_refs_exclude,\n+\t\t\t\t\t\titem->string);\n+\t\t}\n+\n \t\trev->show_decorations = 1;\n+\n \t\tload_ref_decorations(&decoration_filter, decoration_style);\n \t}\n \ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 0f766ba65f5..b5de449e510 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -787,6 +787,9 @@ test_expect_success 'decorate-refs-exclude with glob' '\n \tEOF\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"heads/octopus*\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -801,6 +804,9 @@ test_expect_success 'decorate-refs-exclude without globs' '\n \tEOF\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"tags/reach\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -816,6 +822,14 @@ test_expect_success 'multiple decorate-refs-exclude' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" \\\n+\t\t-c log.excludeDecoration=\"tags/reach\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t--decorate-refs-exclude=\"tags/reach\" \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -831,6 +845,10 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs=\"heads/*\" \\\n \t\t--decorate-refs-exclude=\"heads/oc*\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/oc*\" log \\\n+\t\t--decorate-refs=\"heads/*\" \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -846,6 +864,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"*octopus*\" \\\n \t\t--simplify-by-decoration >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"*octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--simplify-by-decoration >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"395347","messageId":"20200413154945.GA59601@syl.local","threadId":"53219","inReplyTo":"pull.610.git.1586791720114.gitgitgadget@gmail.com","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-13T15:49:45Z","receivedAt":"2020-04-13T15:49:50Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Stolee,\n\nOn Mon, Apr 13, 2020 at 03:28:39PM +0000, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n>\n> In 'git log', the --decorate-refs-exclude option appends a pattern\n> to a string_list. This list is used to prevent showing some refs\n> in the decoration output, or even by --simplify-by-decoration.\n>\n> Users may want to use their refs space to store utility refs that\n> should not appear in the decoration output. For example, Scalar [1]\n> runs a background fetch but places the \"new\" refs inside the\n> refs/scalar/hidden//* refspace instead of refs//*\n> to avoid updating remote refs when the user is not looking. However,\n> these \"hidden\" refs appear during regular 'git log' queries.\n>\n> A similar idea to use \"hidden\" refs is under consideration for core\n> Git [2].\n>\n> Add the 'log.excludeDecoration' config option so users can exclude\n> some refs from decorations by default instead of needing to use\n> --decorate-refs-exclude manually. The config value is multi-valued\n> much like the command-line option.\n>\n> There are several tests in t4202-log.sh that test the\n> --decorate-refs-(include|exclude) options, so these are extended.\n> Since the expected output is already stored as a file, simply add\n> new calls that replace a \"--decorate-refs-exclude\" option with an\n> in-line config setting.\n>\n> [1] https://github.com/microsoft/scalar\n> [2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> log: add log.excludeDecoration config option\n>\n> This was something hinted at in the \"fetch\" step of the background\n> maintenance RFC. Should be a relatively minor addition to our config\n> options.\n>\n> We definitely want this feature for microsoft/git (we would set\n> log.excludeDecoration=refs/scalar/* in all Scalar repos), but we will\n> wait for feedback from the community.\n>\n> Thanks, -Stolee\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/610\n>\n> Documentation/config/log.txt | 5 +++++\n> builtin/log.c | 14 ++++++++++++++\n> t/t4202-log.sh | 22 ++++++++++++++++++++++\n> 3 files changed, 41 insertions(+)\n>\n> diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\n> index e9e1e397f3f..1a158324f79 100644\n> --- a/Documentation/config/log.txt\n> +++ b/Documentation/config/log.txt\n> @@ -18,6 +18,11 @@ log.decorate::\n> \tnames are shown. This is the same as the `--decorate` option\n> \tof the `git log`.\n>\n> +log.excludeDecoration::\n> +\tExclude the specified patterns from the log decorations. This multi-\n> +\tvalued config option is the same as the `--decorate-refs-exclude`\n> +\toption of `git log`.\n> +\n\nThanks for this documentation update. Do you think that it would be\nworth updating the entry for '--decorate-refs-exclude', too? I figure\nthat it would be good, since scripters who look at 'git log's man page\nbefore 'git config's would have a trail to follow in case they want a\npersistent '--decorate-refs-exclude' option.\n\n> log.follow::\n> \tIf `true`, `git log` will act as if the `--follow` option was used when\n> \ta single is given. This has the same limitations as `--follow`,\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 83a4a6188e2..d7d1d5b7143 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n> \t}\n>\n> \tif (decoration_style) {\n> +\t\tconst struct string_list *config_exclude =\n> +\t\t\trepo_config_get_value_multi(the_repository,\n> +\t\t\t\t\t\t \"log.excludeDecoration\");\n> +\n> +\t\tif (config_exclude) {\n> +\t\t\tstruct string_list_item *item;\n> +\t\t\tfor (item = config_exclude->items;\n> +\t\t\t item && item < config_exclude->items + config_exclude->nr;\n> +\t\t\t item++)\n\nAny reason not to use the 'for_each_string_list_item' macro in\n'string-list.h' for this?\n\n> +\t\t\t\tstring_list_append(&decorate_refs_exclude,\n> +\t\t\t\t\t\titem->string);\n> +\t\t}\n> +\n\nOn my first read, I thought that there might be some value in using a\nhash set here, since I (incorrectly) thought that only literal reference\nnames were added here, in which case we'd benefit from a constant time\nlookup.\n\nBut, since we store patterns here, it's not helpful for us to have a\nconstant time lookup, since we don't have anything to look up in the\nfirst place! I guess you could expand out all of these patterns ahead of\ntime, but I don't think that this is universally a good thing to do. We\nmay have a pattern like 'refs/heads/*', but we're only doing a 'git log'\nover a part of history that doesn't have many/any refs pointing at it,\nin which case expanding ahead of time was a bad thing to do.\n\nOf course, 'string_list_append' is going to potentially introduce\nduplicate patterns, but I don't think that should be a huge problem,\nsince 'ref_filter_match' stops as soon as it finds an exclude/include. I\nguess you could use 'string_list_insert', but it makes insertions more\nexpensive without a clear benefit.\n\n> \t\trev->show_decorations = 1;\n> +\n> \t\tload_ref_decorations(&decoration_filter, decoration_style);\n> \t}\n>\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 0f766ba65f5..b5de449e510 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -787,6 +787,9 @@ test_expect_success 'decorate-refs-exclude with glob' '\n> \tEOF\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"heads/octopus*\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> @@ -801,6 +804,9 @@ test_expect_success 'decorate-refs-exclude without globs' '\n> \tEOF\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"tags/reach\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> @@ -816,6 +822,14 @@ test_expect_success 'multiple decorate-refs-exclude' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n> \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" \\\n> +\t\t-c log.excludeDecoration=\"tags/reach\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t--decorate-refs-exclude=\"tags/reach\" \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> @@ -831,6 +845,10 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs=\"heads/*\" \\\n> \t\t--decorate-refs-exclude=\"heads/oc*\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/oc*\" log \\\n> +\t\t--decorate-refs=\"heads/*\" \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> @@ -846,6 +864,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"*octopus*\" \\\n> \t\t--simplify-by-decoration >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"*octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--simplify-by-decoration >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n\nThanks for the tests, they look good. I like your strategy of repeating\nthe test with and without the new config settings.\n\n>\n> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n> --\n> gitgitgadget\n\nThanks,\nTaylor\n"},{"id":"395414","messageId":"0b9e3156-1101-0f19-91eb-36af541519aa@gmail.com","threadId":"53219","inReplyTo":"20200413154945.GA59601@syl.local","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-14T15:10:33Z","receivedAt":"2020-04-14T15:10:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/13/2020 11:49 AM, Taylor Blau wrote:\n> Hi Stolee,\n> \n> On Mon, Apr 13, 2020 at 03:28:39PM +0000, Derrick Stolee via GitGitGadget wrote:\n>> From: Derrick Stolee \n>>\n>> In 'git log', the --decorate-refs-exclude option appends a pattern\n>> to a string_list. This list is used to prevent showing some refs\n>> in the decoration output, or even by --simplify-by-decoration.\n>>\n>> Users may want to use their refs space to store utility refs that\n>> should not appear in the decoration output. For example, Scalar [1]\n>> runs a background fetch but places the \"new\" refs inside the\n>> refs/scalar/hidden//* refspace instead of refs//*\n>> to avoid updating remote refs when the user is not looking. However,\n>> these \"hidden\" refs appear during regular 'git log' queries.\n>>\n>> A similar idea to use \"hidden\" refs is under consideration for core\n>> Git [2].\n>>\n>> Add the 'log.excludeDecoration' config option so users can exclude\n>> some refs from decorations by default instead of needing to use\n>> --decorate-refs-exclude manually. The config value is multi-valued\n>> much like the command-line option.\n>>\n>> There are several tests in t4202-log.sh that test the\n>> --decorate-refs-(include|exclude) options, so these are extended.\n>> Since the expected output is already stored as a file, simply add\n>> new calls that replace a \"--decorate-refs-exclude\" option with an\n>> in-line config setting.\n>>\n>> [1] https://github.com/microsoft/scalar\n>> [2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n>>\n>> Signed-off-by: Derrick Stolee \n>> ---\n>> log: add log.excludeDecoration config option\n>>\n>> This was something hinted at in the \"fetch\" step of the background\n>> maintenance RFC. Should be a relatively minor addition to our config\n>> options.\n>>\n>> We definitely want this feature for microsoft/git (we would set\n>> log.excludeDecoration=refs/scalar/* in all Scalar repos), but we will\n>> wait for feedback from the community.\n>>\n>> Thanks, -Stolee\n>>\n>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/610\n>>\n>> Documentation/config/log.txt | 5 +++++\n>> builtin/log.c | 14 ++++++++++++++\n>> t/t4202-log.sh | 22 ++++++++++++++++++++++\n>> 3 files changed, 41 insertions(+)\n>>\n>> diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\n>> index e9e1e397f3f..1a158324f79 100644\n>> --- a/Documentation/config/log.txt\n>> +++ b/Documentation/config/log.txt\n>> @@ -18,6 +18,11 @@ log.decorate::\n>> \tnames are shown. This is the same as the `--decorate` option\n>> \tof the `git log`.\n>>\n>> +log.excludeDecoration::\n>> +\tExclude the specified patterns from the log decorations. This multi-\n>> +\tvalued config option is the same as the `--decorate-refs-exclude`\n>> +\toption of `git log`.\n>> +\n> \n> Thanks for this documentation update. Do you think that it would be\n> worth updating the entry for '--decorate-refs-exclude', too? I figure\n> that it would be good, since scripters who look at 'git log's man page\n> before 'git config's would have a trail to follow in case they want a\n> persistent '--decorate-refs-exclude' option.\n\nThat seems like a good way to help users.\n\n>> log.follow::\n>> \tIf `true`, `git log` will act as if the `--follow` option was used when\n>> \ta single is given. This has the same limitations as `--follow`,\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 83a4a6188e2..d7d1d5b7143 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>> \t}\n>>\n>> \tif (decoration_style) {\n>> +\t\tconst struct string_list *config_exclude =\n>> +\t\t\trepo_config_get_value_multi(the_repository,\n>> +\t\t\t\t\t\t \"log.excludeDecoration\");\n>> +\n>> +\t\tif (config_exclude) {\n>> +\t\t\tstruct string_list_item *item;\n>> +\t\t\tfor (item = config_exclude->items;\n>> +\t\t\t item && item < config_exclude->items + config_exclude->nr;\n>> +\t\t\t item++)\n> \n> Any reason not to use the 'for_each_string_list_item' macro in\n> 'string-list.h' for this?\n\nThe reason is I forgot about it.\n\nThanks,\n-Stolee\n"},{"id":"395418","messageId":"20200414154513.GB93424@syl.local","threadId":"53219","inReplyTo":"0b9e3156-1101-0f19-91eb-36af541519aa@gmail.com","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-14T15:45:13Z","receivedAt":"2020-04-14T15:45:35Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Apr 14, 2020 at 11:10:33AM -0400, Derrick Stolee wrote:\n> >> log.follow::\n> >> \tIf `true`, `git log` will act as if the `--follow` option was used when\n> >> \ta single is given. This has the same limitations as `--follow`,\n> >> diff --git a/builtin/log.c b/builtin/log.c\n> >> index 83a4a6188e2..d7d1d5b7143 100644\n> >> --- a/builtin/log.c\n> >> +++ b/builtin/log.c\n> >> @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n> >> \t}\n> >>\n> >> \tif (decoration_style) {\n> >> +\t\tconst struct string_list *config_exclude =\n> >> +\t\t\trepo_config_get_value_multi(the_repository,\n> >> +\t\t\t\t\t\t \"log.excludeDecoration\");\n> >> +\n> >> +\t\tif (config_exclude) {\n> >> +\t\t\tstruct string_list_item *item;\n> >> +\t\t\tfor (item = config_exclude->items;\n> >> +\t\t\t item && item < config_exclude->items + config_exclude->nr;\n> >> +\t\t\t item++)\n> >\n> > Any reason not to use the 'for_each_string_list_item' macro in\n> > 'string-list.h' for this?\n>\n> The reason is I forgot about it.\n\nHeh, in fairness I forgot about it, too :). I thought that this code\nlooked familiar, but it was only luck that I had 'string-list.h' open at\nthe time I was reading this.\n\nI don't think that it really matters much each way, but if you're\nalready re-rolling...\n\n> Thanks,\n> -Stolee\n\nThanks,\nTaylor\n"},{"id":"395420","messageId":"31e24f9f-44b0-d0f7-3d81-12365edf7a40@gmail.com","threadId":"53219","inReplyTo":"20200414154513.GB93424@syl.local","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-14T16:00:21Z","receivedAt":"2020-04-14T16:00:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/14/2020 11:45 AM, Taylor Blau wrote:\n> On Tue, Apr 14, 2020 at 11:10:33AM -0400, Derrick Stolee wrote:\n>>>> log.follow::\n>>>> \tIf `true`, `git log` will act as if the `--follow` option was used when\n>>>> \ta single is given. This has the same limitations as `--follow`,\n>>>> diff --git a/builtin/log.c b/builtin/log.c\n>>>> index 83a4a6188e2..d7d1d5b7143 100644\n>>>> --- a/builtin/log.c\n>>>> +++ b/builtin/log.c\n>>>> @@ -236,7 +236,21 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>>>> \t}\n>>>>\n>>>> \tif (decoration_style) {\n>>>> +\t\tconst struct string_list *config_exclude =\n>>>> +\t\t\trepo_config_get_value_multi(the_repository,\n>>>> +\t\t\t\t\t\t \"log.excludeDecoration\");\n>>>> +\n>>>> +\t\tif (config_exclude) {\n>>>> +\t\t\tstruct string_list_item *item;\n>>>> +\t\t\tfor (item = config_exclude->items;\n>>>> +\t\t\t item && item < config_exclude->items + config_exclude->nr;\n>>>> +\t\t\t item++)\n>>>\n>>> Any reason not to use the 'for_each_string_list_item' macro in\n>>> 'string-list.h' for this?\n>>\n>> The reason is I forgot about it.\n> \n> Heh, in fairness I forgot about it, too :). I thought that this code\n> looked familiar, but it was only luck that I had 'string-list.h' open at\n> the time I was reading this.\n> \n> I don't think that it really matters much each way, but if you're\n> already re-rolling...\n\nThis is probably worth a re-roll on its own. I'll give the patch\nsome extra time to simmer before pushing a v2, though.\n\n-Stolee\n"},{"id":"395424","messageId":"xmqqeesq9e8p.fsf@gitster.c.googlers.com","threadId":"53219","inReplyTo":"pull.610.git.1586791720114.gitgitgadget@gmail.com","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-14T17:19:34Z","receivedAt":"2020-04-14T17:19:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> \tif (decoration_style) {\n> +\t\tconst struct string_list *config_exclude =\n> +\t\t\trepo_config_get_value_multi(the_repository,\n> +\t\t\t\t\t\t \"log.excludeDecoration\");\n> +\n> +\t\tif (config_exclude) {\n> +\t\t\tstruct string_list_item *item;\n> +\t\t\tfor (item = config_exclude->items;\n> +\t\t\t item && item < config_exclude->items + config_exclude->nr;\n> +\t\t\t item++)\n> +\t\t\t\tstring_list_append(&decorate_refs_exclude,\n> +\t\t\t\t\t\titem->string);\n> +\t\t}\n> +\n> \t\trev->show_decorations = 1;\n> +\n> \t\tload_ref_decorations(&decoration_filter, decoration_style);\n> \t}\n\nA few random thoughts. Unlike my other usual reviews, please do not\ntake \"should we do X\" as a suggestion (these are purely me wondering\nand nothing more at this point):\n\n * Given that we have command line options to specify what patterns\n to include as well as to exclude, it feels somewhat asymmetric to\n have only the configuration to exclude. Should we also have a\n configuration for including?\n\n * The new code only adds to decorate_refs_exclude, which has the\n patterns that were given with the \"--decorate-refs-exclude\"\n command line option. As refs.c:ref_filter_match() rejects\n anything that match an exclude pattern first before looking at\n the include patterns, there is no way to countermand what is\n configured to be excluded with the configuration from the command\n line, even with --decorate-refs\" option. Should we have a new\n command line option to \"clear\" the exclude list read from the\n configuration? And if we add configuration for including for\n symmetry, should that be cleared as well?\n\n * As this is a multi-valued configuration, there probably are cases\n where you have configured three patterns, and for this single\n invocation you would want to override only one of them. It might\n not be usable if the only way to override were to \"clear\" with a\n new option and then add two that you want from the command line.\n\nWhat if we had (configured) exclusion for X, Y and Z, and then\nallowed the command line to say \"include Y\", that would result in\nthe combination to specify exclusion of X and Z only? Can we get\naway by not having \"include these\" configuration at all, perhaps,\nbecause \"if there is no inclusion pattern, anything that does not\nmatch exclusion patterns is included\" is how the matcher works?\n\nI guess the last one, despite what I said upfront, is the beginning\nof my suggestion. If we take the quoted change as-is, and then\nbefore load_ref_decorations() uses the decoration_filter, perhaps we\ncan see for each pattern in the \"exclude\" list, if there is the same\nentry in the \"include\" list, and remove it from both lists. That\nway, when the users wonder why their \"git log\" does not use certain\nrefs to decorate (let's say, you configured \"refs/heads/*\" in the\nexclusion list), they can countermand by giving \"--decorate-refs\"\nfrom the command line, perhaps? It is still unclear to me how well\nsuch a scheme works, e.g. how should patterns \"refs/tags/*\" and\n\"refs/tags/*-rc*\" interact when they are given as configs and\noptions to include/exclude in various permutations, though.\n\nThanks.\n\n\n\n"},{"id":"395427","messageId":"5c8cd2dc-f1e2-5c93-094c-e15e45e8543e@gmail.com","threadId":"53219","inReplyTo":"xmqqeesq9e8p.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-14T17:49:47Z","receivedAt":"2020-04-14T17:50:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/14/2020 1:19 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> \tif (decoration_style) {\n>> +\t\tconst struct string_list *config_exclude =\n>> +\t\t\trepo_config_get_value_multi(the_repository,\n>> +\t\t\t\t\t\t \"log.excludeDecoration\");\n>> +\n>> +\t\tif (config_exclude) {\n>> +\t\t\tstruct string_list_item *item;\n>> +\t\t\tfor (item = config_exclude->items;\n>> +\t\t\t item && item < config_exclude->items + config_exclude->nr;\n>> +\t\t\t item++)\n>> +\t\t\t\tstring_list_append(&decorate_refs_exclude,\n>> +\t\t\t\t\t\titem->string);\n>> +\t\t}\n>> +\n>> \t\trev->show_decorations = 1;\n>> +\n>> \t\tload_ref_decorations(&decoration_filter, decoration_style);\n>> \t}\n> \n> A few random thoughts. Unlike my other usual reviews, please do not\n> take \"should we do X\" as a suggestion (these are purely me wondering\n> and nothing more at this point):\n> \n> * Given that we have command line options to specify what patterns\n> to include as well as to exclude, it feels somewhat asymmetric to\n> have only the configuration to exclude. Should we also have a\n> configuration for including?\n\nI left the other side out for simplicity and because I didn't know\nthe use case. It seems all refs are included by default.\n\n> * The new code only adds to decorate_refs_exclude, which has the\n> patterns that were given with the \"--decorate-refs-exclude\"\n> command line option. As refs.c:ref_filter_match() rejects\n> anything that match an exclude pattern first before looking at\n> the include patterns, there is no way to countermand what is\n> configured to be excluded with the configuration from the command\n> line, even with --decorate-refs\" option. Should we have a new\n> command line option to \"clear\" the exclude list read from the\n> configuration? And if we add configuration for including for\n> symmetry, should that be cleared as well?\n>\n> * As this is a multi-valued configuration, there probably are cases\n> where you have configured three patterns, and for this single\n> invocation you would want to override only one of them. It might\n> not be usable if the only way to override were to \"clear\" with a\n> new option and then add two that you want from the command line.\n> \n> What if we had (configured) exclusion for X, Y and Z, and then\n> allowed the command line to say \"include Y\", that would result in\n> the combination to specify exclusion of X and Z only? Can we get\n> away by not having \"include these\" configuration at all, perhaps,\n> because \"if there is no inclusion pattern, anything that does not\n> match exclusion patterns is included\" is how the matcher works?\n\nThis is a very good point. We should be able to use command-line\noptions to override configured values. Something like this should\nshow decorations for refs/hidden/origin/master:\n\n git -c log.excludeDecoration=refs/hidden/* log --decorate-refs=refs/hidden/*\n\nBut, the current patch does not.\n \n> I guess the last one, despite what I said upfront, is the beginning\n> of my suggestion. If we take the quoted change as-is, and then\n> before load_ref_decorations() uses the decoration_filter, perhaps we\n> can see for each pattern in the \"exclude\" list, if there is the same\n> entry in the \"include\" list, and remove it from both lists. That\n> way, when the users wonder why their \"git log\" does not use certain\n> refs to decorate (let's say, you configured \"refs/heads/*\" in the\n> exclusion list), they can countermand by giving \"--decorate-refs\"\n> from the command line, perhaps? It is still unclear to me how well\n> such a scheme works, e.g. how should patterns \"refs/tags/*\" and\n> \"refs/tags/*-rc*\" interact when they are given as configs and\n> options to include/exclude in various permutations, though.\n\nMy next version will allow this \"overwrite\" of configured values.\nIt seems like an important use case that I had missed.\n\nWithout getting into the code immediately, I predict the change\nwill be to include a second pass of \"configured patterns\" after\nthe command-line patterns. If the explicit command-line patterns\nhave already included the ref, then the configured exclude\npatterns should not be tested.\n \nThanks!\n-Stolee\n"},{"id":"395429","messageId":"xmqqtv1m7xc5.fsf@gitster.c.googlers.com","threadId":"53219","inReplyTo":"5c8cd2dc-f1e2-5c93-094c-e15e45e8543e@gmail.com","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-14T18:10:02Z","receivedAt":"2020-04-14T18:10:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n>> * Given that we have command line options to specify what patterns\n>> to include as well as to exclude, it feels somewhat asymmetric to\n>> have only the configuration to exclude. Should we also have a\n>> configuration for including?\n>\n> I left the other side out for simplicity and because I didn't know\n> the use case. It seems all refs are included by default.\n\nIt is a bit more subtle than that. \n\nOnce you have an inclusion pattern, nothing is included by default.\nOnly the ones that match an inclusion pattern would be considered.\nWhen there is no inclusion pattern, we behave as if there is a\nmatch-all inclusion pattern.\n"},{"id":"395493","messageId":"707d61f5-02fe-73a9-0719-d7eece4669b4@gmail.com","threadId":"53219","inReplyTo":"xmqqtv1m7xc5.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] log: add log.excludeDecoration config option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-15T14:14:14Z","receivedAt":"2020-04-15T14:14:33Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/14/2020 2:10 PM, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n>>> * Given that we have command line options to specify what patterns\n>>> to include as well as to exclude, it feels somewhat asymmetric to\n>>> have only the configuration to exclude. Should we also have a\n>>> configuration for including?\n>>\n>> I left the other side out for simplicity and because I didn't know\n>> the use case. It seems all refs are included by default.\n> \n> It is a bit more subtle than that. \n> \n> Once you have an inclusion pattern, nothing is included by default.\n> Only the ones that match an inclusion pattern would be considered.\n> When there is no inclusion pattern, we behave as if there is a\n> match-all inclusion pattern.\n\nI did a quick check to verify how things currently work with\nthis diff:\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex b5de449e51..e9c9e59461 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -742,8 +742,24 @@ test_expect_success 'decorate-refs with glob' '\n octopus-a (octopus-a)\n reach\n EOF\n+ cat >expect.no-decorate <<-\\EOF &&\n+ Merge-tag-reach\n+ Merge-tags-octopus-a-and-octopus-b\n+ seventh\n+ octopus-b\n+ octopus-a\n+ reach\n+ EOF\n git log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n --decorate-refs=\"heads/octopus*\" >actual &&\n+ test_cmp expect.decorate actual &&\n+ git log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+ --decorate-refs-exclude=\"heads/octopus*\" \\\n+ --decorate-refs=\"heads/octopus*\" >actual &&\n+ test_cmp expect.no-decorate actual &&\n+ git -c log.excludeDecoration=\"heads/octopus*\" log \\\n+ -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+ --decorate-refs=\"heads/octopus*\" >actual &&\n test_cmp expect.decorate actual\n '\n\nThis test fails at the last test_cmp with the current\npatch.\n\nNote that if we have both --decorate-refs-exclude=X and\n--decorate-refs=X, then the exclusion wins. This means that\nwe will need to split the \"configured\" exclusions from the\n\"command-line\" exclusions and give them different priority.\n\nBut, I believe that if we can get this test to pass, then\nwe will have the correct inclusion/exclusion logic. I will\nget started on this right now.\n\nThanks,\n-Stolee\n"},{"id":"395505","messageId":"pull.610.v2.git.1586965463222.gitgitgadget@gmail.com","threadId":"53219","inReplyTo":"pull.610.git.1586791720114.gitgitgadget@gmail.com","subject":"[PATCH v2] log: add log.excludeDecoration config option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-15T15:44:22Z","receivedAt":"2020-04-15T15:44:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIn 'git log', the --decorate-refs-exclude option appends a pattern\nto a string_list. This list is used to prevent showing some refs\nin the decoration output, or even by --simplify-by-decoration.\n\nUsers may want to use their refs space to store utility refs that\nshould not appear in the decoration output. For example, Scalar [1]\nruns a background fetch but places the \"new\" refs inside the\nrefs/scalar/hidden//* refspace instead of refs//*\nto avoid updating remote refs when the user is not looking. However,\nthese \"hidden\" refs appear during regular 'git log' queries.\n\nA similar idea to use \"hidden\" refs is under consideration for core\nGit [2].\n\nAdd the 'log.excludeDecoration' config option so users can exclude\nsome refs from decorations by default instead of needing to use\n--decorate-refs-exclude manually. The config value is multi-valued\nmuch like the command-line option. The documentation is careful to\npoint out that the config value can be overridden by the\n--decorate-refs option, even though --decorate-refs-exclude would\nalways \"win\" over --decorate-refs.\n\nSince the 'log.excludeDecoration' takes lower precedence to\n--decorate-refs, and --decorate-refs-exclude takes higher\nprecedence, the struct decoration_filter needed another field.\nThis led also to new logic in load_ref_decorations() and\nref_filter_match().\n\nThere are several tests in t4202-log.sh that test the\n--decorate-refs-(include|exclude) options, so these are extended.\nSince the expected output is already stored as a file, most tests\ncould simply replace a \"--decorate-refs-exclude\" option with an\nin-line config setting. Other tests involve the precedence of\nthe config option compared to command-line options and needed more\nmodification.\n\n[1] https://github.com/microsoft/scalar\n[2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n\nSigned-off-by: Derrick Stolee \n---\n log: add log.excludeDecoration config option\n \n This was something hinted at in the \"fetch\" step of the background\n maintenance RFC. Should be a relatively minor addition to our config\n options.\n \n We definitely want this feature for microsoft/git (we would set\n log.excludeDecoration=refs/scalar/* in all Scalar repos), but we will\n wait for feedback from the community.\n \n Updates in v2:\n \n * Use for_each_string_list_item()\n \n \n * Update the matching logic to allow --decorate-refs to override the\n config option.\n \n \n \n Thanks, -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/610\n\nRange-diff vs v1:\n\n 1: 6aa81243c1f ! 1: cbdaef4a8e1 log: add log.excludeDecoration config option\n @@ Commit message\n Add the 'log.excludeDecoration' config option so users can exclude\n some refs from decorations by default instead of needing to use\n --decorate-refs-exclude manually. The config value is multi-valued\n - much like the command-line option.\n + much like the command-line option. The documentation is careful to\n + point out that the config value can be overridden by the\n + --decorate-refs option, even though --decorate-refs-exclude would\n + always \"win\" over --decorate-refs.\n +\n + Since the 'log.excludeDecoration' takes lower precedence to\n + --decorate-refs, and --decorate-refs-exclude takes higher\n + precedence, the struct decoration_filter needed another field.\n + This led also to new logic in load_ref_decorations() and\n + ref_filter_match().\n \n There are several tests in t4202-log.sh that test the\n --decorate-refs-(include|exclude) options, so these are extended.\n - Since the expected output is already stored as a file, simply add\n - new calls that replace a \"--decorate-refs-exclude\" option with an\n - in-line config setting.\n + Since the expected output is already stored as a file, most tests\n + could simply replace a \"--decorate-refs-exclude\" option with an\n + in-line config setting. Other tests involve the precedence of\n + the config option compared to command-line options and needed more\n + modification.\n \n [1] https://github.com/microsoft/scalar\n [2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n @@ Documentation/config/log.txt: log.decorate::\n \tIf `true`, `git log` will act as if the `--follow` option was used when\n \ta single is given. This has the same limitations as `--follow`,\n \n + ## Documentation/git-log.txt ##\n +@@ Documentation/git-log.txt: OPTIONS\n + \tIf no `--decorate-refs` is given, pretend as if all refs were\n + \tincluded. For each candidate, do not use it for decoration if it\n + \tmatches any patterns given to `--decorate-refs-exclude` or if it\n +-\tdoesn't match any of the patterns given to `--decorate-refs`.\n ++\tdoesn't match any of the patterns given to `--decorate-refs`. The\n ++\t`log.excludeDecoration` config option allows excluding refs from\n ++\tthe decorations, but an explicit `--decorate-refs` pattern will\n ++\toverride a match in `log.excludeDecoration`.\n + \n + --source::\n + \tPrint out the ref name given on the command line by which each\n +\n ## builtin/log.c ##\n +@@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n + \tint quiet = 0, source = 0, mailmap;\n + \tstatic struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};\n + \tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n ++\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n + \tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n + \tstruct decoration_filter decoration_filter = {&decorate_refs_include,\n +-\t\t\t\t\t\t &decorate_refs_exclude};\n ++\t\t\t\t\t\t &decorate_refs_exclude,\n ++\t\t\t\t\t\t &decorate_refs_exclude_config};\n + \tstatic struct revision_sources revision_sources;\n + \n + \tconst struct option builtin_log_options[] = {\n @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t}\n \n @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons\n +\n +\t\tif (config_exclude) {\n +\t\t\tstruct string_list_item *item;\n -+\t\t\tfor (item = config_exclude->items;\n -+\t\t\t item && item < config_exclude->items + config_exclude->nr;\n -+\t\t\t item++)\n -+\t\t\t\tstring_list_append(&decorate_refs_exclude,\n -+\t\t\t\t\t\titem->string);\n ++\t\t\tfor_each_string_list_item(item, config_exclude)\n ++\t\t\t\tstring_list_append(&decorate_refs_exclude_config,\n ++\t\t\t\t\t\t item->string);\n +\t\t}\n +\n \t\trev->show_decorations = 1;\n @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons\n \t}\n \n \n + ## log-tree.c ##\n +@@ log-tree.c: static int add_ref_decoration(const char *refname, const struct object_id *oid,\n + \n + \tif (filter && !ref_filter_match(refname,\n + \t\t\t filter->include_ref_pattern,\n +-\t\t\t filter->exclude_ref_pattern))\n ++\t\t\t filter->exclude_ref_pattern,\n ++\t\t\t filter->exclude_ref_config_pattern))\n + \t\treturn 0;\n + \n + \tif (starts_with(refname, git_replace_ref_base)) {\n +@@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flags)\n + \t\t\tfor_each_string_list_item(item, filter->include_ref_pattern) {\n + \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n + \t\t\t}\n ++\t\t\tfor_each_string_list_item(item, filter->exclude_ref_config_pattern) {\n ++\t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n ++\t\t\t}\n + \t\t}\n + \t\tdecoration_loaded = 1;\n + \t\tdecoration_flags = flags;\n +\n + ## log-tree.h ##\n +@@ log-tree.h: struct log_info {\n + };\n + \n + struct decoration_filter {\n +-\tstruct string_list *include_ref_pattern, *exclude_ref_pattern;\n ++\tstruct string_list *include_ref_pattern;\n ++\tstruct string_list *exclude_ref_pattern;\n ++\tstruct string_list *exclude_ref_config_pattern;\n + };\n + \n + int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);\n +\n + ## refs.c ##\n +@@ refs.c: static int match_ref_pattern(const char *refname,\n + \n + int ref_filter_match(const char *refname,\n + \t\t const struct string_list *include_patterns,\n +-\t\t const struct string_list *exclude_patterns)\n ++\t\t const struct string_list *exclude_patterns,\n ++\t\t const struct string_list *exclude_patterns_config)\n + {\n + \tstruct string_list_item *item;\n ++\tint found = 0;\n + \n + \tif (exclude_patterns && exclude_patterns->nr) {\n + \t\tfor_each_string_list_item(item, exclude_patterns) {\n +@@ refs.c: int ref_filter_match(const char *refname,\n + \t}\n + \n + \tif (include_patterns && include_patterns->nr) {\n +-\t\tint found = 0;\n + \t\tfor_each_string_list_item(item, include_patterns) {\n + \t\t\tif (match_ref_pattern(refname, item)) {\n + \t\t\t\tfound = 1;\n +@@ refs.c: int ref_filter_match(const char *refname,\n + \t\tif (!found)\n + \t\t\treturn 0;\n + \t}\n ++\n ++\tif (!found &&\n ++\t exclude_patterns_config &&\n ++\t exclude_patterns_config->nr) {\n ++\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n ++\t\t\tif (match_ref_pattern(refname, item))\n ++\t\t\t\treturn 0;\n ++\t\t}\n ++\t}\n ++\n + \treturn 1;\n + }\n + \n +\n + ## refs.h ##\n +@@ refs.h: void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n + \t\t\tconst char *pattern);\n + \n + /*\n +- * Returns 0 if refname matches any of the exclude_patterns, or if it doesn't\n +- * match any of the include_patterns. Returns 1 otherwise.\n ++ * Returns 0 if the refname matches any of the exclude_patterns.\n ++ *\n ++ * Returns 0 if include_patterns is non-empty but refname does not match\n ++ * any of those patterns.\n ++ *\n ++ * Returns 0 if refname matches a pattern in exclude_patterns_config but\n ++ * does not match any pattern in inclue_patterns.\n ++ *\n ++ * Otherwise, returns 1.\n + *\n +- * If pattern list is NULL or empty, matching against that list is skipped.\n + * This has the effect of matching everything by default, unless the user\n + * specifies rules otherwise.\n + */\n + int ref_filter_match(const char *refname,\n + \t\t const struct string_list *include_patterns,\n +-\t\t const struct string_list *exclude_patterns);\n ++\t\t const struct string_list *exclude_patterns,\n ++\t\t const struct string_list *exclude_patterns_config);\n + \n + static inline const char *has_glob_specials(const char *pattern)\n + {\n +\n ## t/t4202-log.sh ##\n +@@ t/t4202-log.sh: test_expect_success 'decorate-refs with glob' '\n + \toctopus-a (octopus-a)\n + \treach\n + \tEOF\n ++\tcat >expect.no-decorate <<-\\EOF &&\n ++\tMerge-tag-reach\n ++\tMerge-tags-octopus-a-and-octopus-b\n ++\tseventh\n ++\toctopus-b\n ++\toctopus-a\n ++\treach\n ++\tEOF\n ++\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n ++\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n ++\ttest_cmp expect.decorate actual &&\n + \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n ++\t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n ++\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n ++\ttest_cmp expect.no-decorate actual &&\n ++\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n ++\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n + \t\t--decorate-refs=\"heads/octopus*\" >actual &&\n + \ttest_cmp expect.decorate actual\n + '\n @@ t/t4202-log.sh: test_expect_success 'decorate-refs-exclude with glob' '\n \tEOF\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n @@ t/t4202-log.sh: test_expect_success 'multiple decorate-refs-exclude' '\n \ttest_cmp expect.decorate actual\n '\n \n + test_expect_success 'decorate-refs and decorate-refs-exclude' '\n +-\tcat >expect.decorate <<-\\EOF &&\n ++\tcat >expect.no-decorate <<-\\EOF &&\n + \tMerge-tag-reach (master)\n + \tMerge-tags-octopus-a-and-octopus-b\n + \tseventh\n @@ t/t4202-log.sh: test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs=\"heads/*\" \\\n \t\t--decorate-refs-exclude=\"heads/oc*\" >actual &&\n -+\ttest_cmp expect.decorate actual &&\n ++\ttest_cmp expect.no-decorate actual\n ++'\n ++\n ++test_expect_success 'deocrate-refs and log.excludeDecoration' '\n ++\tcat >expect.decorate <<-\\EOF &&\n ++\tMerge-tag-reach (master)\n ++\tMerge-tags-octopus-a-and-octopus-b\n ++\tseventh\n ++\toctopus-b (octopus-b)\n ++\toctopus-a (octopus-a)\n ++\treach (reach)\n ++\tEOF\n +\tgit -c log.excludeDecoration=\"heads/oc*\" log \\\n +\t\t--decorate-refs=\"heads/*\" \\\n +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n\n\n Documentation/config/log.txt | 5 ++++\n Documentation/git-log.txt | 5 +++-\n builtin/log.c | 16 ++++++++++-\n log-tree.c | 6 ++++-\n log-tree.h | 4 ++-\n refs.c | 15 +++++++++--\n refs.h | 15 ++++++++---\n t/t4202-log.sh | 51 +++++++++++++++++++++++++++++++++++-\n 8 files changed, 106 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\nindex e9e1e397f3f..1a158324f79 100644\n--- a/Documentation/config/log.txt\n+++ b/Documentation/config/log.txt\n@@ -18,6 +18,11 @@ log.decorate::\n \tnames are shown. This is the same as the `--decorate` option\n \tof the `git log`.\n \n+log.excludeDecoration::\n+\tExclude the specified patterns from the log decorations. This multi-\n+\tvalued config option is the same as the `--decorate-refs-exclude`\n+\toption of `git log`.\n+\n log.follow::\n \tIf `true`, `git log` will act as if the `--follow` option was used when\n \ta single is given. This has the same limitations as `--follow`,\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex bed09bb09e5..17592234ba4 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -43,7 +43,10 @@ OPTIONS\n \tIf no `--decorate-refs` is given, pretend as if all refs were\n \tincluded. For each candidate, do not use it for decoration if it\n \tmatches any patterns given to `--decorate-refs-exclude` or if it\n-\tdoesn't match any of the patterns given to `--decorate-refs`.\n+\tdoesn't match any of the patterns given to `--decorate-refs`. The\n+\t`log.excludeDecoration` config option allows excluding refs from\n+\tthe decorations, but an explicit `--decorate-refs` pattern will\n+\toverride a match in `log.excludeDecoration`.\n \n --source::\n \tPrint out the ref name given on the command line by which each\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 83a4a6188e2..72192710dcd 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -164,9 +164,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tint quiet = 0, source = 0, mailmap;\n \tstatic struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};\n \tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n+\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n \tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n \tstruct decoration_filter decoration_filter = {&decorate_refs_include,\n-\t\t\t\t\t\t &decorate_refs_exclude};\n+\t\t\t\t\t\t &decorate_refs_exclude,\n+\t\t\t\t\t\t &decorate_refs_exclude_config};\n \tstatic struct revision_sources revision_sources;\n \n \tconst struct option builtin_log_options[] = {\n@@ -236,7 +238,19 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t}\n \n \tif (decoration_style) {\n+\t\tconst struct string_list *config_exclude =\n+\t\t\trepo_config_get_value_multi(the_repository,\n+\t\t\t\t\t\t \"log.excludeDecoration\");\n+\n+\t\tif (config_exclude) {\n+\t\t\tstruct string_list_item *item;\n+\t\t\tfor_each_string_list_item(item, config_exclude)\n+\t\t\t\tstring_list_append(&decorate_refs_exclude_config,\n+\t\t\t\t\t\t item->string);\n+\t\t}\n+\n \t\trev->show_decorations = 1;\n+\n \t\tload_ref_decorations(&decoration_filter, decoration_style);\n \t}\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 52127427ffe..bd8d4c07bb8 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -90,7 +90,8 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,\n \n \tif (filter && !ref_filter_match(refname,\n \t\t\t filter->include_ref_pattern,\n-\t\t\t filter->exclude_ref_pattern))\n+\t\t\t filter->exclude_ref_pattern,\n+\t\t\t filter->exclude_ref_config_pattern))\n \t\treturn 0;\n \n \tif (starts_with(refname, git_replace_ref_base)) {\n@@ -155,6 +156,9 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n \t\t\tfor_each_string_list_item(item, filter->include_ref_pattern) {\n \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n \t\t\t}\n+\t\t\tfor_each_string_list_item(item, filter->exclude_ref_config_pattern) {\n+\t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n+\t\t\t}\n \t\t}\n \t\tdecoration_loaded = 1;\n \t\tdecoration_flags = flags;\ndiff --git a/log-tree.h b/log-tree.h\nindex e6686280746..8fa79289ec6 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -8,7 +8,9 @@ struct log_info {\n };\n \n struct decoration_filter {\n-\tstruct string_list *include_ref_pattern, *exclude_ref_pattern;\n+\tstruct string_list *include_ref_pattern;\n+\tstruct string_list *exclude_ref_pattern;\n+\tstruct string_list *exclude_ref_config_pattern;\n };\n \n int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);\ndiff --git a/refs.c b/refs.c\nindex 1ab0bb54d3d..63d8b569333 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -339,9 +339,11 @@ static int match_ref_pattern(const char *refname,\n \n int ref_filter_match(const char *refname,\n \t\t const struct string_list *include_patterns,\n-\t\t const struct string_list *exclude_patterns)\n+\t\t const struct string_list *exclude_patterns,\n+\t\t const struct string_list *exclude_patterns_config)\n {\n \tstruct string_list_item *item;\n+\tint found = 0;\n \n \tif (exclude_patterns && exclude_patterns->nr) {\n \t\tfor_each_string_list_item(item, exclude_patterns) {\n@@ -351,7 +353,6 @@ int ref_filter_match(const char *refname,\n \t}\n \n \tif (include_patterns && include_patterns->nr) {\n-\t\tint found = 0;\n \t\tfor_each_string_list_item(item, include_patterns) {\n \t\t\tif (match_ref_pattern(refname, item)) {\n \t\t\t\tfound = 1;\n@@ -362,6 +363,16 @@ int ref_filter_match(const char *refname,\n \t\tif (!found)\n \t\t\treturn 0;\n \t}\n+\n+\tif (!found &&\n+\t exclude_patterns_config &&\n+\t exclude_patterns_config->nr) {\n+\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n+\t\t\tif (match_ref_pattern(refname, item))\n+\t\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n \treturn 1;\n }\n \ndiff --git a/refs.h b/refs.h\nindex 545029c6d80..7cec33303d7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -362,16 +362,23 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n \t\t\tconst char *pattern);\n \n /*\n- * Returns 0 if refname matches any of the exclude_patterns, or if it doesn't\n- * match any of the include_patterns. Returns 1 otherwise.\n+ * Returns 0 if the refname matches any of the exclude_patterns.\n+ *\n+ * Returns 0 if include_patterns is non-empty but refname does not match\n+ * any of those patterns.\n+ *\n+ * Returns 0 if refname matches a pattern in exclude_patterns_config but\n+ * does not match any pattern in inclue_patterns.\n+ *\n+ * Otherwise, returns 1.\n *\n- * If pattern list is NULL or empty, matching against that list is skipped.\n * This has the effect of matching everything by default, unless the user\n * specifies rules otherwise.\n */\n int ref_filter_match(const char *refname,\n \t\t const struct string_list *include_patterns,\n-\t\t const struct string_list *exclude_patterns);\n+\t\t const struct string_list *exclude_patterns,\n+\t\t const struct string_list *exclude_patterns_config);\n \n static inline const char *has_glob_specials(const char *pattern)\n {\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 0f766ba65f5..78f9ade6870 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -742,7 +742,23 @@ test_expect_success 'decorate-refs with glob' '\n \toctopus-a (octopus-a)\n \treach\n \tEOF\n+\tcat >expect.no-decorate <<-\\EOF &&\n+\tMerge-tag-reach\n+\tMerge-tags-octopus-a-and-octopus-b\n+\tseventh\n+\toctopus-b\n+\toctopus-a\n+\treach\n+\tEOF\n+\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n+\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n+\ttest_cmp expect.no-decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs=\"heads/octopus*\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n@@ -787,6 +803,9 @@ test_expect_success 'decorate-refs-exclude with glob' '\n \tEOF\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"heads/octopus*\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -801,6 +820,9 @@ test_expect_success 'decorate-refs-exclude without globs' '\n \tEOF\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"tags/reach\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -816,11 +838,19 @@ test_expect_success 'multiple decorate-refs-exclude' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" \\\n+\t\t-c log.excludeDecoration=\"tags/reach\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t--decorate-refs-exclude=\"tags/reach\" \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n test_expect_success 'decorate-refs and decorate-refs-exclude' '\n-\tcat >expect.decorate <<-\\EOF &&\n+\tcat >expect.no-decorate <<-\\EOF &&\n \tMerge-tag-reach (master)\n \tMerge-tags-octopus-a-and-octopus-b\n \tseventh\n@@ -831,6 +861,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs=\"heads/*\" \\\n \t\t--decorate-refs-exclude=\"heads/oc*\" >actual &&\n+\ttest_cmp expect.no-decorate actual\n+'\n+\n+test_expect_success 'deocrate-refs and log.excludeDecoration' '\n+\tcat >expect.decorate <<-\\EOF &&\n+\tMerge-tag-reach (master)\n+\tMerge-tags-octopus-a-and-octopus-b\n+\tseventh\n+\toctopus-b (octopus-b)\n+\toctopus-a (octopus-a)\n+\treach (reach)\n+\tEOF\n+\tgit -c log.excludeDecoration=\"heads/oc*\" log \\\n+\t\t--decorate-refs=\"heads/*\" \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -846,6 +891,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"*octopus*\" \\\n \t\t--simplify-by-decoration >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"*octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--simplify-by-decoration >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"395518","messageId":"20200415165238.GA23527@syl.local","threadId":"53219","inReplyTo":"pull.610.v2.git.1586965463222.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] log: add log.excludeDecoration config option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-04-15T16:52:38Z","receivedAt":"2020-04-15T16:52:47Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Stolee,\n\nOn Wed, Apr 15, 2020 at 03:44:22PM +0000, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n>\n> In 'git log', the --decorate-refs-exclude option appends a pattern\n> to a string_list. This list is used to prevent showing some refs\n> in the decoration output, or even by --simplify-by-decoration.\n>\n> Users may want to use their refs space to store utility refs that\n> should not appear in the decoration output. For example, Scalar [1]\n> runs a background fetch but places the \"new\" refs inside the\n> refs/scalar/hidden//* refspace instead of refs//*\n> to avoid updating remote refs when the user is not looking. However,\n> these \"hidden\" refs appear during regular 'git log' queries.\n>\n> A similar idea to use \"hidden\" refs is under consideration for core\n> Git [2].\n>\n> Add the 'log.excludeDecoration' config option so users can exclude\n> some refs from decorations by default instead of needing to use\n> --decorate-refs-exclude manually. The config value is multi-valued\n> much like the command-line option. The documentation is careful to\n> point out that the config value can be overridden by the\n> --decorate-refs option, even though --decorate-refs-exclude would\n> always \"win\" over --decorate-refs.\n>\n> Since the 'log.excludeDecoration' takes lower precedence to\n> --decorate-refs, and --decorate-refs-exclude takes higher\n> precedence, the struct decoration_filter needed another field.\n> This led also to new logic in load_ref_decorations() and\n> ref_filter_match().\n>\n> There are several tests in t4202-log.sh that test the\n> --decorate-refs-(include|exclude) options, so these are extended.\n> Since the expected output is already stored as a file, most tests\n> could simply replace a \"--decorate-refs-exclude\" option with an\n> in-line config setting. Other tests involve the precedence of\n> the config option compared to command-line options and needed more\n> modification.\n>\n> [1] https://github.com/microsoft/scalar\n> [2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> log: add log.excludeDecoration config option\n>\n> This was something hinted at in the \"fetch\" step of the background\n> maintenance RFC. Should be a relatively minor addition to our config\n> options.\n>\n> We definitely want this feature for microsoft/git (we would set\n> log.excludeDecoration=refs/scalar/* in all Scalar repos), but we will\n> wait for feedback from the community.\n>\n> Updates in v2:\n>\n> * Use for_each_string_list_item()\n>\n>\n> * Update the matching logic to allow --decorate-refs to override the\n> config option.\n>\n>\n>\n> Thanks, -Stolee\n\nThanks for a v2, Stolee.\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/610\n>\n> [ snip range-diff ... ]\n>\n> Documentation/config/log.txt | 5 ++++\n> Documentation/git-log.txt | 5 +++-\n> builtin/log.c | 16 ++++++++++-\n> log-tree.c | 6 ++++-\n> log-tree.h | 4 ++-\n> refs.c | 15 +++++++++--\n> refs.h | 15 ++++++++---\n> t/t4202-log.sh | 51 +++++++++++++++++++++++++++++++++++-\n> 8 files changed, 106 insertions(+), 11 deletions(-)\n>\n> diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\n> index e9e1e397f3f..1a158324f79 100644\n> --- a/Documentation/config/log.txt\n> +++ b/Documentation/config/log.txt\n> @@ -18,6 +18,11 @@ log.decorate::\n> \tnames are shown. This is the same as the `--decorate` option\n> \tof the `git log`.\n>\n> +log.excludeDecoration::\n> +\tExclude the specified patterns from the log decorations. This multi-\n> +\tvalued config option is the same as the `--decorate-refs-exclude`\n> +\toption of `git log`.\n> +\n\nGood, I'm glad that you didn't change this from v1, since I figure the\ninteraction with 'git log' should be explained in 'git log's man page,\nnot this one. Nice.\n\n> log.follow::\n> \tIf `true`, `git log` will act as if the `--follow` option was used when\n> \ta single is given. This has the same limitations as `--follow`,\n> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\n> index bed09bb09e5..17592234ba4 100644\n> --- a/Documentation/git-log.txt\n> +++ b/Documentation/git-log.txt\n> @@ -43,7 +43,10 @@ OPTIONS\n> \tIf no `--decorate-refs` is given, pretend as if all refs were\n> \tincluded. For each candidate, do not use it for decoration if it\n> \tmatches any patterns given to `--decorate-refs-exclude` or if it\n> -\tdoesn't match any of the patterns given to `--decorate-refs`.\n> +\tdoesn't match any of the patterns given to `--decorate-refs`. The\n> +\t`log.excludeDecoration` config option allows excluding refs from\n> +\tthe decorations, but an explicit `--decorate-refs` pattern will\n> +\toverride a match in `log.excludeDecoration`.\n>\n> --source::\n> \tPrint out the ref name given on the command line by which each\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 83a4a6188e2..72192710dcd 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -164,9 +164,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n> \tint quiet = 0, source = 0, mailmap;\n> \tstatic struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};\n> \tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n> +\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n> \tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n> \tstruct decoration_filter decoration_filter = {&decorate_refs_include,\n> -\t\t\t\t\t\t &decorate_refs_exclude};\n> +\t\t\t\t\t\t &decorate_refs_exclude,\n> +\t\t\t\t\t\t &decorate_refs_exclude_config};\n> \tstatic struct revision_sources revision_sources;\n>\n> \tconst struct option builtin_log_options[] = {\n> @@ -236,7 +238,19 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n> \t}\n>\n> \tif (decoration_style) {\n> +\t\tconst struct string_list *config_exclude =\n> +\t\t\trepo_config_get_value_multi(the_repository,\n> +\t\t\t\t\t\t \"log.excludeDecoration\");\n> +\n> +\t\tif (config_exclude) {\n> +\t\t\tstruct string_list_item *item;\n> +\t\t\tfor_each_string_list_item(item, config_exclude)\n> +\t\t\t\tstring_list_append(&decorate_refs_exclude_config,\n> +\t\t\t\t\t\t item->string);\n> +\t\t}\n> +\n> \t\trev->show_decorations = 1;\n> +\n> \t\tload_ref_decorations(&decoration_filter, decoration_style);\n> \t}\n>\n> diff --git a/log-tree.c b/log-tree.c\n> index 52127427ffe..bd8d4c07bb8 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -90,7 +90,8 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,\n>\n> \tif (filter && !ref_filter_match(refname,\n> \t\t\t filter->include_ref_pattern,\n> -\t\t\t filter->exclude_ref_pattern))\n> +\t\t\t filter->exclude_ref_pattern,\n> +\t\t\t filter->exclude_ref_config_pattern))\n> \t\treturn 0;\n>\n> \tif (starts_with(refname, git_replace_ref_base)) {\n> @@ -155,6 +156,9 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n> \t\t\tfor_each_string_list_item(item, filter->include_ref_pattern) {\n> \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n> \t\t\t}\n> +\t\t\tfor_each_string_list_item(item, filter->exclude_ref_config_pattern) {\n> +\t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n> +\t\t\t}\n> \t\t}\n> \t\tdecoration_loaded = 1;\n> \t\tdecoration_flags = flags;\n> diff --git a/log-tree.h b/log-tree.h\n> index e6686280746..8fa79289ec6 100644\n> --- a/log-tree.h\n> +++ b/log-tree.h\n> @@ -8,7 +8,9 @@ struct log_info {\n> };\n>\n> struct decoration_filter {\n> -\tstruct string_list *include_ref_pattern, *exclude_ref_pattern;\n> +\tstruct string_list *include_ref_pattern;\n> +\tstruct string_list *exclude_ref_pattern;\n> +\tstruct string_list *exclude_ref_config_pattern;\n> };\n>\n> int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);\n> diff --git a/refs.c b/refs.c\n> index 1ab0bb54d3d..63d8b569333 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -339,9 +339,11 @@ static int match_ref_pattern(const char *refname,\n>\n> int ref_filter_match(const char *refname,\n> \t\t const struct string_list *include_patterns,\n> -\t\t const struct string_list *exclude_patterns)\n> +\t\t const struct string_list *exclude_patterns,\n> +\t\t const struct string_list *exclude_patterns_config)\n> {\n> \tstruct string_list_item *item;\n> +\tint found = 0;\n>\n> \tif (exclude_patterns && exclude_patterns->nr) {\n> \t\tfor_each_string_list_item(item, exclude_patterns) {\n> @@ -351,7 +353,6 @@ int ref_filter_match(const char *refname,\n> \t}\n>\n> \tif (include_patterns && include_patterns->nr) {\n> -\t\tint found = 0;\n> \t\tfor_each_string_list_item(item, include_patterns) {\n> \t\t\tif (match_ref_pattern(refname, item)) {\n> \t\t\t\tfound = 1;\n> @@ -362,6 +363,16 @@ int ref_filter_match(const char *refname,\n> \t\tif (!found)\n> \t\t\treturn 0;\n> \t}\n> +\n> +\tif (!found &&\n> +\t exclude_patterns_config &&\n> +\t exclude_patterns_config->nr) {\n> +\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n> +\t\t\tif (match_ref_pattern(refname, item))\n> +\t\t\t\treturn 0;\n> +\t\t}\n> +\t}\n> +\n> \treturn 1;\n> }\n>\n> diff --git a/refs.h b/refs.h\n> index 545029c6d80..7cec33303d7 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -362,16 +362,23 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n> \t\t\tconst char *pattern);\n>\n> /*\n> - * Returns 0 if refname matches any of the exclude_patterns, or if it doesn't\n> - * match any of the include_patterns. Returns 1 otherwise.\n> + * Returns 0 if the refname matches any of the exclude_patterns.\n> + *\n> + * Returns 0 if include_patterns is non-empty but refname does not match\n> + * any of those patterns.\n> + *\n> + * Returns 0 if refname matches a pattern in exclude_patterns_config but\n> + * does not match any pattern in inclue_patterns.\n> + *\n> + * Otherwise, returns 1.\n> *\n> - * If pattern list is NULL or empty, matching against that list is skipped.\n> * This has the effect of matching everything by default, unless the user\n> * specifies rules otherwise.\n> */\n> int ref_filter_match(const char *refname,\n> \t\t const struct string_list *include_patterns,\n> -\t\t const struct string_list *exclude_patterns);\n> +\t\t const struct string_list *exclude_patterns,\n> +\t\t const struct string_list *exclude_patterns_config);\n>\n> static inline const char *has_glob_specials(const char *pattern)\n> {\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 0f766ba65f5..78f9ade6870 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -742,7 +742,23 @@ test_expect_success 'decorate-refs with glob' '\n> \toctopus-a (octopus-a)\n> \treach\n> \tEOF\n> +\tcat >expect.no-decorate <<-\\EOF &&\n> +\tMerge-tag-reach\n> +\tMerge-tags-octopus-a-and-octopus-b\n> +\tseventh\n> +\toctopus-b\n> +\toctopus-a\n> +\treach\n> +\tEOF\n> +\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n> +\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n> +\ttest_cmp expect.no-decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs=\"heads/octopus*\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n> @@ -787,6 +803,9 @@ test_expect_success 'decorate-refs-exclude with glob' '\n> \tEOF\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"heads/octopus*\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> @@ -801,6 +820,9 @@ test_expect_success 'decorate-refs-exclude without globs' '\n> \tEOF\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"tags/reach\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> @@ -816,11 +838,19 @@ test_expect_success 'multiple decorate-refs-exclude' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n> \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" \\\n> +\t\t-c log.excludeDecoration=\"tags/reach\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t--decorate-refs-exclude=\"tags/reach\" \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> test_expect_success 'decorate-refs and decorate-refs-exclude' '\n> -\tcat >expect.decorate <<-\\EOF &&\n> +\tcat >expect.no-decorate <<-\\EOF &&\n> \tMerge-tag-reach (master)\n> \tMerge-tags-octopus-a-and-octopus-b\n> \tseventh\n> @@ -831,6 +861,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs=\"heads/*\" \\\n> \t\t--decorate-refs-exclude=\"heads/oc*\" >actual &&\n> +\ttest_cmp expect.no-decorate actual\n> +'\n> +\n> +test_expect_success 'deocrate-refs and log.excludeDecoration' '\n> +\tcat >expect.decorate <<-\\EOF &&\n> +\tMerge-tag-reach (master)\n> +\tMerge-tags-octopus-a-and-octopus-b\n> +\tseventh\n> +\toctopus-b (octopus-b)\n> +\toctopus-a (octopus-a)\n> +\treach (reach)\n> +\tEOF\n> +\tgit -c log.excludeDecoration=\"heads/oc*\" log \\\n> +\t\t--decorate-refs=\"heads/*\" \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n> @@ -846,6 +891,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"*octopus*\" \\\n> \t\t--simplify-by-decoration >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"*octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--simplify-by-decoration >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n>\n>\n> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n> --\n\nThis looks good to me, thanks.\n\n Reviewed-by: Taylor Blau \n\nThanks,\nTaylor\n"},{"id":"395519","messageId":"xmqqftd47jce.fsf@gitster.c.googlers.com","threadId":"53219","inReplyTo":"pull.610.v2.git.1586965463222.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] log: add log.excludeDecoration config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-15T17:24:33Z","receivedAt":"2020-04-15T17:24:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> Add the 'log.excludeDecoration' config option so users can exclude\n> some refs from decorations by default instead of needing to use\n> --decorate-refs-exclude manually. The config value is multi-valued\n> much like the command-line option. The documentation is careful to\n> point out that the config value can be overridden by the\n> --decorate-refs option, even though --decorate-refs-exclude would\n> always \"win\" over --decorate-refs.\n>\n> Since the 'log.excludeDecoration' takes lower precedence to\n> --decorate-refs, and --decorate-refs-exclude takes higher\n> precedence, the struct decoration_filter needed another field.\n> This led also to new logic in load_ref_decorations() and\n> ref_filter_match().\n\nAll of the above makes sense to me.\n\n> diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\n> index e9e1e397f3f..1a158324f79 100644\n> --- a/Documentation/config/log.txt\n> +++ b/Documentation/config/log.txt\n> @@ -18,6 +18,11 @@ log.decorate::\n> \tnames are shown. This is the same as the `--decorate` option\n> \tof the `git log`.\n> \n> +log.excludeDecoration::\n> +\tExclude the specified patterns from the log decorations. This multi-\n> +\tvalued config option is the same as the `--decorate-refs-exclude`\n> +\toption of `git log`.\n\nCan the config still be \"the same as\" that option, though, with the\nnew \"unlike --decorate-refs-exclude that always wins, config is at\nthe lowest precedence\" rule?\n\n> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\n> index bed09bb09e5..17592234ba4 100644\n> --- a/Documentation/git-log.txt\n> +++ b/Documentation/git-log.txt\n> @@ -43,7 +43,10 @@ OPTIONS\n> \tIf no `--decorate-refs` is given, pretend as if all refs were\n> \tincluded. For each candidate, do not use it for decoration if it\n> \tmatches any patterns given to `--decorate-refs-exclude` or if it\n> -\tdoesn't match any of the patterns given to `--decorate-refs`.\n> +\tdoesn't match any of the patterns given to `--decorate-refs`. The\n> +\t`log.excludeDecoration` config option allows excluding refs from\n> +\tthe decorations, but an explicit `--decorate-refs` pattern will\n> +\toverride a match in `log.excludeDecoration`.\n\nThis description does make sense.\n\n> diff --git a/log-tree.c b/log-tree.c\n> index 52127427ffe..bd8d4c07bb8 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -90,7 +90,8 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,\n> \n> \tif (filter && !ref_filter_match(refname,\n> \t\t\t filter->include_ref_pattern,\n> -\t\t\t filter->exclude_ref_pattern))\n> +\t\t\t filter->exclude_ref_pattern,\n> +\t\t\t filter->exclude_ref_config_pattern))\n> \t\treturn 0;\n\nAs there is only one caller of the ref_filter_match() helper, I\nwonder if we want to\n\n (1) move the helper to log-tree.c, make it static and remove its\n definition from refs.h, and optionally rename it so that it is\n clear that this is not part of the \"ref_filter\" API that drives\n \"for-each-ref\" and friends;\n\n (2) instead of adding yet another pattern to the parameter list,\n make the helper take the whole \"filter\" instance as a single\n parameter.\n\nas a clean-up.\n\n> diff --git a/refs.c b/refs.c\n> index 1ab0bb54d3d..63d8b569333 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -339,9 +339,11 @@ static int match_ref_pattern(const char *refname,\n> \n> int ref_filter_match(const char *refname,\n> \t\t const struct string_list *include_patterns,\n> -\t\t const struct string_list *exclude_patterns)\n> +\t\t const struct string_list *exclude_patterns,\n> +\t\t const struct string_list *exclude_patterns_config)\n> {\n> \tstruct string_list_item *item;\n> +\tint found = 0;\n> \n> \tif (exclude_patterns && exclude_patterns->nr) {\n> \t\tfor_each_string_list_item(item, exclude_patterns) {\n> @@ -351,7 +353,6 @@ int ref_filter_match(const char *refname,\n> \t}\n> \n> \tif (include_patterns && include_patterns->nr) {\n> -\t\tint found = 0;\n> \t\tfor_each_string_list_item(item, include_patterns) {\n> \t\t\tif (match_ref_pattern(refname, item)) {\n> \t\t\t\tfound = 1;\n> @@ -362,6 +363,16 @@ int ref_filter_match(const char *refname,\n> \t\tif (!found)\n> \t\t\treturn 0;\n> \t}\n> +\n> +\tif (!found &&\n> +\t exclude_patterns_config &&\n> +\t exclude_patterns_config->nr) {\n> +\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n> +\t\t\tif (match_ref_pattern(refname, item))\n> +\t\t\t\treturn 0;\n> +\t\t}\n> +\t}\n\nHmph. Do we still need \"found\" here? If there are include patterns\ngiven explicitly from the command line, a ref MUST match one of them\nin order to be included, and a ref that matches one of them will be\nincluded no matter that exclude config says.\n\nSo shouldn't the updated logic for the include patterns part be more\nlike\n\n\tif (include_patterns is not empty) {\n\t\tfor_each_string_list_item(item, include_patterns) {\n\t\t\tif (matches)\n\t\t\t\treturn 1; /* matches */\n\t\t}\n\t\treturn 0; /* did not match */\n\t}\n\nThen the new code to deal with \"config\", which will come after the\n\"include\" thing, will only have to deal with the case where there is\nno include patterns given from the command line. And when the\ncontrol reaches there, we need to do exactly the same thing as we do\nto the commad line excludes, i.e. see if any of the config patterns\nmatch, and say \"reject\" if we find a match.\n\nAnd after the loop for the exclude config fails to \"return 0\" early,\nwe will hit the \"return 1\" at the end.\n\nHad I butchered the logic somehow?\n\n> /*\n> + * Returns 0 if the refname matches any of the exclude_patterns.\n> + *\n> + * Returns 0 if include_patterns is non-empty but refname does not match\n> + * any of those patterns.\n> + *\n> + * Returns 0 if refname matches a pattern in exclude_patterns_config but\n> + * does not match any pattern in inclue_patterns.\n> + *\n> + * Otherwise, returns 1.\n> *\n> * This has the effect of matching everything by default, unless the user\n> * specifies rules otherwise.\n> */\n\nThe above is not wrong per-se, but feels somewhat roundabout way to\ndefine what it does, from the viewpoint of somebody who may want to\ncall or understand it. \"What matches one of the exclude patterns is\nexcluded. If the include patterns is empty, what did not match\nexclude patterns is included unless it matches one of the exclude\nconfigs. But if the include patterns is not empty, what did not\nmatch exclude patterns is included only if it matches one of the\ninclude patterns.\"\n> int ref_filter_match(const char *refname,\n> \t\t const struct string_list *include_patterns,\n> -\t\t const struct string_list *exclude_patterns);\n> +\t\t const struct string_list *exclude_patterns,\n> +\t\t const struct string_list *exclude_patterns_config);\n> \n> static inline const char *has_glob_specials(const char *pattern)\n> {\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 0f766ba65f5..78f9ade6870 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -742,7 +742,23 @@ test_expect_success 'decorate-refs with glob' '\n> \toctopus-a (octopus-a)\n> \treach\n> \tEOF\n> +\tcat >expect.no-decorate <<-\\EOF &&\n> +\tMerge-tag-reach\n> +\tMerge-tags-octopus-a-and-octopus-b\n> +\tseventh\n> +\toctopus-b\n> +\toctopus-a\n> +\treach\n> +\tEOF\n> +\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n> +\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n> +\ttest_cmp expect.no-decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs=\"heads/octopus*\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n> @@ -787,6 +803,9 @@ test_expect_success 'decorate-refs-exclude with glob' '\n> \tEOF\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"heads/octopus*\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n> \n> @@ -801,6 +820,9 @@ test_expect_success 'decorate-refs-exclude without globs' '\n> \tEOF\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"tags/reach\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n> \n> @@ -816,11 +838,19 @@ test_expect_success 'multiple decorate-refs-exclude' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n> \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" \\\n> +\t\t-c log.excludeDecoration=\"tags/reach\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n> +\t\t--decorate-refs-exclude=\"tags/reach\" \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n> \n> test_expect_success 'decorate-refs and decorate-refs-exclude' '\n> -\tcat >expect.decorate <<-\\EOF &&\n> +\tcat >expect.no-decorate <<-\\EOF &&\n> \tMerge-tag-reach (master)\n> \tMerge-tags-octopus-a-and-octopus-b\n> \tseventh\n> @@ -831,6 +861,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs=\"heads/*\" \\\n> \t\t--decorate-refs-exclude=\"heads/oc*\" >actual &&\n> +\ttest_cmp expect.no-decorate actual\n> +'\n> +\n> +test_expect_success 'deocrate-refs and log.excludeDecoration' '\n> +\tcat >expect.decorate <<-\\EOF &&\n> +\tMerge-tag-reach (master)\n> +\tMerge-tags-octopus-a-and-octopus-b\n> +\tseventh\n> +\toctopus-b (octopus-b)\n> +\toctopus-a (octopus-a)\n> +\treach (reach)\n> +\tEOF\n> +\tgit -c log.excludeDecoration=\"heads/oc*\" log \\\n> +\t\t--decorate-refs=\"heads/*\" \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n> \n> @@ -846,6 +891,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n> \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> \t\t--decorate-refs-exclude=\"*octopus*\" \\\n> \t\t--simplify-by-decoration >actual &&\n> +\ttest_cmp expect.decorate actual &&\n> +\tgit -c log.excludeDecoration=\"*octopus*\" log \\\n> +\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n> +\t\t--simplify-by-decoration >actual &&\n> \ttest_cmp expect.decorate actual\n> '\n> \n>\n> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n"},{"id":"395520","messageId":"xmqqblns7j4u.fsf@gitster.c.googlers.com","threadId":"53219","inReplyTo":"xmqqftd47jce.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] log: add log.excludeDecoration config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-15T17:29:05Z","receivedAt":"2020-04-15T17:29:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Hmph. Do we still need \"found\" here? If there are include patterns\n> given explicitly from the command line, a ref MUST match one of them\n> in order to be included, and a ref that matches one of them will be\n> included no matter that exclude config says.\n>\n> So shouldn't the updated logic for the include patterns part be more\n> like ...\n\nI still think that the two clean-ups I mentioned are both worth\ndoing, but without them, but with the simplification of the code \nI suggested, the resulting helper becomes like this, which I think\nis quite easy to understand. It seems to pass t4202, which you\nupdated, too.\n\n\n\nint ref_filter_match(const char *refname,\n\t\t const struct string_list *include_patterns,\n\t\t const struct string_list *exclude_patterns,\n\t\t const struct string_list *exclude_patterns_config)\n{\n\tstruct string_list_item *item;\n\n\tif (exclude_patterns && exclude_patterns->nr) {\n\t\tfor_each_string_list_item(item, exclude_patterns) {\n\t\t\tif (match_ref_pattern(refname, item))\n\t\t\t\treturn 0;\n\t\t}\n\t}\n\n\tif (include_patterns && include_patterns->nr) {\n\t\tfor_each_string_list_item(item, include_patterns) {\n\t\t\tif (match_ref_pattern(refname, item)) {\n\t\t\t\treturn 1;\n\t\t\t}\n\t\t}\n\t\treturn 0;\n\t}\n\n\tif (exclude_patterns_config && exclude_patterns_config->nr) {\n\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n\t\t\tif (match_ref_pattern(refname, item))\n\t\t\t\treturn 0;\n\t\t}\n\t}\n\n\treturn 1;\n}\n"},{"id":"395582","messageId":"681fc3a9-9402-44a0-dc73-56c8ad14ba47@gmail.com","threadId":"53219","inReplyTo":"xmqqblns7j4u.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] log: add log.excludeDecoration config option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-16T12:36:50Z","receivedAt":"2020-04-16T12:37:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/15/2020 1:29 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> Hmph. Do we still need \"found\" here? If there are include patterns\n>> given explicitly from the command line, a ref MUST match one of them\n>> in order to be included, and a ref that matches one of them will be\n>> included no matter that exclude config says.\n>>\n>> So shouldn't the updated logic for the include patterns part be more\n>> like ...\n> \n> I still think that the two clean-ups I mentioned are both worth\n> doing, but without them, but with the simplification of the code \n> I suggested, the resulting helper becomes like this, which I think\n> is quite easy to understand. It seems to pass t4202, which you\n> updated, too.\n> \n> \n> \n> int ref_filter_match(const char *refname,\n> \t\t const struct string_list *include_patterns,\n> \t\t const struct string_list *exclude_patterns,\n> \t\t const struct string_list *exclude_patterns_config)\n> {\n> \tstruct string_list_item *item;\n> \n> \tif (exclude_patterns && exclude_patterns->nr) {\n> \t\tfor_each_string_list_item(item, exclude_patterns) {\n> \t\t\tif (match_ref_pattern(refname, item))\n> \t\t\t\treturn 0;\n> \t\t}\n> \t}\n> \n> \tif (include_patterns && include_patterns->nr) {\n> \t\tfor_each_string_list_item(item, include_patterns) {\n> \t\t\tif (match_ref_pattern(refname, item)) {\n> \t\t\t\treturn 1;\n> \t\t\t}\n> \t\t}\n> \t\treturn 0;\n> \t}\n> \n> \tif (exclude_patterns_config && exclude_patterns_config->nr) {\n> \t\tfor_each_string_list_item(item, exclude_patterns_config) {\n> \t\t\tif (match_ref_pattern(refname, item))\n> \t\t\t\treturn 0;\n> \t\t}\n> \t}\n> \n> \treturn 1;\n> }\n\nThis reorganization of the code is much cleaner, and it can\nguide cleaning up the documentation comment in the header file.\n\nI see you included it in a SQUASH??? commit, which I will squash\ninto my branch for v3.\n\nThanks,\n-Stolee\n\n\n"},{"id":"395584","messageId":"a8380ed7-d3a1-873d-7f65-099fd55a3988@gmail.com","threadId":"53219","inReplyTo":"xmqqftd47jce.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] log: add log.excludeDecoration config option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-16T12:46:51Z","receivedAt":"2020-04-16T12:46:59Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/15/2020 1:24 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> Add the 'log.excludeDecoration' config option so users can exclude\n>> some refs from decorations by default instead of needing to use\n>> --decorate-refs-exclude manually. The config value is multi-valued\n>> much like the command-line option. The documentation is careful to\n>> point out that the config value can be overridden by the\n>> --decorate-refs option, even though --decorate-refs-exclude would\n>> always \"win\" over --decorate-refs.\n>>\n>> Since the 'log.excludeDecoration' takes lower precedence to\n>> --decorate-refs, and --decorate-refs-exclude takes higher\n>> precedence, the struct decoration_filter needed another field.\n>> This led also to new logic in load_ref_decorations() and\n>> ref_filter_match().\n> \n> All of the above makes sense to me.\n> \n>> diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\n>> index e9e1e397f3f..1a158324f79 100644\n>> --- a/Documentation/config/log.txt\n>> +++ b/Documentation/config/log.txt\n>> @@ -18,6 +18,11 @@ log.decorate::\n>> \tnames are shown. This is the same as the `--decorate` option\n>> \tof the `git log`.\n>> \n>> +log.excludeDecoration::\n>> +\tExclude the specified patterns from the log decorations. This multi-\n>> +\tvalued config option is the same as the `--decorate-refs-exclude`\n>> +\toption of `git log`.\n> \n> Can the config still be \"the same as\" that option, though, with the\n> new \"unlike --decorate-refs-exclude that always wins, config is at\n> the lowest precedence\" rule?\n\nI thought I had updated this to make that clearer, but it looks like\nI missed it when staging.\n\nWhat I had meant to write was this:\n\nlog.excludeDecoration::\n\tExclude the specified patterns from the log decorations. This is\n\tsimilar to the `--decorate-refs-exclude` command-line option, but\n\tthe config option can be overridden by the `--decorate-refs`\n\toption.\n\n>> diff --git a/log-tree.c b/log-tree.c\n>> index 52127427ffe..bd8d4c07bb8 100644\n>> --- a/log-tree.c\n>> +++ b/log-tree.c\n>> @@ -90,7 +90,8 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,\n>> \n>> \tif (filter && !ref_filter_match(refname,\n>> \t\t\t filter->include_ref_pattern,\n>> -\t\t\t filter->exclude_ref_pattern))\n>> +\t\t\t filter->exclude_ref_pattern,\n>> +\t\t\t filter->exclude_ref_config_pattern))\n>> \t\treturn 0;\n> \n> As there is only one caller of the ref_filter_match() helper, I\n> wonder if we want to\n> \n> (1) move the helper to log-tree.c, make it static and remove its\n> definition from refs.h, and optionally rename it so that it is\n> clear that this is not part of the \"ref_filter\" API that drives\n> \"for-each-ref\" and friends;\n> \n> (2) instead of adding yet another pattern to the parameter list,\n> make the helper take the whole \"filter\" instance as a single\n> parameter.\n> \n> as a clean-up.\n\nThis is a good idea. I was thinking the code was \"smelly\" when I had\nto jump through so many hoops to get it working. I'll add a refactor\npatch in front of this one.\n\n>> diff --git a/refs.c b/refs.c\n>> index 1ab0bb54d3d..63d8b569333 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -339,9 +339,11 @@ static int match_ref_pattern(const char *refname,\n>> \n>> int ref_filter_match(const char *refname,\n>> \t\t const struct string_list *include_patterns,\n>> -\t\t const struct string_list *exclude_patterns)\n>> +\t\t const struct string_list *exclude_patterns,\n>> +\t\t const struct string_list *exclude_patterns_config)\n>> {\n>> \tstruct string_list_item *item;\n>> +\tint found = 0;\n>> \n>> \tif (exclude_patterns && exclude_patterns->nr) {\n>> \t\tfor_each_string_list_item(item, exclude_patterns) {\n>> @@ -351,7 +353,6 @@ int ref_filter_match(const char *refname,\n>> \t}\n>> \n>> \tif (include_patterns && include_patterns->nr) {\n>> -\t\tint found = 0;\n>> \t\tfor_each_string_list_item(item, include_patterns) {\n>> \t\t\tif (match_ref_pattern(refname, item)) {\n>> \t\t\t\tfound = 1;\n>> @@ -362,6 +363,16 @@ int ref_filter_match(const char *refname,\n>> \t\tif (!found)\n>> \t\t\treturn 0;\n>> \t}\n>> +\n>> +\tif (!found &&\n>> +\t exclude_patterns_config &&\n>> +\t exclude_patterns_config->nr) {\n>> +\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n>> +\t\t\tif (match_ref_pattern(refname, item))\n>> +\t\t\t\treturn 0;\n>> +\t\t}\n>> +\t}\n> \n> Hmph. Do we still need \"found\" here? ...\n\nYou included an excellent update in another response, which I have\nsquashed locally.\n\n>> /*\n>> + * Returns 0 if the refname matches any of the exclude_patterns.\n>> + *\n>> + * Returns 0 if include_patterns is non-empty but refname does not match\n>> + * any of those patterns.\n>> + *\n>> + * Returns 0 if refname matches a pattern in exclude_patterns_config but\n>> + * does not match any pattern in inclue_patterns.\n>> + *\n>> + * Otherwise, returns 1.\n>> *\n>> * This has the effect of matching everything by default, unless the user\n>> * specifies rules otherwise.\n>> */\n> \n> The above is not wrong per-se, but feels somewhat roundabout way to\n> define what it does, from the viewpoint of somebody who may want to\n> call or understand it. \"What matches one of the exclude patterns is\n> excluded. If the include patterns is empty, what did not match\n> exclude patterns is included unless it matches one of the exclude\n> configs. But if the include patterns is not empty, what did not\n> match exclude patterns is included only if it matches one of the\n> include patterns.\"\n\nYour new logic for the method makes this a bit simpler to write:\n\n/*\n * Returns 0 if the refname matches any of the exclude_patterns.\n *\n * Otherwise, returns 1 if the refname matches any of the include_patterns.\n *\n * Otherwise, returns 0 if include_patterns is non-empty.\n *\n * Otherwise, returns 0 if the refname matches any of the patterns\n * in exclude_patterns_config.\n *\n * Finally, if none of the above apply, return 1.\n */\n\nHowever, if I pull this method into a static helper method, then\nthe documentation is unnecessary.\n\nThanks for the careful review!\n\n-Stolee\n"},{"id":"395588","messageId":"96c865e9214802021f8e991d2194b7aea6cd6bba.1587046549.git.gitgitgadget@gmail.com","threadId":"53219","inReplyTo":"pull.610.v3.git.1587046549.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] log: add log.excludeDecoration config option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-16T14:15:49Z","receivedAt":"2020-04-16T14:16:07Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nIn 'git log', the --decorate-refs-exclude option appends a pattern\nto a string_list. This list is used to prevent showing some refs\nin the decoration output, or even by --simplify-by-decoration.\n\nUsers may want to use their refs space to store utility refs that\nshould not appear in the decoration output. For example, Scalar [1]\nruns a background fetch but places the \"new\" refs inside the\nrefs/scalar/hidden//* refspace instead of refs//*\nto avoid updating remote refs when the user is not looking. However,\nthese \"hidden\" refs appear during regular 'git log' queries.\n\nA similar idea to use \"hidden\" refs is under consideration for core\nGit [2].\n\nAdd the 'log.excludeDecoration' config option so users can exclude\nsome refs from decorations by default instead of needing to use\n--decorate-refs-exclude manually. The config value is multi-valued\nmuch like the command-line option. The documentation is careful to\npoint out that the config value can be overridden by the\n--decorate-refs option, even though --decorate-refs-exclude would\nalways \"win\" over --decorate-refs.\n\nSince the 'log.excludeDecoration' takes lower precedence to\n--decorate-refs, and --decorate-refs-exclude takes higher\nprecedence, the struct decoration_filter needed another field.\nThis led also to new logic in load_ref_decorations() and\nref_filter_match().\n\nThere are several tests in t4202-log.sh that test the\n--decorate-refs-(include|exclude) options, so these are extended.\nSince the expected output is already stored as a file, most tests\ncould simply replace a \"--decorate-refs-exclude\" option with an\nin-line config setting. Other tests involve the precedence of\nthe config option compared to command-line options and needed more\nmodification.\n\n[1] https://github.com/microsoft/scalar\n[2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n\nHelped-by: Junio C Hamano \nSigned-off-by: Derrick Stolee \n---\n Documentation/config/log.txt | 6 +++++\n Documentation/git-log.txt | 5 +++-\n builtin/log.c | 16 ++++++++++-\n log-tree.c | 19 ++++++++++----\n log-tree.h | 4 ++-\n t/t4202-log.sh | 51 +++++++++++++++++++++++++++++++++++-\n 6 files changed, 92 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config/log.txt b/Documentation/config/log.txt\nindex e9e1e397f3f..208d5fdcaa6 100644\n--- a/Documentation/config/log.txt\n+++ b/Documentation/config/log.txt\n@@ -18,6 +18,12 @@ log.decorate::\n \tnames are shown. This is the same as the `--decorate` option\n \tof the `git log`.\n \n+log.excludeDecoration::\n+\tExclude the specified patterns from the log decorations. This is\n+\tsimilar to the `--decorate-refs-exclude` command-line option, but\n+\tthe config option can be overridden by the `--decorate-refs`\n+\toption.\n+\n log.follow::\n \tIf `true`, `git log` will act as if the `--follow` option was used when\n \ta single is given. This has the same limitations as `--follow`,\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex bed09bb09e5..17592234ba4 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -43,7 +43,10 @@ OPTIONS\n \tIf no `--decorate-refs` is given, pretend as if all refs were\n \tincluded. For each candidate, do not use it for decoration if it\n \tmatches any patterns given to `--decorate-refs-exclude` or if it\n-\tdoesn't match any of the patterns given to `--decorate-refs`.\n+\tdoesn't match any of the patterns given to `--decorate-refs`. The\n+\t`log.excludeDecoration` config option allows excluding refs from\n+\tthe decorations, but an explicit `--decorate-refs` pattern will\n+\toverride a match in `log.excludeDecoration`.\n \n --source::\n \tPrint out the ref name given on the command line by which each\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 83a4a6188e2..72192710dcd 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -164,9 +164,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tint quiet = 0, source = 0, mailmap;\n \tstatic struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};\n \tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n+\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n \tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n \tstruct decoration_filter decoration_filter = {&decorate_refs_include,\n-\t\t\t\t\t\t &decorate_refs_exclude};\n+\t\t\t\t\t\t &decorate_refs_exclude,\n+\t\t\t\t\t\t &decorate_refs_exclude_config};\n \tstatic struct revision_sources revision_sources;\n \n \tconst struct option builtin_log_options[] = {\n@@ -236,7 +238,19 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t}\n \n \tif (decoration_style) {\n+\t\tconst struct string_list *config_exclude =\n+\t\t\trepo_config_get_value_multi(the_repository,\n+\t\t\t\t\t\t \"log.excludeDecoration\");\n+\n+\t\tif (config_exclude) {\n+\t\t\tstruct string_list_item *item;\n+\t\t\tfor_each_string_list_item(item, config_exclude)\n+\t\t\t\tstring_list_append(&decorate_refs_exclude_config,\n+\t\t\t\t\t\t item->string);\n+\t\t}\n+\n \t\trev->show_decorations = 1;\n+\n \t\tload_ref_decorations(&decoration_filter, decoration_style);\n \t}\n \ndiff --git a/log-tree.c b/log-tree.c\nindex ab6d29a746b..fd3fd3316a1 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -103,6 +103,8 @@ static int ref_filter_match(const char *refname,\n \tstruct string_list_item *item;\n \tconst struct string_list *exclude_patterns = filter->exclude_ref_pattern;\n \tconst struct string_list *include_patterns = filter->include_ref_pattern;\n+\tconst struct string_list *exclude_patterns_config =\n+\t\t\t\tfilter->exclude_ref_config_pattern;\n \n \tif (exclude_patterns && exclude_patterns->nr) {\n \t\tfor_each_string_list_item(item, exclude_patterns) {\n@@ -112,17 +114,21 @@ static int ref_filter_match(const char *refname,\n \t}\n \n \tif (include_patterns && include_patterns->nr) {\n-\t\tint found = 0;\n \t\tfor_each_string_list_item(item, include_patterns) {\n \t\t\tif (match_ref_pattern(refname, item)) {\n-\t\t\t\tfound = 1;\n-\t\t\t\tbreak;\n+\t\t\t\treturn 1;\n \t\t\t}\n \t\t}\n+\t\treturn 0;\n+\t}\n \n-\t\tif (!found)\n-\t\t\treturn 0;\n+\tif (exclude_patterns_config && exclude_patterns_config->nr) {\n+\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n+\t\t\tif (match_ref_pattern(refname, item))\n+\t\t\t\treturn 0;\n+\t\t}\n \t}\n+\n \treturn 1;\n }\n \n@@ -198,6 +204,9 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n \t\t\tfor_each_string_list_item(item, filter->include_ref_pattern) {\n \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n \t\t\t}\n+\t\t\tfor_each_string_list_item(item, filter->exclude_ref_config_pattern) {\n+\t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n+\t\t\t}\n \t\t}\n \t\tdecoration_loaded = 1;\n \t\tdecoration_flags = flags;\ndiff --git a/log-tree.h b/log-tree.h\nindex e6686280746..8fa79289ec6 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -8,7 +8,9 @@ struct log_info {\n };\n \n struct decoration_filter {\n-\tstruct string_list *include_ref_pattern, *exclude_ref_pattern;\n+\tstruct string_list *include_ref_pattern;\n+\tstruct string_list *exclude_ref_pattern;\n+\tstruct string_list *exclude_ref_config_pattern;\n };\n \n int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 0f766ba65f5..78f9ade6870 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -742,7 +742,23 @@ test_expect_success 'decorate-refs with glob' '\n \toctopus-a (octopus-a)\n \treach\n \tEOF\n+\tcat >expect.no-decorate <<-\\EOF &&\n+\tMerge-tag-reach\n+\tMerge-tags-octopus-a-and-octopus-b\n+\tseventh\n+\toctopus-b\n+\toctopus-a\n+\treach\n+\tEOF\n+\tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n+\t\t--decorate-refs=\"heads/octopus*\" >actual &&\n+\ttest_cmp expect.no-decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs=\"heads/octopus*\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n@@ -787,6 +803,9 @@ test_expect_success 'decorate-refs-exclude with glob' '\n \tEOF\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"heads/octopus*\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -801,6 +820,9 @@ test_expect_success 'decorate-refs-exclude without globs' '\n \tEOF\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"tags/reach\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -816,11 +838,19 @@ test_expect_success 'multiple decorate-refs-exclude' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"heads/octopus*\" \\\n \t\t--decorate-refs-exclude=\"tags/reach\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" \\\n+\t\t-c log.excludeDecoration=\"tags/reach\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"heads/octopus*\" log \\\n+\t\t--decorate-refs-exclude=\"tags/reach\" \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n test_expect_success 'decorate-refs and decorate-refs-exclude' '\n-\tcat >expect.decorate <<-\\EOF &&\n+\tcat >expect.no-decorate <<-\\EOF &&\n \tMerge-tag-reach (master)\n \tMerge-tags-octopus-a-and-octopus-b\n \tseventh\n@@ -831,6 +861,21 @@ test_expect_success 'decorate-refs and decorate-refs-exclude' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs=\"heads/*\" \\\n \t\t--decorate-refs-exclude=\"heads/oc*\" >actual &&\n+\ttest_cmp expect.no-decorate actual\n+'\n+\n+test_expect_success 'deocrate-refs and log.excludeDecoration' '\n+\tcat >expect.decorate <<-\\EOF &&\n+\tMerge-tag-reach (master)\n+\tMerge-tags-octopus-a-and-octopus-b\n+\tseventh\n+\toctopus-b (octopus-b)\n+\toctopus-a (octopus-a)\n+\treach (reach)\n+\tEOF\n+\tgit -c log.excludeDecoration=\"heads/oc*\" log \\\n+\t\t--decorate-refs=\"heads/*\" \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n@@ -846,6 +891,10 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n \tgit log -n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n \t\t--decorate-refs-exclude=\"*octopus*\" \\\n \t\t--simplify-by-decoration >actual &&\n+\ttest_cmp expect.decorate actual &&\n+\tgit -c log.excludeDecoration=\"*octopus*\" log \\\n+\t\t-n6 --decorate=short --pretty=\"tformat:%f%d\" \\\n+\t\t--simplify-by-decoration >actual &&\n \ttest_cmp expect.decorate actual\n '\n \n-- \ngitgitgadget\n"},{"id":"395589","messageId":"pull.610.v3.git.1587046549.gitgitgadget@gmail.com","threadId":"53219","inReplyTo":"pull.610.v2.git.1586965463222.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] log: add log.excludeDecoration config option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-16T14:15:47Z","receivedAt":"2020-04-16T14:16:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This was something hinted at in the \"fetch\" step of the background\nmaintenance RFC. Should be a relatively minor addition to our config\noptions.\n\nWe definitely want this feature for microsoft/git (we would set\nlog.excludeDecoration=refs/scalar/* in all Scalar repos), but we will wait\nfor feedback from the community.\n\nUpdates in v2:\n\n * Use for_each_string_list_item()\n \n \n * Update the matching logic to allow --decorate-refs to override the config\n option.\n \n \n\nUpdates in v3:\n\n * Moved and refactored the ref_filter_match() in a preparation patch.\n \n \n * Used Junio's new logic in ref_filter_match()\n \n \n * Updated the config documentation to be more clear.\n \n \n\nThanks, -Stolee\n\nDerrick Stolee (2):\n log-tree: make ref_filter_match() a helper method\n log: add log.excludeDecoration config option\n\n Documentation/config/log.txt | 6 ++++\n Documentation/git-log.txt | 5 +++-\n builtin/log.c | 16 +++++++++-\n log-tree.c | 58 ++++++++++++++++++++++++++++++++++--\n log-tree.h | 4 ++-\n refs.c | 44 ---------------------------\n refs.h | 12 --------\n t/t4202-log.sh | 51 ++++++++++++++++++++++++++++++-\n 8 files changed, 133 insertions(+), 63 deletions(-)\n\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-610%2Fderrickstolee%2Flog-exclude-decoration-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-610/derrickstolee/log-exclude-decoration-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/610\n\nRange-diff vs v2:\n\n -: ----------- > 1: 6840f8801e4 log-tree: make ref_filter_match() a helper method\n 1: cbdaef4a8e1 ! 2: 96c865e9214 log: add log.excludeDecoration config option\n @@ Commit message\n [1] https://github.com/microsoft/scalar\n [2] https://lore.kernel.org/git/77b1da5d3063a2404cd750adfe3bb8be9b6c497d.1585946894.git.gitgitgadget@gmail.com/\n \n + Helped-by: Junio C Hamano \n Signed-off-by: Derrick Stolee \n \n ## Documentation/config/log.txt ##\n @@ Documentation/config/log.txt: log.decorate::\n \tof the `git log`.\n \n +log.excludeDecoration::\n -+\tExclude the specified patterns from the log decorations. This multi-\n -+\tvalued config option is the same as the `--decorate-refs-exclude`\n -+\toption of `git log`.\n ++\tExclude the specified patterns from the log decorations. This is\n ++\tsimilar to the `--decorate-refs-exclude` command-line option, but\n ++\tthe config option can be overridden by the `--decorate-refs`\n ++\toption.\n +\n log.follow::\n \tIf `true`, `git log` will act as if the `--follow` option was used when\n @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons\n \n \n ## log-tree.c ##\n -@@ log-tree.c: static int add_ref_decoration(const char *refname, const struct object_id *oid,\n +@@ log-tree.c: static int ref_filter_match(const char *refname,\n + \tstruct string_list_item *item;\n + \tconst struct string_list *exclude_patterns = filter->exclude_ref_pattern;\n + \tconst struct string_list *include_patterns = filter->include_ref_pattern;\n ++\tconst struct string_list *exclude_patterns_config =\n ++\t\t\t\tfilter->exclude_ref_config_pattern;\n + \n + \tif (exclude_patterns && exclude_patterns->nr) {\n + \t\tfor_each_string_list_item(item, exclude_patterns) {\n +@@ log-tree.c: static int ref_filter_match(const char *refname,\n + \t}\n + \n + \tif (include_patterns && include_patterns->nr) {\n +-\t\tint found = 0;\n + \t\tfor_each_string_list_item(item, include_patterns) {\n + \t\t\tif (match_ref_pattern(refname, item)) {\n +-\t\t\t\tfound = 1;\n +-\t\t\t\tbreak;\n ++\t\t\t\treturn 1;\n + \t\t\t}\n + \t\t}\n ++\t\treturn 0;\n ++\t}\n \n - \tif (filter && !ref_filter_match(refname,\n - \t\t\t filter->include_ref_pattern,\n --\t\t\t filter->exclude_ref_pattern))\n -+\t\t\t filter->exclude_ref_pattern,\n -+\t\t\t filter->exclude_ref_config_pattern))\n - \t\treturn 0;\n +-\t\tif (!found)\n +-\t\t\treturn 0;\n ++\tif (exclude_patterns_config && exclude_patterns_config->nr) {\n ++\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n ++\t\t\tif (match_ref_pattern(refname, item))\n ++\t\t\t\treturn 0;\n ++\t\t}\n + \t}\n ++\n + \treturn 1;\n + }\n \n - \tif (starts_with(refname, git_replace_ref_base)) {\n @@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flags)\n \t\t\tfor_each_string_list_item(item, filter->include_ref_pattern) {\n \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n @@ log-tree.h: struct log_info {\n \n int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);\n \n - ## refs.c ##\n -@@ refs.c: static int match_ref_pattern(const char *refname,\n - \n - int ref_filter_match(const char *refname,\n - \t\t const struct string_list *include_patterns,\n --\t\t const struct string_list *exclude_patterns)\n -+\t\t const struct string_list *exclude_patterns,\n -+\t\t const struct string_list *exclude_patterns_config)\n - {\n - \tstruct string_list_item *item;\n -+\tint found = 0;\n - \n - \tif (exclude_patterns && exclude_patterns->nr) {\n - \t\tfor_each_string_list_item(item, exclude_patterns) {\n -@@ refs.c: int ref_filter_match(const char *refname,\n - \t}\n - \n - \tif (include_patterns && include_patterns->nr) {\n --\t\tint found = 0;\n - \t\tfor_each_string_list_item(item, include_patterns) {\n - \t\t\tif (match_ref_pattern(refname, item)) {\n - \t\t\t\tfound = 1;\n -@@ refs.c: int ref_filter_match(const char *refname,\n - \t\tif (!found)\n - \t\t\treturn 0;\n - \t}\n -+\n -+\tif (!found &&\n -+\t exclude_patterns_config &&\n -+\t exclude_patterns_config->nr) {\n -+\t\tfor_each_string_list_item(item, exclude_patterns_config) {\n -+\t\t\tif (match_ref_pattern(refname, item))\n -+\t\t\t\treturn 0;\n -+\t\t}\n -+\t}\n -+\n - \treturn 1;\n - }\n - \n -\n - ## refs.h ##\n -@@ refs.h: void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n - \t\t\tconst char *pattern);\n - \n - /*\n -- * Returns 0 if refname matches any of the exclude_patterns, or if it doesn't\n -- * match any of the include_patterns. Returns 1 otherwise.\n -+ * Returns 0 if the refname matches any of the exclude_patterns.\n -+ *\n -+ * Returns 0 if include_patterns is non-empty but refname does not match\n -+ * any of those patterns.\n -+ *\n -+ * Returns 0 if refname matches a pattern in exclude_patterns_config but\n -+ * does not match any pattern in inclue_patterns.\n -+ *\n -+ * Otherwise, returns 1.\n - *\n -- * If pattern list is NULL or empty, matching against that list is skipped.\n - * This has the effect of matching everything by default, unless the user\n - * specifies rules otherwise.\n - */\n - int ref_filter_match(const char *refname,\n - \t\t const struct string_list *include_patterns,\n --\t\t const struct string_list *exclude_patterns);\n -+\t\t const struct string_list *exclude_patterns,\n -+\t\t const struct string_list *exclude_patterns_config);\n - \n - static inline const char *has_glob_specials(const char *pattern)\n - {\n -\n ## t/t4202-log.sh ##\n @@ t/t4202-log.sh: test_expect_success 'decorate-refs with glob' '\n \toctopus-a (octopus-a)\n\n-- \ngitgitgadget\n"},{"id":"395590","messageId":"6840f8801e43120cc78e9fa937fd54ad26a10180.1587046549.git.gitgitgadget@gmail.com","threadId":"53219","inReplyTo":"pull.610.v3.git.1587046549.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] log-tree: make ref_filter_match() a helper method","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-16T14:15:48Z","receivedAt":"2020-04-16T14:16:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe ref_filter_match() method is defined in refs.h and implemented\nin refs.c, but is only used by add_ref_decoration() in log-tree.c.\nMove it into that file as a static helper method. The\nmatch_ref_pattern() comes along for the ride.\n\nWhile moving the code, also make a slight adjustment to have\nref_filter_match() take a struct decoration_filter pointer instead\nof multiple string lists. This is non-functional, but will make a\nlater change be much cleaner.\n\nThe diff is easier to parse when using the --color-moved option.\n\nReported-by: Junio C Hamano \nSigned-off-by: Derrick Stolee \n---\n log-tree.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---\n refs.c | 44 --------------------------------------------\n refs.h | 12 ------------\n 3 files changed, 46 insertions(+), 59 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 52127427ffe..ab6d29a746b 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -81,6 +81,51 @@ const struct name_decoration *get_name_decoration(const struct object *obj)\n \treturn lookup_decoration(&name_decoration, obj);\n }\n \n+static int match_ref_pattern(const char *refname,\n+\t\t\t const struct string_list_item *item)\n+{\n+\tint matched = 0;\n+\tif (item->util == NULL) {\n+\t\tif (!wildmatch(item->string, refname, 0))\n+\t\t\tmatched = 1;\n+\t} else {\n+\t\tconst char *rest;\n+\t\tif (skip_prefix(refname, item->string, &rest) &&\n+\t\t (!*rest || *rest == '/'))\n+\t\t\tmatched = 1;\n+\t}\n+\treturn matched;\n+}\n+\n+static int ref_filter_match(const char *refname,\n+\t\t\t const struct decoration_filter *filter)\n+{\n+\tstruct string_list_item *item;\n+\tconst struct string_list *exclude_patterns = filter->exclude_ref_pattern;\n+\tconst struct string_list *include_patterns = filter->include_ref_pattern;\n+\n+\tif (exclude_patterns && exclude_patterns->nr) {\n+\t\tfor_each_string_list_item(item, exclude_patterns) {\n+\t\t\tif (match_ref_pattern(refname, item))\n+\t\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n+\tif (include_patterns && include_patterns->nr) {\n+\t\tint found = 0;\n+\t\tfor_each_string_list_item(item, include_patterns) {\n+\t\t\tif (match_ref_pattern(refname, item)) {\n+\t\t\t\tfound = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (!found)\n+\t\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n static int add_ref_decoration(const char *refname, const struct object_id *oid,\n \t\t\t int flags, void *cb_data)\n {\n@@ -88,9 +133,7 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,\n \tenum decoration_type type = DECORATION_NONE;\n \tstruct decoration_filter *filter = (struct decoration_filter *)cb_data;\n \n-\tif (filter && !ref_filter_match(refname,\n-\t\t\t filter->include_ref_pattern,\n-\t\t\t filter->exclude_ref_pattern))\n+\tif (filter && !ref_filter_match(refname, filter))\n \t\treturn 0;\n \n \tif (starts_with(refname, git_replace_ref_base)) {\ndiff --git a/refs.c b/refs.c\nindex 1ab0bb54d3d..28c91d603c2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -321,50 +321,6 @@ int ref_exists(const char *refname)\n \treturn refs_ref_exists(get_main_ref_store(the_repository), refname);\n }\n \n-static int match_ref_pattern(const char *refname,\n-\t\t\t const struct string_list_item *item)\n-{\n-\tint matched = 0;\n-\tif (item->util == NULL) {\n-\t\tif (!wildmatch(item->string, refname, 0))\n-\t\t\tmatched = 1;\n-\t} else {\n-\t\tconst char *rest;\n-\t\tif (skip_prefix(refname, item->string, &rest) &&\n-\t\t (!*rest || *rest == '/'))\n-\t\t\tmatched = 1;\n-\t}\n-\treturn matched;\n-}\n-\n-int ref_filter_match(const char *refname,\n-\t\t const struct string_list *include_patterns,\n-\t\t const struct string_list *exclude_patterns)\n-{\n-\tstruct string_list_item *item;\n-\n-\tif (exclude_patterns && exclude_patterns->nr) {\n-\t\tfor_each_string_list_item(item, exclude_patterns) {\n-\t\t\tif (match_ref_pattern(refname, item))\n-\t\t\t\treturn 0;\n-\t\t}\n-\t}\n-\n-\tif (include_patterns && include_patterns->nr) {\n-\t\tint found = 0;\n-\t\tfor_each_string_list_item(item, include_patterns) {\n-\t\t\tif (match_ref_pattern(refname, item)) {\n-\t\t\t\tfound = 1;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n-\n-\t\tif (!found)\n-\t\t\treturn 0;\n-\t}\n-\treturn 1;\n-}\n-\n static int filter_refs(const char *refname, const struct object_id *oid,\n \t\t\t int flags, void *data)\n {\ndiff --git a/refs.h b/refs.h\nindex 545029c6d80..a92d2c74c83 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -361,18 +361,6 @@ int for_each_rawref(each_ref_fn fn, void *cb_data);\n void normalize_glob_ref(struct string_list_item *item, const char *prefix,\n \t\t\tconst char *pattern);\n \n-/*\n- * Returns 0 if refname matches any of the exclude_patterns, or if it doesn't\n- * match any of the include_patterns. Returns 1 otherwise.\n- *\n- * If pattern list is NULL or empty, matching against that list is skipped.\n- * This has the effect of matching everything by default, unless the user\n- * specifies rules otherwise.\n- */\n-int ref_filter_match(const char *refname,\n-\t\t const struct string_list *include_patterns,\n-\t\t const struct string_list *exclude_patterns);\n-\n static inline const char *has_glob_specials(const char *pattern)\n {\n \treturn strpbrk(pattern, \"?*[\");\n-- \ngitgitgadget\n\n"},{"id":"395608","messageId":"xmqqo8rrz5gn.fsf@gitster.c.googlers.com","threadId":"53219","inReplyTo":"96c865e9214802021f8e991d2194b7aea6cd6bba.1587046549.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] log: add log.excludeDecoration config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-16T17:49:12Z","receivedAt":"2020-04-16T17:49:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> diff --git a/log-tree.c b/log-tree.c\n> index ab6d29a746b..fd3fd3316a1 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -103,6 +103,8 @@ static int ref_filter_match(const char *refname,\n> \tstruct string_list_item *item;\n> \tconst struct string_list *exclude_patterns = filter->exclude_ref_pattern;\n> \tconst struct string_list *include_patterns = filter->include_ref_pattern;\n> +\tconst struct string_list *exclude_patterns_config =\n> +\t\t\t\tfilter->exclude_ref_config_pattern;\n> \n> \tif (exclude_patterns && exclude_patterns->nr) {\n> \t\tfor_each_string_list_item(item, exclude_patterns) {\n> @@ -112,17 +114,21 @@ static int ref_filter_match(const char *refname,\n> \t}\n> \n> \tif (include_patterns && include_patterns->nr) {\n> -\t\tint found = 0;\n> \t\tfor_each_string_list_item(item, include_patterns) {\n> \t\t\tif (match_ref_pattern(refname, item)) {\n> -\t\t\t\tfound = 1;\n> -\t\t\t\tbreak;\n> +\t\t\t\treturn 1;\n> \t\t\t}\n\nMicronit. \n\nLet's mimick the early return in the loop above (for command line\nexcludes) and below (for configured excludes), each of which is just\na single \"return\" statement in a block without {braces} around.\n\nOther than that, looks nicely done.\n\nThe new tests are really appreciated, too.\n"},{"id":"395610","messageId":"xmqqftd3z4sa.fsf@gitster.c.googlers.com","threadId":"53219","inReplyTo":"xmqqo8rrz5gn.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/2] log: add log.excludeDecoration config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-16T18:03:49Z","receivedAt":"2020-04-16T18:03:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> \tif (include_patterns && include_patterns->nr) {\n>> -\t\tint found = 0;\n>> \t\tfor_each_string_list_item(item, include_patterns) {\n>> \t\t\tif (match_ref_pattern(refname, item)) {\n>> -\t\t\t\tfound = 1;\n>> -\t\t\t\tbreak;\n>> +\t\t\t\treturn 1;\n>> \t\t\t}\n>\n> Micronit. \n>\n> Let's mimick the early return in the loop above (for command line\n> excludes) and below (for configured excludes), each of which is just\n> a single \"return\" statement in a block without {braces} around.\n\n... heh, it seems that the nit is mine in the suggested alternative\nupthread. Let me amend while queuing.\n\nThanks.\n"},{"id":"395640","messageId":"481d4e05-c6f8-0554-e8f6-04562d316c4c@gmail.com","threadId":"53219","inReplyTo":"xmqqftd3z4sa.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/2] log: add log.excludeDecoration config option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-04-17T01:53:51Z","receivedAt":"2020-04-17T01:53:56Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/16/2020 2:03 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>>> \tif (include_patterns && include_patterns->nr) {\n>>> -\t\tint found = 0;\n>>> \t\tfor_each_string_list_item(item, include_patterns) {\n>>> \t\t\tif (match_ref_pattern(refname, item)) {\n>>> -\t\t\t\tfound = 1;\n>>> -\t\t\t\tbreak;\n>>> +\t\t\t\treturn 1;\n>>> \t\t\t}\n>>\n>> Micronit. \n>>\n>> Let's mimick the early return in the loop above (for command line\n>> excludes) and below (for configured excludes), each of which is just\n>> a single \"return\" statement in a block without {braces} around.\n> \n> ... heh, it seems that the nit is mine in the suggested alternative\n> upthread. Let me amend while queuing.\n\nShame on me for not noticing. Thanks for the close look!\n\n-Stolee\n"},{"id":"395642","messageId":"xmqqv9lyx43a.fsf@gitster.c.googlers.com","threadId":"53219","inReplyTo":"481d4e05-c6f8-0554-e8f6-04562d316c4c@gmail.com","subject":"Re: [PATCH v3 2/2] log: add log.excludeDecoration config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-17T02:01:45Z","receivedAt":"2020-04-17T02:01:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 4/16/2020 2:03 PM, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>> \n>>>> \tif (include_patterns && include_patterns->nr) {\n>>>> -\t\tint found = 0;\n>>>> \t\tfor_each_string_list_item(item, include_patterns) {\n>>>> \t\t\tif (match_ref_pattern(refname, item)) {\n>>>> -\t\t\t\tfound = 1;\n>>>> -\t\t\t\tbreak;\n>>>> +\t\t\t\treturn 1;\n>>>> \t\t\t}\n>>>\n>>> Micronit. \n>>>\n>>> Let's mimick the early return in the loop above (for command line\n>>> excludes) and below (for configured excludes), each of which is just\n>>> a single \"return\" statement in a block without {braces} around.\n>> \n>> ... heh, it seems that the nit is mine in the suggested alternative\n>> upthread. Let me amend while queuing.\n>\n> Shame on me for not noticing. Thanks for the close look!\n\nHeh, shame is mine, too ;-)\n\nThanks.\n"}]}