| Message ID | 8142b32f023685c78936a618d7b93aaf86ba77fb.1659122979.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | log: create tighter default decoration filter | expand |
On 2022.07.29 19:29, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <derrickstolee@github.com> > > The previous change introduced the --decorate-all option for users who > do not want their decorations limited to a narrow set of ref namespaces. > > Add a config option that is equivalent to specifying --decorate-all by > default. > > Signed-off-by: Derrick Stolee <derrickstolee@github.com> > --- > Documentation/config/log.txt | 5 +++++ > Documentation/git-log.txt | 4 +++- > builtin/log.c | 12 ++++++++++++ > t/t4202-log.sh | 3 +++ > t/t9902-completion.sh | 3 +++ > 5 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt > index 456eb07800c..615cb26e5c9 100644 > --- a/Documentation/config/log.txt > +++ b/Documentation/config/log.txt > @@ -18,6 +18,11 @@ log.decorate:: > names are shown. This is the same as the `--decorate` option > of the `git log`. > > +log.decorateFilter:: > + By default, `git log` only shows decorations for certain known ref > + namespaces. If 'all' is specified, then show all possible ref > + decorations. Default value is 'default'. > + Could we make it more clear here that "all" is the only supported value? IIUC any other value will just get the default behavior. Just reading the doc as-is, I worry that users might expect that they can add specific refs / ref-patterns that would be added to the filter.
On 8/3/2022 6:42 PM, Josh Steadmon wrote: > On 2022.07.29 19:29, Derrick Stolee via GitGitGadget wrote: >> +log.decorateFilter:: >> + By default, `git log` only shows decorations for certain known ref >> + namespaces. If 'all' is specified, then show all possible ref >> + decorations. Default value is 'default'. >> + > > Could we make it more clear here that "all" is the only supported value? > IIUC any other value will just get the default behavior. Just reading > the doc as-is, I worry that users might expect that they can add > specific refs / ref-patterns that would be added to the filter. Perhaps that's a sign that the config key is poorly named. I had considered other options: * log.decorateFilterMode (seemed too long) * log.decorateMode (hard to distinguish from log.decorate) Perhaps we could leave this as a boolean config with the name 'log.decorateHiddenRefs' with default being 'false'? I'd love to explore other options. Thanks, -Stolee
diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt index 456eb07800c..615cb26e5c9 100644 --- a/Documentation/config/log.txt +++ b/Documentation/config/log.txt @@ -18,6 +18,11 @@ log.decorate:: names are shown. This is the same as the `--decorate` option of the `git log`. +log.decorateFilter:: + By default, `git log` only shows decorations for certain known ref + namespaces. If 'all' is specified, then show all possible ref + decorations. Default value is 'default'. + log.excludeDecoration:: Exclude the specified patterns from the log decorations. This is similar to the `--decorate-refs-exclude` command-line option, but diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index 633705bde90..c35f9b8ad9e 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -59,7 +59,9 @@ used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`, --decorate-all:: When specified, this option clears all previous `--decorate-refs` or `--decorate-refs-exclude` options and relaxes the default - decoration filter to include all possible decoration refs. + decoration filter to include all possible decoration refs. This + option is assumed if the config value `log.decorateFilter` is set + to `all`. --source:: Print out the ref name given on the command line by which each diff --git a/builtin/log.c b/builtin/log.c index b7fc4946c35..961fe3ae45b 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -184,6 +184,7 @@ static void cmd_log_init_defaults(struct rev_info *rev) static void set_default_decoration_filter(struct decoration_filter *decoration_filter) { int i; + char *value = NULL; struct string_list *include = decoration_filter->include_ref_pattern; const struct string_list *config_exclude = git_config_get_value_multi("log.excludeDecoration"); @@ -195,6 +196,17 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f item->string); } + /* + * By default, decorate_all is disabled. Enable it if + * log.decorateMode=all. Don't ever disable it by config, since + * the command-line takes precedent. + */ + if (!decorate_all && + !git_config_get_string("log.decoratefilter", &value) && + !strcmp("all", value)) + decorate_all = 1; + free(value); + if (decorate_all || decoration_filter->exclude_ref_pattern->nr || decoration_filter->include_ref_pattern->nr || diff --git a/t/t4202-log.sh b/t/t4202-log.sh index e939ed34ff7..6d96710fdfa 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -1090,6 +1090,9 @@ test_expect_success '--decorate-all overrides defaults' ' EOF git log --decorate=full --pretty="tformat:%f%d" \ --decorate-all >actual && + test_cmp expect.all actual && + git -c log.decorateFilter=all log \ + --decorate=full --pretty="tformat:%f%d" >actual && test_cmp expect.all actual ' diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 31526e6b641..7a0c7b3c372 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -2489,6 +2489,7 @@ test_expect_success 'git config - variable name' ' test_completion "git config log.d" <<-\EOF log.date Z log.decorate Z + log.decorateFilter Z log.diffMerges Z EOF ' @@ -2511,6 +2512,7 @@ test_expect_success 'git -c - variable name' ' test_completion "git -c log.d" <<-\EOF log.date=Z log.decorate=Z + log.decorateFilter=Z log.diffMerges=Z EOF ' @@ -2533,6 +2535,7 @@ test_expect_success 'git clone --config= - variable name' ' test_completion "git clone --config=log.d" <<-\EOF log.date=Z log.decorate=Z + log.decorateFilter=Z log.diffMerges=Z EOF '