* [PATCH v2 01/10] refs: allow "HEAD" as decoration filter
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-03 6:03 ` Junio C Hamano
2022-07-29 19:29 ` [PATCH v2 02/10] t4207: test coloring of grafted decorations Derrick Stolee via GitGitGadget
` (9 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 01/10] refs: allow "HEAD" as decoration filter
2022-07-29 19:29 ` [PATCH v2 01/10] refs: allow "HEAD" as " Derrick Stolee via GitGitGadget
@ 2022-08-03 6:03 ` Junio C Hamano
2022-08-04 13:22 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Junio C Hamano @ 2022-08-03 6:03 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason,
Derrick Stolee
"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)".
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 01/10] refs: allow "HEAD" as decoration filter
2022-08-03 6:03 ` Junio C Hamano
@ 2022-08-04 13:22 ` Derrick Stolee
0 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 13:22 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 02/10] t4207: test coloring of grafted decorations
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
2022-07-29 19:29 ` [PATCH v2 01/10] refs: allow "HEAD" as " Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-03 6:25 ` Ævar Arnfjörð Bjarmason
2022-07-29 19:29 ` [PATCH v2 03/10] refs: add array of ref namespaces Derrick Stolee via GitGitGadget
` (8 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations
2022-07-29 19:29 ` [PATCH v2 02/10] t4207: test coloring of grafted decorations Derrick Stolee via GitGitGadget
@ 2022-08-03 6:25 ` Ævar Arnfjörð Bjarmason
2022-08-03 6:44 ` Eric Sunshine
0 siblings, 1 reply; 79+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-03 6:25 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, gitster, me, vdye, steadmon, Derrick Stolee
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...
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations
2022-08-03 6:25 ` Ævar Arnfjörð Bjarmason
@ 2022-08-03 6:44 ` Eric Sunshine
2022-08-03 13:03 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 79+ messages in thread
From: Eric Sunshine @ 2022-08-03 6:44 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Derrick Stolee via GitGitGadget, Git List, Junio C Hamano,
Taylor Blau, Victoria Dye, Josh Steadmon, Derrick Stolee
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations
2022-08-03 6:44 ` Eric Sunshine
@ 2022-08-03 13:03 ` Ævar Arnfjörð Bjarmason
2022-08-04 4:05 ` Eric Sunshine
0 siblings, 1 reply; 79+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-03 13:03 UTC (permalink / raw)
To: Eric Sunshine
Cc: Derrick Stolee via GitGitGadget, Git List, Junio C Hamano,
Taylor Blau, Victoria Dye, Josh Steadmon, Derrick Stolee
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).
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations
2022-08-03 13:03 ` Ævar Arnfjörð Bjarmason
@ 2022-08-04 4:05 ` Eric Sunshine
2022-08-04 17:23 ` Junio C Hamano
0 siblings, 1 reply; 79+ messages in thread
From: Eric Sunshine @ 2022-08-04 4:05 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Derrick Stolee via GitGitGadget, Git List, Junio C Hamano,
Taylor Blau, Victoria Dye, Josh Steadmon, Derrick Stolee
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.).
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations
2022-08-04 4:05 ` Eric Sunshine
@ 2022-08-04 17:23 ` Junio C Hamano
2022-08-05 14:21 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Junio C Hamano @ 2022-08-04 17:23 UTC (permalink / raw)
To: Eric Sunshine
Cc: Ævar Arnfjörð Bjarmason,
Derrick Stolee via GitGitGadget, Git List, Taylor Blau,
Victoria Dye, Josh Steadmon, Derrick Stolee
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 02/10] t4207: test coloring of grafted decorations
2022-08-04 17:23 ` Junio C Hamano
@ 2022-08-05 14:21 ` Derrick Stolee
0 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee @ 2022-08-05 14:21 UTC (permalink / raw)
To: Junio C Hamano, Eric Sunshine
Cc: Ævar Arnfjörð Bjarmason,
Derrick Stolee via GitGitGadget, Git List, Taylor Blau,
Victoria Dye, Josh Steadmon
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 03/10] refs: add array of ref namespaces
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
2022-07-29 19:29 ` [PATCH v2 01/10] refs: allow "HEAD" as " Derrick Stolee via GitGitGadget
2022-07-29 19:29 ` [PATCH v2 02/10] t4207: test coloring of grafted decorations Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-03 6:16 ` Junio C Hamano
2022-08-03 22:39 ` Josh Steadmon
2022-07-29 19:29 ` [PATCH v2 04/10] refs: use ref_namespaces for replace refs base Derrick Stolee via GitGitGadget
` (7 subsequent siblings)
10 siblings, 2 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 03/10] refs: add array of ref namespaces
2022-07-29 19:29 ` [PATCH v2 03/10] refs: add array of ref namespaces Derrick Stolee via GitGitGadget
@ 2022-08-03 6:16 ` Junio C Hamano
2022-08-04 13:29 ` Derrick Stolee
2022-08-03 22:39 ` Josh Steadmon
1 sibling, 1 reply; 79+ messages in thread
From: Junio C Hamano @ 2022-08-03 6:16 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason,
Derrick Stolee
"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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 03/10] refs: add array of ref namespaces
2022-08-03 6:16 ` Junio C Hamano
@ 2022-08-04 13:29 ` Derrick Stolee
2022-08-04 16:16 ` Junio C Hamano
0 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 13:29 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 03/10] refs: add array of ref namespaces
2022-08-04 13:29 ` Derrick Stolee
@ 2022-08-04 16:16 ` Junio C Hamano
0 siblings, 0 replies; 79+ messages in thread
From: Junio C Hamano @ 2022-08-04 16:16 UTC (permalink / raw)
To: Derrick Stolee
Cc: Derrick Stolee via GitGitGadget, git, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 03/10] refs: add array of ref namespaces
2022-07-29 19:29 ` [PATCH v2 03/10] refs: add array of ref namespaces Derrick Stolee via GitGitGadget
2022-08-03 6:16 ` Junio C Hamano
@ 2022-08-03 22:39 ` Josh Steadmon
2022-08-04 13:29 ` Derrick Stolee
1 sibling, 1 reply; 79+ messages in thread
From: Josh Steadmon @ 2022-08-03 22:39 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, gitster, me, vdye, Ævar Arnfjörð Bjarmason,
Derrick Stolee
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
>
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 03/10] refs: add array of ref namespaces
2022-08-03 22:39 ` Josh Steadmon
@ 2022-08-04 13:29 ` Derrick Stolee
0 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 13:29 UTC (permalink / raw)
To: Josh Steadmon, Derrick Stolee via GitGitGadget, git, gitster, me,
vdye, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 04/10] refs: use ref_namespaces for replace refs base
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (2 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 03/10] refs: add array of ref namespaces Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-03 22:38 ` Josh Steadmon
2022-07-29 19:29 ` [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if Derrick Stolee via GitGitGadget
` (6 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 04/10] refs: use ref_namespaces for replace refs base
2022-07-29 19:29 ` [PATCH v2 04/10] refs: use ref_namespaces for replace refs base Derrick Stolee via GitGitGadget
@ 2022-08-03 22:38 ` Josh Steadmon
2022-08-04 13:30 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Josh Steadmon @ 2022-08-03 22:38 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, gitster, me, vdye, Ævar Arnfjörð Bjarmason,
Derrick Stolee
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)
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 04/10] refs: use ref_namespaces for replace refs base
2022-08-03 22:38 ` Josh Steadmon
@ 2022-08-04 13:30 ` Derrick Stolee
0 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 13:30 UTC (permalink / raw)
To: Josh Steadmon, Derrick Stolee via GitGitGadget, git, gitster, me,
vdye, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (3 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 04/10] refs: use ref_namespaces for replace refs base Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-03 6:31 ` Junio C Hamano
2022-07-29 19:29 ` [PATCH v2 06/10] log: add default decoration filter Derrick Stolee via GitGitGadget
` (5 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
2022-07-29 19:29 ` [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if Derrick Stolee via GitGitGadget
@ 2022-08-03 6:31 ` Junio C Hamano
2022-08-04 13:31 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Junio C Hamano @ 2022-08-03 6:31 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason,
Derrick Stolee
"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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
2022-08-03 6:31 ` Junio C Hamano
@ 2022-08-04 13:31 ` Derrick Stolee
2022-08-04 14:34 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 13:31 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
2022-08-04 13:31 ` Derrick Stolee
@ 2022-08-04 14:34 ` Derrick Stolee
2022-08-04 14:50 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 14:34 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
2022-08-04 14:34 ` Derrick Stolee
@ 2022-08-04 14:50 ` Derrick Stolee
2022-08-04 17:40 ` Junio C Hamano
0 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 14:50 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
2022-08-04 14:50 ` Derrick Stolee
@ 2022-08-04 17:40 ` Junio C Hamano
2022-08-04 20:17 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Junio C Hamano @ 2022-08-04 17:40 UTC (permalink / raw)
To: Derrick Stolee
Cc: Derrick Stolee via GitGitGadget, git, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
2022-08-04 17:40 ` Junio C Hamano
@ 2022-08-04 20:17 ` Derrick Stolee
0 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 20:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Derrick Stolee via GitGitGadget, git, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 06/10] log: add default decoration filter
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (4 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-04 7:08 ` Ævar Arnfjörð Bjarmason
2022-07-29 19:29 ` [PATCH v2 07/10] log: add --decorate-all option Derrick Stolee via GitGitGadget
` (4 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 06/10] log: add default decoration filter
2022-07-29 19:29 ` [PATCH v2 06/10] log: add default decoration filter Derrick Stolee via GitGitGadget
@ 2022-08-04 7:08 ` Ævar Arnfjörð Bjarmason
2022-08-05 14:27 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-04 7:08 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, gitster, me, vdye, steadmon, Derrick Stolee
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?
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 06/10] log: add default decoration filter
2022-08-04 7:08 ` Ævar Arnfjörð Bjarmason
@ 2022-08-05 14:27 ` Derrick Stolee
2022-08-05 14:50 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-08-05 14:27 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, Derrick Stolee via GitGitGadget
Cc: git, gitster, me, vdye, steadmon
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 06/10] log: add default decoration filter
2022-08-05 14:27 ` Derrick Stolee
@ 2022-08-05 14:50 ` Ævar Arnfjörð Bjarmason
2022-08-05 15:09 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-05 14:50 UTC (permalink / raw)
To: Derrick Stolee
Cc: Derrick Stolee via GitGitGadget, git, gitster, me, vdye, steadmon
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 06/10] log: add default decoration filter
2022-08-05 14:50 ` Ævar Arnfjörð Bjarmason
@ 2022-08-05 15:09 ` Derrick Stolee
2022-08-11 19:30 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-08-05 15:09 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Derrick Stolee via GitGitGadget, git, gitster, me, vdye, steadmon
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 06/10] log: add default decoration filter
2022-08-05 15:09 ` Derrick Stolee
@ 2022-08-11 19:30 ` Ævar Arnfjörð Bjarmason
2022-08-12 19:37 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-11 19:30 UTC (permalink / raw)
To: Derrick Stolee
Cc: Derrick Stolee via GitGitGadget, git, gitster, me, vdye, steadmon
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 06/10] log: add default decoration filter
2022-08-11 19:30 ` Ævar Arnfjörð Bjarmason
@ 2022-08-12 19:37 ` Derrick Stolee
0 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee @ 2022-08-12 19:37 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Derrick Stolee via GitGitGadget, git, gitster, me, vdye, steadmon
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 07/10] log: add --decorate-all option
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (5 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 06/10] log: add default decoration filter Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-04 16:57 ` Junio C Hamano
2022-07-29 19:29 ` [PATCH v2 08/10] log: create log.decorateFilter=all Derrick Stolee via GitGitGadget
` (3 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 07/10] log: add --decorate-all option
2022-07-29 19:29 ` [PATCH v2 07/10] log: add --decorate-all option Derrick Stolee via GitGitGadget
@ 2022-08-04 16:57 ` Junio C Hamano
0 siblings, 0 replies; 79+ messages in thread
From: Junio C Hamano @ 2022-08-04 16:57 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason,
Derrick Stolee
"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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 08/10] log: create log.decorateFilter=all
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (6 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 07/10] log: add --decorate-all option Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-03 22:42 ` Josh Steadmon
2022-07-29 19:29 ` [PATCH v2 09/10] maintenance: stop writing log.excludeDecoration Derrick Stolee via GitGitGadget
` (2 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 08/10] log: create log.decorateFilter=all
2022-07-29 19:29 ` [PATCH v2 08/10] log: create log.decorateFilter=all Derrick Stolee via GitGitGadget
@ 2022-08-03 22:42 ` Josh Steadmon
2022-08-04 13:38 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Josh Steadmon @ 2022-08-03 22:42 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, gitster, me, vdye, Ævar Arnfjörð Bjarmason,
Derrick Stolee
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v2 08/10] log: create log.decorateFilter=all
2022-08-03 22:42 ` Josh Steadmon
@ 2022-08-04 13:38 ` Derrick Stolee
0 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee @ 2022-08-04 13:38 UTC (permalink / raw)
To: Josh Steadmon, Derrick Stolee via GitGitGadget, git, gitster, me,
vdye, Ævar Arnfjörð Bjarmason
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 09/10] maintenance: stop writing log.excludeDecoration
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (7 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 08/10] log: create log.decorateFilter=all Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-03 6:36 ` Junio C Hamano
2022-07-29 19:29 ` [PATCH v2 10/10] fetch: use ref_namespaces during prefetch Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v2 09/10] maintenance: stop writing log.excludeDecoration
2022-07-29 19:29 ` [PATCH v2 09/10] maintenance: stop writing log.excludeDecoration Derrick Stolee via GitGitGadget
@ 2022-08-03 6:36 ` Junio C Hamano
0 siblings, 0 replies; 79+ messages in thread
From: Junio C Hamano @ 2022-08-03 6:36 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, me, vdye, steadmon, Ævar Arnfjörð Bjarmason,
Derrick Stolee
"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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v2 10/10] fetch: use ref_namespaces during prefetch
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (8 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 09/10] maintenance: stop writing log.excludeDecoration Derrick Stolee via GitGitGadget
@ 2022-07-29 19:29 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-07-29 19:29 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 00/11] log: create tighter default decoration filter
2022-07-29 19:29 ` [PATCH v2 00/10] " Derrick Stolee via GitGitGadget
` (9 preceding siblings ...)
2022-07-29 19:29 ` [PATCH v2 10/10] fetch: use ref_namespaces during prefetch Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 01/11] refs: allow "HEAD" as " Derrick Stolee via GitGitGadget
` (10 more replies)
10 siblings, 11 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v3 01/11] refs: allow "HEAD" as decoration filter
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 02/11] t4207: modernize test Derrick Stolee via GitGitGadget
` (9 subsequent siblings)
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 02/11] t4207: modernize test
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 01/11] refs: allow "HEAD" as " Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 03/11] t4207: test coloring of grafted decorations Derrick Stolee via GitGitGadget
` (8 subsequent siblings)
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 03/11] t4207: test coloring of grafted decorations
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 01/11] refs: allow "HEAD" as " Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 02/11] t4207: modernize test Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 04/11] refs: add array of ref namespaces Derrick Stolee via GitGitGadget
` (7 subsequent siblings)
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 04/11] refs: add array of ref namespaces
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (2 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 03/11] t4207: test coloring of grafted decorations Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 05/11] refs: use ref_namespaces for replace refs base Derrick Stolee via GitGitGadget
` (6 subsequent siblings)
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 05/11] refs: use ref_namespaces for replace refs base
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (3 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 04/11] refs: add array of ref namespaces Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 06/11] log-tree: use ref_namespaces instead of if/else-if Derrick Stolee via GitGitGadget
` (5 subsequent siblings)
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 06/11] log-tree: use ref_namespaces instead of if/else-if
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (4 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 05/11] refs: use ref_namespaces for replace refs base Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 07/11] log: add default decoration filter Derrick Stolee via GitGitGadget
` (4 subsequent siblings)
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 07/11] log: add default decoration filter
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (5 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 06/11] log-tree: use ref_namespaces instead of if/else-if Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-09-08 21:13 ` Glen Choo
2022-08-05 17:58 ` [PATCH v3 08/11] log: add --clear-decorations option Derrick Stolee via GitGitGadget
` (3 subsequent siblings)
10 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH v3 07/11] log: add default decoration filter
2022-08-05 17:58 ` [PATCH v3 07/11] log: add default decoration filter Derrick Stolee via GitGitGadget
@ 2022-09-08 21:13 ` Glen Choo
2022-09-09 12:23 ` Derrick Stolee
0 siblings, 1 reply; 79+ messages in thread
From: Glen Choo @ 2022-09-08 21:13 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget, git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee
"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?
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v3 07/11] log: add default decoration filter
2022-09-08 21:13 ` Glen Choo
@ 2022-09-09 12:23 ` Derrick Stolee
2022-09-09 16:19 ` Junio C Hamano
0 siblings, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-09-09 12:23 UTC (permalink / raw)
To: Glen Choo, Derrick Stolee via GitGitGadget, git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v3 07/11] log: add default decoration filter
2022-09-09 12:23 ` Derrick Stolee
@ 2022-09-09 16:19 ` Junio C Hamano
2022-09-09 16:40 ` Derrick Stolee
2022-09-09 17:12 ` Glen Choo
0 siblings, 2 replies; 79+ messages in thread
From: Junio C Hamano @ 2022-09-09 16:19 UTC (permalink / raw)
To: Derrick Stolee
Cc: Glen Choo, Derrick Stolee via GitGitGadget, git, me, vdye,
steadmon, Ævar Arnfjörð Bjarmason, Jeff King,
Eric Sunshine
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v3 07/11] log: add default decoration filter
2022-09-09 16:19 ` Junio C Hamano
@ 2022-09-09 16:40 ` Derrick Stolee
2022-09-09 17:41 ` Junio C Hamano
2022-09-09 17:12 ` Glen Choo
1 sibling, 1 reply; 79+ messages in thread
From: Derrick Stolee @ 2022-09-09 16:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Glen Choo, Derrick Stolee via GitGitGadget, git, me, vdye,
steadmon, Ævar Arnfjörð Bjarmason, Jeff King,
Eric Sunshine
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
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v3 07/11] log: add default decoration filter
2022-09-09 16:40 ` Derrick Stolee
@ 2022-09-09 17:41 ` Junio C Hamano
0 siblings, 0 replies; 79+ messages in thread
From: Junio C Hamano @ 2022-09-09 17:41 UTC (permalink / raw)
To: Derrick Stolee
Cc: Glen Choo, Derrick Stolee via GitGitGadget, git, me, vdye,
steadmon, Ævar Arnfjörð Bjarmason, Jeff King,
Eric Sunshine
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH v3 07/11] log: add default decoration filter
2022-09-09 16:19 ` Junio C Hamano
2022-09-09 16:40 ` Derrick Stolee
@ 2022-09-09 17:12 ` Glen Choo
1 sibling, 0 replies; 79+ messages in thread
From: Glen Choo @ 2022-09-09 17:12 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee
Cc: Derrick Stolee via GitGitGadget, git, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine
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.
^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH v3 08/11] log: add --clear-decorations option
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (6 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 07/11] log: add default decoration filter Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 09/11] log: create log.initialDecorationSet=all Derrick Stolee via GitGitGadget
` (2 subsequent siblings)
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 09/11] log: create log.initialDecorationSet=all
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (7 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 08/11] log: add --clear-decorations option Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 10/11] maintenance: stop writing log.excludeDecoration Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 11/11] fetch: use ref_namespaces during prefetch Derrick Stolee via GitGitGadget
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 10/11] maintenance: stop writing log.excludeDecoration
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (8 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 09/11] log: create log.initialDecorationSet=all Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
2022-08-05 17:58 ` [PATCH v3 11/11] fetch: use ref_namespaces during prefetch Derrick Stolee via GitGitGadget
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH v3 11/11] fetch: use ref_namespaces during prefetch
2022-08-05 17:58 ` [PATCH v3 00/11] log: create tighter default decoration filter Derrick Stolee via GitGitGadget
` (9 preceding siblings ...)
2022-08-05 17:58 ` [PATCH v3 10/11] maintenance: stop writing log.excludeDecoration Derrick Stolee via GitGitGadget
@ 2022-08-05 17:58 ` Derrick Stolee via GitGitGadget
10 siblings, 0 replies; 79+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2022-08-05 17:58 UTC (permalink / raw)
To: git
Cc: gitster, me, vdye, steadmon,
Ævar Arnfjörð Bjarmason, Jeff King, Eric Sunshine,
Derrick Stolee, Derrick Stolee
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
^ permalink raw reply related [flat|nested] 79+ messages in thread