{"thread":{"id":"59270","subject":"[PATCH] log: omit tag prefix for color decoration","startedAt":"2023-02-19T17:50:53Z","lastAt":"2023-02-21T21:29:11Z","messageCount":3,"participants":["Andy Koppe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"472316","messageId":"20230219174631.1040-1-andy.koppe@gmail.com","threadId":"59270","inReplyTo":null,"subject":"[PATCH] log: omit tag prefix for color decoration","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2023-02-19T17:46:30Z","receivedAt":"2023-02-19T17:50:53Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"Omit the \"tag: \" prefix for tags in commit decorations when coloring is\nenabled. The prefix isn't necessary as such when tags are distinguished\nby color already, and omitting it saves a bit of space and visual noise.\n\nIt also avoids a problem: when the %d or %D placeholders are used while\na %w wrapping specifier is in force in a format string, lines can be\nbroken between the prefix and the tag name. In that case, the prefix\ngets colored correctly, but the tag name gets the default color instead.\n\nAlso remove the tag prefix from test t4207 for color decorations, and\nadd --no-color to a test t6002 check of the output of rev-list --bisect\nthat expects tag prefixes to be present.\n\nSigned-off-by: Andy Koppe <andy.koppe@gmail.com>\n---\n\nCI run: https://github.com/ak2/git/actions/runs/4211957381\n\n log-tree.c                       |  3 ++-\n t/t4207-log-decoration-colors.sh | 22 +++++++++++-----------\n t/t6002-rev-list-bisect.sh       |  2 +-\n 3 files changed, 14 insertions(+), 13 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 1dd5fcbf7b..64ea15e0a0 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -324,7 +324,8 @@ void format_decorations_extended(struct strbuf *sb,\n \t\t\tstrbuf_addstr(sb, prefix);\n \t\t\tstrbuf_addstr(sb, color_reset);\n \t\t\tstrbuf_addstr(sb, decorate_get_color(use_color, decoration->type));\n-\t\t\tif (decoration->type == DECORATION_REF_TAG)\n+\t\t\tif (!use_color &&\n+\t\t\t    decoration->type == DECORATION_REF_TAG)\n \t\t\t\tstrbuf_addstr(sb, \"tag: \");\n \n \t\t\tshow_name(sb, decoration);\ndiff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh\nindex ded33a82e2..a5ee7b19d7 100755\n--- a/t/t4207-log-decoration-colors.sh\n+++ b/t/t4207-log-decoration-colors.sh\n@@ -55,13 +55,13 @@ test_expect_success 'commit decorations colored correctly' '\n \tcat >expect <<-EOF &&\n \t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \\\n ${c_reset}${c_branch}main${c_reset}${c_commit}, \\\n-${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit}, \\\n-${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B\n-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B\n+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}A1${c_reset}${c_commit}, \\\n ${c_reset}${c_remoteBranch}other/main${c_reset}${c_commit})${c_reset} A1\n \t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset} \\\n On main: Changes to A.t\n-\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A\n \tEOF\n \n \tgit log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&\n@@ -78,10 +78,10 @@ test_expect_success 'test coloring with replace-objects' '\n \tcat >expect <<-EOF &&\n \t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \\\n ${c_reset}${c_branch}main${c_reset}${c_commit}, \\\n-${c_reset}${c_tag}tag: D${c_reset}${c_commit})${c_reset} D\n-\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: C${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}D${c_reset}${c_commit})${c_reset} D\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}C${c_reset}${c_commit}, \\\n ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B\n-\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A\n EOF\n \n \tgit log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&\n@@ -102,11 +102,11 @@ test_expect_success 'test coloring with grafted commit' '\n \tcat >expect <<-EOF &&\n \t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD -> \\\n ${c_reset}${c_branch}main${c_reset}${c_commit}, \\\n-${c_reset}${c_tag}tag: D${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}D${c_reset}${c_commit}, \\\n ${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D\n-\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit}, \\\n-${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B\n-\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A\n \tEOF\n \n \tgit log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&\ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex 162cf50778..924923afaa 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -305,7 +305,7 @@ test_expect_success '--bisect-all --first-parent' '\n \t# expect results to be ordered by distance (descending),\n \t# commit hash (ascending)\n \tsort -k4,4r -k1,1 expect.unsorted >expect &&\n-\tgit rev-list --bisect-all --first-parent E ^F >actual &&\n+\tgit rev-list --bisect-all --first-parent --no-color E ^F >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.39.0\n\n"},{"id":"472357","messageId":"xmqqv8jwggsp.fsf@gitster.g","threadId":"59270","inReplyTo":"20230219174631.1040-1-andy.koppe@gmail.com","subject":"Re: [PATCH] log: omit tag prefix for color decoration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-20T21:36:54Z","receivedAt":"2023-02-20T21:37:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Koppe <andy.koppe@gmail.com> writes:\n\n> Omit the \"tag: \" prefix for tags in commit decorations when coloring is\n> enabled. The prefix isn't necessary as such when tags are distinguished\n> by color already, and omitting it saves a bit of space and visual noise.\n\nAs somebody whose color perception is weaker than other people, I am\ntorn about this line of changes.  Accessibiilty folks often warn us\nagainst designing UI that differentiates two things only by painting\nthem in two different colors \"to be inclusive\", and the above goes\ndirectly against their guidelines.\n\nThe only saving grace in this case is that I expect that tags are\nnamed very differently from topics in well managed projects, and\nusers can tell withotu \"tag:\" prefix even in the output without\ncolors, and that si why I said \"I am torn\", not I am not outright\n100% negative, but still I doubt that the upside of shortening 4\ndisplay spaces is worth going directly against inclusion advocates.\n\nSo...\n"},{"id":"472402","messageId":"CAHWeT-bqnwUxcaGOK7XD0wmmc6BaW2x6Gqhu107vZeU=U6Rm8g@mail.gmail.com","threadId":"59270","inReplyTo":"xmqqv8jwggsp.fsf@gitster.g","subject":"Re: [PATCH] log: omit tag prefix for color decoration","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2023-02-21T21:28:25Z","receivedAt":"2023-02-21T21:29:11Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"On Mon, 20 Feb 2023 at 21:36, Junio C Hamano wrote:\n>\n> Andy Koppe writes:\n>\n> > Omit the \"tag: \" prefix for tags in commit decorations when coloring is\n> > enabled. The prefix isn't necessary as such when tags are distinguished\n> > by color already, and omitting it saves a bit of space and visual noise.\n>\n> As somebody whose color perception is weaker than other people, I am\n> torn about this line of changes.  Accessibiilty folks often warn us\n> against designing UI that differentiates two things only by painting\n> them in two different colors \"to be inclusive\", and the above goes\n> directly against their guidelines.\n>\n> The only saving grace in this case is that I expect that tags are\n> named very differently from topics in well managed projects, and\n> users can tell withotu \"tag:\" prefix even in the output without\n> colors, and that si why I said \"I am torn\", not I am not outright\n> 100% negative, but still I doubt that the upside of shortening 4\n> display spaces is worth going directly against inclusion advocates.\n\nI can't argue with that, particularly when changing default behavior.\n\nWould you welcome a patch instead that implements a new %r pretty\nformat placeholder alongside %d and %D that lists the refs separated\nby spaces only, i.e. without parentheses, commas or tag prefixes?\n\n               %d\n                   ref names, like the --decorate option of git-log(1)\n\n               %D\n                   ref names without the \" (\", \")\" wrapping.\n\n               %r\n                   ref names only\n\n(Having looked at the relevant code, I think it should still be a\nfairly small change.)\n"}]}