{"thread":{"id":"59183","subject":"[PATCH 0/3] blame: allow blame.coloring to specify both --color-lines and --color-by-age","startedAt":"2023-02-02T19:16:53Z","lastAt":"2023-02-02T19:16:57Z","messageCount":4,"participants":["Robert Estelle via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"471382","messageId":"pull.1449.git.git.1675365406.gitgitgadget@gmail.com","threadId":"59183","inReplyTo":null,"subject":"[PATCH 0/3] blame: allow blame.coloring to specify both --color-lines and --color-by-age","fromName":"Robert Estelle via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-02T19:16:43Z","receivedAt":"2023-02-02T19:16:53Z","isPatch":true,"sender":{"key":"robertestelle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/447615?v=4"},"body":"The flags --color-lines and --color-by-age are not mutually exclusive, and\nboth affect the output format of git blame. This adds a test verifying git\nblame's output when run with both, where previously they were only tested\nindependently.\n\nblame is then taught to accept multiple values from blame.coloring, so that\nboth modes can be set by configuration. Previously, the config only\npermitted one (repeatedLines) or the other (highlightRecent), despite the\ncommand line accepting both flags independently. This is a\nbackwards-compatible change that does not alter the interpretation of\nexisting configuration.\n\nThe list-parsing logic was derived from that of parse_color_moved_ws in\ndiff.c.\n\nRobert Estelle (3):\n blame: add test for --color-lines + --color-by-age\n blame: document --color-{lines,by-age} config\n blame: support multiple values in blame.coloring\n\n Documentation/blame-options.txt | 4 ++\n Documentation/config/blame.txt | 1 +\n builtin/blame.c | 47 ++++++++++++++++------\n t/t8012-blame-colors.sh | 71 +++++++++++++++++++++++++++++++++\n 4 files changed, 111 insertions(+), 12 deletions(-)\n\n\nbase-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1449%2Frwe%2Fblame-color-config-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1449/rwe/blame-color-config-v1\nPull-Request: https://github.com/git/git/pull/1449\n-- \ngitgitgadget\n"},{"id":"471383","messageId":"01112de2b6ab7188e9fa9ef00aa06948ead74172.1675365406.git.gitgitgadget@gmail.com","threadId":"59183","inReplyTo":"pull.1449.git.git.1675365406.gitgitgadget@gmail.com","subject":"[PATCH 1/3] blame: add test for --color-lines + --color-by-age","fromName":"Robert Estelle via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-02T19:16:44Z","receivedAt":"2023-02-02T19:16:54Z","isPatch":true,"sender":{"key":"robertestelle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/447615?v=4"},"body":"From: Robert Estelle \n\nThis adds a test verifying `git blame`'s output when run with both\n`--color-lines` and `--color-by-age`. Those flags are not mutually\nexclusive and both affect the output format. They were previously only\ntested independently.\n\nSigned-off-by: Robert Estelle \n---\n t/t8012-blame-colors.sh | 63 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 63 insertions(+)\n\ndiff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh\nindex c3a5f6d01ff..820f86c3aed 100755\n--- a/t/t8012-blame-colors.sh\n+++ b/t/t8012-blame-colors.sh\n@@ -49,4 +49,67 @@ test_expect_success 'blame color by age: new code is different' '\n \tgrep qfunc colored\n '\n \n+# shellcheck disable=SC2317\n+re_normalize_color_decoded_blame() {\n+\t# Construct the regex used by `normalize_color_decoded_blame`.\n+\t# This is simply for documentation: line comments aren't permitted in\n+\t# backslash-continuation lines, and POSIX sh does't support 'x+=' syntax.\n+\tprintf '%s' '\\(<[^>]*>\\)' # 1: capture the \"\" etc\n+\tprintf '%s' ' *' # -- discard any spaces\n+\tprintf '%s' '[0-9a-f]\\{1,\\}' # -- discard the commit ID\n+\tprintf '%s' ' *' # -- discard any spaces\n+\tprintf '%s' '(' # -- left paren\n+\tprintf '%s' '\\(.\\)' # 2: capture the single-char A/F/G/etc\n+\tprintf '%s' '[^\\)]*' # -- discard author and timestamp stuff\n+\tprintf '%s' '\\([0-9]\\)\\{1,\\}' # 3: capture the line number\n+\tprintf '%s' ')' # -- right paren\n+\tprintf '%s' ' *' # -- discard leading spaces\n+\tprintf '%s' '\\(.*$\\)' # 4: capture the remainder\n+}\n+\n+# shellcheck disable=SC2317\n+normalize_color_decoded_blame() {\n+\t# Reads from stdin and writes to stdout.\n+\t# Removes the commit ID and author/timestamp from blame output after\n+\t# \"test_decode_color\" has run.\n+\t#\n+\t# This is simply to make expected outputs easier to describe\n+\t# and compare without having to refer to magic line counts.\n+\tre=\"$(re_normalize_color_decoded_blame)\" || return $?\n+\tsed -e 's/^'\"${re}\"'/\\1 (\\2 \\3) \\4/'\n+}\n+\n+test_expect_success 'blame color by age and lines' '\n+\tgit \\\n+\t\t-c color.blame.repeatedLines=blue \\\n+\t\t-c color.blame.highlightRecent=\"yellow,1 month ago, cyan\" \\\n+\t\tblame \\\n+\t\t--color-lines \\\n+\t\t--color-by-age \\\n+\t\thello.c \\\n+\t\t>actual.raw &&\n+\n+\ttest_decode_color actual &&\n+\tnormalize_color_decoded_blame actual.norm &&\n+\n+\tnormalize_color_decoded_blame >expected.norm <<-EOF &&\n+\t\t11111111 (H ... 1) #include \n+\t\t22222222 (F ... 2) int main(int argc, const char *argv[])\n+\t\t 22222222 (F ... 3) {\n+\t\t 22222222 (F ... 4) \tputs(\"hello\");\n+\t\t33333333 (G ... 5) \tputs(\"goodbye\");\n+\t\t22222222 (F ... 6) }\n+\t\t11111111 (H ... 7) void mail()\n+\t\t 11111111 (H ... 8) {\n+\t\t 11111111 (H ... 9) \tputs(\"mail\");\n+\t\t 11111111 (H ... 10) }\n+\t\t 44444444 (A ... 11) void qfunc();\n+\t\tEOF\n+\n+\ttest_cmp actual.norm expected.norm &&\n+\n+\tgrep \"\" sanity-check &&\n+\ttest_line_count = 5 sanity-check\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"471384","messageId":"6b0bf073cf0e06e4cfe51f4c10f632344c163921.1675365406.git.gitgitgadget@gmail.com","threadId":"59183","inReplyTo":"pull.1449.git.git.1675365406.gitgitgadget@gmail.com","subject":"[PATCH 2/3] blame: document --color-{lines,by-age} config","fromName":"Robert Estelle via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-02T19:16:45Z","receivedAt":"2023-02-02T19:16:56Z","isPatch":true,"sender":{"key":"robertestelle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/447615?v=4"},"body":"From: Robert Estelle \n\nThese flags correspond to specific values of the `blame.coloring` config\noption. This adds a small note mentioning this correspondence.\n\nSigned-off-by: Robert Estelle \n---\n Documentation/blame-options.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 9a663535f44..170016c941e 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -141,11 +141,15 @@ take effect.\n \tthe same commit as the preceding line. This makes it easier to distinguish\n \tcode blocks introduced by different commits. The color defaults to cyan and\n \tcan be adjusted using the `color.blame.repeatedLines` config option.\n+\tIt corresponds to the value `repeatedLines` in the `blame.coloring`\n+\tconfiguration setting in linkgit:git-config[1] .\n \n --color-by-age::\n \tColor line annotations depending on the age of the line in the default format.\n \tThe `color.blame.highlightRecent` config option controls what color is used for\n \teach range of age.\n+\tIt corresponds to the value `highlightRecent` in the `blame.coloring`\n+\tconfiguration setting in linkgit:git-config[1] .\n \n -h::\n \tShow help message.\n-- \ngitgitgadget\n\n"},{"id":"471385","messageId":"e33ae071e27faf16f90f4a71e80840c7f3897b50.1675365406.git.gitgitgadget@gmail.com","threadId":"59183","inReplyTo":"pull.1449.git.git.1675365406.gitgitgadget@gmail.com","subject":"[PATCH 3/3] blame: support multiple values in blame.coloring","fromName":"Robert Estelle via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-02-02T19:16:46Z","receivedAt":"2023-02-02T19:16:57Z","isPatch":true,"sender":{"key":"robertestelle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/447615?v=4"},"body":"From: Robert Estelle \n\n`blame.coloring` is now parsed as a comma-separated list, so that both\ncolorizers (\"by age\" and \"by repeated lines\") can be enabled via\nconfiguration. Previously, they could be specified together on the\ncommand line via --color-by-age and --color-lines, but not by config.\n\nSigned-off-by: Robert Estelle \n---\n Documentation/config/blame.txt | 1 +\n builtin/blame.c | 47 +++++++++++++++++++++++++---------\n t/t8012-blame-colors.sh | 8 ++++++\n 3 files changed, 44 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/config/blame.txt b/Documentation/config/blame.txt\nindex 4d047c17908..8624c4dc5d4 100644\n--- a/Documentation/config/blame.txt\n+++ b/Documentation/config/blame.txt\n@@ -6,6 +6,7 @@ blame.coloring::\n \tThis determines the coloring scheme to be applied to blame\n \toutput. It can be 'repeatedLines', 'highlightRecent',\n \tor 'none' which is the default.\n+\tMultiple values may be separated by commas.\n \n blame.date::\n \tSpecifies the format used to output dates in linkgit:git-blame[1].\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 71f925e456c..eea1418d209 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -685,6 +685,39 @@ static const char *add_prefix(const char *prefix, const char *path)\n \treturn prefix_path(prefix, prefix ? strlen(prefix) : 0, path);\n }\n \n+static unsigned parse_blame_coloring_mode(const char *arg)\n+{\n+\tint ret = 0;\n+\tstruct string_list l = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *i;\n+\n+\tstring_list_split(&l, arg, ',', -1);\n+\n+\tfor_each_string_list_item(i, &l) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tstrbuf_addstr(&sb, i->string);\n+\t\tstrbuf_trim(&sb);\n+\n+\t\tif (!strcmp(sb.buf, \"none\")) {\n+\t\t\tret = 0;\n+\t\t} else if (!strcmp(sb.buf, \"repeatedLines\")) {\n+\t\t\tret |= OUTPUT_COLOR_LINE;\n+\t\t} else if (!strcmp(sb.buf, \"highlightRecent\")) {\n+\t\t\tret |= OUTPUT_SHOW_AGE_WITH_COLOR;\n+\t\t} else {\n+\t\t\twarning(_(\"invalid value for '%s': '%s'\"),\n+\t\t\t\t\"blame.coloring\", sb.buf);\n+\t\t\tret = 0;\n+\t\t}\n+\n+\t\tstrbuf_release(&sb);\n+\t}\n+\n+\tstring_list_clear(&l, 0);\n+\n+\treturn ret;\n+}\n+\n static int git_blame_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"blame.showroot\")) {\n@@ -739,18 +772,8 @@ static int git_blame_config(const char *var, const char *value, void *cb)\n \t}\n \n \tif (!strcmp(var, \"blame.coloring\")) {\n-\t\tif (!strcmp(value, \"repeatedLines\")) {\n-\t\t\tcoloring_mode |= OUTPUT_COLOR_LINE;\n-\t\t} else if (!strcmp(value, \"highlightRecent\")) {\n-\t\t\tcoloring_mode |= OUTPUT_SHOW_AGE_WITH_COLOR;\n-\t\t} else if (!strcmp(value, \"none\")) {\n-\t\t\tcoloring_mode &= ~(OUTPUT_COLOR_LINE |\n-\t\t\t\t\t OUTPUT_SHOW_AGE_WITH_COLOR);\n-\t\t} else {\n-\t\t\twarning(_(\"invalid value for '%s': '%s'\"),\n-\t\t\t\t\"blame.coloring\", value);\n-\t\t\treturn 0;\n-\t\t}\n+\t\tcoloring_mode = parse_blame_coloring_mode(value);\n+\t\treturn 0;\n \t}\n \n \tif (git_diff_heuristic_config(var, value, cb) < 0)\ndiff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh\nindex 820f86c3aed..051f734ea03 100755\n--- a/t/t8012-blame-colors.sh\n+++ b/t/t8012-blame-colors.sh\n@@ -89,6 +89,14 @@ test_expect_success 'blame color by age and lines' '\n \t\thello.c \\\n \t\t>actual.raw &&\n \n+\tgit \\\n+\t\t-c color.blame.repeatedLines=blue \\\n+\t\t-c color.blame.highlightRecent=\"yellow,1 month ago, cyan\" \\\n+\t\t-c blame.coloring=highlightRecent,repeatedLines \\\n+\t\tblame hello.c \\\n+\t\t>actual.raw.2 &&\n+\ttest_cmp actual.raw actual.raw.2 &&\n+\n \ttest_decode_color actual &&\n \tnormalize_color_decoded_blame actual.norm &&\n \n-- \ngitgitgadget\n"}]}