[PATCH 0/3] log: create tighter default decoration filter

[PATCH 0/3] log: create tighter default decoration filter

[Derrick Stolee via GitGitGadget]

When running 'git log', the default is to report any refs within refs/* as
decorations. This series reduces the default set to be more specific to a
set of known-useful namespaces.

This was previously reduced by adding the log.excludeDecoration config
option and modifying that config in git maintenance's prefetch task (to hide
refs/prefetch/*). I then followed that pattern again for the bundle URI
feature [1], but this caught some reviewers by surprise as an unfortunate
side-effect. This series is a way to roll back the previous decision to use
log.excludeDecoration and instead use tighter filters by default.

[1]
https://lore.kernel.org/git/a217e9a0640b45d21ef971d6e91cee3f1993f383.1656535245.git.gitgitgadget@gmail.com/

As noted in the last patch, the current design ignores the new filters if
there are any previously-specified filters. This includes the
log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
This means that users who ran that command in their repo will not get the
benefits of the more strict filters. While we stop writing
log.excludeDecorations, we don't remove existing instances of it.

I'm interested if anyone has another way around this issue, or if we
consider adding the default filter as long as no --decorate=refs options are
specified.

Thanks, -Stolee

Derrick Stolee (3):
  refs: allow "HEAD" as decoration filter
  log: add default decoration filter
  maintenance: stop writing log.excludeDecoration

 Documentation/git-log.txt |  7 ++++--
 builtin/gc.c              |  6 -----
 builtin/log.c             | 53 +++++++++++++++++++++++++++++----------
 refs.c                    |  4 +--
 t/t4202-log.sh            | 29 +++++++++++++++++++++
 t/t7900-maintenance.sh    | 21 ----------------
 6 files changed, 76 insertions(+), 44 deletions(-)


base-commit: 6a475b71f8c4ce708d69fdc9317aefbde3769e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1301%2Fderrickstolee%2Fdecorations-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1301/derrickstolee/decorations-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1301
-- 
gitgitgadget

[PATCH 1/3] refs: allow "HEAD" as decoration filter

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The normalize_glob_ref() method was introduced in 65516f586b693 (log:
add option to choose which refs to decorate, 2017-11-21) to help with
decoration filters such as --decorate-refs=<filter> and
--decorate-refs-exclude=<filter>. The method has not been used anywhere
else.

At the moment, it is impossible to specify HEAD as a decoration filter
since normalize_glob_ref() prepends "refs/" to the filter if it isn't
already there.

Allow adding HEAD as a decoration filter by allowing the exact string
"HEAD" to not be prepended with "refs/". Add a test in t4202-log.sh that
would previously fail since the HEAD decoration would exist in the
output.

It is sufficient to only cover "HEAD" here and not include other special
refs like REBASE_HEAD. This is because HEAD is the only ref outside of
refs/* that is added to the list of decorations.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 refs.c         | 4 ++--
 t/t4202-log.sh | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 90bcb271687..ec3134e4842 100644
--- a/refs.c
+++ b/refs.c
@@ -457,8 +457,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
 
 	if (prefix) {
 		strbuf_addstr(&normalized_pattern, prefix);
-	}
-	else if (!starts_with(pattern, "refs/"))
+	} else if (!starts_with(pattern, "refs/") &&
+		   strcmp(pattern, "HEAD"))
 		strbuf_addstr(&normalized_pattern, "refs/");
 	strbuf_addstr(&normalized_pattern, pattern);
 	strbuf_strip_suffix(&normalized_pattern, "/");
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6e663525582..6b7d8e269f7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1025,6 +1025,12 @@ test_expect_success 'decorate-refs and simplify-by-decoration without output' '
 	test_cmp expect actual
 '
 
+test_expect_success 'decorate-refs-exclude HEAD' '
+	git log --decorate=full --oneline \
+		--decorate-refs-exclude="HEAD" >actual &&
+	! grep HEAD actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
-- 
gitgitgadget

Re: [PATCH 1/3] refs: allow "HEAD" as decoration filter

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
>
> The normalize_glob_ref() method was introduced in 65516f586b693 (log:
> add option to choose which refs to decorate, 2017-11-21) to help with
> decoration filters such as --decorate-refs=<filter> and
> --decorate-refs-exclude=<filter>. The method has not been used anywhere
> else.
>
> At the moment, it is impossible to specify HEAD as a decoration filter
> since normalize_glob_ref() prepends "refs/" to the filter if it isn't
> already there.
>
> Allow adding HEAD as a decoration filter by allowing the exact string
> "HEAD" to not be prepended with "refs/". Add a test in t4202-log.sh that
> would previously fail since the HEAD decoration would exist in the
> output.
>
> It is sufficient to only cover "HEAD" here and not include other special
> refs like REBASE_HEAD. This is because HEAD is the only ref outside of
> refs/* that is added to the list of decorations.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  refs.c         | 4 ++--
>  t/t4202-log.sh | 6 ++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 90bcb271687..ec3134e4842 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -457,8 +457,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
>  
>  	if (prefix) {
>  		strbuf_addstr(&normalized_pattern, prefix);
> -	}
> -	else if (!starts_with(pattern, "refs/"))
> +	} else if (!starts_with(pattern, "refs/") &&
> +		   strcmp(pattern, "HEAD"))
>  		strbuf_addstr(&normalized_pattern, "refs/");

Arguably a digression for this series, but I think it would be very nice
to have a pre-step where we'd create some pre-step to centrally declare
these magical refs in some struct whose flags would cover these various
cases.

E.g. we have this hardcoded in should_autocreate_reflog(), in log-tree.c
(which AFAICT is the reference to "HEAD" here), refs.c etc. etc.

Re: [PATCH 1/3] refs: allow "HEAD" as decoration filter

[Derrick Stolee]

On 7/26/2022 11:10 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:
>>  	if (prefix) {
>>  		strbuf_addstr(&normalized_pattern, prefix);
>> -	}
>> -	else if (!starts_with(pattern, "refs/"))
>> +	} else if (!starts_with(pattern, "refs/") &&
>> +		   strcmp(pattern, "HEAD"))
>>  		strbuf_addstr(&normalized_pattern, "refs/");
> 
> Arguably a digression for this series, but I think it would be very nice
> to have a pre-step where we'd create some pre-step to centrally declare
> these magical refs in some struct whose flags would cover these various
> cases.
> 
> E.g. we have this hardcoded in should_autocreate_reflog(), in log-tree.c
> (which AFAICT is the reference to "HEAD" here), refs.c etc. etc.
 
It's a good idea to collect our known ref namespaces into a
single place so we can update them together when a new feature
is added. This could help avoid the inconsistencies that already
exist (such as not filtering "HEAD" or not coloring "refs/notes/").

Thanks,
-Stolee

[PATCH 2/3] log: add default decoration filter

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

When a user runs 'git log', they expect a certain set of helpful
decorations. This includes:

* The HEAD ref
* Branches (refs/heads/)
* Notes (refs/notes/)
* Stashes (refs/stash/)
* Tags (refs/tags/)
* Remote branches (refs/remotes/)

However, there are other refs that are less helpful to show as
decoration:

* Prefetch refs (refs/prefetch/)
* Rebase refs (refs/rebase-merge/ and refs/rebase-apply/)
* Bundle refs (refs/bundle/) [!]

In the case of prefetch refs, 96eaffebbf3d0 (maintenance: set
log.excludeDecoration durin prefetch, 2021-01-19) added logic to add
refs/prefetch/ to the log.excludeDecoration config option. Additional
feedback pointed out that having such a side-effect can be confusing and
perhaps not helpful to users. Instead, we should hide these ref
namespaces that are being used by Git for internal reasons but are not
helpful for the users to see.

The way to provide a seamless user experience without setting the config
is to modify the default decoration filters to match our expectation of
what refs the user actually wants to see.

In builtin/log.c, after parsing the --decorate-refs and
--decorate-refs-exclude options from the command-line, call
set_default_decoration_filter(). This method populates the exclusions
from log.excludeDecoration, then checks if the list of pattern
modifications are empty. If none are specified, then the default set is
restricted to the set of inclusions mentioned earlier (HEAD, branches,
etc.).

Note that the logic in ref_filter_match() in log-tree.c follows this
matching pattern:

 1. If there are exclusion patterns and the ref matches one, then ignore
    the decoration.

 2. If there are inclusion patterns and the ref matches one, then
    definitely include the decoration.

 3. If there are config-based exclusions from log.excludeDecoration and
    the ref matches one, then ignore the decoration.

With this logic in mind, we need to ensure that we do not populate our
new defaults if any of these filters are manually set. Specifically, if
a user runs

	git -c log.excludeDecoration=HEAD log

then we expect the HEAD decoration to not appear. If we left the default
inclusions in the set, then HEAD would match that inclusion before
reaching the config-based exclusions.

A potential alternative would be to check the list of default inclusions
at the end, after the config-based exclusions. This would still create a
behavior change for some uses of --decorate-refs-exclude=<X>, and could
be overwritten somewhat with --decorate-refs=refs/ and
--decorate-refs=HEAD. However, it no longer becomes possible to include
refs outside of the defaults while also excluding some using
log.excludeDecoration.

Add a test that checks that the defaults are not added when
--decorate-refs is specified. We verify this by showing that HEAD is not
included as it normally would.  Also add a test that shows that the
default filter avoids the unwanted decorations from refs/prefetch,
refs/rebase-merge,
and refs/bundle.

There are several tests throughout the codebase that verify that
--decorate-refs, --decorate-refs-exclude, and log.excludeDecoration work
as designed and the tests continue to pass without intervention.

[!] The bundle refs are part of a parallel series that bootstraps a repo
from a bundle file, storing the bundle's refs into the repo's
refs/bundle/ namespace.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-log.txt |  7 ++++--
 builtin/log.c             | 53 +++++++++++++++++++++++++++++----------
 t/t4202-log.sh            | 23 +++++++++++++++++
 3 files changed, 68 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 20e87cecf49..14b67f3d058 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -45,13 +45,16 @@ OPTIONS
 
 --decorate-refs=<pattern>::
 --decorate-refs-exclude=<pattern>::
-	If no `--decorate-refs` is given, pretend as if all refs were
-	included.  For each candidate, do not use it for decoration if it
+	For each candidate reference, do not use it for decoration if it
 	matches any patterns given to `--decorate-refs-exclude` or if it
 	doesn't match any of the patterns given to `--decorate-refs`. The
 	`log.excludeDecoration` config option allows excluding refs from
 	the decorations, but an explicit `--decorate-refs` pattern will
 	override a match in `log.excludeDecoration`.
++
+If none of these options or config settings are given, then references are
+used as decoration if they match `HEAD`, `refs/heads/`, `refs/notes/`,
+`refs/remotes/`, `refs/stash/`, or `refs/tags/`.
 
 --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 88a5e98875a..dca06612b8e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -162,6 +162,40 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 		parse_date_format(default_date_mode, &rev->date_mode);
 }
 
+static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
+{
+	const struct string_list *config_exclude =
+		repo_config_get_value_multi(the_repository, "log.excludeDecoration");
+
+	if (config_exclude) {
+		struct string_list_item *item;
+		for_each_string_list_item(item, config_exclude)
+			string_list_append(decoration_filter->exclude_ref_config_pattern,
+					   item->string);
+	}
+
+	if (!decoration_filter->exclude_ref_pattern->nr &&
+	    !decoration_filter->include_ref_pattern->nr &&
+	    !decoration_filter->exclude_ref_config_pattern->nr) {
+		/*
+		 * No command-line or config options were given, so
+		 * populate with sensible defaults.
+		 */
+		string_list_append(decoration_filter->include_ref_pattern,
+				   "refs/heads/");
+		string_list_append(decoration_filter->include_ref_pattern,
+				   "refs/notes/");
+		string_list_append(decoration_filter->include_ref_pattern,
+				   "refs/stash/");
+		string_list_append(decoration_filter->include_ref_pattern,
+				   "refs/tags/");
+		string_list_append(decoration_filter->include_ref_pattern,
+				   "refs/remotes/");
+		string_list_append(decoration_filter->include_ref_pattern,
+				   "HEAD");
+	}
+}
+
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
@@ -171,9 +205,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
 	static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
 	static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
-	struct decoration_filter decoration_filter = {&decorate_refs_include,
-						      &decorate_refs_exclude,
-						      &decorate_refs_exclude_config};
+	struct decoration_filter decoration_filter = {
+		.exclude_ref_pattern = &decorate_refs_exclude,
+		.include_ref_pattern = &decorate_refs_include,
+		.exclude_ref_config_pattern = &decorate_refs_exclude_config,
+	};
 	static struct revision_sources revision_sources;
 
 	const struct option builtin_log_options[] = {
@@ -265,16 +301,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	}
 
 	if (decoration_style || rev->simplify_by_decoration) {
-		const struct string_list *config_exclude =
-			repo_config_get_value_multi(the_repository,
-						    "log.excludeDecoration");
-
-		if (config_exclude) {
-			struct string_list_item *item;
-			for_each_string_list_item(item, config_exclude)
-				string_list_append(&decorate_refs_exclude_config,
-						   item->string);
-		}
+		set_default_decoration_filter(&decoration_filter);
 
 		if (decoration_style)
 			rev->show_decorations = 1;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6b7d8e269f7..1a41096c8e2 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1031,6 +1031,12 @@ test_expect_success 'decorate-refs-exclude HEAD' '
 	! grep HEAD actual
 '
 
+test_expect_success 'decorate-refs focus from default' '
+	git log --decorate=full --oneline \
+		--decorate-refs="refs/heads" >actual &&
+	! grep HEAD actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
@@ -2198,6 +2204,23 @@ test_expect_success 'log --decorate includes all levels of tag annotated tags' '
 	test_cmp expect actual
 '
 
+test_expect_success 'log --decorate does not include things outside filter' '
+	reflist="refs/prefetch/ refs/rebase-merge/ refs/bundle/" &&
+
+	for ref in $reflist
+	do
+		mkdir -p .git/$ref &&
+		echo $(git rev-parse HEAD) >.git/$ref/fake || return 1
+	done &&
+
+	git log --decorate=full --oneline >actual &&
+
+	for ref in $reflist
+	do
+		! grep $ref/fake actual || return 1
+	done
+'
+
 test_expect_success 'log --end-of-options' '
        git update-ref refs/heads/--source HEAD &&
        git log --end-of-options --source >actual &&
-- 
gitgitgadget

Re: [PATCH 2/3] log: add default decoration filter

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
> [...]
>  test_expect_success 'log.decorate config parsing' '
>  	git log --oneline --decorate=full >expect.full &&
>  	git log --oneline --decorate=short >expect.short &&
> @@ -2198,6 +2204,23 @@ test_expect_success 'log --decorate includes all levels of tag annotated tags' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'log --decorate does not include things outside filter' '
> +	reflist="refs/prefetch/ refs/rebase-merge/ refs/bundle/" &&
> +
> +	for ref in $reflist
> +	do
> +		mkdir -p .git/$ref &&

Let's not use -p, and fail if .git doesn't exist?

> +		echo $(git rev-parse HEAD) >.git/$ref/fake || return 1

Hiding the exit code of the "git rev-parse here, but aside fram that why
is the echo needed at all, can't we just:

	git rev-parse HEAD >.git/ref/fake

But even more generally can't we:

	git update-ref $ref/fake HEAD

?

If we need to manually munge the ref store let's add REFFILES prereq.

Re: [PATCH 2/3] log: add default decoration filter

[Derrick Stolee]

On 7/26/2022 10:39 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>> [...]
>>  test_expect_success 'log.decorate config parsing' '
>>  	git log --oneline --decorate=full >expect.full &&
>>  	git log --oneline --decorate=short >expect.short &&
>> @@ -2198,6 +2204,23 @@ test_expect_success 'log --decorate includes all levels of tag annotated tags' '
>>  	test_cmp expect actual
>>  '
>>  
>> +test_expect_success 'log --decorate does not include things outside filter' '
>> +	reflist="refs/prefetch/ refs/rebase-merge/ refs/bundle/" &&
>> +
>> +	for ref in $reflist
>> +	do
>> +		mkdir -p .git/$ref &&
> 
> Let's not use -p, and fail if .git doesn't exist?

The reflist contains multi-leveled refs. However, they don't go more than
one level beyond .git/refs/, so -p shouldn't be necessary since .git/refs/
should exist.
 
>> +		echo $(git rev-parse HEAD) >.git/$ref/fake || return 1
> 
> Hiding the exit code of the "git rev-parse here, but aside fram that why
> is the echo needed at all, can't we just:
> 
> 	git rev-parse HEAD >.git/ref/fake
> 
> But even more generally can't we:
> 
> 	git update-ref $ref/fake HEAD
> 
> ?

This was just a brain fart on my part.

> If we need to manually munge the ref store let's add REFFILES prereq.

We don't. Thanks.

-Stolee

Re: [PATCH 2/3] log: add default decoration filter

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:

> +static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
> +{
> +	const struct string_list *config_exclude =
> +		repo_config_get_value_multi(the_repository, "log.excludeDecoration");

Just use git_config_get_value_multi. It supplies the_repository.

> +	if (config_exclude) {
> +		struct string_list_item *item;
> +		for_each_string_list_item(item, config_exclude)
> +			string_list_append(decoration_filter->exclude_ref_config_pattern,
> +					   item->string);
> +	}

mm, not the first time I've wanted the equivalent of strbuf_addbuf but
for a string_list...

> +
> +	if (!decoration_filter->exclude_ref_pattern->nr &&
> +	    !decoration_filter->include_ref_pattern->nr &&
> +	    !decoration_filter->exclude_ref_config_pattern->nr) {

Could return if this isn't true, and save ourselves the subsequent
indentation...

> +		/*
> +		 * No command-line or config options were given, so
> +		 * populate with sensible defaults.
> +		 */
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/heads/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/notes/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/stash/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/tags/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/remotes/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "HEAD");
> +	}
> +}

Nit: This would be much easier to read if early on there was a:

	struct string_list *list = decoration_filter->include_ref_pattern;

You'd also be able to avoid much of the line wrapping.

Re: [PATCH 2/3] log: add default decoration filter

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> When a user runs 'git log', they expect a certain set of helpful
> decorations. This includes:
>
> * The HEAD ref

OK.

> * Branches (refs/heads/)

OK.

> * Notes (refs/notes/)

Questionable.  None of the notes commits are on the primary history,
so it is unlikely to be even used for decoration, so it would not
hurt to have them as part of possible decoration sources, but on the
other hand, the value of using them is dubious.

> * Stashes (refs/stash/)

Questionable, for the same reason as 'notes'.

> * Tags (refs/tags/)

Abolutely useful.

> * Remote branches (refs/remotes/)

Ditto.

>  --decorate-refs=<pattern>::
>  --decorate-refs-exclude=<pattern>::
> -	If no `--decorate-refs` is given, pretend as if all refs were
> -	included.  For each candidate, do not use it for decoration if it
> +	For each candidate reference, do not use it for decoration if it
>  	matches any patterns given to `--decorate-refs-exclude` or if it
>  	doesn't match any of the patterns given to `--decorate-refs`. The
>  	`log.excludeDecoration` config option allows excluding refs from
>  	the decorations, but an explicit `--decorate-refs` pattern will
>  	override a match in `log.excludeDecoration`.
> ++
> +If none of these options or config settings are given, then references are
> +used as decoration if they match `HEAD`, `refs/heads/`, `refs/notes/`,
> +`refs/remotes/`, `refs/stash/`, or `refs/tags/`.
>  
>  --source::
>  	Print out the ref name given on the command line by which each

OK.

Thanks.

Re: [PATCH 2/3] log: add default decoration filter

[Derrick Stolee]

On 7/27/2022 10:50 AM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>>
>> When a user runs 'git log', they expect a certain set of helpful
>> decorations. This includes:
>>
>> * The HEAD ref
> 
> OK.
> 
>> * Branches (refs/heads/)
> 
> OK.
> 
>> * Notes (refs/notes/)
> 
> Questionable.  None of the notes commits are on the primary history,
> so it is unlikely to be even used for decoration, so it would not
> hurt to have them as part of possible decoration sources, but on the
> other hand, the value of using them is dubious.

I had originally left this out of the filter, but then noticed that test
4013.102 (git log --decorate --all # magic is (not used)) includes an
instance of refs/notes/. Perhaps this is just old test cruft and could be
changed.

I could remove this from the filter and change the old test to match the
new behavior.

>> * Stashes (refs/stash/)
> 
> Questionable, for the same reason as 'notes'.

I had also left this out initially, but t4207-log-decoration-colors.sh
fails, and it's because 'stash' is one of the supported "slots" in the
color.decorate.<slot> config option. This elevates the importance of the
stash refs. (Notes are not a slot in this config option, for instance.)

The other slot that isn't included in my list is "grafted". This includes
grafts _and_ replace-objects.  Grafts appear to add decorations in a
different path that side-steps this filter, but replace-objects are
subject to this filter. Things are a bit more complicated here because of
the GIT_REPLACE_REF_BASE environment variable.

I'll add replace-objects in the next re-roll using the value of
GIT_REPLACE_REF_BASE to construct the filter. I'll add some tests to keep
the existing behavior, too, especially around color.decorate.grafted.

>> * Tags (refs/tags/)
> 
> Abolutely useful.
> 
>> * Remote branches (refs/remotes/)
> 
> Ditto.

Thanks,
-Stolee

Re: [PATCH 2/3] log: add default decoration filter

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

>>> * Notes (refs/notes/)
>> 
>> Questionable.  None of the notes commits are on the primary history,
>> so it is unlikely to be even used for decoration, so it would not
>> hurt to have them as part of possible decoration sources, but on the
>> other hand, the value of using them is dubious.
>
> I had originally left this out of the filter, but then noticed that test
> 4013.102 (git log --decorate --all # magic is (not used)) includes an
> instance of refs/notes/. Perhaps this is just old test cruft and could be
> changed.
>
> I could remove this from the filter and change the old test to match the
> new behavior.

I can summarize what I said to "Keeping this will not hurt, but it
is unclear if anybody depends on it"---and I am OK to err on the
safe side, i.e. keep notes and stash in the default set.

Re: [PATCH 2/3] log: add default decoration filter

[Jeff King]

On Wed, Jul 27, 2022 at 07:50:51AM -0700, Junio C Hamano wrote:

> > * Notes (refs/notes/)
> 
> Questionable.  None of the notes commits are on the primary history,
> so it is unlikely to be even used for decoration, so it would not
> hurt to have them as part of possible decoration sources, but on the
> other hand, the value of using them is dubious.
> 
> > * Stashes (refs/stash/)
> 
> Questionable, for the same reason as 'notes'.

Even more so, IMHO, because we are not even decorating all of the
stashes. Because the stash is kept in a reflog, it is really just
decorating stash@{0}.

As you said elsewhere, I'm OK with keeping it in the name of "it's
probably not hurting", but I'd be really surprised if it has ever helped
anyone.

-Peff

[PATCH 3/3] maintenance: stop writing log.excludeDecoration

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

This reverts commit 96eaffebbf3d0 (maintenance: set
log.excludeDecoration durin prefetch, 2021-01-19).

The previous change created a default decoration filter that does not
include refs/prefetch/, so this modification of the config is no longer
needed.

One issue that can happen from this point on is that users who ran the
prefetch task on previous versions of Git will still have a
log.excludeDecoration value and that will prevent the new default
decoration filter from being active. Thus, when we add the refs/bundle/
namespace as part of the bundle URI feature, those users will see
refs/bundle/ decorations.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 builtin/gc.c           |  6 ------
 t/t7900-maintenance.sh | 21 ---------------------
 2 files changed, 27 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index eeff2b760e0..6c222052177 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -910,12 +910,6 @@ static int fetch_remote(struct remote *remote, void *cbdata)
 
 static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
 {
-	git_config_set_multivar_gently("log.excludedecoration",
-					"refs/prefetch/",
-					"refs/prefetch/",
-					CONFIG_FLAGS_FIXED_VALUE |
-					CONFIG_FLAGS_MULTI_REPLACE);
-
 	if (for_each_remote(fetch_remote, opts)) {
 		error(_("failed to prefetch remotes"));
 		return 1;
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 74aa6384755..62ed694a404 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -162,7 +162,6 @@ test_expect_success 'prefetch multiple remotes' '
 	test_cmp_rev refs/remotes/remote1/one refs/prefetch/remotes/remote1/one &&
 	test_cmp_rev refs/remotes/remote2/two refs/prefetch/remotes/remote2/two &&
 
-	test_cmp_config refs/prefetch/ log.excludedecoration &&
 	git log --oneline --decorate --all >log &&
 	! grep "prefetch" log &&
 
@@ -173,26 +172,6 @@ test_expect_success 'prefetch multiple remotes' '
 	test_subcommand git fetch remote2 $fetchargs <skip-remote1.txt
 '
 
-test_expect_success 'prefetch and existing log.excludeDecoration values' '
-	git config --unset-all log.excludeDecoration &&
-	git config log.excludeDecoration refs/remotes/remote1/ &&
-	git maintenance run --task=prefetch &&
-
-	git config --get-all log.excludeDecoration >out &&
-	grep refs/remotes/remote1/ out &&
-	grep refs/prefetch/ out &&
-
-	git log --oneline --decorate --all >log &&
-	! grep "prefetch" log &&
-	! grep "remote1" log &&
-	grep "remote2" log &&
-
-	# a second run does not change the config
-	git maintenance run --task=prefetch &&
-	git log --oneline --decorate --all >log2 &&
-	test_cmp log log2
-'
-
 test_expect_success 'loose-objects task' '
 	# Repack everything so we know the state of the object dir
 	git repack -adk &&
-- 
gitgitgadget

Re: [PATCH 3/3] maintenance: stop writing log.excludeDecoration

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> This reverts commit 96eaffebbf3d0 (maintenance: set
> log.excludeDecoration durin prefetch, 2021-01-19).
>
> The previous change created a default decoration filter that does not
> include refs/prefetch/, so this modification of the config is no longer
> needed.
>
> One issue that can happen from this point on is that users who ran the
> prefetch task on previous versions of Git will still have a
> log.excludeDecoration value and that will prevent the new default
> decoration filter from being active. Thus, when we add the refs/bundle/
> namespace as part of the bundle URI feature, those users will see
> refs/bundle/ decorations.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  builtin/gc.c           |  6 ------
>  t/t7900-maintenance.sh | 21 ---------------------
>  2 files changed, 27 deletions(-)

Nice.

Re: [PATCH 0/3] log: create tighter default decoration filter

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:

> This was previously reduced by adding the log.excludeDecoration config
> option and modifying that config in git maintenance's prefetch task (to hide
> refs/prefetch/*). I then followed that pattern again for the bundle URI
> feature [1], but this caught some reviewers by surprise as an unfortunate
> side-effect. This series is a way to roll back the previous decision to use
> log.excludeDecoration and instead use tighter filters by default.
>
> As noted in the last patch, the current design ignores the new filters if
> there are any previously-specified filters. This includes the
> log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
> This means that users who ran that command in their repo will not get the
> benefits of the more strict filters. While we stop writing
> log.excludeDecorations, we don't remove existing instances of it.

Leaving aside the question of these magic refs, and if we need new ones
(e.g. refs/bundle/*) I have sometimes made use of out-of-standard
refspace refs.

E.g. when I build git I create refs/built-tags/* tag object refs
(i.e. not in refs/tags/*), which is a neat way to get "git tag -l" and
the like to ignore it.

But to still have it show decorated in logs (e.g. I'll see what my
"private" branch is at), and "for-each-ref --contains" still knows about
it.

Now, that's a rather obscure use-case, and I suspect other "special
refs" are similarly obscure (e.g. GitLab's refs/keep-around/* comes to
mind).

But I think this change is going about it the wrong way, let's have a
list of refs that Git knows about as magical, instead of assuming that
we can ignore everything that's not on a small list of things we're
including.

Wouldn't that give you what you want, and not exclude these sorts of
custom refs unexpectedly for users?

> I'm interested if anyone has another way around this issue, or if we
> consider adding the default filter as long as no --decorate=refs options are
> specified.

I think the resulting UX here is bad, in that we ship hardcoded list of
these if you don't specify the config in 2/3. So I can do:

      -c log.excludeDecoration=this-will-never-match

To "clear" the list, but not this:

      -c log.excludeDecoration=

Which we usually have as a synonym for "false", is it just adding a
prefix of "" that matches everything to my exclusion list (but HEAD
still shows up...).

Speaking of funny refnames isn't your:

	git -c log.excludeDecoration=HEAD log

Going to exclude refs like "HEAD/foo/bar" too? I.e. it's always a
prefix.

I suspect a much better way out of this, which I think fixes the problem
you're raising here is:

	log.useBuiltinDecorationExclusions=[bool]

Or whatever, i.e. we ship a hardcoded list of exclusions.

Leaving aside whether that's a blacklist or whitelist, or whether we
turn that to "true" or "false" by default it wouldn't have the behavior
of "flushing out" the built-in config.

Another way of doing that if you really want to use one variable would
be to have:

      -c log.excludeDecoration=foo

Add to the built-in list, but:

      -c log.excludeDecoration=
      -c log.excludeDecoration=foo

clear it, we have a couple of other list config variables that work that
way.

Re: [PATCH 0/3] log: create tighter default decoration filter

[Derrick Stolee]

On 7/26/2022 10:44 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:
> 
>> This was previously reduced by adding the log.excludeDecoration config
>> option and modifying that config in git maintenance's prefetch task (to hide
>> refs/prefetch/*). I then followed that pattern again for the bundle URI
>> feature [1], but this caught some reviewers by surprise as an unfortunate
>> side-effect. This series is a way to roll back the previous decision to use
>> log.excludeDecoration and instead use tighter filters by default.
>>
>> As noted in the last patch, the current design ignores the new filters if
>> there are any previously-specified filters. This includes the
>> log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
>> This means that users who ran that command in their repo will not get the
>> benefits of the more strict filters. While we stop writing
>> log.excludeDecorations, we don't remove existing instances of it.
> 
> Leaving aside the question of these magic refs, and if we need new ones
> (e.g. refs/bundle/*) I have sometimes made use of out-of-standard
> refspace refs.
> 
> E.g. when I build git I create refs/built-tags/* tag object refs
> (i.e. not in refs/tags/*), which is a neat way to get "git tag -l" and
> the like to ignore it.
> 
> But to still have it show decorated in logs (e.g. I'll see what my
> "private" branch is at), and "for-each-ref --contains" still knows about
> it.

You also have the unfortunate UX of having the refs spelled out entirely
("refs/<special-place>/..." instead of "<special-place>/..." like how
"refs/remotes/" is dropped from remote refs) and not having special color.
But that's beside the point.

> Now, that's a rather obscure use-case, and I suspect other "special
> refs" are similarly obscure (e.g. GitLab's refs/keep-around/* comes to
> mind).
> 
> But I think this change is going about it the wrong way, let's have a
> list of refs that Git knows about as magical, instead of assuming that
> we can ignore everything that's not on a small list of things we're
> including.
> 
> Wouldn't that give you what you want, and not exclude these sorts of
> custom refs unexpectedly for users?

Instead of keeping track of an ever-growing list of exclusions, instead
making a clear list of "this is what most users will want for their
decorations" is a better approach.

Users who know how to create custom refs outside of this space have the
capability to figure out how to show their special refs. My general ideas
for designing these kinds of features is to have a default that is focused
on the typical user while giving config options for experts to tweak those
defaults.

You're right that this series perhaps leaves something to be desired in
that second part, since there isn't an easy _config-based_ way to enable
all decorations (or a small additional subset).

>> I'm interested if anyone has another way around this issue, or if we
>> consider adding the default filter as long as no --decorate=refs options are
>> specified.
> 
> I think the resulting UX here is bad, in that we ship hardcoded list of
> these if you don't specify the config in 2/3. So I can do:
> 
>       -c log.excludeDecoration=this-will-never-match
> 
> To "clear" the list, but not this:
> 
>       -c log.excludeDecoration=

The thing that I forgot to do, but had considered was adding a
--decorate-all option to allow clearing the default filters from the
command line. You can already do "--decorate-refs=refs" to get everything
(except HEAD).

As far as config goes, we could also create a log.includeDecoration key,
but we'd want to consider it to populate the same part of the filtering
algorithm. Similar to having any instance of log.excludeDecoration, this
would clear the default list. To get all decorations again, you could add
this to your config file:

	[log]
		includeDecoration = refs
		includeDecoration = HEAD

Alternatively, we could instead create a "filter default" option such as
"log.decorateFilter = (core|all)" where "core" is the default set being
considered by this series, and "all" is the empty filter.

Thanks,
-Stolee

Re: [PATCH 0/3] log: create tighter default decoration filter

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 26 2022, Derrick Stolee wrote:

> On 7/26/2022 10:44 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:
>> 
>>> This was previously reduced by adding the log.excludeDecoration config
>>> option and modifying that config in git maintenance's prefetch task (to hide
>>> refs/prefetch/*). I then followed that pattern again for the bundle URI
>>> feature [1], but this caught some reviewers by surprise as an unfortunate
>>> side-effect. This series is a way to roll back the previous decision to use
>>> log.excludeDecoration and instead use tighter filters by default.
>>>
>>> As noted in the last patch, the current design ignores the new filters if
>>> there are any previously-specified filters. This includes the
>>> log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
>>> This means that users who ran that command in their repo will not get the
>>> benefits of the more strict filters. While we stop writing
>>> log.excludeDecorations, we don't remove existing instances of it.
>> 
>> Leaving aside the question of these magic refs, and if we need new ones
>> (e.g. refs/bundle/*) I have sometimes made use of out-of-standard
>> refspace refs.
>> 
>> E.g. when I build git I create refs/built-tags/* tag object refs
>> (i.e. not in refs/tags/*), which is a neat way to get "git tag -l" and
>> the like to ignore it.
>> 
>> But to still have it show decorated in logs (e.g. I'll see what my
>> "private" branch is at), and "for-each-ref --contains" still knows about
>> it.
>
> You also have the unfortunate UX of having the refs spelled out entirely
> ("refs/<special-place>/..." instead of "<special-place>/..." like how
> "refs/remotes/" is dropped from remote refs) and not having special color.
> But that's beside the point.

Whether that's unfortunate UX or not is debatable, since we expose the
"refs/" prefix explicitly in other parts of the UX, and e.g. "HEAD" or
"ORIG_HEAD" or whatever in in that "/"-relation to
"refs/heads/whatever", and not "heads/whatever".

E.g. try:

	mkdir .git/FOO &&
	git rev-parse origin/seen >.git/FOO/BAR &&
	git push origin FOO/BAR:refs/FOO/BAR

And then:

	mkdir .git/refs/FOO &&
	git rev-parse origin/next >.git/refs/FOO/BAR
	$ git rev-parse FOO/BAR
	warning: object-name.c:970: refname 'FOO/BAR' is ambiguous.
	[seen SHA1]
	$ git rev-parse refs/FOO/BAR
	[next SHA1]

I.e. we really do understand FOO/BAR as meaning .git/FOO/BAR, as opposed
to .git/refs/FOO/BAR. You really can't assume that you can strip a
"refs/" from a multi-level ref and not end up with ambiguities. It's
*not* the case that we just have the .git/<UPPER-CASE-HERE> and
.git/refs/* namespaces, and nothing else.

Are we *almost* there? Yes, I vaguely recall experimenting with trying
to get us 100% of the way (just locally, didn't make it on-list) a long
time ago, and there were some tricky edge cases, and we don't know who
in the wild is relying on it.

I think it would be interesting to explore getting there, but I don't
think eliding information on the display end like that would be the
right way to start.

Although to be fair e.g. "git for-each-ref" excludes these entirely, but
that's a ref-filter.c specific edge-case, per the above try it on
e.g. "git push", you'll find that you can push one or the other.

I think I was poking at this at the time because I wanted to have "git
show head" mean the same as "git show HEAD", or at least for us to start
enforcing the rule that thou shalt not use refnames that are head, HeAd
or whatever mixed-case version we have of our magical all-upper special
refs. This matters because you have commands like "git rev-parse head"
that'll work on case-insensitive FS's that promptly fail on
case-sensitive FS's.

>> Now, that's a rather obscure use-case, and I suspect other "special
>> refs" are similarly obscure (e.g. GitLab's refs/keep-around/* comes to
>> mind).
>> 
>> But I think this change is going about it the wrong way, let's have a
>> list of refs that Git knows about as magical, instead of assuming that
>> we can ignore everything that's not on a small list of things we're
>> including.
>> 
>> Wouldn't that give you what you want, and not exclude these sorts of
>> custom refs unexpectedly for users?
>
> Instead of keeping track of an ever-growing list of exclusions, instead
> making a clear list of "this is what most users will want for their
> decorations" is a better approach.
>
> Users who know how to create custom refs outside of this space have the
> capability to figure out how to show their special refs. My general ideas
> for designing these kinds of features is to have a default that is focused
> on the typical user while giving config options for experts to tweak those
> defaults.
>
> You're right that this series perhaps leaves something to be desired in
> that second part, since there isn't an easy _config-based_ way to enable
> all decorations (or a small additional subset).

Yes, but this is just side-stepping the issue. Your X-Y problem is that
you want to exclude certain refs that we're specifically creating.

I think that's fair enough, but I don't see why we're not specifically
excluding just those then.

It'll just be these bundle refs, filter refs, or whatever other magic we
want excluded. Why would we extend that to refs that we don't know about?

It seems like a safe assumption that if we don't know what it is we
should include it.

Both because that's what we do now (least chance of breaking workflows
in the wild), and because the entire point of the feature is to display
the log in relation to points on the ref namespace. If we don't *know*
that they're meaningless let's leave them out of the exclusion, no?

I'd also think that we'd want to consider this holistically. I haven't
checked, but aside from HEAD (which I think is the only special-case,
but maybe I'm wrong) isn't there a one-to-one mapping between what we'll
show in decorations, and what "git for-each-ref" diplays?

>>> I'm interested if anyone has another way around this issue, or if we
>>> consider adding the default filter as long as no --decorate=refs options are
>>> specified.
>> 
>> I think the resulting UX here is bad, in that we ship hardcoded list of
>> these if you don't specify the config in 2/3. So I can do:
>> 
>>       -c log.excludeDecoration=this-will-never-match
>> 
>> To "clear" the list, but not this:
>> 
>>       -c log.excludeDecoration=
>
> The thing that I forgot to do, but had considered was adding a
> --decorate-all option to allow clearing the default filters from the
> command line. You can already do "--decorate-refs=refs" to get everything
> (except HEAD).

...and some other exclusions, e.g. it won't show ORIG_HEAD even with
--decorate-refs=ORIG_HEAD, but perhaps I'm doing it the wrong way...

> As far as config goes, we could also create a log.includeDecoration key,
> but we'd want to consider it to populate the same part of the filtering
> algorithm. Similar to having any instance of log.excludeDecoration, this
> would clear the default list. To get all decorations again, you could add
> this to your config file:
>
> 	[log]
> 		includeDecoration = refs
> 		includeDecoration = HEAD

If we're considering new config I'd think anything that worked like this
instead would be *much less* confusing:

	some.config = refs/*
	some.config = HEAD

I.e. per the above your HEAD will presumably either match HEAD/BLAH, or
treat "refs" magically as "refs/" and not "refs".

Now, I don't think you can delete .git/HEAD and/or make .git/refs a file
and still have a functioning repository, but the same general rule
applies to "FOO" v.s. "FOO/BAR" and "custom-refs" or whatever.

We use that wildcard syntax unambiguously for the refspec matching, we
could just use the same machinery to match this.

> Alternatively, we could instead create a "filter default" option such as
> "log.decorateFilter = (core|all)" where "core" is the default set being
> considered by this series, and "all" is the empty filter.

Sure, for any key that takes N arguments we could have some way to say
"if you do this it's the same as enumerating these X values", as long as
it's not ambiguous whether you mean a special "aggregate label" or a
name of your own...

Re: [PATCH 0/3] log: create tighter default decoration filter

[Derrick Stolee]

On 7/26/22 2:19 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Tue, Jul 26 2022, Derrick Stolee wrote:
> 
>> On 7/26/2022 10:44 AM, Ævar Arnfjörð Bjarmason wrote:

>>> But I think this change is going about it the wrong way, let's have a
>>> list of refs that Git knows about as magical, instead of assuming that
>>> we can ignore everything that's not on a small list of things we're
>>> including.
>>>
>>> Wouldn't that give you what you want, and not exclude these sorts of
>>> custom refs unexpectedly for users?
>>
>> Instead of keeping track of an ever-growing list of exclusions, instead
>> making a clear list of "this is what most users will want for their
>> decorations" is a better approach.
>>
>> Users who know how to create custom refs outside of this space have the
>> capability to figure out how to show their special refs. My general ideas
>> for designing these kinds of features is to have a default that is focused
>> on the typical user while giving config options for experts to tweak those
>> defaults.
>>
>> You're right that this series perhaps leaves something to be desired in
>> that second part, since there isn't an easy _config-based_ way to enable
>> all decorations (or a small additional subset).
> 
> Yes, but this is just side-stepping the issue. Your X-Y problem is that
> you want to exclude certain refs that we're specifically creating.
> 
> I think that's fair enough, but I don't see why we're not specifically
> excluding just those then.

I'm advocating that we make a one-time change to have a set of "known
useful refs" as showing up in the decorations. Perhaps some users (like
yourself) need to react to that change, but it happens _once_.

Changing the rules repeatedly as new "hidden" namespaces are added is
more likely to cause confusion multiple times.

Thanks,
-Stolee

Re: [PATCH 0/3] log: create tighter default decoration filter

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 27 2022, Derrick Stolee wrote:

> On 7/26/22 2:19 PM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Tue, Jul 26 2022, Derrick Stolee wrote:
>> 
>>> On 7/26/2022 10:44 AM, Ævar Arnfjörð Bjarmason wrote:
>
>>>> But I think this change is going about it the wrong way, let's have a
>>>> list of refs that Git knows about as magical, instead of assuming that
>>>> we can ignore everything that's not on a small list of things we're
>>>> including.
>>>>
>>>> Wouldn't that give you what you want, and not exclude these sorts of
>>>> custom refs unexpectedly for users?
>>>
>>> Instead of keeping track of an ever-growing list of exclusions, instead
>>> making a clear list of "this is what most users will want for their
>>> decorations" is a better approach.
>>>
>>> Users who know how to create custom refs outside of this space have the
>>> capability to figure out how to show their special refs. My general ideas
>>> for designing these kinds of features is to have a default that is focused
>>> on the typical user while giving config options for experts to tweak those
>>> defaults.
>>>
>>> You're right that this series perhaps leaves something to be desired in
>>> that second part, since there isn't an easy _config-based_ way to enable
>>> all decorations (or a small additional subset).
>> 
>> Yes, but this is just side-stepping the issue. Your X-Y problem is that
>> you want to exclude certain refs that we're specifically creating.
>> 
>> I think that's fair enough, but I don't see why we're not specifically
>> excluding just those then.
>
> I'm advocating that we make a one-time change to have a set of "known
> useful refs"

You can't know the set of known-useful refs. Unless we explicitly lay
claim to something in the namespace it's the user's.

This series seems to have started out by observing that "git log" is
showing certain "useless" refs added in another topic, since they're
owned by git, and "internal-only", which is fair enough.

But I don't see how we can or should make the leap to things that are
not on a limited "not-useless" list git knows about should be the only
thing we should display.

Wouldn't carrying a list of what to exclude achive the same goal (but
see below...).

> as showing up in the decorations. Perhaps some users (like yourself)
> need to react to that change, but it happens _once_.

It's long-established behavior, we try not to change that sort of
thing. In this case I think we *could*, i.e. I don't imagine it would be
a *huge* disruption if we had a good reason, e.g. the use-cases of
machine-parsing this are fairly obscure.

But the disruption seems unwarranted if it's just to hide the likes of
refs/bundle/, when we can just address those specifically.

And it's not "once", it's N times for each downstream user that'll run
into this, whereas not changing the behavior is effort among a small
group of git developers...

> Changing the rules repeatedly as new "hidden" namespaces are added is
> more likely to cause confusion multiple times.

I really don't see the worry about a maintenance burden of having a
thing in refs.c (or whatever) that's just:

	hide_from_decorate[] = {
		"refs/bundle/*"
                [...],
                NULL
	};

Worst case we'll create another "magic" ref, forget to add to the list,
and someone will notice and we'll add it.

Which at that point will be a one-line change, and preferrable to N
users having to chase down some new config which explains why their
local refs aren't shown anymore with decorate...

[PATCH v2 00/10] log: create tighter default decoration filter

[Derrick Stolee via GitGitGadget]

When running 'git log', the default is to report any refs within refs/* as
decorations. This series reduces the default set to be more specific to a
set of known-useful namespaces.

This was previously reduced by adding the log.excludeDecoration config
option and modifying that config in git maintenance's prefetch task (to hide
refs/prefetch/*). I then followed that pattern again for the bundle URI
feature [1], but this caught some reviewers by surprise as an unfortunate
side-effect. This series is a way to roll back the previous decision to use
log.excludeDecoration and instead use tighter filters by default.

[1]
https://lore.kernel.org/git/a217e9a0640b45d21ef971d6e91cee3f1993f383.1656535245.git.gitgitgadget@gmail.com/

The current design ignores the new filters if there are any
previously-specified filters. This includes the
log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
This means that users who ran that command in their repo will not get the
benefits of the more strict filters. While we stop writing
log.excludeDecorations, we don't remove existing instances of it. This
should be fine at least for the refs/bundles/ namespace in the future, since
those refs will typically be created for new clones that have not yet had
the log.excludeDecoration setting.

To make it easy for users to opt-in to the previous behavior, add a new git
log --decorate-all option and log.decorateFilter=all config option.


Updates in v2
=============

 * As discussed in the previous version, there are a lot of places that care
   about different subsets of the special ref namespaces. This version
   creates a new ref_namespaces array that centralizes and describes how
   these known ref namespaces interact with other Git features.

 * Some patches are added to help centralize previously-unrelated features
   onto the ref_namespaces array.

 * One thing that I noticed while doing this work was that notes come from a
   commit history that is pointed by a single ref, not multiple refs in a
   namespace. Also, that ref can change based on config and environment
   variables. This version updates that knowledge to allow our
   ref_namespaces[NAMESPACE_NOTES] value change with these settings. Since
   there is only one ref, I also chose to remove it from the default
   decorations. As Junio mentioned, this should not make any difference
   since a user won't see it unless they purposefully start git log from the
   notes ref.

 * Despite feedback to the opposite, I maintain my opinion that this default
   filter should be a small list of known-useful namespaces. For expert
   users who use other namespaces, I added patches that make it easier to
   show custom namespaces (specifically --decorate-all and
   log.decorateFilter=all). The logic for creating the default filter now
   uses the ref_namespaces array in such a way that it would be simple to
   reverse this decision, and I include that alternate implementation in a
   commit message.

 * This version includes several code and test style improvements.

 * The final patch demonstrates how we can remove some duplicate string
   literals for these namespaces. It is focused on the prefetch namespace
   (and an adjacent use of the tags namespace) because of how this series
   changes our handling of that namespace in particular. This makes it
   easier for a future change to modify the namespace via config or
   environment variables. There are opportunities to do this kind of update
   more widely, but I don't want to set that as a critical refactor to do
   right now.

Thanks, -Stolee

Derrick Stolee (10):
  refs: allow "HEAD" as decoration filter
  t4207: test coloring of grafted decorations
  refs: add array of ref namespaces
  refs: use ref_namespaces for replace refs base
  log-tree: use ref_namespaces instead of if/else-if
  log: add default decoration filter
  log: add --decorate-all option
  log: create log.decorateFilter=all
  maintenance: stop writing log.excludeDecoration
  fetch: use ref_namespaces during prefetch

 Documentation/config/log.txt                  |   5 +
 Documentation/git-log.txt                     |  14 +-
 builtin/fetch.c                               |   6 +-
 builtin/gc.c                                  |   6 -
 builtin/log.c                                 |  87 +++++++++---
 builtin/replace.c                             |   3 +
 cache.h                                       |   1 -
 environment.c                                 |   5 +-
 log-tree.c                                    |  28 ++--
 notes.c                                       |   1 +
 refs.c                                        |  88 +++++++++++-
 refs.h                                        |  46 ++++++
 t/t4013-diff-various.sh                       |   2 +
 t/t4013/diff.log_--decorate=full_--all        |   2 +-
 ...f.log_--decorate=full_--decorate-all_--all |  61 ++++++++
 t/t4013/diff.log_--decorate_--all             |   2 +-
 .../diff.log_--decorate_--decorate-all_--all  |  61 ++++++++
 t/t4202-log.sh                                | 134 +++++++++++++++++-
 t/t4207-log-decoration-colors.sh              |  59 ++++++++
 t/t7900-maintenance.sh                        |  21 ---
 t/t9902-completion.sh                         |   3 +
 21 files changed, 568 insertions(+), 67 deletions(-)
 create mode 100644 t/t4013/diff.log_--decorate=full_--decorate-all_--all
 create mode 100644 t/t4013/diff.log_--decorate_--decorate-all_--all


base-commit: 6a475b71f8c4ce708d69fdc9317aefbde3769e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1301%2Fderrickstolee%2Fdecorations-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1301/derrickstolee/decorations-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1301

Range-diff vs v1:

  1:  c2e5a0b3a50 =  1:  c2e5a0b3a50 refs: allow "HEAD" as decoration filter
  -:  ----------- >  2:  b5eb110958b t4207: test coloring of grafted decorations
  -:  ----------- >  3:  d7486390d57 refs: add array of ref namespaces
  -:  ----------- >  4:  8da0b0a3181 refs: use ref_namespaces for replace refs base
  -:  ----------- >  5:  53b15a0b793 log-tree: use ref_namespaces instead of if/else-if
  2:  6b40b84773c !  6:  bec532fb8c6 log: add default decoration filter
     @@ Commit message
      
          * The HEAD ref
          * Branches (refs/heads/)
     -    * Notes (refs/notes/)
     -    * Stashes (refs/stash/)
     +    * Stashes (refs/stash)
          * Tags (refs/tags/)
          * Remote branches (refs/remotes/)
     +    * Replace refs (refs/replace/ or $GIT_REPLACE_REF_BASE)
     +
     +    Each of these namespaces was selected due to existing test cases that
     +    verify these namespaces appear in the decorations. In particular,
     +    stashes and replace refs can have custom colors from the
     +    color.decorate.<slot> config option.
     +
     +    While one test checks for a decoration from notes, it only applies to
     +    the tip of refs/notes/commit (or its configured ref name). Notes form
     +    their own kind of decoration instead. Modify the expected output for the
     +    tests in t4013 that expect this note decoration.  There are several
     +    tests throughout the codebase that verify that --decorate-refs,
     +    --decorate-refs-exclude, and log.excludeDecoration work as designed and
     +    the tests continue to pass without intervention.
      
          However, there are other refs that are less helpful to show as
          decoration:
     @@ Commit message
          * Rebase refs (refs/rebase-merge/ and refs/rebase-apply/)
          * Bundle refs (refs/bundle/) [!]
      
     +    [!] The bundle refs are part of a parallel series that bootstraps a repo
     +        from a bundle file, storing the bundle's refs into the repo's
     +        refs/bundle/ namespace.
     +
          In the case of prefetch refs, 96eaffebbf3d0 (maintenance: set
          log.excludeDecoration durin prefetch, 2021-01-19) added logic to add
          refs/prefetch/ to the log.excludeDecoration config option. Additional
     @@ Commit message
          from log.excludeDecoration, then checks if the list of pattern
          modifications are empty. If none are specified, then the default set is
          restricted to the set of inclusions mentioned earlier (HEAD, branches,
     -    etc.).
     +    etc.).  A previous change introduced the ref_namespaces array, which
     +    includes all of these currently-used namespaces. The 'decoration' value
     +    is non-zero when that namespace is associated with a special coloring
     +    and fits into the list of "expected" decorations as described above,
     +    which makes the implementation of this filter very simple.
      
          Note that the logic in ref_filter_match() in log-tree.c follows this
          matching pattern:
     @@ Commit message
          refs outside of the defaults while also excluding some using
          log.excludeDecoration.
      
     +    Another alternative would be to exclude the known namespaces that are
     +    not intended to be shown. This would reduce the visible effect of the
     +    change for expert users who use their own custom ref namespaces. The
     +    implementation change would be very simple to swap due to our use of
     +    ref_namespaces:
     +
     +            int i;
     +            struct string_list *exclude = decoration_filter->exclude_ref_pattern;
     +
     +            /*
     +             * No command-line or config options were given, so
     +             * populate with sensible defaults.
     +             */
     +            for (i = 0; i < NAMESPACE__COUNT; i++) {
     +                    if (ref_namespaces[i].decoration)
     +                            continue;
     +
     +                    string_list_append(exclude, ref_namespaces[i].ref);
     +            }
     +
     +    The main downside of this approach is that we expect to add new hidden
     +    namespaces in the future, and that means that Git versions will be less
     +    stable in how they behave as those namespaces are added.
     +
     +    It is critical that we provide ways for expert users to disable this
     +    behavior change via command-line options and config keys. These changes
     +    will be implemented in a future change.
     +
          Add a test that checks that the defaults are not added when
          --decorate-refs is specified. We verify this by showing that HEAD is not
          included as it normally would.  Also add a test that shows that the
     @@ Commit message
          refs/rebase-merge,
          and refs/bundle.
      
     -    There are several tests throughout the codebase that verify that
     -    --decorate-refs, --decorate-refs-exclude, and log.excludeDecoration work
     -    as designed and the tests continue to pass without intervention.
     -
     -    [!] The bundle refs are part of a parallel series that bootstraps a repo
     -    from a bundle file, storing the bundle's refs into the repo's
     -    refs/bundle/ namespace.
     -
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
       ## Documentation/git-log.txt ##
     @@ Documentation/git-log.txt: OPTIONS
       	override a match in `log.excludeDecoration`.
      ++
      +If none of these options or config settings are given, then references are
     -+used as decoration if they match `HEAD`, `refs/heads/`, `refs/notes/`,
     -+`refs/remotes/`, `refs/stash/`, or `refs/tags/`.
     ++used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
     ++`refs/stash/`, or `refs/tags/`.
       
       --source::
       	Print out the ref name given on the command line by which each
     @@ builtin/log.c: static void cmd_log_init_defaults(struct rev_info *rev)
       
      +static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
      +{
     ++	int i;
     ++	struct string_list *include = decoration_filter->include_ref_pattern;
      +	const struct string_list *config_exclude =
     -+		repo_config_get_value_multi(the_repository, "log.excludeDecoration");
     ++			git_config_get_value_multi("log.excludeDecoration");
      +
      +	if (config_exclude) {
      +		struct string_list_item *item;
     @@ builtin/log.c: static void cmd_log_init_defaults(struct rev_info *rev)
      +					   item->string);
      +	}
      +
     -+	if (!decoration_filter->exclude_ref_pattern->nr &&
     -+	    !decoration_filter->include_ref_pattern->nr &&
     -+	    !decoration_filter->exclude_ref_config_pattern->nr) {
     -+		/*
     -+		 * No command-line or config options were given, so
     -+		 * populate with sensible defaults.
     -+		 */
     -+		string_list_append(decoration_filter->include_ref_pattern,
     -+				   "refs/heads/");
     -+		string_list_append(decoration_filter->include_ref_pattern,
     -+				   "refs/notes/");
     -+		string_list_append(decoration_filter->include_ref_pattern,
     -+				   "refs/stash/");
     -+		string_list_append(decoration_filter->include_ref_pattern,
     -+				   "refs/tags/");
     -+		string_list_append(decoration_filter->include_ref_pattern,
     -+				   "refs/remotes/");
     -+		string_list_append(decoration_filter->include_ref_pattern,
     -+				   "HEAD");
     ++	if (decoration_filter->exclude_ref_pattern->nr ||
     ++	    decoration_filter->include_ref_pattern->nr ||
     ++	    decoration_filter->exclude_ref_config_pattern->nr)
     ++		return;
     ++
     ++	/*
     ++	 * No command-line or config options were given, so
     ++	 * populate with sensible defaults.
     ++	 */
     ++	for (i = 0; i < NAMESPACE__COUNT; i++) {
     ++		if (!ref_namespaces[i].decoration)
     ++			continue;
     ++
     ++		string_list_append(include, ref_namespaces[i].ref);
      +	}
      +}
      +
     @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons
       		if (decoration_style)
       			rev->show_decorations = 1;
      
     + ## t/t4013/diff.log_--decorate=full_--all ##
     +@@ t/t4013/diff.log_--decorate=full_--all: Date:   Mon Jun 26 00:06:00 2006 +0000
     + 
     +     Rearranged lines in dir/sub
     + 
     +-commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
     ++commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0
     + Author: A U Thor <author@example.com>
     + Date:   Mon Jun 26 00:06:00 2006 +0000
     + 
     +
     + ## t/t4013/diff.log_--decorate_--all ##
     +@@ t/t4013/diff.log_--decorate_--all: Date:   Mon Jun 26 00:06:00 2006 +0000
     + 
     +     Rearranged lines in dir/sub
     + 
     +-commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
     ++commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0
     + Author: A U Thor <author@example.com>
     + Date:   Mon Jun 26 00:06:00 2006 +0000
     + 
     +
       ## t/t4202-log.sh ##
      @@ t/t4202-log.sh: test_expect_success 'decorate-refs-exclude HEAD' '
       	! grep HEAD actual
     @@ t/t4202-log.sh: test_expect_success 'log --decorate includes all levels of tag a
       '
       
      +test_expect_success 'log --decorate does not include things outside filter' '
     -+	reflist="refs/prefetch/ refs/rebase-merge/ refs/bundle/" &&
     ++	reflist="refs/prefetch refs/rebase-merge refs/bundle" &&
      +
      +	for ref in $reflist
      +	do
     -+		mkdir -p .git/$ref &&
     -+		echo $(git rev-parse HEAD) >.git/$ref/fake || return 1
     ++		git update-ref $ref/fake HEAD || return 1
      +	done &&
      +
      +	git log --decorate=full --oneline >actual &&
  -:  ----------- >  7:  64ee889369d log: add --decorate-all option
  -:  ----------- >  8:  8142b32f023 log: create log.decorateFilter=all
  3:  84fbf16613d =  9:  318269dfe27 maintenance: stop writing log.excludeDecoration
  -:  ----------- > 10:  8599bb55045 fetch: use ref_namespaces during prefetch

-- 
gitgitgadget

[PATCH v2 01/10] refs: allow "HEAD" as decoration filter

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The normalize_glob_ref() method was introduced in 65516f586b693 (log:
add option to choose which refs to decorate, 2017-11-21) to help with
decoration filters such as --decorate-refs=<filter> and
--decorate-refs-exclude=<filter>. The method has not been used anywhere
else.

At the moment, it is impossible to specify HEAD as a decoration filter
since normalize_glob_ref() prepends "refs/" to the filter if it isn't
already there.

Allow adding HEAD as a decoration filter by allowing the exact string
"HEAD" to not be prepended with "refs/". Add a test in t4202-log.sh that
would previously fail since the HEAD decoration would exist in the
output.

It is sufficient to only cover "HEAD" here and not include other special
refs like REBASE_HEAD. This is because HEAD is the only ref outside of
refs/* that is added to the list of decorations.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 refs.c         | 4 ++--
 t/t4202-log.sh | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 90bcb271687..ec3134e4842 100644
--- a/refs.c
+++ b/refs.c
@@ -457,8 +457,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
 
 	if (prefix) {
 		strbuf_addstr(&normalized_pattern, prefix);
-	}
-	else if (!starts_with(pattern, "refs/"))
+	} else if (!starts_with(pattern, "refs/") &&
+		   strcmp(pattern, "HEAD"))
 		strbuf_addstr(&normalized_pattern, "refs/");
 	strbuf_addstr(&normalized_pattern, pattern);
 	strbuf_strip_suffix(&normalized_pattern, "/");
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6e663525582..6b7d8e269f7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1025,6 +1025,12 @@ test_expect_success 'decorate-refs and simplify-by-decoration without output' '
 	test_cmp expect actual
 '
 
+test_expect_success 'decorate-refs-exclude HEAD' '
+	git log --decorate=full --oneline \
+		--decorate-refs-exclude="HEAD" >actual &&
+	! grep HEAD actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
-- 
gitgitgadget

Re: [PATCH v2 01/10] refs: allow "HEAD" as decoration filter

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  	if (prefix) {
>  		strbuf_addstr(&normalized_pattern, prefix);
> -	}
> -	else if (!starts_with(pattern, "refs/"))
> +	} else if (!starts_with(pattern, "refs/") &&
> +		   strcmp(pattern, "HEAD"))

Perhaps leave a needswork comment to remind us to later consider
covering all the pseudorefs like MERGE_HEAD, CHERRY_PICK_HEAD etc.?

>  		strbuf_addstr(&normalized_pattern, "refs/");

Style:

If you plan to add more code to the bodies of this if/elseif, then
have {} around all arms.  Otherwise, let's lose the {} around the
body of "if (prefix)".

Re: [PATCH v2 01/10] refs: allow "HEAD" as decoration filter

[Derrick Stolee]

On 8/3/2022 2:03 AM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>>  	if (prefix) {
>>  		strbuf_addstr(&normalized_pattern, prefix);
>> -	}
>> -	else if (!starts_with(pattern, "refs/"))
>> +	} else if (!starts_with(pattern, "refs/") &&
>> +		   strcmp(pattern, "HEAD"))
> 
> Perhaps leave a needswork comment to remind us to later consider
> covering all the pseudorefs like MERGE_HEAD, CHERRY_PICK_HEAD etc.?

A comment is a good idea. We should leave it for later because it
would make a visible change from the user's perspective.

>>  		strbuf_addstr(&normalized_pattern, "refs/");
> 
> Style:
> 
> If you plan to add more code to the bodies of this if/elseif, then
> have {} around all arms.  Otherwise, let's lose the {} around the
> body of "if (prefix)".

Yes, thanks for the close eyes.

Thanks,
-Stolee

[PATCH v2 02/10] t4207: test coloring of grafted decorations

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The color.decorate.<slot> config option added the 'grafted' slot in
09c4ba410b0f (log-tree: allow to customize 'grafted' color, 2018-05-26)
but included no tests for this behavior. When modifying some logic
around decorations, this ref namespace was ignored and could have been
lost as a default namespace for 'git log' decorations by default.

Add two tests to t4207 that check that the replaced objects are
correctly decorated. Use "black" as the color since it is distinct from
the other colors already in the test. The first test uses regular
replace-objects while the second creates a commit graft.

Be sure to test both modes with GIT_REPLACE_REF_BASE unset and set to an
alternative base.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t4207-log-decoration-colors.sh | 59 ++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 36ac6aff1e4..69f8ac602d6 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
 	git config color.decorate.tag "reverse bold yellow" &&
 	git config color.decorate.stash magenta &&
 	git config color.decorate.HEAD cyan &&
+	git config color.decorate.grafted black &&
 
 	c_reset="<RESET>" &&
 
@@ -27,6 +28,7 @@ test_expect_success setup '
 	c_tag="<BOLD;REVERSE;YELLOW>" &&
 	c_stash="<MAGENTA>" &&
 	c_HEAD="<CYAN>" &&
+	c_grafted="<BLACK>" &&
 
 	test_commit A &&
 	git clone . other &&
@@ -63,4 +65,61 @@ test_expect_success 'Commit Decorations Colored Correctly' '
 	test_cmp expected out
 '
 
+cat >expected <<EOF
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
+ ${c_reset}${c_branch}main${c_reset}${c_commit},\
+ ${c_reset}${c_tag}tag: D${c_reset}${c_commit})${c_reset} D
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: C${c_reset}${c_commit},\
+ ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+EOF
+
+test_expect_success 'test coloring with replace-objects' '
+	test_when_finished rm -rf .git/refs/replace* &&
+	test_commit C &&
+	test_commit D &&
+
+	git replace HEAD~1 HEAD~2 &&
+	git log --first-parent --abbrev=10 --decorate --oneline --color=always HEAD |
+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
+	test_decode_color >out &&
+	test_cmp expected out &&
+	git replace -d HEAD~1 &&
+
+	GIT_REPLACE_REF_BASE=refs/replace2/ git replace HEAD~1 HEAD~2 &&
+	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent --abbrev=10 \
+		--decorate --oneline --color=always HEAD |
+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
+	test_decode_color >out &&
+	test_cmp expected out
+'
+
+cat >expected <<EOF
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
+ ${c_reset}${c_branch}main${c_reset}${c_commit},\
+ ${c_reset}${c_tag}tag: D${c_reset}${c_commit},\
+ ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
+ ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+EOF
+
+test_expect_success 'test coloring with grafted commit' '
+	test_when_finished rm -rf .git/refs/replace* &&
+
+	git replace --graft HEAD HEAD~2 &&
+	git log --first-parent --abbrev=10 --decorate --oneline --color=always HEAD |
+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
+	test_decode_color >out &&
+	test_cmp expected out &&
+	git replace -d HEAD &&
+
+	GIT_REPLACE_REF_BASE=refs/replace2/ git replace --graft HEAD HEAD~2 &&
+	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent --abbrev=10 \
+		--decorate --oneline --color=always HEAD |
+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
+	test_decode_color >out &&
+	test_cmp expected out
+'
+
 test_done
-- 
gitgitgadget

Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 29 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
>
> The color.decorate.<slot> config option added the 'grafted' slot in
> 09c4ba410b0f (log-tree: allow to customize 'grafted' color, 2018-05-26)
> but included no tests for this behavior. When modifying some logic
> around decorations, this ref namespace was ignored and could have been
> lost as a default namespace for 'git log' decorations by default.
>
> Add two tests to t4207 that check that the replaced objects are
> correctly decorated. Use "black" as the color since it is distinct from
> the other colors already in the test. The first test uses regular
> replace-objects while the second creates a commit graft.
>
> Be sure to test both modes with GIT_REPLACE_REF_BASE unset and set to an
> alternative base.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  t/t4207-log-decoration-colors.sh | 59 ++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>
> diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
> index 36ac6aff1e4..69f8ac602d6 100755
> --- a/t/t4207-log-decoration-colors.sh
> +++ b/t/t4207-log-decoration-colors.sh
> @@ -18,6 +18,7 @@ test_expect_success setup '
>  	git config color.decorate.tag "reverse bold yellow" &&
>  	git config color.decorate.stash magenta &&
>  	git config color.decorate.HEAD cyan &&
> +	git config color.decorate.grafted black &&
>  
>  	c_reset="<RESET>" &&
>  
> @@ -27,6 +28,7 @@ test_expect_success setup '
>  	c_tag="<BOLD;REVERSE;YELLOW>" &&
>  	c_stash="<MAGENTA>" &&
>  	c_HEAD="<CYAN>" &&
> +	c_grafted="<BLACK>" &&
>  
>  	test_commit A &&
>  	git clone . other &&
> @@ -63,4 +65,61 @@ test_expect_success 'Commit Decorations Colored Correctly' '
>  	test_cmp expected out
>  '
>  
> +cat >expected <<EOF
> +${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
> + ${c_reset}${c_branch}main${c_reset}${c_commit},\
> + ${c_reset}${c_tag}tag: D${c_reset}${c_commit})${c_reset} D
> +${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: C${c_reset}${c_commit},\
> + ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
> +${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
> +EOF

I see this is used by (one) existing test, but for new test code let's
add such setup in the test_expect_success block.

A small issue, but e.g. if the partition our trash directories is on
fills up this sort of thing outside of test_expect_success hides the
source of the "first" error.

More generally (just eyeballing this v.s. the existing one) maybe we can
combine them a bit & share some of these lines?

> +test_expect_success 'test coloring with replace-objects' '
> +	test_when_finished rm -rf .git/refs/replace* &&
> +	test_commit C &&
> +	test_commit D &&
> +
> +	git replace HEAD~1 HEAD~2 &&
> +	git log --first-parent --abbrev=10 --decorate --oneline --color=always HEAD |

This hides segfaults, abort() etc. in git log, let's use an intermediate
file rather than git on the LHS of a pipe.

> +	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |

{10,10} in a regex is just {10}, no?

> +	test_decode_color >out &&
> +	test_cmp expected out &&
> +	git replace -d HEAD~1 &&
> +
> +	GIT_REPLACE_REF_BASE=refs/replace2/ git replace HEAD~1 HEAD~2 &&
> +	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent --abbrev=10 \
> +		--decorate --oneline --color=always HEAD |

Ditto LHS.

> +	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |

Ditto RX, but at this point I see it's copy/pasted from the existing
test in the file, might be worth starting by factoring the duplicate
bits out into a function...

> +	test_decode_color >out &&
> +	test_cmp expected out
> +'
> +
> +cat >expected <<EOF
> +${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
> + ${c_reset}${c_branch}main${c_reset}${c_commit},\
> + ${c_reset}${c_tag}tag: D${c_reset}${c_commit},\
> + ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
> +${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
> + ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
> +${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
> +EOF
> +
> +test_expect_success 'test coloring with grafted commit' '
> +	test_when_finished rm -rf .git/refs/replace* &&
> +
> +	git replace --graft HEAD HEAD~2 &&
> +	git log --first-parent --abbrev=10 --decorate --oneline --color=always HEAD |
> +	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
> +	test_decode_color >out &&
> +	test_cmp expected out &&
> +	git replace -d HEAD &&
> +
> +	GIT_REPLACE_REF_BASE=refs/replace2/ git replace --graft HEAD HEAD~2 &&
> +	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent --abbrev=10 \
> +		--decorate --oneline --color=always HEAD |
> +	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
> +	test_decode_color >out &&
> +	test_cmp expected out
> +'
> +
>  test_done

...as we had one of these, and now have 3x...

Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations

[Eric Sunshine]

On Wed, Aug 3, 2022 at 2:32 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Fri, Jul 29 2022, Derrick Stolee via GitGitGadget wrote:
> > +     sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
>
> {10,10} in a regex is just {10}, no?

I'm more than a little surprised that this regex repeat-count notation
works on macOS `sed` which, in the BSD tradition, is rather feature
poor. Testing it, though, I find that it does work, even on my
relatively old version of macOS. However, I'd still worry about other
BSD `sed`s in the wild.

Instead of using the repeat-count notation, we could model this after
the way `OID_REGEX` is defined in t/test-lib.sh which, through
automation, defines a highly portable regex. Alternatively, just use
"$_x05$_x05", also from test-lib.sh, to define a regex matcher for ten
hex digits.

Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations

[Ævar Arnfjörð Bjarmason]

On Wed, Aug 03 2022, Eric Sunshine wrote:

> On Wed, Aug 3, 2022 at 2:32 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> On Fri, Jul 29 2022, Derrick Stolee via GitGitGadget wrote:
>> > +     sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
>>
>> {10,10} in a regex is just {10}, no?
>
> I'm more than a little surprised that this regex repeat-count notation
> works on macOS `sed` which, in the BSD tradition, is rather feature
> poor. Testing it, though, I find that it does work, even on my
> relatively old version of macOS. However, I'd still worry about other
> BSD `sed`s in the wild.

It seems you missed it but we already have this code tested "in the
wild", i.e. the "new" code here is really just copy/pasting test setup
from above.

So maybe we want to change (and you have some good suggestions here),
but it's already shown itself to be portable enough. We've had this
"sed" command since v1.7.2, or 567102819ac (Add test for correct
coloring of git log --decoration, 2010-06-29).

Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations

[Eric Sunshine]

On Wed, Aug 3, 2022 at 9:05 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Wed, Aug 03 2022, Eric Sunshine wrote:
> > On Wed, Aug 3, 2022 at 2:32 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> >> On Fri, Jul 29 2022, Derrick Stolee via GitGitGadget wrote:
> >> > +     sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
> >>
> >> {10,10} in a regex is just {10}, no?
> >
> > I'm more than a little surprised that this regex repeat-count notation
> > works on macOS `sed` which, in the BSD tradition, is rather feature
> > poor. Testing it, though, I find that it does work, even on my
> > relatively old version of macOS. However, I'd still worry about other
> > BSD `sed`s in the wild.
>
> It seems you missed it but we already have this code tested "in the
> wild", i.e. the "new" code here is really just copy/pasting test setup
> from above.
>
> So maybe we want to change (and you have some good suggestions here),
> but it's already shown itself to be portable enough. We've had this
> "sed" command since v1.7.2, or 567102819ac (Add test for correct
> coloring of git log --decoration, 2010-06-29).

No, I didn't miss it; I saw your mention that it was copied from
existing code in the script, but I assumed the existing code was
recent -- hence my surprise. Since it's been around so long, I agree,
it's hardly a cause for concern at this point (though, I still
reflexively write the more portable notation as used by `OID_REGEX`
and `_x05`, etc.).

Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> it's hardly a cause for concern at this point (though, I still
> reflexively write the more portable notation as used by `OID_REGEX`
> and `_x05`, etc.).

Yeah, use of predefined constants we have in our test framework
releaves us from having to worry about the issue, which is why
I too am in favor of using them.

Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations

[Derrick Stolee]

On 8/4/2022 1:23 PM, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
>> it's hardly a cause for concern at this point (though, I still
>> reflexively write the more portable notation as used by `OID_REGEX`
>> and `_x05`, etc.).
> 
> Yeah, use of predefined constants we have in our test framework
> releaves us from having to worry about the issue, which is why
> I too am in favor of using them.

Thanks. Using $OID_REGEX (along with --no-abbrev) will work
perfectly for this script.

-Stolee

[PATCH v2 03/10] refs: add array of ref namespaces

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

Git interprets different meanings to different refs based on their
names. Some meanings are cosmetic, like how refs in  'refs/remotes/*'
are colored differently from refs in 'refs/heads/*'. Others are more
critical, such as how replace refs are interpreted.

Before making behavior changes based on ref namespaces, collect all
known ref namespaces into a array of ref_namespace_info structs. This
array is indexed by the new ref_namespace enum for quick access.

As of this change, this array is purely documentation. Future changes
will add dependencies on this array.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 environment.c |  2 ++
 notes.c       |  1 +
 refs.c        | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
 refs.h        | 46 ++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+)

diff --git a/environment.c b/environment.c
index b3296ce7d15..9eb22f975c7 100644
--- a/environment.c
+++ b/environment.c
@@ -185,6 +185,8 @@ void setup_git_env(const char *git_dir)
 	free(git_replace_ref_base);
 	git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
 							  : "refs/replace/");
+	update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
+
 	free(git_namespace);
 	git_namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
 	shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
diff --git a/notes.c b/notes.c
index 7452e71cc8d..7bade6d8f69 100644
--- a/notes.c
+++ b/notes.c
@@ -1005,6 +1005,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
 
 	if (!notes_ref)
 		notes_ref = default_notes_ref();
+	update_ref_namespace(NAMESPACE_NOTES, xstrdup(notes_ref));
 
 	if (!combine_notes)
 		combine_notes = combine_notes_concatenate;
diff --git a/refs.c b/refs.c
index ec3134e4842..8e87cc7efb0 100644
--- a/refs.c
+++ b/refs.c
@@ -20,6 +20,7 @@
 #include "repository.h"
 #include "sigchain.h"
 #include "date.h"
+#include "commit.h"
 
 /*
  * List of all available backends
@@ -56,6 +57,88 @@ static unsigned char refname_disposition[256] = {
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
 };
 
+struct ref_namespace_info ref_namespaces[] = {
+	[NAMESPACE_HEAD] = {
+		.ref = "HEAD",
+		.decoration = DECORATION_REF_HEAD,
+		.exact = 1,
+	},
+	[NAMESPACE_BRANCHES] = {
+		.ref = "refs/heads/",
+		.decoration = DECORATION_REF_LOCAL,
+	},
+	[NAMESPACE_TAGS] = {
+		.ref = "refs/tags/",
+		.decoration = DECORATION_REF_TAG,
+	},
+	[NAMESPACE_REMOTE_REFS] = {
+		/*
+		 * The default refspec for new remotes copies refs from
+		 * refs/heads/ on the remote into refs/remotes/<remote>/.
+		 * As such, "refs/remotes/" has special handling.
+		 */
+		.ref = "refs/remotes/",
+		.decoration = DECORATION_REF_REMOTE,
+	},
+	[NAMESPACE_STASH] = {
+		/*
+		 * The single ref "refs/stash" stores the latest stash.
+		 * Older stashes can be found in the reflog.
+		 */
+		.ref = "refs/stash",
+		.exact = 1,
+		.decoration = DECORATION_REF_STASH,
+	},
+	[NAMESPACE_REPLACE] = {
+		/*
+		 * This namespace allows Git to act as if one object ID
+		 * points to the content of another. Unlike the other
+		 * ref namespaces, this one can be changed by the
+		 * GIT_REPLACE_REF_BASE environment variable. This
+		 * .namespace value will be overwritten in setup_git_env().
+		 */
+		.ref = "refs/replace/",
+		.decoration = DECORATION_GRAFTED,
+	},
+	[NAMESPACE_NOTES] = {
+		/*
+		 * The refs/notes/commit ref points to the tip of a
+		 * parallel commit history that adds metadata to commits
+		 * in the normal history. This branch can be overwritten
+		 * by the core.notesRef config variable or the
+		 * GIT_NOTES_REFS environment variable.
+		 */
+		.ref = "refs/notes/commit",
+		.exact = 1,
+	},
+	[NAMESPACE_PREFETCH] = {
+		/*
+		 * Prefetch refs are written by the background 'fetch'
+		 * maintenance task. It allows faster foreground fetches
+		 * by advertising these previously-downloaded tips without
+		 * updating refs/remotes/ without user intervention.
+		 */
+		.ref = "refs/prefetch/",
+	},
+	[NAMESPACE_REWRITTEN] = {
+		/*
+		 * Rewritten refs are used by the 'label' command in the
+		 * sequencer. These are particularly useful during an
+		 * interactive rebase that uses the 'merge' command.
+		 */
+		.ref = "refs/rewritten/",
+	},
+};
+
+void update_ref_namespace(enum ref_namespace namespace, char *ref)
+{
+	struct ref_namespace_info *info = &ref_namespaces[namespace];
+	if (info->ref_updated)
+		free(info->ref);
+	info->ref = ref;
+	info->ref_updated = 1;
+}
+
 /*
  * Try to read one refname component from the front of refname.
  * Return the length of the component found, or -1 if the component is
diff --git a/refs.h b/refs.h
index 47cb9edbaa8..94e8dedf939 100644
--- a/refs.h
+++ b/refs.h
@@ -2,6 +2,7 @@
 #define REFS_H
 
 #include "cache.h"
+#include "commit.h"
 
 struct object_id;
 struct ref_store;
@@ -930,4 +931,49 @@ struct ref_store *get_main_ref_store(struct repository *r);
 struct ref_store *get_submodule_ref_store(const char *submodule);
 struct ref_store *get_worktree_ref_store(const struct worktree *wt);
 
+/*
+ * Some of the names specified by refs have special meaning to Git.
+ * Organize these namespaces in a comon 'ref_namespaces' array for
+ * reference from multiple places in the codebase.
+ */
+
+struct ref_namespace_info {
+	char *ref;
+	enum decoration_type decoration;
+
+	/*
+	 * If 'exact' is true, then we must match the 'ref' exactly.
+	 * Otherwise, use a prefix match.
+	 *
+	 * 'orig_ref' is for internal use. It represents whether the
+	 * 'ref' value was replaced from its original literal version.
+	 */
+	unsigned exact:1,
+		 ref_updated:1;
+};
+
+enum ref_namespace {
+	NAMESPACE_HEAD,
+	NAMESPACE_BRANCHES,
+	NAMESPACE_TAGS,
+	NAMESPACE_REMOTE_REFS,
+	NAMESPACE_STASH,
+	NAMESPACE_REPLACE,
+	NAMESPACE_NOTES,
+	NAMESPACE_PREFETCH,
+	NAMESPACE_REWRITTEN,
+
+	/* Must be last */
+	NAMESPACE__COUNT
+};
+
+/* See refs.c for the contents of this array. */
+extern struct ref_namespace_info ref_namespaces[];
+
+/*
+ * Some ref namespaces can be modified by config values or environment
+ * variables. Modify a namespace as specified by its ref_namespace key.
+ */
+void update_ref_namespace(enum ref_namespace namespace, char *ref);
+
 #endif /* REFS_H */
-- 
gitgitgadget

Re: [PATCH v2 03/10] refs: add array of ref namespaces

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +	[NAMESPACE_REPLACE] = {
> +		/*
> +		 * This namespace allows Git to act as if one object ID
> +		 * points to the content of another. Unlike the other
> +		 * ref namespaces, this one can be changed by the
> +		 * GIT_REPLACE_REF_BASE environment variable. This
> +		 * .namespace value will be overwritten in setup_git_env().
> +		 */

Thanks---it is a shame that we have unnecessary flexibility that
requires us to have this code.

> +		.ref = "refs/replace/",
> +		.decoration = DECORATION_GRAFTED,
> +	},
> +	[NAMESPACE_NOTES] = {
> +		/*
> +		 * The refs/notes/commit ref points to the tip of a
> +		 * parallel commit history that adds metadata to commits
> +		 * in the normal history. This branch can be overwritten

This is not "branch" but is a ref.


> +		 * by the core.notesRef config variable or the
> +		 * GIT_NOTES_REFS environment variable.
> +		 */
> +		.ref = "refs/notes/commit",
> +		.exact = 1,

Allowing just "the default" to be replaced by another "custom
default" is a good start, but we probably want to support more than
one notes refs, to parallel how "struct display_notes_opt" has
extra_notes_refs to allow multiple notes refs to decorate objects.

Re: [PATCH v2 03/10] refs: add array of ref namespaces

[Derrick Stolee]

On 8/3/2022 2:16 AM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> +	[NAMESPACE_REPLACE] = {
>> +		/*
>> +		 * This namespace allows Git to act as if one object ID
>> +		 * points to the content of another. Unlike the other
>> +		 * ref namespaces, this one can be changed by the
>> +		 * GIT_REPLACE_REF_BASE environment variable. This
>> +		 * .namespace value will be overwritten in setup_git_env().
>> +		 */
> 
> Thanks---it is a shame that we have unnecessary flexibility that
> requires us to have this code.
> 
>> +		.ref = "refs/replace/",
>> +		.decoration = DECORATION_GRAFTED,
>> +	},
>> +	[NAMESPACE_NOTES] = {
>> +		/*
>> +		 * The refs/notes/commit ref points to the tip of a
>> +		 * parallel commit history that adds metadata to commits
>> +		 * in the normal history. This branch can be overwritten
> 
> This is not "branch" but is a ref.

Thanks!

>> +		 * by the core.notesRef config variable or the
>> +		 * GIT_NOTES_REFS environment variable.
>> +		 */
>> +		.ref = "refs/notes/commit",
>> +		.exact = 1,
> 
> Allowing just "the default" to be replaced by another "custom
> default" is a good start, but we probably want to support more than
> one notes refs, to parallel how "struct display_notes_opt" has
> extra_notes_refs to allow multiple notes refs to decorate objects.

I imagine that if we allowed multiple notes refs, then we would need
to use a ref prefix to define a namespace. If we relaxed that, then
we could modify this as follows:

	.ref = "refs/notes/",
	/* .exact = 0, */

(The comment is included just to illustrate the change.)

There is additional logic in init_notes() to track multiple notes
refs, but the change to things that use ref_namespaces would be
minimal.

Thanks,
-Stolee

Re: [PATCH v2 03/10] refs: add array of ref namespaces

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

>>> +		 * by the core.notesRef config variable or the
>>> +		 * GIT_NOTES_REFS environment variable.
>>> +		 */
>>> +		.ref = "refs/notes/commit",
>>> +		.exact = 1,
>> 
>> Allowing just "the default" to be replaced by another "custom
>> default" is a good start, but we probably want to support more than
>> one notes refs, to parallel how "struct display_notes_opt" has
>> extra_notes_refs to allow multiple notes refs to decorate objects.
>
> I imagine that if we allowed multiple notes refs, then we would need
> to use a ref prefix to define a namespace. If we relaxed that, then
> we could modify this as follows:
>
> 	.ref = "refs/notes/",
> 	/* .exact = 0, */
>
> (The comment is included just to illustrate the change.)

As I do not think it is so bad to decorate a commit with, say,
refs/notes/amlog in "git log --notes" (not "git log --notes=amlog")
output if such a commit happens to appear, so limiting to the single
notes ref that is given by notes.displayRef and ignoring others like
the posted patch felt like an over-engineering that may result in
even a negative gain.  Treating it just like "tags" and "remotes"
hierarchy would match intuition better for people, I suspect.

Thanks.

Re: [PATCH v2 03/10] refs: add array of ref namespaces

[Josh Steadmon]

On 2022.07.29 19:29, Derrick Stolee via GitGitGadget wrote:
> diff --git a/refs.h b/refs.h
> index 47cb9edbaa8..94e8dedf939 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -2,6 +2,7 @@
>  #define REFS_H
>  
>  #include "cache.h"
> +#include "commit.h"
>  
>  struct object_id;
>  struct ref_store;
> @@ -930,4 +931,49 @@ struct ref_store *get_main_ref_store(struct repository *r);
>  struct ref_store *get_submodule_ref_store(const char *submodule);
>  struct ref_store *get_worktree_ref_store(const struct worktree *wt);
>  
> +/*
> + * Some of the names specified by refs have special meaning to Git.
> + * Organize these namespaces in a comon 'ref_namespaces' array for
> + * reference from multiple places in the codebase.
> + */
> +
> +struct ref_namespace_info {
> +	char *ref;
> +	enum decoration_type decoration;
> +
> +	/*
> +	 * If 'exact' is true, then we must match the 'ref' exactly.
> +	 * Otherwise, use a prefix match.
> +	 *
> +	 * 'orig_ref' is for internal use. It represents whether the

s/orig_ref/ref_updated/ here I think.


> +	 * 'ref' value was replaced from its original literal version.
> +	 */
> +	unsigned exact:1,
> +		 ref_updated:1;
> +};
> +
> +enum ref_namespace {
> +	NAMESPACE_HEAD,
> +	NAMESPACE_BRANCHES,
> +	NAMESPACE_TAGS,
> +	NAMESPACE_REMOTE_REFS,
> +	NAMESPACE_STASH,
> +	NAMESPACE_REPLACE,
> +	NAMESPACE_NOTES,
> +	NAMESPACE_PREFETCH,
> +	NAMESPACE_REWRITTEN,
> +
> +	/* Must be last */
> +	NAMESPACE__COUNT
> +};
> +
> +/* See refs.c for the contents of this array. */
> +extern struct ref_namespace_info ref_namespaces[];
> +
> +/*
> + * Some ref namespaces can be modified by config values or environment
> + * variables. Modify a namespace as specified by its ref_namespace key.
> + */
> +void update_ref_namespace(enum ref_namespace namespace, char *ref);
> +
>  #endif /* REFS_H */
> -- 
> gitgitgadget
> 

Re: [PATCH v2 03/10] refs: add array of ref namespaces

[Derrick Stolee]

On 8/3/2022 6:39 PM, Josh Steadmon wrote:
> On 2022.07.29 19:29, Derrick Stolee via GitGitGadget wrote:

>> +	/*
>> +	 * If 'exact' is true, then we must match the 'ref' exactly.
>> +	 * Otherwise, use a prefix match.
>> +	 *
>> +	 * 'orig_ref' is for internal use. It represents whether the
> 
> s/orig_ref/ref_updated/ here I think.

Thanks!
-Stolee

[PATCH v2 04/10] refs: use ref_namespaces for replace refs base

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The git_replace_ref_base global is used to store the value of the
GIT_REPLACE_REF_BASE environment variable or the default of
"refs/replace/". This is initialized within setup_git_env().

The ref_namespaces array is a new centralized location for information
such as the ref namespace used for replace refs. Instead of having this
namespace stored in two places, use the ref_namespaces array instead.

For simplicity, create a local git_replace_ref_base variable wherever
the global was previously used.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 builtin/replace.c | 3 +++
 cache.h           | 1 -
 environment.c     | 3 +--
 log-tree.c        | 2 ++
 refs.c            | 1 +
 5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 583702a0980..6b97ef2348b 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -106,6 +106,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
 	size_t base_len;
 	int had_error = 0;
 	struct object_id oid;
+	const char *git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
 
 	strbuf_addstr(&ref, git_replace_ref_base);
 	base_len = ref.len;
@@ -147,6 +148,8 @@ static int check_ref_valid(struct object_id *object,
 			    struct strbuf *ref,
 			    int force)
 {
+	const char *git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
+
 	strbuf_reset(ref);
 	strbuf_addf(ref, "%s%s", git_replace_ref_base, oid_to_hex(object));
 	if (check_refname_format(ref->buf, 0))
diff --git a/cache.h b/cache.h
index ac5ab4ef9d3..631a4f388d6 100644
--- a/cache.h
+++ b/cache.h
@@ -1008,7 +1008,6 @@ void reset_shared_repository(void);
  * commands that do not want replace references to be active.
  */
 extern int read_replace_refs;
-extern char *git_replace_ref_base;
 
 /*
  * These values are used to help identify parts of a repository to fsync.
diff --git a/environment.c b/environment.c
index 9eb22f975c7..b2004437dce 100644
--- a/environment.c
+++ b/environment.c
@@ -56,7 +56,6 @@ const char *askpass_program;
 const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 int read_replace_refs = 1;
-char *git_replace_ref_base;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding = "SHIFT-JIS";
@@ -162,6 +161,7 @@ const char *getenv_safe(struct strvec *argv, const char *name)
 
 void setup_git_env(const char *git_dir)
 {
+	char *git_replace_ref_base;
 	const char *shallow_file;
 	const char *replace_ref_base;
 	struct set_gitdir_args args = { NULL };
@@ -182,7 +182,6 @@ void setup_git_env(const char *git_dir)
 	if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
 		read_replace_refs = 0;
 	replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
-	free(git_replace_ref_base);
 	git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
 							  : "refs/replace/");
 	update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
diff --git a/log-tree.c b/log-tree.c
index d0ac0a6327a..bb80f1a94d2 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -141,10 +141,12 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
 	enum object_type objtype;
 	enum decoration_type deco_type = DECORATION_NONE;
 	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
+	const char *git_replace_ref_base;
 
 	if (filter && !ref_filter_match(refname, filter))
 		return 0;
 
+	git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
 	if (starts_with(refname, git_replace_ref_base)) {
 		struct object_id original_oid;
 		if (!read_replace_refs)
diff --git a/refs.c b/refs.c
index 8e87cc7efb0..0da089d760b 100644
--- a/refs.c
+++ b/refs.c
@@ -1607,6 +1607,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)
 {
+	const char *git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
 	return do_for_each_repo_ref(r, git_replace_ref_base, fn,
 				    strlen(git_replace_ref_base),
 				    DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
-- 
gitgitgadget

Re: [PATCH v2 04/10] refs: use ref_namespaces for replace refs base

[Josh Steadmon]

On 2022.07.29 19:29, Derrick Stolee via GitGitGadget wrote:
> diff --git a/log-tree.c b/log-tree.c
> index d0ac0a6327a..bb80f1a94d2 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -141,10 +141,12 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
>  	enum object_type objtype;
>  	enum decoration_type deco_type = DECORATION_NONE;
>  	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
> +	const char *git_replace_ref_base;
>  
>  	if (filter && !ref_filter_match(refname, filter))
>  		return 0;
>  
> +	git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;

Is there a reason not to initialize this as soon as we declare it?


>  	if (starts_with(refname, git_replace_ref_base)) {
>  		struct object_id original_oid;
>  		if (!read_replace_refs)

Re: [PATCH v2 04/10] refs: use ref_namespaces for replace refs base

[Derrick Stolee]

On 8/3/2022 6:38 PM, Josh Steadmon wrote:
> On 2022.07.29 19:29, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/log-tree.c b/log-tree.c
>> index d0ac0a6327a..bb80f1a94d2 100644
>> --- a/log-tree.c
>> +++ b/log-tree.c
>> @@ -141,10 +141,12 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
>>  	enum object_type objtype;
>>  	enum decoration_type deco_type = DECORATION_NONE;
>>  	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
>> +	const char *git_replace_ref_base;
>>  
>>  	if (filter && !ref_filter_match(refname, filter))
>>  		return 0;
>>  
>> +	git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
> 
> Is there a reason not to initialize this as soon as we declare it?

No, there is not. Thanks!
-Stolee

[PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The add_ref_decoration() method uses an if/else-if chain to determine if
a ref matches a known ref namespace that has a special decoration
category. That decoration type is later used to assign a color when
writing to stdout.

The newly-added ref_namespaces array contains all namespaces, along
with information about their decoration type. Check this array instead
of this if/else-if chain. This reduces our dependency on string literals
being embedded in the decoration logic.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 log-tree.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index bb80f1a94d2..6075bdd334e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -137,6 +137,7 @@ static int ref_filter_match(const char *refname,
 static int add_ref_decoration(const char *refname, const struct object_id *oid,
 			      int flags, void *cb_data)
 {
+	int i;
 	struct object *obj;
 	enum object_type objtype;
 	enum decoration_type deco_type = DECORATION_NONE;
@@ -167,16 +168,21 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
 		return 0;
 	obj = lookup_object_by_type(the_repository, oid, objtype);
 
-	if (starts_with(refname, "refs/heads/"))
-		deco_type = DECORATION_REF_LOCAL;
-	else if (starts_with(refname, "refs/remotes/"))
-		deco_type = DECORATION_REF_REMOTE;
-	else if (starts_with(refname, "refs/tags/"))
-		deco_type = DECORATION_REF_TAG;
-	else if (!strcmp(refname, "refs/stash"))
-		deco_type = DECORATION_REF_STASH;
-	else if (!strcmp(refname, "HEAD"))
-		deco_type = DECORATION_REF_HEAD;
+	for (i = 0; i < NAMESPACE__COUNT; i++) {
+		struct ref_namespace_info *info = &ref_namespaces[i];
+
+		if (!info->decoration)
+			continue;
+		if (info->exact) {
+			if (!strcmp(refname, info->ref)) {
+				deco_type = info->decoration;
+				break;
+			}
+		} else if (starts_with(refname, info->ref)) {
+			deco_type = info->decoration;
+			break;
+		}
+	}
 
 	add_name_decoration(deco_type, refname, obj);
 	while (obj->type == OBJ_TAG) {
-- 
gitgitgadget

Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> The add_ref_decoration() method uses an if/else-if chain to determine if
> a ref matches a known ref namespace that has a special decoration
> category. That decoration type is later used to assign a color when
> writing to stdout.
>
> The newly-added ref_namespaces array contains all namespaces, along
> with information about their decoration type. Check this array instead
> of this if/else-if chain. This reduces our dependency on string literals
> being embedded in the decoration logic.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  log-tree.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index bb80f1a94d2..6075bdd334e 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -137,6 +137,7 @@ static int ref_filter_match(const char *refname,
>  static int add_ref_decoration(const char *refname, const struct object_id *oid,
>  			      int flags, void *cb_data)
>  {
> +	int i;
>  	struct object *obj;
>  	enum object_type objtype;
>  	enum decoration_type deco_type = DECORATION_NONE;
> @@ -167,16 +168,21 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
>  		return 0;
>  	obj = lookup_object_by_type(the_repository, oid, objtype);
>  
> -	if (starts_with(refname, "refs/heads/"))
> -		deco_type = DECORATION_REF_LOCAL;
> -	else if (starts_with(refname, "refs/remotes/"))
> -		deco_type = DECORATION_REF_REMOTE;
> -	else if (starts_with(refname, "refs/tags/"))
> -		deco_type = DECORATION_REF_TAG;
> -	else if (!strcmp(refname, "refs/stash"))
> -		deco_type = DECORATION_REF_STASH;
> -	else if (!strcmp(refname, "HEAD"))
> -		deco_type = DECORATION_REF_HEAD;
> +	for (i = 0; i < NAMESPACE__COUNT; i++) {
> +		struct ref_namespace_info *info = &ref_namespaces[i];
> +
> +		if (!info->decoration)
> +			continue;
> +		if (info->exact) {
> +			if (!strcmp(refname, info->ref)) {
> +				deco_type = info->decoration;
> +				break;
> +			}
> +		} else if (starts_with(refname, info->ref)) {
> +			deco_type = info->decoration;
> +			break;
> +		}
> +	}

Very nice.  The double-dash in the NAMESPACE__COUNT constant somehow
looks strange.  As we scan through ref_namespace[] array densely,

	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++)
		...

without having to use the constant would probably be more in line
with the way how the rest of the codebase works.

Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Derrick Stolee]

On 8/3/2022 2:31 AM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>> +	for (i = 0; i < NAMESPACE__COUNT; i++) {
>> +		struct ref_namespace_info *info = &ref_namespaces[i];
>> +
>> +		if (!info->decoration)
>> +			continue;
>> +		if (info->exact) {
>> +			if (!strcmp(refname, info->ref)) {
>> +				deco_type = info->decoration;
>> +				break;
>> +			}
>> +		} else if (starts_with(refname, info->ref)) {
>> +			deco_type = info->decoration;
>> +			break;
>> +		}
>> +	}
> 
> Very nice.  The double-dash in the NAMESPACE__COUNT constant somehow
> looks strange.  As we scan through ref_namespace[] array densely,
> 
> 	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++)
> 		...
> 
> without having to use the constant would probably be more in line
> with the way how the rest of the codebase works.

Ah, I did not know about that trick. Thanks!

-Stolee

Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Derrick Stolee]

On 8/4/2022 9:31 AM, Derrick Stolee wrote:
> On 8/3/2022 2:31 AM, Junio C Hamano wrote:
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> From: Derrick Stolee <derrickstolee@github.com>
>>> +	for (i = 0; i < NAMESPACE__COUNT; i++) {
>>> +		struct ref_namespace_info *info = &ref_namespaces[i];
>>> +
>>> +		if (!info->decoration)
>>> +			continue;
>>> +		if (info->exact) {
>>> +			if (!strcmp(refname, info->ref)) {
>>> +				deco_type = info->decoration;
>>> +				break;
>>> +			}
>>> +		} else if (starts_with(refname, info->ref)) {
>>> +			deco_type = info->decoration;
>>> +			break;
>>> +		}
>>> +	}
>>
>> Very nice.  The double-dash in the NAMESPACE__COUNT constant somehow
>> looks strange.  As we scan through ref_namespace[] array densely,
>>
>> 	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++)
>> 		...
>>
>> without having to use the constant would probably be more in line
>> with the way how the rest of the codebase works.
> 
> Ah, I did not know about that trick. Thanks!

...except that it doesn't work because the array is declared as
'extern' so we don't know its size outside of refs.c.

This motivates the use of NAMESPACE__COUNT (and the double
underscores differentiates the "COUNT" from other namespaces, so
there is no confusion about "COUNT" being a namespace). If there
is another way around this, then I would love to hear it!

Thanks,
-Stolee

Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Derrick Stolee]

On 8/4/2022 10:34 AM, Derrick Stolee wrote:
> On 8/4/2022 9:31 AM, Derrick Stolee wrote:
>> On 8/3/2022 2:31 AM, Junio C Hamano wrote:

>>> Very nice.  The double-dash in the NAMESPACE__COUNT constant somehow
>>> looks strange.  As we scan through ref_namespace[] array densely,
>>>
>>> 	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++)
>>> 		...
>>>
>>> without having to use the constant would probably be more in line
>>> with the way how the rest of the codebase works.
>>
>> Ah, I did not know about that trick. Thanks!
> 
> ...except that it doesn't work because the array is declared as
> 'extern' so we don't know its size outside of refs.c.
> 
> This motivates the use of NAMESPACE__COUNT (and the double
> underscores differentiates the "COUNT" from other namespaces, so
> there is no confusion about "COUNT" being a namespace). If there
> is another way around this, then I would love to hear it!

My current workaround is to define the size of the array in the
header file:

---

enum ref_namespace {
	NAMESPACE_HEAD,
	NAMESPACE_BRANCHES,
	NAMESPACE_TAGS,
	NAMESPACE_REMOTE_REFS,
	NAMESPACE_STASH,
	NAMESPACE_REPLACE,
	NAMESPACE_NOTES,
	NAMESPACE_PREFETCH,
	NAMESPACE_REWRITTEN,

	/* Must be last */
	NAMESPACE__COUNT
};

/* See refs.c for the contents of this array. */
extern struct ref_namespace_info ref_namespaces[NAMESPACE__COUNT];

---

Then ARRAY_SIZE(ref_namespaces) works properly.

Thanks,
-Stolee

Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

> My current workaround is to define the size of the array in the
> header file:
>
> ---
>
> enum ref_namespace {
> 	NAMESPACE_HEAD,
> 	NAMESPACE_BRANCHES,
> 	NAMESPACE_TAGS,
> 	NAMESPACE_REMOTE_REFS,
> 	NAMESPACE_STASH,
> 	NAMESPACE_REPLACE,
> 	NAMESPACE_NOTES,
> 	NAMESPACE_PREFETCH,
> 	NAMESPACE_REWRITTEN,
>
> 	/* Must be last */
> 	NAMESPACE__COUNT
> };
>
> /* See refs.c for the contents of this array. */
> extern struct ref_namespace_info ref_namespaces[NAMESPACE__COUNT];

Because there is no reason why we want to keep the size of the array
opaque in this particular case, it may even be preferrable over the
original ref_namespace[] array of unspecified size.  Nice.

BTW, I prefer to name my arrays with a singular noun, when the
predominant use of it in the code that is an API customer is to name
an individual element and use it.

In this case, many API users do things like

   ref_namespace[NAMESPACE_NOTES}.ref = ...;

so a singular name would be more appropriate.  thing[4] that means
the fourth thing feels more intuitive than things[4], at least to
me.

On the other hand, when API customers mostly pass the whole thing
around as an almost opaque collection to the API function, then a
plural name is more appropriate.  The reason why I focus on API
customers is because API implementation of course has to go in to
each individual element of the array and work on it at some level,
and "individual access means singular name" rule would become non
workable.  The names are mostly to help API customers, so if an
array is perceived by them as mostly a collection of things, then
naming it in plural would make it more natural.

Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Derrick Stolee]

On 8/4/2022 1:40 PM, Junio C Hamano wrote:
> Derrick Stolee <derrickstolee@github.com> writes:
> 
>> My current workaround is to define the size of the array in the
>> header file:
>>
>> ---
>>
>> enum ref_namespace {
>> 	NAMESPACE_HEAD,
>> 	NAMESPACE_BRANCHES,
>> 	NAMESPACE_TAGS,
>> 	NAMESPACE_REMOTE_REFS,
>> 	NAMESPACE_STASH,
>> 	NAMESPACE_REPLACE,
>> 	NAMESPACE_NOTES,
>> 	NAMESPACE_PREFETCH,
>> 	NAMESPACE_REWRITTEN,
>>
>> 	/* Must be last */
>> 	NAMESPACE__COUNT
>> };
>>
>> /* See refs.c for the contents of this array. */
>> extern struct ref_namespace_info ref_namespaces[NAMESPACE__COUNT];
> 
> Because there is no reason why we want to keep the size of the array
> opaque in this particular case, it may even be preferrable over the
> original ref_namespace[] array of unspecified size.  Nice.
> 
> BTW, I prefer to name my arrays with a singular noun, when the
> predominant use of it in the code that is an API customer is to name
> an individual element and use it.
> 
> In this case, many API users do things like
> 
>    ref_namespace[NAMESPACE_NOTES}.ref = ...;
> 
> so a singular name would be more appropriate.  thing[4] that means
> the fourth thing feels more intuitive than things[4], at least to
> me.

This makes sense to me.

> On the other hand, when API customers mostly pass the whole thing
> around as an almost opaque collection to the API function, then a
> plural name is more appropriate.  The reason why I focus on API
> customers is because API implementation of course has to go in to
> each individual element of the array and work on it at some level,
> and "individual access means singular name" rule would become non
> workable.  The names are mostly to help API customers, so if an
> array is perceived by them as mostly a collection of things, then
> naming it in plural would make it more natural.

Understood. I think this array does not seem to fit this second
case, so I will change it to be singular.

Thanks,
-Stolee

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

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

When a user runs 'git log', they expect a certain set of helpful
decorations. This includes:

* The HEAD ref
* Branches (refs/heads/)
* Stashes (refs/stash)
* Tags (refs/tags/)
* Remote branches (refs/remotes/)
* Replace refs (refs/replace/ or $GIT_REPLACE_REF_BASE)

Each of these namespaces was selected due to existing test cases that
verify these namespaces appear in the decorations. In particular,
stashes and replace refs can have custom colors from the
color.decorate.<slot> config option.

While one test checks for a decoration from notes, it only applies to
the tip of refs/notes/commit (or its configured ref name). Notes form
their own kind of decoration instead. Modify the expected output for the
tests in t4013 that expect this note decoration.  There are several
tests throughout the codebase that verify that --decorate-refs,
--decorate-refs-exclude, and log.excludeDecoration work as designed and
the tests continue to pass without intervention.

However, there are other refs that are less helpful to show as
decoration:

* Prefetch refs (refs/prefetch/)
* Rebase refs (refs/rebase-merge/ and refs/rebase-apply/)
* Bundle refs (refs/bundle/) [!]

[!] The bundle refs are part of a parallel series that bootstraps a repo
    from a bundle file, storing the bundle's refs into the repo's
    refs/bundle/ namespace.

In the case of prefetch refs, 96eaffebbf3d0 (maintenance: set
log.excludeDecoration durin prefetch, 2021-01-19) added logic to add
refs/prefetch/ to the log.excludeDecoration config option. Additional
feedback pointed out that having such a side-effect can be confusing and
perhaps not helpful to users. Instead, we should hide these ref
namespaces that are being used by Git for internal reasons but are not
helpful for the users to see.

The way to provide a seamless user experience without setting the config
is to modify the default decoration filters to match our expectation of
what refs the user actually wants to see.

In builtin/log.c, after parsing the --decorate-refs and
--decorate-refs-exclude options from the command-line, call
set_default_decoration_filter(). This method populates the exclusions
from log.excludeDecoration, then checks if the list of pattern
modifications are empty. If none are specified, then the default set is
restricted to the set of inclusions mentioned earlier (HEAD, branches,
etc.).  A previous change introduced the ref_namespaces array, which
includes all of these currently-used namespaces. The 'decoration' value
is non-zero when that namespace is associated with a special coloring
and fits into the list of "expected" decorations as described above,
which makes the implementation of this filter very simple.

Note that the logic in ref_filter_match() in log-tree.c follows this
matching pattern:

 1. If there are exclusion patterns and the ref matches one, then ignore
    the decoration.

 2. If there are inclusion patterns and the ref matches one, then
    definitely include the decoration.

 3. If there are config-based exclusions from log.excludeDecoration and
    the ref matches one, then ignore the decoration.

With this logic in mind, we need to ensure that we do not populate our
new defaults if any of these filters are manually set. Specifically, if
a user runs

	git -c log.excludeDecoration=HEAD log

then we expect the HEAD decoration to not appear. If we left the default
inclusions in the set, then HEAD would match that inclusion before
reaching the config-based exclusions.

A potential alternative would be to check the list of default inclusions
at the end, after the config-based exclusions. This would still create a
behavior change for some uses of --decorate-refs-exclude=<X>, and could
be overwritten somewhat with --decorate-refs=refs/ and
--decorate-refs=HEAD. However, it no longer becomes possible to include
refs outside of the defaults while also excluding some using
log.excludeDecoration.

Another alternative would be to exclude the known namespaces that are
not intended to be shown. This would reduce the visible effect of the
change for expert users who use their own custom ref namespaces. The
implementation change would be very simple to swap due to our use of
ref_namespaces:

	int i;
	struct string_list *exclude = decoration_filter->exclude_ref_pattern;

	/*
	 * No command-line or config options were given, so
	 * populate with sensible defaults.
	 */
	for (i = 0; i < NAMESPACE__COUNT; i++) {
		if (ref_namespaces[i].decoration)
			continue;

		string_list_append(exclude, ref_namespaces[i].ref);
	}

The main downside of this approach is that we expect to add new hidden
namespaces in the future, and that means that Git versions will be less
stable in how they behave as those namespaces are added.

It is critical that we provide ways for expert users to disable this
behavior change via command-line options and config keys. These changes
will be implemented in a future change.

Add a test that checks that the defaults are not added when
--decorate-refs is specified. We verify this by showing that HEAD is not
included as it normally would.  Also add a test that shows that the
default filter avoids the unwanted decorations from refs/prefetch,
refs/rebase-merge,
and refs/bundle.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-log.txt              |  7 ++--
 builtin/log.c                          | 50 +++++++++++++++++++-------
 t/t4013/diff.log_--decorate=full_--all |  2 +-
 t/t4013/diff.log_--decorate_--all      |  2 +-
 t/t4202-log.sh                         | 22 ++++++++++++
 5 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 20e87cecf49..b2ac89dfafc 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -45,13 +45,16 @@ OPTIONS
 
 --decorate-refs=<pattern>::
 --decorate-refs-exclude=<pattern>::
-	If no `--decorate-refs` is given, pretend as if all refs were
-	included.  For each candidate, do not use it for decoration if it
+	For each candidate reference, do not use it for decoration if it
 	matches any patterns given to `--decorate-refs-exclude` or if it
 	doesn't match any of the patterns given to `--decorate-refs`. The
 	`log.excludeDecoration` config option allows excluding refs from
 	the decorations, but an explicit `--decorate-refs` pattern will
 	override a match in `log.excludeDecoration`.
++
+If none of these options or config settings are given, then references are
+used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
+`refs/stash/`, or `refs/tags/`.
 
 --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 88a5e98875a..0b5efc9434c 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -162,6 +162,37 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 		parse_date_format(default_date_mode, &rev->date_mode);
 }
 
+static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
+{
+	int i;
+	struct string_list *include = decoration_filter->include_ref_pattern;
+	const struct string_list *config_exclude =
+			git_config_get_value_multi("log.excludeDecoration");
+
+	if (config_exclude) {
+		struct string_list_item *item;
+		for_each_string_list_item(item, config_exclude)
+			string_list_append(decoration_filter->exclude_ref_config_pattern,
+					   item->string);
+	}
+
+	if (decoration_filter->exclude_ref_pattern->nr ||
+	    decoration_filter->include_ref_pattern->nr ||
+	    decoration_filter->exclude_ref_config_pattern->nr)
+		return;
+
+	/*
+	 * No command-line or config options were given, so
+	 * populate with sensible defaults.
+	 */
+	for (i = 0; i < NAMESPACE__COUNT; i++) {
+		if (!ref_namespaces[i].decoration)
+			continue;
+
+		string_list_append(include, ref_namespaces[i].ref);
+	}
+}
+
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
@@ -171,9 +202,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
 	static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
 	static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
-	struct decoration_filter decoration_filter = {&decorate_refs_include,
-						      &decorate_refs_exclude,
-						      &decorate_refs_exclude_config};
+	struct decoration_filter decoration_filter = {
+		.exclude_ref_pattern = &decorate_refs_exclude,
+		.include_ref_pattern = &decorate_refs_include,
+		.exclude_ref_config_pattern = &decorate_refs_exclude_config,
+	};
 	static struct revision_sources revision_sources;
 
 	const struct option builtin_log_options[] = {
@@ -265,16 +298,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	}
 
 	if (decoration_style || rev->simplify_by_decoration) {
-		const struct string_list *config_exclude =
-			repo_config_get_value_multi(the_repository,
-						    "log.excludeDecoration");
-
-		if (config_exclude) {
-			struct string_list_item *item;
-			for_each_string_list_item(item, config_exclude)
-				string_list_append(&decorate_refs_exclude_config,
-						   item->string);
-		}
+		set_default_decoration_filter(&decoration_filter);
 
 		if (decoration_style)
 			rev->show_decorations = 1;
diff --git a/t/t4013/diff.log_--decorate=full_--all b/t/t4013/diff.log_--decorate=full_--all
index 3f9b872eceb..6b0b334a5d6 100644
--- a/t/t4013/diff.log_--decorate=full_--all
+++ b/t/t4013/diff.log_--decorate=full_--all
@@ -20,7 +20,7 @@ Date:   Mon Jun 26 00:06:00 2006 +0000
 
     Rearranged lines in dir/sub
 
-commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:06:00 2006 +0000
 
diff --git a/t/t4013/diff.log_--decorate_--all b/t/t4013/diff.log_--decorate_--all
index f5e20e1e14a..c7df1f58141 100644
--- a/t/t4013/diff.log_--decorate_--all
+++ b/t/t4013/diff.log_--decorate_--all
@@ -20,7 +20,7 @@ Date:   Mon Jun 26 00:06:00 2006 +0000
 
     Rearranged lines in dir/sub
 
-commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:06:00 2006 +0000
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6b7d8e269f7..9ea9e835d43 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1031,6 +1031,12 @@ test_expect_success 'decorate-refs-exclude HEAD' '
 	! grep HEAD actual
 '
 
+test_expect_success 'decorate-refs focus from default' '
+	git log --decorate=full --oneline \
+		--decorate-refs="refs/heads" >actual &&
+	! grep HEAD actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
@@ -2198,6 +2204,22 @@ test_expect_success 'log --decorate includes all levels of tag annotated tags' '
 	test_cmp expect actual
 '
 
+test_expect_success 'log --decorate does not include things outside filter' '
+	reflist="refs/prefetch refs/rebase-merge refs/bundle" &&
+
+	for ref in $reflist
+	do
+		git update-ref $ref/fake HEAD || return 1
+	done &&
+
+	git log --decorate=full --oneline >actual &&
+
+	for ref in $reflist
+	do
+		! grep $ref/fake actual || return 1
+	done
+'
+
 test_expect_success 'log --end-of-options' '
        git update-ref refs/heads/--source HEAD &&
        git log --end-of-options --source >actual &&
-- 
gitgitgadget

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

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 29 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>

Thanks for the re-roll in general, there's a lot of good stuff here & I
hope I find more time to comment on it in more detail, but just focusing
on things that would be hard to back out of once changed:

> [...]
> Another alternative would be to exclude the known namespaces that are
> not intended to be shown. This would reduce the visible effect of the
> change for expert users who use their own custom ref namespaces. The
> implementation change would be very simple to swap due to our use of
> ref_namespaces:
>
> 	int i;
> 	struct string_list *exclude = decoration_filter->exclude_ref_pattern;
>
> 	/*
> 	 * No command-line or config options were given, so
> 	 * populate with sensible defaults.
> 	 */
> 	for (i = 0; i < NAMESPACE__COUNT; i++) {
> 		if (ref_namespaces[i].decoration)
> 			continue;
>
> 		string_list_append(exclude, ref_namespaces[i].ref);
> 	}
>
> The main downside of this approach is that we expect to add new hidden
> namespaces in the future, and that means that Git versions will be less
> stable in how they behave as those namespaces are added.

I see that as a feature, and not a downside. If we simply explain this
in the documentation as e.g.:

	When adding decorations git will by default exclude certain
	"internal" ref namespaces that it treats specially, such as
	refs/magical-1/*, refs/magical-2/* (or whatever). Other such
	special namespaces may be reserved in the future.

There's no lack of "stability", because the ref hiding only act on
what's known to be something we can ignore, because our git version
knows about it.

If git v2.40 knows about refs/magical-1/* but not refs/magical-2/*, but
git v2.50 knows about both it's not a lack of stability that v2.40 shows
one decorated by default, but v2.50 shows neither.

To v2.40 one of them isn't a magical "I know what this is, it's internal
& I can hide it" ref.

I'm aware that we disagree, and some of this was discussed in v1. I'm
not intending to just repeat what I said before.

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

 * Know that we have (admittedly probably rare) in-the-wild use of
   non-standard and custom namespaces, and that this series would change
   long-standing log output.

   If I could see a good reason to change the existing "log" behavior
   here I'd probably be sold on it. We try not to change existing output
   in general, but this part isn't "plumbing", and arguably not a very
   "stable" part of the log output either (decorations being optional
   etc).

   But it does rub me the wrong way to sell a change in the name of
   "stability" when it's from the outset doing the exact opposite,
   to....

 * ... are willing to make that one-time change so that we can have
   stability for users that are relying on "decorate" working
   consistently across versions once we're in the new world order,
   because we *might* add new magical refs.

Since the latter group of users don't exist today by definition (this
having not been integrated) why isn't it a better win-win solution to
give those users some --decorate-only-known-refs option/config?

They'd get their "stability" going forward, and without the needless
breaking of existing behavior, no?

> +test_expect_success 'log --decorate does not include things outside filter' '
> +	reflist="refs/prefetch refs/rebase-merge refs/bundle" &&
> +
> +	for ref in $reflist
> +	do
> +		git update-ref $ref/fake HEAD || return 1
> +	done &&
> +
> +	git log --decorate=full --oneline >actual &&
> +
> +	for ref in $reflist
> +	do
> +		! grep $ref/fake actual || return 1
> +	done

I haven't tested, but isn't that last for-loop replacable with:

	! grep /fake actual

?

Or do we have other "/fake" refs we want to include?

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

[Derrick Stolee]

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

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

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

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

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

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

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

This is a nice efficient replacement. Thanks.

-Stolee

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

[Ævar Arnfjörð Bjarmason]

On Fri, Aug 05 2022, Derrick Stolee wrote:

> On 8/4/2022 3:08 AM, Ævar Arnfjörð Bjarmason wrote:
>
>> There's no lack of "stability", because the ref hiding only act on
>> what's known to be something we can ignore, because our git version
>> knows about it.
>> 
>> If git v2.40 knows about refs/magical-1/* but not refs/magical-2/*, but
>> git v2.50 knows about both it's not a lack of stability that v2.40 shows
>> one decorated by default, but v2.50 shows neither.
>
> You are describing how the behavior changes between these versions on
> the same repository. That's what I mean by lack of stability.

If you want that forward-stability wouldn't another way to accomplish
this to put all of these new magical refs in the same refs/git-internal/
namespace, e.g. refs/git-internal/foo/, refs/git-internal/bar/?

>> But it's not just that I disagree, I genuinely don't get your POV
>> here.
>
> I'm optimizing for non-experts who never need any refs outside of the
> standard set.

Clearly, I'm wondering if we can find a way forward that doesn't change
existing long-statnding behavior and caters to the "stability" of future
inter-version behavior with this new class of refs.

> Now that this version removed the notes ref from the
> decoration, the stance for inclusion is simple:
>
>   If Git offers to color the namespace with color.decoration.<slot>,
>   then Git decorates with that namespace by default.

I'm a bit confused, sorry.

So aside from "notes", if we have a color.decoration.<slot> applying to
a ref now, it's a bug in your series if it's not showing up anymore?

I still find that inclusion criteria to be a bit odd, we've usually
considered colors optional sugar. Just because nobody bothered to add a
coloring config for it doesn't seem like a good reason to not show
something at all.

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

[Derrick Stolee]

On 8/5/2022 10:50 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Aug 05 2022, Derrick Stolee wrote:

>> Now that this version removed the notes ref from the
>> decoration, the stance for inclusion is simple:
>>
>>   If Git offers to color the namespace with color.decoration.<slot>,
>>   then Git decorates with that namespace by default.
> 
> I'm a bit confused, sorry.
> 
> So aside from "notes", if we have a color.decoration.<slot> applying to
> a ref now, it's a bug in your series if it's not showing up anymore?

The possible slots are:

 * branch (refs/heads/)
 * remoteBranch (refs/remotes/)
 * tag (refs/tags/)
 * stash (refs/stash)
 * HEAD (HEAD)
 * grafted (refs/replace/ or GIT_REPLACE_REF_BASE)

These are exactly the namespaces that are now shown by default in
this series.

If someone adds a new color slot, then that would need to be
justified with a reason why that slot is important. We've already
had discussions as to why showing a decoration for notes is not
valuable to an end user. The stability of this config option (the
last addition of 'grafted' was in 2011) is good evidence that this
core set of namespaces is all users will need.

By contrast, the use of "hidden" namespaces is relatively new and
is expected to increase in the near future.

-Stolee

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

[Ævar Arnfjörð Bjarmason]

On Fri, Aug 05 2022, Derrick Stolee wrote:

> On 8/5/2022 10:50 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Fri, Aug 05 2022, Derrick Stolee wrote:
>
>>> Now that this version removed the notes ref from the
>>> decoration, the stance for inclusion is simple:
>>>
>>>   If Git offers to color the namespace with color.decoration.<slot>,
>>>   then Git decorates with that namespace by default.
>> 
>> I'm a bit confused, sorry.
>> 
>> So aside from "notes", if we have a color.decoration.<slot> applying to
>> a ref now, it's a bug in your series if it's not showing up anymore?
>
> The possible slots are:
>
>  * branch (refs/heads/)
>  * remoteBranch (refs/remotes/)
>  * tag (refs/tags/)
>  * stash (refs/stash)
>  * HEAD (HEAD)
>  * grafted (refs/replace/ or GIT_REPLACE_REF_BASE)
>
> These are exactly the namespaces that are now shown by default in
> this series.

No, e.g. "tag" doesn't mean "refs/tags/*", it means *a tag object*. Try
this on master:

	git update-ref refs/archived-tags/v2.36.0 refs/tags/v2.36.0

Then on master:

	./git -P -c color.decorate.tag="bold blue" log --oneline -1 v2.36.0
	6cd33dceed6 (tag: v2.36.0, gitster/yw/cmake-use-pcre2, gitgitgadget/yw/cmake-use-pcre2, tag: refs/archived-tags/v2.36.0) Git 2.36

But on "seen" currently:

	$ ./git -P -c color.decorate.tag="bold blue" log --oneline -1 v2.36.0
	6cd33dceed6 (tag: v2.36.0, gitster/yw/cmake-use-pcre2, gitgitgadget/yw/cmake-use-pcre2) Git 2.36

Before that "bold blue" applied to *tag objects*, but your series has
made it apply to the refs/tags/* namespace.

I noted this (indirectly) before in
https://lore.kernel.org/git/220726.86tu73ncf8.gmgdl@evledraar.gmail.com/;
I.e. that I have a "refs/built-tags/" namespace.

So that specifically seems like a regression by the criteria you've
established for inclusion. I.e. we have objects that are impacted by
existing coloring config now that your series is hiding, seemingly
because you've conflated "tag object" with "a name in in refs/tags/".

I *also* think it's overzelous to hide *uknown* things by default
because we think we might add more *known* internal things in the
future, but that's a distinct topic from this more narrow case, which
seems to be a clear regression by criteria you're establishing &
advocating for.

> If someone adds a new color slot, then that would need to be
> justified with a reason why that slot is important. We've already
> had discussions as to why showing a decoration for notes is not
> valuable to an end user. The stability of this config option (the
> last addition of 'grafted' was in 2011) is good evidence that this
> core set of namespaces is all users will need.
>
> By contrast, the use of "hidden" namespaces is relatively new and
> is expected to increase in the near future.

I really don't see how you're making the leap that because nobody's
bothered to customize the coloring for things in custom namespaces that
it's OK to hide them entirely.

I just leave everything at the default color settings, aside from (after
checking my ~/.gitconfig) one bit of diff coloring default that I found
annoying.

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

[Derrick Stolee]

On 8/11/2022 3:30 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Aug 05 2022, Derrick Stolee wrote:
> 
>> On 8/5/2022 10:50 AM, Ævar Arnfjörð Bjarmason wrote:
>>>
>>> On Fri, Aug 05 2022, Derrick Stolee wrote:
>>
>>>> Now that this version removed the notes ref from the
>>>> decoration, the stance for inclusion is simple:
>>>>
>>>>   If Git offers to color the namespace with color.decoration.<slot>,
>>>>   then Git decorates with that namespace by default.
>>>
>>> I'm a bit confused, sorry.
>>>
>>> So aside from "notes", if we have a color.decoration.<slot> applying to
>>> a ref now, it's a bug in your series if it's not showing up anymore?
>>
>> The possible slots are:
>>
>>  * branch (refs/heads/)
>>  * remoteBranch (refs/remotes/)
>>  * tag (refs/tags/)
>>  * stash (refs/stash)
>>  * HEAD (HEAD)
>>  * grafted (refs/replace/ or GIT_REPLACE_REF_BASE)
>>
>> These are exactly the namespaces that are now shown by default in
>> this series.
> 
> No, e.g. "tag" doesn't mean "refs/tags/*", it means *a tag object*.

You are half right. It means "refs/tags/*" _or_ "a tag object". A
lightweight tag is still counted based only on its ref namespace.

> Try
> this on master:
> 
> 	git update-ref refs/archived-tags/v2.36.0 refs/tags/v2.36.0
> 
> Then on master:
> 
> 	./git -P -c color.decorate.tag="bold blue" log --oneline -1 v2.36.0
> 	6cd33dceed6 (tag: v2.36.0, gitster/yw/cmake-use-pcre2, gitgitgadget/yw/cmake-use-pcre2, tag: refs/archived-tags/v2.36.0) Git 2.36
> 
> But on "seen" currently:
> 
> 	$ ./git -P -c color.decorate.tag="bold blue" log --oneline -1 v2.36.0
> 	6cd33dceed6 (tag: v2.36.0, gitster/yw/cmake-use-pcre2, gitgitgadget/yw/cmake-use-pcre2) Git 2.36

You initially made me think there was a bug that was not covered
by our existing test infrastructure. We may still be missing a
test about this "annotated tag outside of refs/tags/*" behavior,
but it is not broken with this series.

The change of behavior here is that refs/archived-tags is excluded
from the list of decorations. If you applied --clear-decorations to
your log command, then it would appear. It would also receive the
bold blue coloring (and the "tag:" prefix) since it points to an
annotated tag object.

I also tested non-annotated tags in the refs/tags/* space and the
coloring continues to work correctly before and after this change.

Thanks,
-Stolee

[PATCH v2 07/10] log: add --decorate-all option

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-log.txt                     |   5 +
 builtin/log.c                                 |  27 ++++-
 t/t4013-diff-various.sh                       |   2 +
 ...f.log_--decorate=full_--decorate-all_--all |  61 +++++++++++
 .../diff.log_--decorate_--decorate-all_--all  |  61 +++++++++++
 t/t4202-log.sh                                | 103 +++++++++++++++++-
 6 files changed, 252 insertions(+), 7 deletions(-)
 create mode 100644 t/t4013/diff.log_--decorate=full_--decorate-all_--all
 create mode 100644 t/t4013/diff.log_--decorate_--decorate-all_--all

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index b2ac89dfafc..633705bde90 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -56,6 +56,11 @@ If none of these options or config settings are given, then references are
 used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
 `refs/stash/`, or `refs/tags/`.
 
+--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.
+
 --source::
 	Print out the ref name given on the command line by which each
 	commit was reached.
diff --git a/builtin/log.c b/builtin/log.c
index 0b5efc9434c..b7fc4946c35 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -101,6 +101,25 @@ static int parse_decoration_style(const char *value)
 	return -1;
 }
 
+static int decorate_all;
+static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
+static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
+static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
+
+static int decorate_all_callback(const struct option *opt,
+				 const char *arg, int unset)
+{
+	if (unset) {
+		decorate_all = 0;
+		return 0;
+	}
+
+	string_list_clear(&decorate_refs_include, 0);
+	string_list_clear(&decorate_refs_exclude, 0);
+	decorate_all = 1;
+	return 0;
+}
+
 static int decorate_callback(const struct option *opt, const char *arg, int unset)
 {
 	if (unset)
@@ -176,7 +195,8 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
 					   item->string);
 	}
 
-	if (decoration_filter->exclude_ref_pattern->nr ||
+	if (decorate_all ||
+	    decoration_filter->exclude_ref_pattern->nr ||
 	    decoration_filter->include_ref_pattern->nr ||
 	    decoration_filter->exclude_ref_config_pattern->nr)
 		return;
@@ -199,9 +219,6 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	struct userformat_want w;
 	int quiet = 0, source = 0, mailmap;
 	static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};
-	static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
-	static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
-	static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
 	struct decoration_filter decoration_filter = {
 		.exclude_ref_pattern = &decorate_refs_exclude,
 		.include_ref_pattern = &decorate_refs_include,
@@ -214,6 +231,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		OPT_BOOL(0, "source", &source, N_("show source")),
 		OPT_BOOL(0, "use-mailmap", &mailmap, N_("use mail map file")),
 		OPT_ALIAS(0, "mailmap", "use-mailmap"),
+		OPT_CALLBACK_F(0, "decorate-all", NULL, NULL, N_("how all ref decorations"),
+			       PARSE_OPT_NOARG, decorate_all_callback),
 		OPT_STRING_LIST(0, "decorate-refs", &decorate_refs_include,
 				N_("pattern"), N_("only decorate refs that match <pattern>")),
 		OPT_STRING_LIST(0, "decorate-refs-exclude", &decorate_refs_exclude,
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 056e922164d..577221c71ed 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -352,6 +352,8 @@ log -GF -p --pickaxe-all master
 log -IA -IB -I1 -I2 -p master
 log --decorate --all
 log --decorate=full --all
+log --decorate --decorate-all --all
+log --decorate=full --decorate-all --all
 
 rev-list --parents HEAD
 rev-list --children HEAD
diff --git a/t/t4013/diff.log_--decorate=full_--decorate-all_--all b/t/t4013/diff.log_--decorate=full_--decorate-all_--all
new file mode 100644
index 00000000000..d6e79287846
--- /dev/null
+++ b/t/t4013/diff.log_--decorate=full_--decorate-all_--all
@@ -0,0 +1,61 @@
+$ git log --decorate=full --decorate-all --all
+commit b7e0bc69303b488b47deca799a7d723971dfa6cd (refs/heads/mode)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode
+
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (refs/heads/note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
+commit cd4e72fd96faed3f0ba949dc42967430374e2290 (refs/heads/rearrange)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Rearranged lines in dir/sub
+
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> refs/heads/master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
diff --git a/t/t4013/diff.log_--decorate_--decorate-all_--all b/t/t4013/diff.log_--decorate_--decorate-all_--all
new file mode 100644
index 00000000000..5d22618bb60
--- /dev/null
+++ b/t/t4013/diff.log_--decorate_--decorate-all_--all
@@ -0,0 +1,61 @@
+$ git log --decorate --decorate-all --all
+commit b7e0bc69303b488b47deca799a7d723971dfa6cd (mode)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode
+
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
+commit cd4e72fd96faed3f0ba949dc42967430374e2290 (rearrange)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Rearranged lines in dir/sub
+
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 9ea9e835d43..e939ed34ff7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -704,9 +704,12 @@ test_expect_success 'set up more tangled history' '
 	git checkout -b tangle HEAD~6 &&
 	test_commit tangle-a tangle-a a &&
 	git merge main~3 &&
+	git update-ref refs/prefetch/merge HEAD &&
 	git merge side~1 &&
+	git update-ref refs/rewritten/merge HEAD &&
 	git checkout main &&
 	git merge tangle &&
+	git update-ref refs/hidden/tangle HEAD &&
 	git checkout -b reach &&
 	test_commit reach &&
 	git checkout main &&
@@ -974,9 +977,9 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '
 	Merge-tag-reach (HEAD -> main)
 	reach (tag: reach, reach)
 	seventh (tag: seventh)
-	Merge-branch-tangle
-	Merge-branch-side-early-part-into-tangle (tangle)
-	tangle-a (tag: tangle-a)
+	Merge-branch-tangle (refs/hidden/tangle)
+	Merge-branch-side-early-part-into-tangle (refs/rewritten/merge, tangle)
+	Merge-branch-main-early-part-into-tangle (refs/prefetch/merge)
 	EOF
 	git log -n6 --decorate=short --pretty="tformat:%f%d" \
 		--decorate-refs-exclude="*octopus*" \
@@ -1037,6 +1040,100 @@ test_expect_success 'decorate-refs focus from default' '
 	! grep HEAD actual
 '
 
+test_expect_success '--decorate-all overrides defaults' '
+	cat >expect.default <<-\EOF &&
+	Merge-tag-reach (HEAD -> refs/heads/main)
+	Merge-tags-octopus-a-and-octopus-b
+	seventh (tag: refs/tags/seventh)
+	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
+	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
+	reach (tag: refs/tags/reach, refs/heads/reach)
+	Merge-branch-tangle
+	Merge-branch-side-early-part-into-tangle (refs/heads/tangle)
+	Merge-branch-main-early-part-into-tangle
+	tangle-a (tag: refs/tags/tangle-a)
+	Merge-branch-side
+	side-2 (tag: refs/tags/side-2, refs/heads/side)
+	side-1 (tag: refs/tags/side-1)
+	Second
+	sixth
+	fifth
+	fourth
+	third
+	second
+	initial
+	EOF
+	git log --decorate=full --pretty="tformat:%f%d" >actual &&
+	test_cmp expect.default actual &&
+
+	cat >expect.all <<-\EOF &&
+	Merge-tag-reach (HEAD -> refs/heads/main)
+	Merge-tags-octopus-a-and-octopus-b
+	seventh (tag: refs/tags/seventh)
+	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
+	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
+	reach (tag: refs/tags/reach, refs/heads/reach)
+	Merge-branch-tangle (refs/hidden/tangle)
+	Merge-branch-side-early-part-into-tangle (refs/rewritten/merge, refs/heads/tangle)
+	Merge-branch-main-early-part-into-tangle (refs/prefetch/merge)
+	tangle-a (tag: refs/tags/tangle-a)
+	Merge-branch-side
+	side-2 (tag: refs/tags/side-2, refs/heads/side)
+	side-1 (tag: refs/tags/side-1)
+	Second
+	sixth
+	fifth
+	fourth
+	third
+	second
+	initial
+	EOF
+	git log --decorate=full --pretty="tformat:%f%d" \
+		--decorate-all >actual &&
+	test_cmp expect.all actual
+'
+
+test_expect_success '--decorate-all clears previous exclusions' '
+	cat >expect.all <<-\EOF &&
+	Merge-tag-reach (HEAD -> refs/heads/main)
+	reach (tag: refs/tags/reach, refs/heads/reach)
+	Merge-tags-octopus-a-and-octopus-b
+	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
+	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
+	seventh (tag: refs/tags/seventh)
+	Merge-branch-tangle (refs/hidden/tangle)
+	Merge-branch-side-early-part-into-tangle (refs/rewritten/merge, refs/heads/tangle)
+	Merge-branch-main-early-part-into-tangle (refs/prefetch/merge)
+	tangle-a (tag: refs/tags/tangle-a)
+	side-2 (tag: refs/tags/side-2, refs/heads/side)
+	side-1 (tag: refs/tags/side-1)
+	initial
+	EOF
+
+	git log --decorate=full --pretty="tformat:%f%d" \
+		--simplify-by-decoration \
+		--decorate-refs-exclude="heads/octopus*" \
+		--decorate-refs="heads" \
+		--decorate-all >actual &&
+	test_cmp expect.all actual &&
+
+	cat >expect.filtered <<-\EOF &&
+	Merge-tags-octopus-a-and-octopus-b
+	octopus-b (refs/heads/octopus-b)
+	octopus-a (refs/heads/octopus-a)
+	initial
+	EOF
+
+	git log --decorate=full --pretty="tformat:%f%d" \
+		--simplify-by-decoration \
+		--decorate-refs-exclude="heads/octopus" \
+		--decorate-refs="heads" \
+		--decorate-all \
+		--decorate-refs-exclude="tags/" \
+		--decorate-refs="heads/octopus*" >actual &&
+	test_cmp expect.filtered actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
-- 
gitgitgadget

Re: [PATCH v2 07/10] log: add --decorate-all option

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static int decorate_all;
> +static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
> +static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
> +static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
> +
> +static int decorate_all_callback(const struct option *opt,
> +				 const char *arg, int unset)
> +{
> +	if (unset) {
> +		decorate_all = 0;
> +		return 0;
> +	}
> +
> +	string_list_clear(&decorate_refs_include, 0);
> +	string_list_clear(&decorate_refs_exclude, 0);
> +	decorate_all = 1;

Here, we clear include and exclude list, and set the flag.

> +	return 0;
> +}
> +
>  static int decorate_callback(const struct option *opt, const char *arg, int unset)
>  {
>  	if (unset)
> @@ -176,7 +195,8 @@ static void set_default_d
>  					   item->string);
>  	}
>  
> -	if (decoration_filter->exclude_ref_pattern->nr ||
> +	if (decorate_all ||
> +	    decoration_filter->exclude_ref_pattern->nr ||
>  	    decoration_filter->include_ref_pattern->nr ||
>  	    decoration_filter->exclude_ref_config_pattern->nr)
>  		return;

In the pre-context of this hunk, we still pay attention to the
configuration variable log.excludedDecoration and stuff them in
exclude_ref_config_pattern.  Presense of decorate_all option makes
us leave early from this function, skipping the addition of
hardcoded default patterns from the ref_namespace[] array.

Most notably, clearing of exclude_ref_pattern and
include_ref_pattern is done when "--decorate-all" was parsed, and
not here.  So we may have patterns there if 

	git log --decorate-all \
		--decorate-refs=<pattern> \
		--decorate-refs-exclude=<pattern>

Later, log-tree.c::load_ref_decoration() uses these patterns to
decide what to exclude and to include.

So to me, this command option does not look like "all" at all.  It
is more like "clear", with a side effect visible and noticeable only
to implementors.

If we explain the decoration feature to end users in such a way to
form this mental model, then this option becomes truly "clear the
inclusion and exclusion patterns for ref decoration".

    "log --decorate" uses lists of include and exclude patterns to
    decide which refs are used for decoration.  Before the user
    gives any patterns, the system adds the default include and
    exclude patterns in the list.  The user can use --decorate-refs
    and --decorate-refs-exclude options to add more of them to the
    lists.

Then this option can be explained to "clear what has been
accumulated in the lists of include and exclude patterns" and that
also gets rid of what the system adds by default.

I am not sure what to call and how to explain the corresponding
configuration variable introduced in the next step, though.

Thanks.

[PATCH v2 08/10] log: create log.decorateFilter=all

[Derrick Stolee via GitGitGadget]

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'.
+
 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
 '
-- 
gitgitgadget

Re: [PATCH v2 08/10] log: create log.decorateFilter=all

[Josh Steadmon]

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.

Re: [PATCH v2 08/10] log: create log.decorateFilter=all

[Derrick Stolee]

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

[PATCH v2 09/10] maintenance: stop writing log.excludeDecoration

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

This reverts commit 96eaffebbf3d0 (maintenance: set
log.excludeDecoration durin prefetch, 2021-01-19).

The previous change created a default decoration filter that does not
include refs/prefetch/, so this modification of the config is no longer
needed.

One issue that can happen from this point on is that users who ran the
prefetch task on previous versions of Git will still have a
log.excludeDecoration value and that will prevent the new default
decoration filter from being active. Thus, when we add the refs/bundle/
namespace as part of the bundle URI feature, those users will see
refs/bundle/ decorations.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 builtin/gc.c           |  6 ------
 t/t7900-maintenance.sh | 21 ---------------------
 2 files changed, 27 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index eeff2b760e0..6c222052177 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -910,12 +910,6 @@ static int fetch_remote(struct remote *remote, void *cbdata)
 
 static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
 {
-	git_config_set_multivar_gently("log.excludedecoration",
-					"refs/prefetch/",
-					"refs/prefetch/",
-					CONFIG_FLAGS_FIXED_VALUE |
-					CONFIG_FLAGS_MULTI_REPLACE);
-
 	if (for_each_remote(fetch_remote, opts)) {
 		error(_("failed to prefetch remotes"));
 		return 1;
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 74aa6384755..62ed694a404 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -162,7 +162,6 @@ test_expect_success 'prefetch multiple remotes' '
 	test_cmp_rev refs/remotes/remote1/one refs/prefetch/remotes/remote1/one &&
 	test_cmp_rev refs/remotes/remote2/two refs/prefetch/remotes/remote2/two &&
 
-	test_cmp_config refs/prefetch/ log.excludedecoration &&
 	git log --oneline --decorate --all >log &&
 	! grep "prefetch" log &&
 
@@ -173,26 +172,6 @@ test_expect_success 'prefetch multiple remotes' '
 	test_subcommand git fetch remote2 $fetchargs <skip-remote1.txt
 '
 
-test_expect_success 'prefetch and existing log.excludeDecoration values' '
-	git config --unset-all log.excludeDecoration &&
-	git config log.excludeDecoration refs/remotes/remote1/ &&
-	git maintenance run --task=prefetch &&
-
-	git config --get-all log.excludeDecoration >out &&
-	grep refs/remotes/remote1/ out &&
-	grep refs/prefetch/ out &&
-
-	git log --oneline --decorate --all >log &&
-	! grep "prefetch" log &&
-	! grep "remote1" log &&
-	grep "remote2" log &&
-
-	# a second run does not change the config
-	git maintenance run --task=prefetch &&
-	git log --oneline --decorate --all >log2 &&
-	test_cmp log log2
-'
-
 test_expect_success 'loose-objects task' '
 	# Repack everything so we know the state of the object dir
 	git repack -adk &&
-- 
gitgitgadget

Re: [PATCH v2 09/10] maintenance: stop writing log.excludeDecoration

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> This reverts commit 96eaffebbf3d0 (maintenance: set
> log.excludeDecoration durin prefetch, 2021-01-19).
>
> The previous change created a default decoration filter that does not
> include refs/prefetch/, so this modification of the config is no longer
> needed.
>
> One issue that can happen from this point on is that users who ran the
> prefetch task on previous versions of Git will still have a
> log.excludeDecoration value and that will prevent the new default
> decoration filter from being active. Thus, when we add the refs/bundle/
> namespace as part of the bundle URI feature, those users will see
> refs/bundle/ decorations.

Yup, and if they find it disturbing, they can fix it, but it is
unlikely to be a problem, I would think, since decoration works only
for exact point in history.  If this topic were to choose what refs
the "describe" abbreviation uses, it would have been much more
annoying to see some unwanted refs used.

[PATCH v2 10/10] fetch: use ref_namespaces during prefetch

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The "refs/prefetch/" namespace is used by 'git fetch --prefetch' as a
replacement of the destination of the refpsec for a remote. Git also
removes refspecs that include tags.

Instead of using string literals for the 'refs/tags/ and
'refs/prefetch/' namespaces, use the entries in the ref_namespaces
array.

This kind of change could be done in many places around the codebase,
but we are isolating only to this change because of the way the
refs/prefetch/ namespace somewhat motivated the creation of the
ref_namespaces array.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 builtin/fetch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc5cecb4835..004d92b3554 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -490,7 +490,9 @@ static void filter_prefetch_refspec(struct refspec *rs)
 			continue;
 		if (!rs->items[i].dst ||
 		    (rs->items[i].src &&
-		     !strncmp(rs->items[i].src, "refs/tags/", 10))) {
+		     !strncmp(rs->items[i].src,
+			      ref_namespaces[NAMESPACE_TAGS].ref,
+			      strlen(ref_namespaces[NAMESPACE_TAGS].ref)))) {
 			int j;
 
 			free(rs->items[i].src);
@@ -506,7 +508,7 @@ static void filter_prefetch_refspec(struct refspec *rs)
 		}
 
 		old_dst = rs->items[i].dst;
-		strbuf_addstr(&new_dst, "refs/prefetch/");
+		strbuf_addstr(&new_dst, ref_namespaces[NAMESPACE_PREFETCH].ref);
 
 		/*
 		 * If old_dst starts with "refs/", then place
-- 
gitgitgadget

[PATCH v3 00/11] log: create tighter default decoration filter

[Derrick Stolee via GitGitGadget]

When running 'git log', the default is to report any refs within refs/* as
decorations. This series reduces the default set to be more specific to a
set of known-useful namespaces.

This was previously reduced by adding the log.excludeDecoration config
option and modifying that config in git maintenance's prefetch task (to hide
refs/prefetch/*). I then followed that pattern again for the bundle URI
feature [1], but this caught some reviewers by surprise as an unfortunate
side-effect. This series is a way to roll back the previous decision to use
log.excludeDecoration and instead use tighter filters by default.

[1]
https://lore.kernel.org/git/a217e9a0640b45d21ef971d6e91cee3f1993f383.1656535245.git.gitgitgadget@gmail.com/

The current design ignores the new filters if there are any
previously-specified filters. This includes the
log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
This means that users who ran that command in their repo will not get the
benefits of the more strict filters. While we stop writing
log.excludeDecorations, we don't remove existing instances of it. This
should be fine at least for the refs/bundles/ namespace in the future, since
those refs will typically be created for new clones that have not yet had
the log.excludeDecoration setting.

To make it easy for users to opt-in to the previous behavior, add a new git
log --decorate-all option and log.decorateFilter=all config option.


Updates in v3
=============

 * Fixed braces in if/else-if in patch 1. Also added a NEEDSWORK comment.
 * Added a patch that modernizes t4207 and converts some logic to a helper.
 * Updated the new tests in t4207 to use the new helper.
 * Used singular ref_namespace instead of ref_namespaces.
 * Update definition of refs_namespace to have fixed size so we can use
   ARRAY_SIZE() in a loop.
 * Renamed the CLI option to --clear-decorations and the config option to
   log.initialDecorationSet.
 * Several recommended style updates and typo fixes.


Updates in v2
=============

 * As discussed in the previous version, there are a lot of places that care
   about different subsets of the special ref namespaces. This version
   creates a new ref_namespaces array that centralizes and describes how
   these known ref namespaces interact with other Git features.

 * Some patches are added to help centralize previously-unrelated features
   onto the ref_namespaces array.

 * One thing that I noticed while doing this work was that notes come from a
   commit history that is pointed by a single ref, not multiple refs in a
   namespace. Also, that ref can change based on config and environment
   variables. This version updates that knowledge to allow our
   ref_namespaces[NAMESPACE_NOTES] value change with these settings. Since
   there is only one ref, I also chose to remove it from the default
   decorations. As Junio mentioned, this should not make any difference
   since a user won't see it unless they purposefully start git log from the
   notes ref.

 * Despite feedback to the opposite, I maintain my opinion that this default
   filter should be a small list of known-useful namespaces. For expert
   users who use other namespaces, I added patches that make it easier to
   show custom namespaces (specifically --decorate-all and
   log.decorateFilter=all). The logic for creating the default filter now
   uses the ref_namespaces array in such a way that it would be simple to
   reverse this decision, and I include that alternate implementation in a
   commit message.

 * This version includes several code and test style improvements.

 * The final patch demonstrates how we can remove some duplicate string
   literals for these namespaces. It is focused on the prefetch namespace
   (and an adjacent use of the tags namespace) because of how this series
   changes our handling of that namespace in particular. This makes it
   easier for a future change to modify the namespace via config or
   environment variables. There are opportunities to do this kind of update
   more widely, but I don't want to set that as a critical refactor to do
   right now.

Thanks, -Stolee

Derrick Stolee (11):
  refs: allow "HEAD" as decoration filter
  t4207: modernize test
  t4207: test coloring of grafted decorations
  refs: add array of ref namespaces
  refs: use ref_namespaces for replace refs base
  log-tree: use ref_namespaces instead of if/else-if
  log: add default decoration filter
  log: add --clear-decorations option
  log: create log.initialDecorationSet=all
  maintenance: stop writing log.excludeDecoration
  fetch: use ref_namespaces during prefetch

 Documentation/config/log.txt                  |   5 +
 Documentation/git-log.txt                     |  14 +-
 builtin/fetch.c                               |   6 +-
 builtin/gc.c                                  |   6 -
 builtin/log.c                                 |  84 ++++++++---
 builtin/replace.c                             |   3 +
 cache.h                                       |   1 -
 environment.c                                 |   5 +-
 log-tree.c                                    |  27 ++--
 notes.c                                       |   1 +
 refs.c                                        |  95 ++++++++++++-
 refs.h                                        |  46 ++++++
 t/t4013-diff-various.sh                       |   2 +
 t/t4013/diff.log_--decorate=full_--all        |   2 +-
 ..._--decorate=full_--clear-decorations_--all |  61 ++++++++
 ...f.log_--decorate=full_--decorate-all_--all |  61 ++++++++
 t/t4013/diff.log_--decorate_--all             |   2 +-
 ...f.log_--decorate_--clear-decorations_--all |  61 ++++++++
 .../diff.log_--decorate_--decorate-all_--all  |  61 ++++++++
 t/t4202-log.sh                                | 132 +++++++++++++++++-
 t/t4207-log-decoration-colors.sh              |  90 +++++++++---
 t/t7900-maintenance.sh                        |  21 ---
 22 files changed, 701 insertions(+), 85 deletions(-)
 create mode 100644 t/t4013/diff.log_--decorate=full_--clear-decorations_--all
 create mode 100644 t/t4013/diff.log_--decorate=full_--decorate-all_--all
 create mode 100644 t/t4013/diff.log_--decorate_--clear-decorations_--all
 create mode 100644 t/t4013/diff.log_--decorate_--decorate-all_--all


base-commit: 6a475b71f8c4ce708d69fdc9317aefbde3769e25
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1301%2Fderrickstolee%2Fdecorations-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1301/derrickstolee/decorations-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1301

Range-diff vs v2:

  1:  c2e5a0b3a50 !  1:  1522889352f refs: allow "HEAD" as decoration filter
     @@ Commit message
      
          It is sufficient to only cover "HEAD" here and not include other special
          refs like REBASE_HEAD. This is because HEAD is the only ref outside of
     -    refs/* that is added to the list of decorations.
     +    refs/* that is added to the list of decorations. However, we may want to
     +    special-case these other refs in normalize_glob_ref() in the future.
     +    Leave a NEEDSWORK comment for now.
      
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
       ## refs.c ##
      @@ refs.c: void normalize_glob_ref(struct string_list_item *item, const char *prefix,
     + 	if (*pattern == '/')
     + 		BUG("pattern must not start with '/'");
       
     - 	if (prefix) {
     +-	if (prefix) {
     ++	if (prefix)
       		strbuf_addstr(&normalized_pattern, prefix);
      -	}
      -	else if (!starts_with(pattern, "refs/"))
     -+	} else if (!starts_with(pattern, "refs/") &&
     ++	else if (!starts_with(pattern, "refs/") &&
      +		   strcmp(pattern, "HEAD"))
       		strbuf_addstr(&normalized_pattern, "refs/");
     ++	/*
     ++	 * NEEDSWORK: Special case other symrefs such as REBASE_HEAD,
     ++	 * MERGE_HEAD, etc.
     ++	 */
     ++
       	strbuf_addstr(&normalized_pattern, pattern);
       	strbuf_strip_suffix(&normalized_pattern, "/");
     + 
      
       ## t/t4202-log.sh ##
      @@ t/t4202-log.sh: test_expect_success 'decorate-refs and simplify-by-decoration without output' '
  -:  ----------- >  2:  0633f8403db t4207: modernize test
  2:  b5eb110958b !  3:  fafe20c1e82 t4207: test coloring of grafted decorations
     @@ Commit message
      
       ## t/t4207-log-decoration-colors.sh ##
      @@ t/t4207-log-decoration-colors.sh: test_expect_success setup '
     + 	git config color.decorate.remoteBranch red &&
       	git config color.decorate.tag "reverse bold yellow" &&
       	git config color.decorate.stash magenta &&
     - 	git config color.decorate.HEAD cyan &&
      +	git config color.decorate.grafted black &&
     + 	git config color.decorate.HEAD cyan &&
       
       	c_reset="<RESET>" &&
     - 
      @@ t/t4207-log-decoration-colors.sh: test_expect_success setup '
       	c_tag="<BOLD;REVERSE;YELLOW>" &&
       	c_stash="<MAGENTA>" &&
     @@ t/t4207-log-decoration-colors.sh: test_expect_success setup '
       
       	test_commit A &&
       	git clone . other &&
     -@@ t/t4207-log-decoration-colors.sh: test_expect_success 'Commit Decorations Colored Correctly' '
     - 	test_cmp expected out
     +@@ t/t4207-log-decoration-colors.sh: On main: Changes to A.t
     + 	cmp_filtered_decorations
       '
       
     -+cat >expected <<EOF
     -+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
     -+ ${c_reset}${c_branch}main${c_reset}${c_commit},\
     -+ ${c_reset}${c_tag}tag: D${c_reset}${c_commit})${c_reset} D
     -+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: C${c_reset}${c_commit},\
     -+ ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
     -+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
     -+EOF
     -+
      +test_expect_success 'test coloring with replace-objects' '
      +	test_when_finished rm -rf .git/refs/replace* &&
      +	test_commit C &&
      +	test_commit D &&
      +
      +	git replace HEAD~1 HEAD~2 &&
     -+	git log --first-parent --abbrev=10 --decorate --oneline --color=always HEAD |
     -+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
     -+	test_decode_color >out &&
     -+	test_cmp expected out &&
     ++
     ++	cat >expect <<-EOF &&
     ++	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \
     ++${c_reset}${c_branch}main${c_reset}${c_commit}, \
     ++${c_reset}${c_tag}tag: D${c_reset}${c_commit})${c_reset} D
     ++	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: C${c_reset}${c_commit}, \
     ++${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
     ++	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
     ++EOF
     ++
     ++	git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
     ++	cmp_filtered_decorations &&
      +	git replace -d HEAD~1 &&
      +
      +	GIT_REPLACE_REF_BASE=refs/replace2/ git replace HEAD~1 HEAD~2 &&
     -+	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent --abbrev=10 \
     -+		--decorate --oneline --color=always HEAD |
     -+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
     -+	test_decode_color >out &&
     -+	test_cmp expected out
     ++	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent \
     ++		--no-abbrev --decorate --oneline --color=always HEAD >actual &&
     ++	cmp_filtered_decorations
      +'
      +
     -+cat >expected <<EOF
     -+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
     -+ ${c_reset}${c_branch}main${c_reset}${c_commit},\
     -+ ${c_reset}${c_tag}tag: D${c_reset}${c_commit},\
     -+ ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
     -+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
     -+ ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
     -+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
     -+EOF
     -+
      +test_expect_success 'test coloring with grafted commit' '
      +	test_when_finished rm -rf .git/refs/replace* &&
      +
      +	git replace --graft HEAD HEAD~2 &&
     -+	git log --first-parent --abbrev=10 --decorate --oneline --color=always HEAD |
     -+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
     -+	test_decode_color >out &&
     -+	test_cmp expected out &&
     ++
     ++	cat >expect <<-EOF &&
     ++	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \
     ++${c_reset}${c_branch}main${c_reset}${c_commit}, \
     ++${c_reset}${c_tag}tag: D${c_reset}${c_commit}, \
     ++${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
     ++	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit}, \
     ++${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
     ++	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
     ++	EOF
     ++
     ++	git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
     ++	cmp_filtered_decorations &&
      +	git replace -d HEAD &&
      +
      +	GIT_REPLACE_REF_BASE=refs/replace2/ git replace --graft HEAD HEAD~2 &&
     -+	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent --abbrev=10 \
     -+		--decorate --oneline --color=always HEAD |
     -+	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
     -+	test_decode_color >out &&
     -+	test_cmp expected out
     ++	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent \
     ++		--no-abbrev --decorate --oneline --color=always HEAD >actual &&
     ++	cmp_filtered_decorations
      +'
      +
       test_done
  3:  d7486390d57 !  4:  475ebca6310 refs: add array of ref namespaces
     @@ refs.c: static unsigned char refname_disposition[256] = {
       	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
       };
       
     -+struct ref_namespace_info ref_namespaces[] = {
     ++struct ref_namespace_info ref_namespace[] = {
      +	[NAMESPACE_HEAD] = {
      +		.ref = "HEAD",
      +		.decoration = DECORATION_REF_HEAD,
     @@ refs.c: static unsigned char refname_disposition[256] = {
      +		/*
      +		 * The refs/notes/commit ref points to the tip of a
      +		 * parallel commit history that adds metadata to commits
     -+		 * in the normal history. This branch can be overwritten
     ++		 * in the normal history. This ref can be overwritten
      +		 * by the core.notesRef config variable or the
      +		 * GIT_NOTES_REFS environment variable.
      +		 */
     @@ refs.c: static unsigned char refname_disposition[256] = {
      +
      +void update_ref_namespace(enum ref_namespace namespace, char *ref)
      +{
     -+	struct ref_namespace_info *info = &ref_namespaces[namespace];
     ++	struct ref_namespace_info *info = &ref_namespace[namespace];
      +	if (info->ref_updated)
      +		free(info->ref);
      +	info->ref = ref;
     @@ refs.h: struct ref_store *get_main_ref_store(struct repository *r);
       
      +/*
      + * Some of the names specified by refs have special meaning to Git.
     -+ * Organize these namespaces in a comon 'ref_namespaces' array for
     ++ * Organize these namespaces in a comon 'ref_namespace' array for
      + * reference from multiple places in the codebase.
      + */
      +
     @@ refs.h: struct ref_store *get_main_ref_store(struct repository *r);
      +	 * If 'exact' is true, then we must match the 'ref' exactly.
      +	 * Otherwise, use a prefix match.
      +	 *
     -+	 * 'orig_ref' is for internal use. It represents whether the
     ++	 * 'ref_updated' is for internal use. It represents whether the
      +	 * 'ref' value was replaced from its original literal version.
      +	 */
      +	unsigned exact:1,
     @@ refs.h: struct ref_store *get_main_ref_store(struct repository *r);
      +};
      +
      +/* See refs.c for the contents of this array. */
     -+extern struct ref_namespace_info ref_namespaces[];
     ++extern struct ref_namespace_info ref_namespace[NAMESPACE__COUNT];
      +
      +/*
      + * Some ref namespaces can be modified by config values or environment
  4:  8da0b0a3181 !  5:  45e3ce94d0d refs: use ref_namespaces for replace refs base
     @@ builtin/replace.c: static int for_each_replace_name(const char **argv, each_repl
       	size_t base_len;
       	int had_error = 0;
       	struct object_id oid;
     -+	const char *git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
     ++	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
       
       	strbuf_addstr(&ref, git_replace_ref_base);
       	base_len = ref.len;
     @@ builtin/replace.c: static int check_ref_valid(struct object_id *object,
       			    struct strbuf *ref,
       			    int force)
       {
     -+	const char *git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
     ++	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
      +
       	strbuf_reset(ref);
       	strbuf_addf(ref, "%s%s", git_replace_ref_base, oid_to_hex(object));
     @@ log-tree.c: static int add_ref_decoration(const char *refname, const struct obje
       	enum object_type objtype;
       	enum decoration_type deco_type = DECORATION_NONE;
       	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
     -+	const char *git_replace_ref_base;
     ++	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
       
       	if (filter && !ref_filter_match(refname, filter))
       		return 0;
     - 
     -+	git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
     - 	if (starts_with(refname, git_replace_ref_base)) {
     - 		struct object_id original_oid;
     - 		if (!read_replace_refs)
      
       ## refs.c ##
      @@ refs.c: int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
       
       int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)
       {
     -+	const char *git_replace_ref_base = ref_namespaces[NAMESPACE_REPLACE].ref;
     ++	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
       	return do_for_each_repo_ref(r, git_replace_ref_base, fn,
       				    strlen(git_replace_ref_base),
       				    DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
  5:  53b15a0b793 !  6:  063e5bac2ce log-tree: use ref_namespaces instead of if/else-if
     @@ log-tree.c: static int add_ref_decoration(const char *refname, const struct obje
      -		deco_type = DECORATION_REF_STASH;
      -	else if (!strcmp(refname, "HEAD"))
      -		deco_type = DECORATION_REF_HEAD;
     -+	for (i = 0; i < NAMESPACE__COUNT; i++) {
     -+		struct ref_namespace_info *info = &ref_namespaces[i];
     ++	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) {
     ++		struct ref_namespace_info *info = &ref_namespace[i];
      +
      +		if (!info->decoration)
      +			continue;
  6:  bec532fb8c6 !  7:  c249bface2a log: add default decoration filter
     @@ builtin/log.c: static void cmd_log_init_defaults(struct rev_info *rev)
      +	 * No command-line or config options were given, so
      +	 * populate with sensible defaults.
      +	 */
     -+	for (i = 0; i < NAMESPACE__COUNT; i++) {
     -+		if (!ref_namespaces[i].decoration)
     ++	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) {
     ++		if (!ref_namespace[i].decoration)
      +			continue;
      +
     -+		string_list_append(include, ref_namespaces[i].ref);
     ++		string_list_append(include, ref_namespace[i].ref);
      +	}
      +}
      +
     @@ t/t4202-log.sh: test_expect_success 'log --decorate includes all levels of tag a
      +
      +	git log --decorate=full --oneline >actual &&
      +
     -+	for ref in $reflist
     -+	do
     -+		! grep $ref/fake actual || return 1
     -+	done
     ++	# None of the refs are visible:
     ++	! grep /fake actual
      +'
      +
       test_expect_success 'log --end-of-options' '
  7:  64ee889369d !  8:  1b12374ff1b log: add --decorate-all option
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    log: add --decorate-all option
     +    log: add --clear-decorations option
     +
     +    The previous changes introduced a new default ref filter for decorations
     +    in the 'git log' command. This can be overridden using
     +    --decorate-refs=HEAD and --decorate-refs=refs/, but that is cumbersome
     +    for users.
     +
     +    Instead, add a --clear-decorations option that resets all previous
     +    filters to a blank filter that accepts all refs.
      
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
     @@ Documentation/git-log.txt: If none of these options or config settings are given
       used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
       `refs/stash/`, or `refs/tags/`.
       
     -+--decorate-all::
     ++--clear-decorations::
      +	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 references.
      +
       --source::
       	Print out the ref name given on the command line by which each
     @@ builtin/log.c: static int parse_decoration_style(const char *value)
       	return -1;
       }
       
     -+static int decorate_all;
     ++static int use_default_decoration_filter = 1;
      +static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
      +static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
      +static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
      +
     -+static int decorate_all_callback(const struct option *opt,
     -+				 const char *arg, int unset)
     ++static int clear_decorations_callback(const struct option *opt,
     ++					    const char *arg, int unset)
      +{
     -+	if (unset) {
     -+		decorate_all = 0;
     -+		return 0;
     -+	}
     -+
      +	string_list_clear(&decorate_refs_include, 0);
      +	string_list_clear(&decorate_refs_exclude, 0);
     -+	decorate_all = 1;
     ++	use_default_decoration_filter = 0;
      +	return 0;
      +}
      +
     @@ builtin/log.c: static void set_default_decoration_filter(struct decoration_filte
       	}
       
      -	if (decoration_filter->exclude_ref_pattern->nr ||
     -+	if (decorate_all ||
     ++	if (!use_default_decoration_filter ||
      +	    decoration_filter->exclude_ref_pattern->nr ||
       	    decoration_filter->include_ref_pattern->nr ||
       	    decoration_filter->exclude_ref_config_pattern->nr)
     @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons
       		OPT_BOOL(0, "source", &source, N_("show source")),
       		OPT_BOOL(0, "use-mailmap", &mailmap, N_("use mail map file")),
       		OPT_ALIAS(0, "mailmap", "use-mailmap"),
     -+		OPT_CALLBACK_F(0, "decorate-all", NULL, NULL, N_("how all ref decorations"),
     -+			       PARSE_OPT_NOARG, decorate_all_callback),
     ++		OPT_CALLBACK_F(0, "clear-decorations", NULL, NULL,
     ++			       N_("clear all previously-defined decoration filters"),
     ++			       PARSE_OPT_NOARG | PARSE_OPT_NONEG,
     ++			       clear_decorations_callback),
       		OPT_STRING_LIST(0, "decorate-refs", &decorate_refs_include,
       				N_("pattern"), N_("only decorate refs that match <pattern>")),
       		OPT_STRING_LIST(0, "decorate-refs-exclude", &decorate_refs_exclude,
     @@ t/t4013-diff-various.sh: log -GF -p --pickaxe-all master
       log -IA -IB -I1 -I2 -p master
       log --decorate --all
       log --decorate=full --all
     -+log --decorate --decorate-all --all
     -+log --decorate=full --decorate-all --all
     ++log --decorate --clear-decorations --all
     ++log --decorate=full --clear-decorations --all
       
       rev-list --parents HEAD
       rev-list --children HEAD
      
     + ## t/t4013/diff.log_--decorate=full_--clear-decorations_--all (new) ##
     +@@
     ++$ git log --decorate=full --clear-decorations --all
     ++commit b7e0bc69303b488b47deca799a7d723971dfa6cd (refs/heads/mode)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    update mode
     ++
     ++commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (refs/heads/note)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    update mode (file2)
     ++
     ++Notes:
     ++    note
     ++
     ++commit cd4e72fd96faed3f0ba949dc42967430374e2290 (refs/heads/rearrange)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    Rearranged lines in dir/sub
     ++
     ++commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    Notes added by 'git notes add'
     ++
     ++commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> refs/heads/master)
     ++Merge: 9a6d494 c7a2ab9
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:04:00 2006 +0000
     ++
     ++    Merge branch 'side'
     ++
     ++commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:03:00 2006 +0000
     ++
     ++    Side
     ++
     ++commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:02:00 2006 +0000
     ++
     ++    Third
     ++
     ++commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:01:00 2006 +0000
     ++
     ++    Second
     ++    
     ++    This is the second commit.
     ++
     ++commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:00:00 2006 +0000
     ++
     ++    Initial
     ++$
     +
       ## t/t4013/diff.log_--decorate=full_--decorate-all_--all (new) ##
      @@
      +$ git log --decorate=full --decorate-all --all
     @@ t/t4013/diff.log_--decorate=full_--decorate-all_--all (new)
      +Date:   Mon Jun 26 00:00:00 2006 +0000
      +
      +    Initial
     ++$
     +
     + ## t/t4013/diff.log_--decorate_--clear-decorations_--all (new) ##
     +@@
     ++$ git log --decorate --clear-decorations --all
     ++commit b7e0bc69303b488b47deca799a7d723971dfa6cd (mode)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    update mode
     ++
     ++commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (note)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    update mode (file2)
     ++
     ++Notes:
     ++    note
     ++
     ++commit cd4e72fd96faed3f0ba949dc42967430374e2290 (rearrange)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    Rearranged lines in dir/sub
     ++
     ++commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:06:00 2006 +0000
     ++
     ++    Notes added by 'git notes add'
     ++
     ++commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> master)
     ++Merge: 9a6d494 c7a2ab9
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:04:00 2006 +0000
     ++
     ++    Merge branch 'side'
     ++
     ++commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:03:00 2006 +0000
     ++
     ++    Side
     ++
     ++commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:02:00 2006 +0000
     ++
     ++    Third
     ++
     ++commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:01:00 2006 +0000
     ++
     ++    Second
     ++    
     ++    This is the second commit.
     ++
     ++commit 444ac553ac7612cc88969031b02b3767fb8a353a (initial)
     ++Author: A U Thor <author@example.com>
     ++Date:   Mon Jun 26 00:00:00 2006 +0000
     ++
     ++    Initial
      +$
      
       ## t/t4013/diff.log_--decorate_--decorate-all_--all (new) ##
     @@ t/t4202-log.sh: test_expect_success 'decorate-refs focus from default' '
       	! grep HEAD actual
       '
       
     -+test_expect_success '--decorate-all overrides defaults' '
     ++test_expect_success '--clear-decorations overrides defaults' '
      +	cat >expect.default <<-\EOF &&
      +	Merge-tag-reach (HEAD -> refs/heads/main)
      +	Merge-tags-octopus-a-and-octopus-b
     @@ t/t4202-log.sh: test_expect_success 'decorate-refs focus from default' '
      +	initial
      +	EOF
      +	git log --decorate=full --pretty="tformat:%f%d" \
     -+		--decorate-all >actual &&
     ++		--clear-decorations >actual &&
      +	test_cmp expect.all actual
      +'
      +
     -+test_expect_success '--decorate-all clears previous exclusions' '
     ++test_expect_success '--clear-decorations clears previous exclusions' '
      +	cat >expect.all <<-\EOF &&
      +	Merge-tag-reach (HEAD -> refs/heads/main)
      +	reach (tag: refs/tags/reach, refs/heads/reach)
     @@ t/t4202-log.sh: test_expect_success 'decorate-refs focus from default' '
      +		--simplify-by-decoration \
      +		--decorate-refs-exclude="heads/octopus*" \
      +		--decorate-refs="heads" \
     -+		--decorate-all >actual &&
     ++		--clear-decorations >actual &&
      +	test_cmp expect.all actual &&
      +
      +	cat >expect.filtered <<-\EOF &&
     @@ t/t4202-log.sh: test_expect_success 'decorate-refs focus from default' '
      +		--simplify-by-decoration \
      +		--decorate-refs-exclude="heads/octopus" \
      +		--decorate-refs="heads" \
     -+		--decorate-all \
     ++		--clear-decorations \
      +		--decorate-refs-exclude="tags/" \
      +		--decorate-refs="heads/octopus*" >actual &&
      +	test_cmp expect.filtered actual
  8:  8142b32f023 !  9:  7ec2578b90a log: create log.decorateFilter=all
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    log: create log.decorateFilter=all
     +    log: create log.initialDecorationSet=all
      
     -    The previous change introduced the --decorate-all option for users who
     -    do not want their decorations limited to a narrow set of ref namespaces.
     +    The previous change introduced the --clear-decorations 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.
     +    Add a config option that is equivalent to specifying --clear-decorations
     +    by default.
      
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
     @@ Documentation/config/log.txt: log.decorate::
       	names are shown. This is the same as the `--decorate` option
       	of the `git log`.
       
     -+log.decorateFilter::
     ++log.initialDecorationSet::
      +	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'.
     ++	namespaces. If 'all' is specified, then show all refs as
     ++	decorations.
      +
       log.excludeDecoration::
       	Exclude the specified patterns from the log decorations. This is
     @@ Documentation/config/log.txt: log.decorate::
      
       ## Documentation/git-log.txt ##
      @@ Documentation/git-log.txt: used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
     - --decorate-all::
     + --clear-decorations::
       	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`.
     +-	decoration filter to include all references.
     ++	decoration filter to include all references. This option is
     ++	assumed if the config value `log.initialDecorationSet` is set to
     ++	`all`.
       
       --source::
       	Print out the ref name given on the command line by which each
     @@ builtin/log.c: static void set_default_decoration_filter(struct decoration_filte
       
      +	/*
      +	 * 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.
     ++	 * log.initialDecorationSet=all. Don't ever disable it by config,
     ++	 * since the command-line takes precedent.
      +	 */
     -+	if (!decorate_all &&
     -+	    !git_config_get_string("log.decoratefilter", &value) &&
     ++	if (use_default_decoration_filter &&
     ++	    !git_config_get_string("log.initialdecorationset", &value) &&
      +	    !strcmp("all", value))
     -+		decorate_all = 1;
     ++		use_default_decoration_filter = 0;
      +	free(value);
      +
     - 	if (decorate_all ||
     + 	if (!use_default_decoration_filter ||
       	    decoration_filter->exclude_ref_pattern->nr ||
       	    decoration_filter->include_ref_pattern->nr ||
      
       ## t/t4202-log.sh ##
     -@@ t/t4202-log.sh: test_expect_success '--decorate-all overrides defaults' '
     +@@ t/t4202-log.sh: test_expect_success '--clear-decorations overrides defaults' '
       	EOF
       	git log --decorate=full --pretty="tformat:%f%d" \
     - 		--decorate-all >actual &&
     + 		--clear-decorations >actual &&
      +	test_cmp expect.all actual &&
     -+	git -c log.decorateFilter=all log \
     ++	git -c log.initialDecorationSet=all log \
      +		--decorate=full --pretty="tformat:%f%d" >actual &&
       	test_cmp expect.all actual
       '
       
     -
     - ## t/t9902-completion.sh ##
     -@@ t/t9902-completion.sh: 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
     - '
     -@@ t/t9902-completion.sh: 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
     - '
     -@@ t/t9902-completion.sh: 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
     - '
  9:  318269dfe27 = 10:  1f4d9bc4b3f maintenance: stop writing log.excludeDecoration
 10:  8599bb55045 ! 11:  bd2ffac88ac fetch: use ref_namespaces during prefetch
     @@ builtin/fetch.c: static void filter_prefetch_refspec(struct refspec *rs)
       		    (rs->items[i].src &&
      -		     !strncmp(rs->items[i].src, "refs/tags/", 10))) {
      +		     !strncmp(rs->items[i].src,
     -+			      ref_namespaces[NAMESPACE_TAGS].ref,
     -+			      strlen(ref_namespaces[NAMESPACE_TAGS].ref)))) {
     ++			      ref_namespace[NAMESPACE_TAGS].ref,
     ++			      strlen(ref_namespace[NAMESPACE_TAGS].ref)))) {
       			int j;
       
       			free(rs->items[i].src);
     @@ builtin/fetch.c: static void filter_prefetch_refspec(struct refspec *rs)
       
       		old_dst = rs->items[i].dst;
      -		strbuf_addstr(&new_dst, "refs/prefetch/");
     -+		strbuf_addstr(&new_dst, ref_namespaces[NAMESPACE_PREFETCH].ref);
     ++		strbuf_addstr(&new_dst, ref_namespace[NAMESPACE_PREFETCH].ref);
       
       		/*
       		 * If old_dst starts with "refs/", then place

-- 
gitgitgadget

[PATCH v3 01/11] refs: allow "HEAD" as decoration filter

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The normalize_glob_ref() method was introduced in 65516f586b693 (log:
add option to choose which refs to decorate, 2017-11-21) to help with
decoration filters such as --decorate-refs=<filter> and
--decorate-refs-exclude=<filter>. The method has not been used anywhere
else.

At the moment, it is impossible to specify HEAD as a decoration filter
since normalize_glob_ref() prepends "refs/" to the filter if it isn't
already there.

Allow adding HEAD as a decoration filter by allowing the exact string
"HEAD" to not be prepended with "refs/". Add a test in t4202-log.sh that
would previously fail since the HEAD decoration would exist in the
output.

It is sufficient to only cover "HEAD" here and not include other special
refs like REBASE_HEAD. This is because HEAD is the only ref outside of
refs/* that is added to the list of decorations. However, we may want to
special-case these other refs in normalize_glob_ref() in the future.
Leave a NEEDSWORK comment for now.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 refs.c         | 11 ++++++++---
 t/t4202-log.sh |  6 ++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 90bcb271687..3fdfa86a5b9 100644
--- a/refs.c
+++ b/refs.c
@@ -455,11 +455,16 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
 	if (*pattern == '/')
 		BUG("pattern must not start with '/'");
 
-	if (prefix) {
+	if (prefix)
 		strbuf_addstr(&normalized_pattern, prefix);
-	}
-	else if (!starts_with(pattern, "refs/"))
+	else if (!starts_with(pattern, "refs/") &&
+		   strcmp(pattern, "HEAD"))
 		strbuf_addstr(&normalized_pattern, "refs/");
+	/*
+	 * NEEDSWORK: Special case other symrefs such as REBASE_HEAD,
+	 * MERGE_HEAD, etc.
+	 */
+
 	strbuf_addstr(&normalized_pattern, pattern);
 	strbuf_strip_suffix(&normalized_pattern, "/");
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6e663525582..6b7d8e269f7 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1025,6 +1025,12 @@ test_expect_success 'decorate-refs and simplify-by-decoration without output' '
 	test_cmp expect actual
 '
 
+test_expect_success 'decorate-refs-exclude HEAD' '
+	git log --decorate=full --oneline \
+		--decorate-refs-exclude="HEAD" >actual &&
+	! grep HEAD actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
-- 
gitgitgadget

[PATCH v3 02/11] t4207: modernize test

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

Before adding new tests to t4207-log-decoration-colors.sh, update the
existing test to use modern test conventions. This includes:

1. Use lowercase in test names.

2. Keep all test setup inside the test_expect_success blocks. We need
   to be careful about left whitespace in the broken lines of the input
   file.

3. Do not use 'git' commands on the left side of a pipe.

4. Create a cmp_filtered_decorations helper to perform the 'log', 'sed',
   and test_decode_color manipulations. Move the '--all' option to be an
   argument so we can change that value in future tests.

5. Modify the 'sed' command to use a simpler form that is more
   portable.

The next change will introduce new tests usinge these new conventions.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t4207-log-decoration-colors.sh | 37 +++++++++++++++++---------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 36ac6aff1e4..324412d0839 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2010 Nazri Ramliy
 #
 
-test_description='Test for "git log --decorate" colors'
+test_description='test "git log --decorate" colors'
 
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
@@ -42,25 +42,28 @@ test_expect_success setup '
 	git stash save Changes to A.t
 '
 
-cat >expected <<EOF
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
- ${c_reset}${c_branch}main${c_reset}${c_commit},\
- ${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
- ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit},\
- ${c_reset}${c_remoteBranch}other/main${c_reset}${c_commit})${c_reset} A1
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\
- On main: Changes to A.t
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
-EOF
+cmp_filtered_decorations () {
+	sed "s/$OID_REGEX/COMMIT_ID/" actual | test_decode_color >filtered &&
+	test_cmp expect filtered
+}
 
 # We want log to show all, but the second parent to refs/stash is irrelevant
 # to this test since it does not contain any decoration, hence --first-parent
-test_expect_success 'Commit Decorations Colored Correctly' '
-	git log --first-parent --abbrev=10 --all --decorate --oneline --color=always |
-	sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
-	test_decode_color >out &&
-	test_cmp expected out
+test_expect_success 'commit decorations colored correctly' '
+	cat >expect <<-EOF &&
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \
+${c_reset}${c_branch}main${c_reset}${c_commit}, \
+${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit}, \
+${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit}, \
+${c_reset}${c_remoteBranch}other/main${c_reset}${c_commit})${c_reset} A1
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset} \
+On main: Changes to A.t
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+	EOF
+
+	git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
+	cmp_filtered_decorations
 '
 
 test_done
-- 
gitgitgadget

[PATCH v3 03/11] t4207: test coloring of grafted decorations

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The color.decorate.<slot> config option added the 'grafted' slot in
09c4ba410b0f (log-tree: allow to customize 'grafted' color, 2018-05-26)
but included no tests for this behavior. When modifying some logic
around decorations, this ref namespace was ignored and could have been
lost as a default namespace for 'git log' decorations by default.

Add two tests to t4207 that check that the replaced objects are
correctly decorated. Use "black" as the color since it is distinct from
the other colors already in the test. The first test uses regular
replace-objects while the second creates a commit graft.

Be sure to test both modes with GIT_REPLACE_REF_BASE unset and set to an
alternative base.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t4207-log-decoration-colors.sh | 53 ++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 324412d0839..ded33a82e2c 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -17,6 +17,7 @@ test_expect_success setup '
 	git config color.decorate.remoteBranch red &&
 	git config color.decorate.tag "reverse bold yellow" &&
 	git config color.decorate.stash magenta &&
+	git config color.decorate.grafted black &&
 	git config color.decorate.HEAD cyan &&
 
 	c_reset="<RESET>" &&
@@ -27,6 +28,7 @@ test_expect_success setup '
 	c_tag="<BOLD;REVERSE;YELLOW>" &&
 	c_stash="<MAGENTA>" &&
 	c_HEAD="<CYAN>" &&
+	c_grafted="<BLACK>" &&
 
 	test_commit A &&
 	git clone . other &&
@@ -66,4 +68,55 @@ On main: Changes to A.t
 	cmp_filtered_decorations
 '
 
+test_expect_success 'test coloring with replace-objects' '
+	test_when_finished rm -rf .git/refs/replace* &&
+	test_commit C &&
+	test_commit D &&
+
+	git replace HEAD~1 HEAD~2 &&
+
+	cat >expect <<-EOF &&
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \
+${c_reset}${c_branch}main${c_reset}${c_commit}, \
+${c_reset}${c_tag}tag: D${c_reset}${c_commit})${c_reset} D
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: C${c_reset}${c_commit}, \
+${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+EOF
+
+	git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
+	cmp_filtered_decorations &&
+	git replace -d HEAD~1 &&
+
+	GIT_REPLACE_REF_BASE=refs/replace2/ git replace HEAD~1 HEAD~2 &&
+	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent \
+		--no-abbrev --decorate --oneline --color=always HEAD >actual &&
+	cmp_filtered_decorations
+'
+
+test_expect_success 'test coloring with grafted commit' '
+	test_when_finished rm -rf .git/refs/replace* &&
+
+	git replace --graft HEAD HEAD~2 &&
+
+	cat >expect <<-EOF &&
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \
+${c_reset}${c_branch}main${c_reset}${c_commit}, \
+${c_reset}${c_tag}tag: D${c_reset}${c_commit}, \
+${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit}, \
+${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
+	${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+	EOF
+
+	git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
+	cmp_filtered_decorations &&
+	git replace -d HEAD &&
+
+	GIT_REPLACE_REF_BASE=refs/replace2/ git replace --graft HEAD HEAD~2 &&
+	GIT_REPLACE_REF_BASE=refs/replace2/ git log --first-parent \
+		--no-abbrev --decorate --oneline --color=always HEAD >actual &&
+	cmp_filtered_decorations
+'
+
 test_done
-- 
gitgitgadget

[PATCH v3 04/11] refs: add array of ref namespaces

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

Git interprets different meanings to different refs based on their
names. Some meanings are cosmetic, like how refs in  'refs/remotes/*'
are colored differently from refs in 'refs/heads/*'. Others are more
critical, such as how replace refs are interpreted.

Before making behavior changes based on ref namespaces, collect all
known ref namespaces into a array of ref_namespace_info structs. This
array is indexed by the new ref_namespace enum for quick access.

As of this change, this array is purely documentation. Future changes
will add dependencies on this array.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 environment.c |  2 ++
 notes.c       |  1 +
 refs.c        | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
 refs.h        | 46 ++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+)

diff --git a/environment.c b/environment.c
index b3296ce7d15..9eb22f975c7 100644
--- a/environment.c
+++ b/environment.c
@@ -185,6 +185,8 @@ void setup_git_env(const char *git_dir)
 	free(git_replace_ref_base);
 	git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
 							  : "refs/replace/");
+	update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
+
 	free(git_namespace);
 	git_namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
 	shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
diff --git a/notes.c b/notes.c
index 7452e71cc8d..7bade6d8f69 100644
--- a/notes.c
+++ b/notes.c
@@ -1005,6 +1005,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
 
 	if (!notes_ref)
 		notes_ref = default_notes_ref();
+	update_ref_namespace(NAMESPACE_NOTES, xstrdup(notes_ref));
 
 	if (!combine_notes)
 		combine_notes = combine_notes_concatenate;
diff --git a/refs.c b/refs.c
index 3fdfa86a5b9..65decf25d09 100644
--- a/refs.c
+++ b/refs.c
@@ -20,6 +20,7 @@
 #include "repository.h"
 #include "sigchain.h"
 #include "date.h"
+#include "commit.h"
 
 /*
  * List of all available backends
@@ -56,6 +57,88 @@ static unsigned char refname_disposition[256] = {
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
 };
 
+struct ref_namespace_info ref_namespace[] = {
+	[NAMESPACE_HEAD] = {
+		.ref = "HEAD",
+		.decoration = DECORATION_REF_HEAD,
+		.exact = 1,
+	},
+	[NAMESPACE_BRANCHES] = {
+		.ref = "refs/heads/",
+		.decoration = DECORATION_REF_LOCAL,
+	},
+	[NAMESPACE_TAGS] = {
+		.ref = "refs/tags/",
+		.decoration = DECORATION_REF_TAG,
+	},
+	[NAMESPACE_REMOTE_REFS] = {
+		/*
+		 * The default refspec for new remotes copies refs from
+		 * refs/heads/ on the remote into refs/remotes/<remote>/.
+		 * As such, "refs/remotes/" has special handling.
+		 */
+		.ref = "refs/remotes/",
+		.decoration = DECORATION_REF_REMOTE,
+	},
+	[NAMESPACE_STASH] = {
+		/*
+		 * The single ref "refs/stash" stores the latest stash.
+		 * Older stashes can be found in the reflog.
+		 */
+		.ref = "refs/stash",
+		.exact = 1,
+		.decoration = DECORATION_REF_STASH,
+	},
+	[NAMESPACE_REPLACE] = {
+		/*
+		 * This namespace allows Git to act as if one object ID
+		 * points to the content of another. Unlike the other
+		 * ref namespaces, this one can be changed by the
+		 * GIT_REPLACE_REF_BASE environment variable. This
+		 * .namespace value will be overwritten in setup_git_env().
+		 */
+		.ref = "refs/replace/",
+		.decoration = DECORATION_GRAFTED,
+	},
+	[NAMESPACE_NOTES] = {
+		/*
+		 * The refs/notes/commit ref points to the tip of a
+		 * parallel commit history that adds metadata to commits
+		 * in the normal history. This ref can be overwritten
+		 * by the core.notesRef config variable or the
+		 * GIT_NOTES_REFS environment variable.
+		 */
+		.ref = "refs/notes/commit",
+		.exact = 1,
+	},
+	[NAMESPACE_PREFETCH] = {
+		/*
+		 * Prefetch refs are written by the background 'fetch'
+		 * maintenance task. It allows faster foreground fetches
+		 * by advertising these previously-downloaded tips without
+		 * updating refs/remotes/ without user intervention.
+		 */
+		.ref = "refs/prefetch/",
+	},
+	[NAMESPACE_REWRITTEN] = {
+		/*
+		 * Rewritten refs are used by the 'label' command in the
+		 * sequencer. These are particularly useful during an
+		 * interactive rebase that uses the 'merge' command.
+		 */
+		.ref = "refs/rewritten/",
+	},
+};
+
+void update_ref_namespace(enum ref_namespace namespace, char *ref)
+{
+	struct ref_namespace_info *info = &ref_namespace[namespace];
+	if (info->ref_updated)
+		free(info->ref);
+	info->ref = ref;
+	info->ref_updated = 1;
+}
+
 /*
  * Try to read one refname component from the front of refname.
  * Return the length of the component found, or -1 if the component is
diff --git a/refs.h b/refs.h
index 47cb9edbaa8..d6575b8c2bd 100644
--- a/refs.h
+++ b/refs.h
@@ -2,6 +2,7 @@
 #define REFS_H
 
 #include "cache.h"
+#include "commit.h"
 
 struct object_id;
 struct ref_store;
@@ -930,4 +931,49 @@ struct ref_store *get_main_ref_store(struct repository *r);
 struct ref_store *get_submodule_ref_store(const char *submodule);
 struct ref_store *get_worktree_ref_store(const struct worktree *wt);
 
+/*
+ * Some of the names specified by refs have special meaning to Git.
+ * Organize these namespaces in a comon 'ref_namespace' array for
+ * reference from multiple places in the codebase.
+ */
+
+struct ref_namespace_info {
+	char *ref;
+	enum decoration_type decoration;
+
+	/*
+	 * If 'exact' is true, then we must match the 'ref' exactly.
+	 * Otherwise, use a prefix match.
+	 *
+	 * 'ref_updated' is for internal use. It represents whether the
+	 * 'ref' value was replaced from its original literal version.
+	 */
+	unsigned exact:1,
+		 ref_updated:1;
+};
+
+enum ref_namespace {
+	NAMESPACE_HEAD,
+	NAMESPACE_BRANCHES,
+	NAMESPACE_TAGS,
+	NAMESPACE_REMOTE_REFS,
+	NAMESPACE_STASH,
+	NAMESPACE_REPLACE,
+	NAMESPACE_NOTES,
+	NAMESPACE_PREFETCH,
+	NAMESPACE_REWRITTEN,
+
+	/* Must be last */
+	NAMESPACE__COUNT
+};
+
+/* See refs.c for the contents of this array. */
+extern struct ref_namespace_info ref_namespace[NAMESPACE__COUNT];
+
+/*
+ * Some ref namespaces can be modified by config values or environment
+ * variables. Modify a namespace as specified by its ref_namespace key.
+ */
+void update_ref_namespace(enum ref_namespace namespace, char *ref);
+
 #endif /* REFS_H */
-- 
gitgitgadget

[PATCH v3 05/11] refs: use ref_namespaces for replace refs base

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The git_replace_ref_base global is used to store the value of the
GIT_REPLACE_REF_BASE environment variable or the default of
"refs/replace/". This is initialized within setup_git_env().

The ref_namespaces array is a new centralized location for information
such as the ref namespace used for replace refs. Instead of having this
namespace stored in two places, use the ref_namespaces array instead.

For simplicity, create a local git_replace_ref_base variable wherever
the global was previously used.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 builtin/replace.c | 3 +++
 cache.h           | 1 -
 environment.c     | 3 +--
 log-tree.c        | 1 +
 refs.c            | 1 +
 5 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 583702a0980..a29e911d309 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -106,6 +106,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
 	size_t base_len;
 	int had_error = 0;
 	struct object_id oid;
+	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
 
 	strbuf_addstr(&ref, git_replace_ref_base);
 	base_len = ref.len;
@@ -147,6 +148,8 @@ static int check_ref_valid(struct object_id *object,
 			    struct strbuf *ref,
 			    int force)
 {
+	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
+
 	strbuf_reset(ref);
 	strbuf_addf(ref, "%s%s", git_replace_ref_base, oid_to_hex(object));
 	if (check_refname_format(ref->buf, 0))
diff --git a/cache.h b/cache.h
index ac5ab4ef9d3..631a4f388d6 100644
--- a/cache.h
+++ b/cache.h
@@ -1008,7 +1008,6 @@ void reset_shared_repository(void);
  * commands that do not want replace references to be active.
  */
 extern int read_replace_refs;
-extern char *git_replace_ref_base;
 
 /*
  * These values are used to help identify parts of a repository to fsync.
diff --git a/environment.c b/environment.c
index 9eb22f975c7..b2004437dce 100644
--- a/environment.c
+++ b/environment.c
@@ -56,7 +56,6 @@ const char *askpass_program;
 const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 int read_replace_refs = 1;
-char *git_replace_ref_base;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
 char *check_roundtrip_encoding = "SHIFT-JIS";
@@ -162,6 +161,7 @@ const char *getenv_safe(struct strvec *argv, const char *name)
 
 void setup_git_env(const char *git_dir)
 {
+	char *git_replace_ref_base;
 	const char *shallow_file;
 	const char *replace_ref_base;
 	struct set_gitdir_args args = { NULL };
@@ -182,7 +182,6 @@ void setup_git_env(const char *git_dir)
 	if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
 		read_replace_refs = 0;
 	replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
-	free(git_replace_ref_base);
 	git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
 							  : "refs/replace/");
 	update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
diff --git a/log-tree.c b/log-tree.c
index d0ac0a6327a..1b2c76c5bb9 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -141,6 +141,7 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
 	enum object_type objtype;
 	enum decoration_type deco_type = DECORATION_NONE;
 	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
+	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
 
 	if (filter && !ref_filter_match(refname, filter))
 		return 0;
diff --git a/refs.c b/refs.c
index 65decf25d09..92819732ab7 100644
--- a/refs.c
+++ b/refs.c
@@ -1612,6 +1612,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)
 {
+	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
 	return do_for_each_repo_ref(r, git_replace_ref_base, fn,
 				    strlen(git_replace_ref_base),
 				    DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
-- 
gitgitgadget

[PATCH v3 06/11] log-tree: use ref_namespaces instead of if/else-if

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The add_ref_decoration() method uses an if/else-if chain to determine if
a ref matches a known ref namespace that has a special decoration
category. That decoration type is later used to assign a color when
writing to stdout.

The newly-added ref_namespaces array contains all namespaces, along
with information about their decoration type. Check this array instead
of this if/else-if chain. This reduces our dependency on string literals
being embedded in the decoration logic.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 log-tree.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 1b2c76c5bb9..bb6cbceee63 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -137,6 +137,7 @@ static int ref_filter_match(const char *refname,
 static int add_ref_decoration(const char *refname, const struct object_id *oid,
 			      int flags, void *cb_data)
 {
+	int i;
 	struct object *obj;
 	enum object_type objtype;
 	enum decoration_type deco_type = DECORATION_NONE;
@@ -166,16 +167,21 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
 		return 0;
 	obj = lookup_object_by_type(the_repository, oid, objtype);
 
-	if (starts_with(refname, "refs/heads/"))
-		deco_type = DECORATION_REF_LOCAL;
-	else if (starts_with(refname, "refs/remotes/"))
-		deco_type = DECORATION_REF_REMOTE;
-	else if (starts_with(refname, "refs/tags/"))
-		deco_type = DECORATION_REF_TAG;
-	else if (!strcmp(refname, "refs/stash"))
-		deco_type = DECORATION_REF_STASH;
-	else if (!strcmp(refname, "HEAD"))
-		deco_type = DECORATION_REF_HEAD;
+	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) {
+		struct ref_namespace_info *info = &ref_namespace[i];
+
+		if (!info->decoration)
+			continue;
+		if (info->exact) {
+			if (!strcmp(refname, info->ref)) {
+				deco_type = info->decoration;
+				break;
+			}
+		} else if (starts_with(refname, info->ref)) {
+			deco_type = info->decoration;
+			break;
+		}
+	}
 
 	add_name_decoration(deco_type, refname, obj);
 	while (obj->type == OBJ_TAG) {
-- 
gitgitgadget

[PATCH v3 07/11] log: add default decoration filter

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

When a user runs 'git log', they expect a certain set of helpful
decorations. This includes:

* The HEAD ref
* Branches (refs/heads/)
* Stashes (refs/stash)
* Tags (refs/tags/)
* Remote branches (refs/remotes/)
* Replace refs (refs/replace/ or $GIT_REPLACE_REF_BASE)

Each of these namespaces was selected due to existing test cases that
verify these namespaces appear in the decorations. In particular,
stashes and replace refs can have custom colors from the
color.decorate.<slot> config option.

While one test checks for a decoration from notes, it only applies to
the tip of refs/notes/commit (or its configured ref name). Notes form
their own kind of decoration instead. Modify the expected output for the
tests in t4013 that expect this note decoration.  There are several
tests throughout the codebase that verify that --decorate-refs,
--decorate-refs-exclude, and log.excludeDecoration work as designed and
the tests continue to pass without intervention.

However, there are other refs that are less helpful to show as
decoration:

* Prefetch refs (refs/prefetch/)
* Rebase refs (refs/rebase-merge/ and refs/rebase-apply/)
* Bundle refs (refs/bundle/) [!]

[!] The bundle refs are part of a parallel series that bootstraps a repo
    from a bundle file, storing the bundle's refs into the repo's
    refs/bundle/ namespace.

In the case of prefetch refs, 96eaffebbf3d0 (maintenance: set
log.excludeDecoration durin prefetch, 2021-01-19) added logic to add
refs/prefetch/ to the log.excludeDecoration config option. Additional
feedback pointed out that having such a side-effect can be confusing and
perhaps not helpful to users. Instead, we should hide these ref
namespaces that are being used by Git for internal reasons but are not
helpful for the users to see.

The way to provide a seamless user experience without setting the config
is to modify the default decoration filters to match our expectation of
what refs the user actually wants to see.

In builtin/log.c, after parsing the --decorate-refs and
--decorate-refs-exclude options from the command-line, call
set_default_decoration_filter(). This method populates the exclusions
from log.excludeDecoration, then checks if the list of pattern
modifications are empty. If none are specified, then the default set is
restricted to the set of inclusions mentioned earlier (HEAD, branches,
etc.).  A previous change introduced the ref_namespaces array, which
includes all of these currently-used namespaces. The 'decoration' value
is non-zero when that namespace is associated with a special coloring
and fits into the list of "expected" decorations as described above,
which makes the implementation of this filter very simple.

Note that the logic in ref_filter_match() in log-tree.c follows this
matching pattern:

 1. If there are exclusion patterns and the ref matches one, then ignore
    the decoration.

 2. If there are inclusion patterns and the ref matches one, then
    definitely include the decoration.

 3. If there are config-based exclusions from log.excludeDecoration and
    the ref matches one, then ignore the decoration.

With this logic in mind, we need to ensure that we do not populate our
new defaults if any of these filters are manually set. Specifically, if
a user runs

	git -c log.excludeDecoration=HEAD log

then we expect the HEAD decoration to not appear. If we left the default
inclusions in the set, then HEAD would match that inclusion before
reaching the config-based exclusions.

A potential alternative would be to check the list of default inclusions
at the end, after the config-based exclusions. This would still create a
behavior change for some uses of --decorate-refs-exclude=<X>, and could
be overwritten somewhat with --decorate-refs=refs/ and
--decorate-refs=HEAD. However, it no longer becomes possible to include
refs outside of the defaults while also excluding some using
log.excludeDecoration.

Another alternative would be to exclude the known namespaces that are
not intended to be shown. This would reduce the visible effect of the
change for expert users who use their own custom ref namespaces. The
implementation change would be very simple to swap due to our use of
ref_namespaces:

	int i;
	struct string_list *exclude = decoration_filter->exclude_ref_pattern;

	/*
	 * No command-line or config options were given, so
	 * populate with sensible defaults.
	 */
	for (i = 0; i < NAMESPACE__COUNT; i++) {
		if (ref_namespaces[i].decoration)
			continue;

		string_list_append(exclude, ref_namespaces[i].ref);
	}

The main downside of this approach is that we expect to add new hidden
namespaces in the future, and that means that Git versions will be less
stable in how they behave as those namespaces are added.

It is critical that we provide ways for expert users to disable this
behavior change via command-line options and config keys. These changes
will be implemented in a future change.

Add a test that checks that the defaults are not added when
--decorate-refs is specified. We verify this by showing that HEAD is not
included as it normally would.  Also add a test that shows that the
default filter avoids the unwanted decorations from refs/prefetch,
refs/rebase-merge,
and refs/bundle.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-log.txt              |  7 ++--
 builtin/log.c                          | 50 +++++++++++++++++++-------
 t/t4013/diff.log_--decorate=full_--all |  2 +-
 t/t4013/diff.log_--decorate_--all      |  2 +-
 t/t4202-log.sh                         | 20 +++++++++++
 5 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 20e87cecf49..b2ac89dfafc 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -45,13 +45,16 @@ OPTIONS
 
 --decorate-refs=<pattern>::
 --decorate-refs-exclude=<pattern>::
-	If no `--decorate-refs` is given, pretend as if all refs were
-	included.  For each candidate, do not use it for decoration if it
+	For each candidate reference, do not use it for decoration if it
 	matches any patterns given to `--decorate-refs-exclude` or if it
 	doesn't match any of the patterns given to `--decorate-refs`. The
 	`log.excludeDecoration` config option allows excluding refs from
 	the decorations, but an explicit `--decorate-refs` pattern will
 	override a match in `log.excludeDecoration`.
++
+If none of these options or config settings are given, then references are
+used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
+`refs/stash/`, or `refs/tags/`.
 
 --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 88a5e98875a..6683e5ff134 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -162,6 +162,37 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 		parse_date_format(default_date_mode, &rev->date_mode);
 }
 
+static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
+{
+	int i;
+	struct string_list *include = decoration_filter->include_ref_pattern;
+	const struct string_list *config_exclude =
+			git_config_get_value_multi("log.excludeDecoration");
+
+	if (config_exclude) {
+		struct string_list_item *item;
+		for_each_string_list_item(item, config_exclude)
+			string_list_append(decoration_filter->exclude_ref_config_pattern,
+					   item->string);
+	}
+
+	if (decoration_filter->exclude_ref_pattern->nr ||
+	    decoration_filter->include_ref_pattern->nr ||
+	    decoration_filter->exclude_ref_config_pattern->nr)
+		return;
+
+	/*
+	 * No command-line or config options were given, so
+	 * populate with sensible defaults.
+	 */
+	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) {
+		if (!ref_namespace[i].decoration)
+			continue;
+
+		string_list_append(include, ref_namespace[i].ref);
+	}
+}
+
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
@@ -171,9 +202,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
 	static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
 	static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
-	struct decoration_filter decoration_filter = {&decorate_refs_include,
-						      &decorate_refs_exclude,
-						      &decorate_refs_exclude_config};
+	struct decoration_filter decoration_filter = {
+		.exclude_ref_pattern = &decorate_refs_exclude,
+		.include_ref_pattern = &decorate_refs_include,
+		.exclude_ref_config_pattern = &decorate_refs_exclude_config,
+	};
 	static struct revision_sources revision_sources;
 
 	const struct option builtin_log_options[] = {
@@ -265,16 +298,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	}
 
 	if (decoration_style || rev->simplify_by_decoration) {
-		const struct string_list *config_exclude =
-			repo_config_get_value_multi(the_repository,
-						    "log.excludeDecoration");
-
-		if (config_exclude) {
-			struct string_list_item *item;
-			for_each_string_list_item(item, config_exclude)
-				string_list_append(&decorate_refs_exclude_config,
-						   item->string);
-		}
+		set_default_decoration_filter(&decoration_filter);
 
 		if (decoration_style)
 			rev->show_decorations = 1;
diff --git a/t/t4013/diff.log_--decorate=full_--all b/t/t4013/diff.log_--decorate=full_--all
index 3f9b872eceb..6b0b334a5d6 100644
--- a/t/t4013/diff.log_--decorate=full_--all
+++ b/t/t4013/diff.log_--decorate=full_--all
@@ -20,7 +20,7 @@ Date:   Mon Jun 26 00:06:00 2006 +0000
 
     Rearranged lines in dir/sub
 
-commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:06:00 2006 +0000
 
diff --git a/t/t4013/diff.log_--decorate_--all b/t/t4013/diff.log_--decorate_--all
index f5e20e1e14a..c7df1f58141 100644
--- a/t/t4013/diff.log_--decorate_--all
+++ b/t/t4013/diff.log_--decorate_--all
@@ -20,7 +20,7 @@ Date:   Mon Jun 26 00:06:00 2006 +0000
 
     Rearranged lines in dir/sub
 
-commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:06:00 2006 +0000
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6b7d8e269f7..4c3ce863074 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1031,6 +1031,12 @@ test_expect_success 'decorate-refs-exclude HEAD' '
 	! grep HEAD actual
 '
 
+test_expect_success 'decorate-refs focus from default' '
+	git log --decorate=full --oneline \
+		--decorate-refs="refs/heads" >actual &&
+	! grep HEAD actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
@@ -2198,6 +2204,20 @@ test_expect_success 'log --decorate includes all levels of tag annotated tags' '
 	test_cmp expect actual
 '
 
+test_expect_success 'log --decorate does not include things outside filter' '
+	reflist="refs/prefetch refs/rebase-merge refs/bundle" &&
+
+	for ref in $reflist
+	do
+		git update-ref $ref/fake HEAD || return 1
+	done &&
+
+	git log --decorate=full --oneline >actual &&
+
+	# None of the refs are visible:
+	! grep /fake actual
+'
+
 test_expect_success 'log --end-of-options' '
        git update-ref refs/heads/--source HEAD &&
        git log --end-of-options --source >actual &&
-- 
gitgitgadget

Re: [PATCH v3 07/11] log: add default decoration filter

[Glen Choo]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> When a user runs 'git log', they expect a certain set of helpful
> decorations. This includes:
>
> * The HEAD ref
> * Branches (refs/heads/)
> * Stashes (refs/stash)
> * Tags (refs/tags/)
> * Remote branches (refs/remotes/)
> * Replace refs (refs/replace/ or $GIT_REPLACE_REF_BASE)
>
> Each of these namespaces was selected due to existing test cases that
> verify these namespaces appear in the decorations. In particular,
> stashes and replace refs can have custom colors from the
> color.decorate.<slot> config option.

I _just_ noticed that refs/bisect/* isn't part of this list, but I'd
presume that users want to see those decorations (or I do, at least).
Was that an intentional omission?

Re: [PATCH v3 07/11] log: add default decoration filter

[Derrick Stolee]

On 9/8/2022 5:13 PM, Glen Choo wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>>
>> When a user runs 'git log', they expect a certain set of helpful
>> decorations. This includes:
>>
>> * The HEAD ref
>> * Branches (refs/heads/)
>> * Stashes (refs/stash)
>> * Tags (refs/tags/)
>> * Remote branches (refs/remotes/)
>> * Replace refs (refs/replace/ or $GIT_REPLACE_REF_BASE)
>>
>> Each of these namespaces was selected due to existing test cases that
>> verify these namespaces appear in the decorations. In particular,
>> stashes and replace refs can have custom colors from the
>> color.decorate.<slot> config option.
> 
> I _just_ noticed that refs/bisect/* isn't part of this list, but I'd
> presume that users want to see those decorations (or I do, at least).
> Was that an intentional omission?

It was an intentional omission because the refs/bisect/* references
are not part of the color.decorate.<slot> category.

Looking into it further, the bisect refs look pretty ugly (especially
the ones like "refs/bisect/good-<hash>").

If you would like to include these in the default filter, then I
would recommend also adding a color.decorate.<slot> category for them
and possibly replace the "refs/bisect" with just "bisect". Alternatively,
you could take a hint from replace objects and just use an indicator
like "bisect good" or "bisect bad" instead of listing the full ref name.

Thanks,
-Stolee

Re: [PATCH v3 07/11] log: add default decoration filter

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

> It was an intentional omission because the refs/bisect/* references
> are not part of the color.decorate.<slot> category.
>
> Looking into it further, the bisect refs look pretty ugly (especially
> the ones like "refs/bisect/good-<hash>").
>
> If you would like to include these in the default filter, then I
> would recommend also adding a color.decorate.<slot> category for them
> and possibly replace the "refs/bisect" with just "bisect". Alternatively,
> you could take a hint from replace objects and just use an indicator
> like "bisect good" or "bisect bad" instead of listing the full ref name.

I personally do not think bisect/bad and bisect/good-* need to be
part of the default set of refs to use for decoration.

I suspect that the suggestion to use them for decoration is based on
the gut feeling: "People during their bisect session would want to
know which points they already examined and what their states are."

But during bisection, there is a specific command to give them
exactly that information: "bisect visualize".  It is roughly
equilvalent to:

    git log refs/bisect/bad \
	$(git for-each-ref --format='^%(refname)' refs/bisect/good-\*

i.e. show the history surrounded by all the known-to-be-good commits
and the known-to-be-bad commit we currently are chasing.  Bad and good
commits are at the boundary you can tell them without decoration.

But if we still want to see bisect/bad and bisect/good-* in a larger
graph (i.e. showing descendants of bad and ancestors of good), then
I do not think good-<object name> being a long label is something we
should special case and shorten.  Especially because the user is not
using "bisect visualize", which is readable without decoration, they
may be seeking more information in the names, perhaps cutting and
pasting the object names to feed "git show" running on a separate
terminal, or something.

Re: [PATCH v3 07/11] log: add default decoration filter

[Derrick Stolee]

On 9/9/2022 12:19 PM, Junio C Hamano wrote:

> But during bisection, there is a specific command to give them
> exactly that information: "bisect visualize".  It is roughly
> equilvalent to:
> 
>     git log refs/bisect/bad \
> 	$(git for-each-ref --format='^%(refname)' refs/bisect/good-\*
> 
> i.e. show the history surrounded by all the known-to-be-good commits
> and the known-to-be-bad commit we currently are chasing.  Bad and good
> commits are at the boundary you can tell them without decoration.

Thank you for recommending this as the expected behavior.
 
> But if we still want to see bisect/bad and bisect/good-* in a larger
> graph (i.e. showing descendants of bad and ancestors of good), then
> I do not think good-<object name> being a long label is something we
> should special case and shorten.  Especially because the user is not
> using "bisect visualize", which is readable without decoration, they
> may be seeking more information in the names, perhaps cutting and
> pasting the object names to feed "git show" running on a separate
> terminal, or something.

The object names should be visible as part of the "git log" output
(perhaps abbreviated) and that provides a way to get that information
without needing an unabbreviated hash showing up in a decoration slot.

Thanks,
-Stolee

Re: [PATCH v3 07/11] log: add default decoration filter

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

> The object names should be visible as part of the "git log" output
> (perhaps abbreviated) and that provides a way to get that information
> without needing an unabbreviated hash showing up in a decoration slot.

OK.

Re: [PATCH v3 07/11] log: add default decoration filter

[Glen Choo]

Junio C Hamano <gitster@pobox.com> writes:

> Derrick Stolee <derrickstolee@github.com> writes:
>
>> It was an intentional omission because the refs/bisect/* references
>> are not part of the color.decorate.<slot> category.
>>
>> Looking into it further, the bisect refs look pretty ugly (especially
>> the ones like "refs/bisect/good-<hash>").
>>
>> If you would like to include these in the default filter, then I
>> would recommend also adding a color.decorate.<slot> category for them
>> and possibly replace the "refs/bisect" with just "bisect". Alternatively,
>> you could take a hint from replace objects and just use an indicator
>> like "bisect good" or "bisect bad" instead of listing the full ref name.
>
> I suspect that the suggestion to use them for decoration is based on
> the gut feeling: "People during their bisect session would want to
> know which points they already examined and what their states are."
>
> But during bisection, there is a specific command to give them
> exactly that information: "bisect visualize".  It is roughly
> equilvalent to:
>
>     git log refs/bisect/bad \
> 	$(git for-each-ref --format='^%(refname)' refs/bisect/good-\*
>
> i.e. show the history surrounded by all the known-to-be-good commits
> and the known-to-be-bad commit we currently are chasing.  Bad and good
> commits are at the boundary you can tell them without decoration.

Thanks, both. I think "bisect visualize" covers the use case I was
thinking of (seeing the bisect result in the commit history), so
omitting refs/bisect/* sounds fine. And if users are unhappy about the
change, it seems simple enough to add it to the list.

[PATCH v3 08/11] log: add --clear-decorations option

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The previous changes introduced a new default ref filter for decorations
in the 'git log' command. This can be overridden using
--decorate-refs=HEAD and --decorate-refs=refs/, but that is cumbersome
for users.

Instead, add a --clear-decorations option that resets all previous
filters to a blank filter that accepts all refs.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-log.txt                     |   5 +
 builtin/log.c                                 |  24 +++-
 t/t4013-diff-various.sh                       |   2 +
 ..._--decorate=full_--clear-decorations_--all |  61 +++++++++++
 ...f.log_--decorate=full_--decorate-all_--all |  61 +++++++++++
 ...f.log_--decorate_--clear-decorations_--all |  61 +++++++++++
 .../diff.log_--decorate_--decorate-all_--all  |  61 +++++++++++
 t/t4202-log.sh                                | 103 +++++++++++++++++-
 8 files changed, 371 insertions(+), 7 deletions(-)
 create mode 100644 t/t4013/diff.log_--decorate=full_--clear-decorations_--all
 create mode 100644 t/t4013/diff.log_--decorate=full_--decorate-all_--all
 create mode 100644 t/t4013/diff.log_--decorate_--clear-decorations_--all
 create mode 100644 t/t4013/diff.log_--decorate_--decorate-all_--all

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index b2ac89dfafc..f2ce16fba71 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -56,6 +56,11 @@ If none of these options or config settings are given, then references are
 used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
 `refs/stash/`, or `refs/tags/`.
 
+--clear-decorations::
+	When specified, this option clears all previous `--decorate-refs`
+	or `--decorate-refs-exclude` options and relaxes the default
+	decoration filter to include all references.
+
 --source::
 	Print out the ref name given on the command line by which each
 	commit was reached.
diff --git a/builtin/log.c b/builtin/log.c
index 6683e5ff134..7d35d1ecab1 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -101,6 +101,20 @@ static int parse_decoration_style(const char *value)
 	return -1;
 }
 
+static int use_default_decoration_filter = 1;
+static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
+static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
+static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
+
+static int clear_decorations_callback(const struct option *opt,
+					    const char *arg, int unset)
+{
+	string_list_clear(&decorate_refs_include, 0);
+	string_list_clear(&decorate_refs_exclude, 0);
+	use_default_decoration_filter = 0;
+	return 0;
+}
+
 static int decorate_callback(const struct option *opt, const char *arg, int unset)
 {
 	if (unset)
@@ -176,7 +190,8 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
 					   item->string);
 	}
 
-	if (decoration_filter->exclude_ref_pattern->nr ||
+	if (!use_default_decoration_filter ||
+	    decoration_filter->exclude_ref_pattern->nr ||
 	    decoration_filter->include_ref_pattern->nr ||
 	    decoration_filter->exclude_ref_config_pattern->nr)
 		return;
@@ -199,9 +214,6 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	struct userformat_want w;
 	int quiet = 0, source = 0, mailmap;
 	static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};
-	static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
-	static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
-	static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
 	struct decoration_filter decoration_filter = {
 		.exclude_ref_pattern = &decorate_refs_exclude,
 		.include_ref_pattern = &decorate_refs_include,
@@ -214,6 +226,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		OPT_BOOL(0, "source", &source, N_("show source")),
 		OPT_BOOL(0, "use-mailmap", &mailmap, N_("use mail map file")),
 		OPT_ALIAS(0, "mailmap", "use-mailmap"),
+		OPT_CALLBACK_F(0, "clear-decorations", NULL, NULL,
+			       N_("clear all previously-defined decoration filters"),
+			       PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+			       clear_decorations_callback),
 		OPT_STRING_LIST(0, "decorate-refs", &decorate_refs_include,
 				N_("pattern"), N_("only decorate refs that match <pattern>")),
 		OPT_STRING_LIST(0, "decorate-refs-exclude", &decorate_refs_exclude,
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 056e922164d..dfcf3a0aaae 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -352,6 +352,8 @@ log -GF -p --pickaxe-all master
 log -IA -IB -I1 -I2 -p master
 log --decorate --all
 log --decorate=full --all
+log --decorate --clear-decorations --all
+log --decorate=full --clear-decorations --all
 
 rev-list --parents HEAD
 rev-list --children HEAD
diff --git a/t/t4013/diff.log_--decorate=full_--clear-decorations_--all b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
new file mode 100644
index 00000000000..1c030a6554e
--- /dev/null
+++ b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
@@ -0,0 +1,61 @@
+$ git log --decorate=full --clear-decorations --all
+commit b7e0bc69303b488b47deca799a7d723971dfa6cd (refs/heads/mode)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode
+
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (refs/heads/note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
+commit cd4e72fd96faed3f0ba949dc42967430374e2290 (refs/heads/rearrange)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Rearranged lines in dir/sub
+
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> refs/heads/master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
diff --git a/t/t4013/diff.log_--decorate=full_--decorate-all_--all b/t/t4013/diff.log_--decorate=full_--decorate-all_--all
new file mode 100644
index 00000000000..d6e79287846
--- /dev/null
+++ b/t/t4013/diff.log_--decorate=full_--decorate-all_--all
@@ -0,0 +1,61 @@
+$ git log --decorate=full --decorate-all --all
+commit b7e0bc69303b488b47deca799a7d723971dfa6cd (refs/heads/mode)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode
+
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (refs/heads/note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
+commit cd4e72fd96faed3f0ba949dc42967430374e2290 (refs/heads/rearrange)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Rearranged lines in dir/sub
+
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> refs/heads/master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
diff --git a/t/t4013/diff.log_--decorate_--clear-decorations_--all b/t/t4013/diff.log_--decorate_--clear-decorations_--all
new file mode 100644
index 00000000000..88be82cce31
--- /dev/null
+++ b/t/t4013/diff.log_--decorate_--clear-decorations_--all
@@ -0,0 +1,61 @@
+$ git log --decorate --clear-decorations --all
+commit b7e0bc69303b488b47deca799a7d723971dfa6cd (mode)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode
+
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
+commit cd4e72fd96faed3f0ba949dc42967430374e2290 (rearrange)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Rearranged lines in dir/sub
+
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
diff --git a/t/t4013/diff.log_--decorate_--decorate-all_--all b/t/t4013/diff.log_--decorate_--decorate-all_--all
new file mode 100644
index 00000000000..5d22618bb60
--- /dev/null
+++ b/t/t4013/diff.log_--decorate_--decorate-all_--all
@@ -0,0 +1,61 @@
+$ git log --decorate --decorate-all --all
+commit b7e0bc69303b488b47deca799a7d723971dfa6cd (mode)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode
+
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
+commit cd4e72fd96faed3f0ba949dc42967430374e2290 (rearrange)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Rearranged lines in dir/sub
+
+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> master)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (initial)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 4c3ce863074..90800feb0f1 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -704,9 +704,12 @@ test_expect_success 'set up more tangled history' '
 	git checkout -b tangle HEAD~6 &&
 	test_commit tangle-a tangle-a a &&
 	git merge main~3 &&
+	git update-ref refs/prefetch/merge HEAD &&
 	git merge side~1 &&
+	git update-ref refs/rewritten/merge HEAD &&
 	git checkout main &&
 	git merge tangle &&
+	git update-ref refs/hidden/tangle HEAD &&
 	git checkout -b reach &&
 	test_commit reach &&
 	git checkout main &&
@@ -974,9 +977,9 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '
 	Merge-tag-reach (HEAD -> main)
 	reach (tag: reach, reach)
 	seventh (tag: seventh)
-	Merge-branch-tangle
-	Merge-branch-side-early-part-into-tangle (tangle)
-	tangle-a (tag: tangle-a)
+	Merge-branch-tangle (refs/hidden/tangle)
+	Merge-branch-side-early-part-into-tangle (refs/rewritten/merge, tangle)
+	Merge-branch-main-early-part-into-tangle (refs/prefetch/merge)
 	EOF
 	git log -n6 --decorate=short --pretty="tformat:%f%d" \
 		--decorate-refs-exclude="*octopus*" \
@@ -1037,6 +1040,100 @@ test_expect_success 'decorate-refs focus from default' '
 	! grep HEAD actual
 '
 
+test_expect_success '--clear-decorations overrides defaults' '
+	cat >expect.default <<-\EOF &&
+	Merge-tag-reach (HEAD -> refs/heads/main)
+	Merge-tags-octopus-a-and-octopus-b
+	seventh (tag: refs/tags/seventh)
+	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
+	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
+	reach (tag: refs/tags/reach, refs/heads/reach)
+	Merge-branch-tangle
+	Merge-branch-side-early-part-into-tangle (refs/heads/tangle)
+	Merge-branch-main-early-part-into-tangle
+	tangle-a (tag: refs/tags/tangle-a)
+	Merge-branch-side
+	side-2 (tag: refs/tags/side-2, refs/heads/side)
+	side-1 (tag: refs/tags/side-1)
+	Second
+	sixth
+	fifth
+	fourth
+	third
+	second
+	initial
+	EOF
+	git log --decorate=full --pretty="tformat:%f%d" >actual &&
+	test_cmp expect.default actual &&
+
+	cat >expect.all <<-\EOF &&
+	Merge-tag-reach (HEAD -> refs/heads/main)
+	Merge-tags-octopus-a-and-octopus-b
+	seventh (tag: refs/tags/seventh)
+	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
+	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
+	reach (tag: refs/tags/reach, refs/heads/reach)
+	Merge-branch-tangle (refs/hidden/tangle)
+	Merge-branch-side-early-part-into-tangle (refs/rewritten/merge, refs/heads/tangle)
+	Merge-branch-main-early-part-into-tangle (refs/prefetch/merge)
+	tangle-a (tag: refs/tags/tangle-a)
+	Merge-branch-side
+	side-2 (tag: refs/tags/side-2, refs/heads/side)
+	side-1 (tag: refs/tags/side-1)
+	Second
+	sixth
+	fifth
+	fourth
+	third
+	second
+	initial
+	EOF
+	git log --decorate=full --pretty="tformat:%f%d" \
+		--clear-decorations >actual &&
+	test_cmp expect.all actual
+'
+
+test_expect_success '--clear-decorations clears previous exclusions' '
+	cat >expect.all <<-\EOF &&
+	Merge-tag-reach (HEAD -> refs/heads/main)
+	reach (tag: refs/tags/reach, refs/heads/reach)
+	Merge-tags-octopus-a-and-octopus-b
+	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
+	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
+	seventh (tag: refs/tags/seventh)
+	Merge-branch-tangle (refs/hidden/tangle)
+	Merge-branch-side-early-part-into-tangle (refs/rewritten/merge, refs/heads/tangle)
+	Merge-branch-main-early-part-into-tangle (refs/prefetch/merge)
+	tangle-a (tag: refs/tags/tangle-a)
+	side-2 (tag: refs/tags/side-2, refs/heads/side)
+	side-1 (tag: refs/tags/side-1)
+	initial
+	EOF
+
+	git log --decorate=full --pretty="tformat:%f%d" \
+		--simplify-by-decoration \
+		--decorate-refs-exclude="heads/octopus*" \
+		--decorate-refs="heads" \
+		--clear-decorations >actual &&
+	test_cmp expect.all actual &&
+
+	cat >expect.filtered <<-\EOF &&
+	Merge-tags-octopus-a-and-octopus-b
+	octopus-b (refs/heads/octopus-b)
+	octopus-a (refs/heads/octopus-a)
+	initial
+	EOF
+
+	git log --decorate=full --pretty="tformat:%f%d" \
+		--simplify-by-decoration \
+		--decorate-refs-exclude="heads/octopus" \
+		--decorate-refs="heads" \
+		--clear-decorations \
+		--decorate-refs-exclude="tags/" \
+		--decorate-refs="heads/octopus*" >actual &&
+	test_cmp expect.filtered actual
+'
+
 test_expect_success 'log.decorate config parsing' '
 	git log --oneline --decorate=full >expect.full &&
 	git log --oneline --decorate=short >expect.short &&
-- 
gitgitgadget

[PATCH v3 09/11] log: create log.initialDecorationSet=all

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The previous change introduced the --clear-decorations 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 --clear-decorations
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 +++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt
index 456eb07800c..5250ba45fb4 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.initialDecorationSet::
+	By default, `git log` only shows decorations for certain known ref
+	namespaces. If 'all' is specified, then show all refs as
+	decorations.
+
 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 f2ce16fba71..b1285aee3c2 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/`,
 --clear-decorations::
 	When specified, this option clears all previous `--decorate-refs`
 	or `--decorate-refs-exclude` options and relaxes the default
-	decoration filter to include all references.
+	decoration filter to include all references. This option is
+	assumed if the config value `log.initialDecorationSet` 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 7d35d1ecab1..2e2136020e5 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -179,6 +179,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");
@@ -190,6 +191,17 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
 					   item->string);
 	}
 
+	/*
+	 * By default, decorate_all is disabled. Enable it if
+	 * log.initialDecorationSet=all. Don't ever disable it by config,
+	 * since the command-line takes precedent.
+	 */
+	if (use_default_decoration_filter &&
+	    !git_config_get_string("log.initialdecorationset", &value) &&
+	    !strcmp("all", value))
+		use_default_decoration_filter = 0;
+	free(value);
+
 	if (!use_default_decoration_filter ||
 	    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 90800feb0f1..4b2d642d34c 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1090,6 +1090,9 @@ test_expect_success '--clear-decorations overrides defaults' '
 	EOF
 	git log --decorate=full --pretty="tformat:%f%d" \
 		--clear-decorations >actual &&
+	test_cmp expect.all actual &&
+	git -c log.initialDecorationSet=all log \
+		--decorate=full --pretty="tformat:%f%d" >actual &&
 	test_cmp expect.all actual
 '
 
-- 
gitgitgadget

[PATCH v3 10/11] maintenance: stop writing log.excludeDecoration

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

This reverts commit 96eaffebbf3d0 (maintenance: set
log.excludeDecoration durin prefetch, 2021-01-19).

The previous change created a default decoration filter that does not
include refs/prefetch/, so this modification of the config is no longer
needed.

One issue that can happen from this point on is that users who ran the
prefetch task on previous versions of Git will still have a
log.excludeDecoration value and that will prevent the new default
decoration filter from being active. Thus, when we add the refs/bundle/
namespace as part of the bundle URI feature, those users will see
refs/bundle/ decorations.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 builtin/gc.c           |  6 ------
 t/t7900-maintenance.sh | 21 ---------------------
 2 files changed, 27 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index eeff2b760e0..6c222052177 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -910,12 +910,6 @@ static int fetch_remote(struct remote *remote, void *cbdata)
 
 static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
 {
-	git_config_set_multivar_gently("log.excludedecoration",
-					"refs/prefetch/",
-					"refs/prefetch/",
-					CONFIG_FLAGS_FIXED_VALUE |
-					CONFIG_FLAGS_MULTI_REPLACE);
-
 	if (for_each_remote(fetch_remote, opts)) {
 		error(_("failed to prefetch remotes"));
 		return 1;
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 74aa6384755..62ed694a404 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -162,7 +162,6 @@ test_expect_success 'prefetch multiple remotes' '
 	test_cmp_rev refs/remotes/remote1/one refs/prefetch/remotes/remote1/one &&
 	test_cmp_rev refs/remotes/remote2/two refs/prefetch/remotes/remote2/two &&
 
-	test_cmp_config refs/prefetch/ log.excludedecoration &&
 	git log --oneline --decorate --all >log &&
 	! grep "prefetch" log &&
 
@@ -173,26 +172,6 @@ test_expect_success 'prefetch multiple remotes' '
 	test_subcommand git fetch remote2 $fetchargs <skip-remote1.txt
 '
 
-test_expect_success 'prefetch and existing log.excludeDecoration values' '
-	git config --unset-all log.excludeDecoration &&
-	git config log.excludeDecoration refs/remotes/remote1/ &&
-	git maintenance run --task=prefetch &&
-
-	git config --get-all log.excludeDecoration >out &&
-	grep refs/remotes/remote1/ out &&
-	grep refs/prefetch/ out &&
-
-	git log --oneline --decorate --all >log &&
-	! grep "prefetch" log &&
-	! grep "remote1" log &&
-	grep "remote2" log &&
-
-	# a second run does not change the config
-	git maintenance run --task=prefetch &&
-	git log --oneline --decorate --all >log2 &&
-	test_cmp log log2
-'
-
 test_expect_success 'loose-objects task' '
 	# Repack everything so we know the state of the object dir
 	git repack -adk &&
-- 
gitgitgadget

[PATCH v3 11/11] fetch: use ref_namespaces during prefetch

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The "refs/prefetch/" namespace is used by 'git fetch --prefetch' as a
replacement of the destination of the refpsec for a remote. Git also
removes refspecs that include tags.

Instead of using string literals for the 'refs/tags/ and
'refs/prefetch/' namespaces, use the entries in the ref_namespaces
array.

This kind of change could be done in many places around the codebase,
but we are isolating only to this change because of the way the
refs/prefetch/ namespace somewhat motivated the creation of the
ref_namespaces array.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 builtin/fetch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc5cecb4835..368a0f5329c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -490,7 +490,9 @@ static void filter_prefetch_refspec(struct refspec *rs)
 			continue;
 		if (!rs->items[i].dst ||
 		    (rs->items[i].src &&
-		     !strncmp(rs->items[i].src, "refs/tags/", 10))) {
+		     !strncmp(rs->items[i].src,
+			      ref_namespace[NAMESPACE_TAGS].ref,
+			      strlen(ref_namespace[NAMESPACE_TAGS].ref)))) {
 			int j;
 
 			free(rs->items[i].src);
@@ -506,7 +508,7 @@ static void filter_prefetch_refspec(struct refspec *rs)
 		}
 
 		old_dst = rs->items[i].dst;
-		strbuf_addstr(&new_dst, "refs/prefetch/");
+		strbuf_addstr(&new_dst, ref_namespace[NAMESPACE_PREFETCH].ref);
 
 		/*
 		 * If old_dst starts with "refs/", then place
-- 
gitgitgadget