{"thread":{"id":"59990","subject":"[PATCH] pretty: add %(decorate[:]) format","startedAt":"2023-07-15T10:39:02Z","lastAt":"2023-09-01T21:50:07Z","messageCount":59,"participants":["Andy Koppe","Junio C Hamano","Glen Choo","Phillip Wood","Taylor Blau","Jeff King","rsbecker@nexbridge.com","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479544","messageId":"20230715103758.3862-1-andy.koppe@gmail.com","threadId":"59990","inReplyTo":null,"subject":"[PATCH] pretty: add %(decorate[:]) format","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2023-07-15T10:37:56Z","receivedAt":"2023-07-15T10:39:02Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"This lists ref names in the same way as the %d decoration format, but\nallows all the otherwise fixed strings printed around the ref names to\nbe customized, namely prefix, suffix, separator, the \"tag:\" annotation\nand the arrow used to show where HEAD points.\n\nExamples:\n- %decorate(prefix=,suffix=) removes the enclosing parentheses, like %D.\n- %decorate(prefix=,suffix=,separator=,tag=,arrow=->) produces a\n space-separated list without wrapping, tag annotations or spaces\n around the arrow.\n- %(decorate:prefix=[,suffix=],separator=%x2C,arrow=%x2C,tag=) produces\n a comma-separated list enclosed in square brackets where the arrow is\n replaced by a comma as well.\n\nAdd functions parse_decoration_option(), parse_decoration_options() and\nfree_decoration_options() to help implement the format. Test it in\nt4205-log-pretty-formats.sh and document it in pretty-formats.txt.\n\nRefactor format_decorations() to take a struct decoration_options\nargument specifying those strings, whereby NULL entries select the\ndefault. Avoid emitting color sequences for empty strings.\n\nWrap tag annotations in separate color sequences from tag names, because\notherwise tag names can end up uncolored when %w width formatting breaks\nlines between annotation and name. Amend t4207-log-decoration-colors.sh\naccordingly.\n\nSigned-off-by: Andy Koppe \n---\nCI: https://github.com/ak2/git/actions/runs/5561647197\n\n Documentation/pretty-formats.txt | 19 ++++++++-\n log-tree.c | 69 ++++++++++++++++++++------------\n log-tree.h | 17 ++++----\n pretty.c | 62 +++++++++++++++++++++++++++-\n t/t4205-log-pretty-formats.sh | 21 ++++++++++\n t/t4207-log-decoration-colors.sh | 32 +++++++++------\n 6 files changed, 171 insertions(+), 49 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 3b71334459..c08aba15af 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -222,7 +222,22 @@ The placeholders are:\n \tlinkgit:git-rev-list[1])\n '%d':: ref names, like the --decorate option of linkgit:git-log[1]\n '%D':: ref names without the \" (\", \")\" wrapping.\n-'%(describe[:options])':: human-readable name, like\n+'%(decorate[:])':: ref names with custom decorations.\n+\t\t\t The `decorate` string may be followed by a colon\n+\t\t\t and zero or more comma-separated options.\n+\t\t\t Option values may contain literal formatting codes.\n+\t\t\t These must be used for commas (`%x2C`) and closing\n+\t\t\t parentheses (`%x29`), due to their role in the option\n+\t\t\t syntax.\n++\n+** 'prefix=': Shown before the list of ref names. Defaults to \" (\".\n+** 'suffix=': Shown after the list of ref names. Defaults to \")\".\n+** 'separator=': Shown between ref names. Defaults to \", \".\n+** 'arrow=': Shown between HEAD and the branch it points to, if any.\n+\t\t Defaults to \" \\-> \".\n+** 'tag=': Shown before tag names. Defaults to \"tag: \".\n+\n+'%(describe[:])':: human-readable name, like\n \t\t\t linkgit:git-describe[1]; empty string for\n \t\t\t undescribable commits. The `describe` string\n \t\t\t may be followed by a colon and zero or more\n@@ -281,7 +296,7 @@ endif::git-rev-list[]\n '%gE':: reflog identity email (respecting .mailmap, see\n \tlinkgit:git-shortlog[1] or linkgit:git-blame[1])\n '%gs':: reflog subject\n-'%(trailers[:options])':: display the trailers of the body as\n+'%(trailers[:])':: display the trailers of the body as\n \t\t\t interpreted by\n \t\t\t linkgit:git-interpret-trailers[1]. The\n \t\t\t `trailers` string may be followed by a colon\ndiff --git a/log-tree.c b/log-tree.c\nindex f4b22a60cc..4b46884ef6 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -301,27 +301,34 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio\n \n /*\n * The caller makes sure there is no funny color before calling.\n- * format_decorations_extended makes sure the same after return.\n+ * format_decorations ensures the same after return.\n */\n-void format_decorations_extended(struct strbuf *sb,\n+void format_decorations(struct strbuf *sb,\n \t\t\tconst struct commit *commit,\n \t\t\tint use_color,\n-\t\t\tconst char *prefix,\n-\t\t\tconst char *separator,\n-\t\t\tconst char *suffix)\n+\t\t\tconst struct decoration_options *opts)\n {\n-\tconst struct name_decoration *decoration;\n-\tconst struct name_decoration *current_and_HEAD;\n-\tconst char *color_commit =\n-\t\tdiff_get_color(use_color, DIFF_COMMIT);\n-\tconst char *color_reset =\n-\t\tdecorate_get_color(use_color, DECORATION_NONE);\n+\tconst char *color_commit, *color_reset;\n+\tconst char *prefix, *suffix, *separator, *arrow, *tag;\n+\n+\tconst struct name_decoration *current_and_HEAD;\n+\tconst struct name_decoration *decoration =\n+\t\tget_name_decoration(&commit->object);\n \n-\tdecoration = get_name_decoration(&commit->object);\n \tif (!decoration)\n \t\treturn;\n \n+\tcolor_commit = diff_get_color(use_color, DIFF_COMMIT);\n+\tcolor_reset = decorate_get_color(use_color, DECORATION_NONE);\n+\n+\tprefix = (opts && opts->prefix) ? opts->prefix : \" (\";\n+\tsuffix = (opts && opts->suffix) ? opts->suffix : \")\";\n+\tseparator = (opts && opts->separator) ? opts->separator : \", \";\n+\tarrow = (opts && opts->arrow) ? opts->arrow : \" -> \";\n+\ttag = (opts && opts->tag) ? opts->tag : \"tag: \";\n+\n \tcurrent_and_HEAD = current_pointed_by_HEAD(decoration);\n+\n \twhile (decoration) {\n \t\t/*\n \t\t * When both current and HEAD are there, only\n@@ -329,20 +336,29 @@ void format_decorations_extended(struct strbuf *sb,\n \t\t * appeared, skipping the entry for current.\n \t\t */\n \t\tif (decoration != current_and_HEAD) {\n-\t\t\tstrbuf_addstr(sb, color_commit);\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\t\tstrbuf_addstr(sb, \"tag: \");\n+\t\t\tconst char *color =\n+\t\t\t\tdecorate_get_color(use_color, decoration->type);\n \n+\t\t\tif (*prefix) {\n+\t\t\t\tstrbuf_addstr(sb, color_commit);\n+\t\t\t\tstrbuf_addstr(sb, prefix);\n+\t\t\t\tstrbuf_addstr(sb, color_reset);\n+\t\t\t}\n+\n+\t\t\tif (*tag && decoration->type == DECORATION_REF_TAG) {\n+\t\t\t\tstrbuf_addstr(sb, color);\n+\t\t\t\tstrbuf_addstr(sb, tag);\n+\t\t\t\tstrbuf_addstr(sb, color_reset);\n+\t\t\t}\n+\t\t\tstrbuf_addstr(sb, color);\n \t\t\tshow_name(sb, decoration);\n \n-\t\t\tif (current_and_HEAD &&\n+\t\t\tif (*arrow && current_and_HEAD &&\n \t\t\t decoration->type == DECORATION_REF_HEAD) {\n-\t\t\t\tstrbuf_addstr(sb, \" -> \");\n+\t\t\t\tstrbuf_addstr(sb, arrow);\n \t\t\t\tstrbuf_addstr(sb, color_reset);\n-\t\t\t\tstrbuf_addstr(sb, decorate_get_color(use_color, current_and_HEAD->type));\n+\t\t\t\tstrbuf_addstr(sb, decorate_get_color(\n+\t\t\t\t\tuse_color, current_and_HEAD->type));\n \t\t\t\tshow_name(sb, current_and_HEAD);\n \t\t\t}\n \t\t\tstrbuf_addstr(sb, color_reset);\n@@ -351,9 +367,12 @@ void format_decorations_extended(struct strbuf *sb,\n \t\t}\n \t\tdecoration = decoration->next;\n \t}\n-\tstrbuf_addstr(sb, color_commit);\n-\tstrbuf_addstr(sb, suffix);\n-\tstrbuf_addstr(sb, color_reset);\n+\n+\tif (*suffix) {\n+\t\tstrbuf_addstr(sb, color_commit);\n+\t\tstrbuf_addstr(sb, suffix);\n+\t\tstrbuf_addstr(sb, color_reset);\n+\t}\n }\n \n void show_decorations(struct rev_info *opt, struct commit *commit)\n@@ -368,7 +387,7 @@ void show_decorations(struct rev_info *opt, struct commit *commit)\n \t}\n \tif (!opt->show_decorations)\n \t\treturn;\n-\tformat_decorations(&sb, commit, opt->diffopt.use_color);\n+\tformat_decorations(&sb, commit, opt->diffopt.use_color, NULL);\n \tfputs(sb.buf, opt->diffopt.file);\n \tstrbuf_release(&sb);\n }\ndiff --git a/log-tree.h b/log-tree.h\nindex e7e4641cf8..39ab06a3ca 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -13,17 +13,20 @@ struct decoration_filter {\n \tstruct string_list *exclude_ref_config_pattern;\n };\n \n+struct decoration_options {\n+\tchar *prefix;\n+\tchar *suffix;\n+\tchar *separator;\n+\tchar *arrow;\n+\tchar *tag;\n+};\n+\n int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);\n int log_tree_diff_flush(struct rev_info *);\n int log_tree_commit(struct rev_info *, struct commit *);\n void show_log(struct rev_info *opt);\n-void format_decorations_extended(struct strbuf *sb, const struct commit *commit,\n-\t\t\t int use_color,\n-\t\t\t const char *prefix,\n-\t\t\t const char *separator,\n-\t\t\t const char *suffix);\n-#define format_decorations(strbuf, commit, color) \\\n-\t\t\t format_decorations_extended((strbuf), (commit), (color), \" (\", \", \", \")\")\n+void format_decorations(struct strbuf *sb, const struct commit *commit,\n+\t\t\tint use_color, const struct decoration_options *opts);\n void show_decorations(struct rev_info *opt, struct commit *commit);\n void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t const char **extra_headers_p,\ndiff --git a/pretty.c b/pretty.c\nindex 0bb938021b..a59b7f0dbc 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1373,6 +1373,46 @@ static size_t parse_describe_args(const char *start, struct strvec *args)\n \treturn arg - start;\n }\n \n+\n+static int parse_decoration_option(const char **arg,\n+\t\t\t\t const char *name,\n+\t\t\t\t char **opt)\n+{\n+\tconst char *argval;\n+\tsize_t arglen;\n+\n+\tif (match_placeholder_arg_value(*arg, name, arg, &argval, &arglen)) {\n+\t\tchar *val = xstrndup(argval, arglen);\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tstrbuf_expand(&sb, val, strbuf_expand_literal_cb, NULL);\n+\t\tfree(val);\n+\t\t*opt = strbuf_detach(&sb, NULL);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static void parse_decoration_options(const char **arg,\n+\t\t\t\t struct decoration_options *opts)\n+{\n+\twhile (parse_decoration_option(arg, \"prefix\", &opts->prefix) ||\n+\t parse_decoration_option(arg, \"suffix\", &opts->suffix) ||\n+\t parse_decoration_option(arg, \"separator\", &opts->separator) ||\n+\t parse_decoration_option(arg, \"arrow\", &opts->arrow) ||\n+\t parse_decoration_option(arg, \"tag\", &opts->tag))\n+\t\t;\n+}\n+\n+static void free_decoration_options(const struct decoration_options *opts)\n+{\n+\tfree(opts->prefix);\n+\tfree(opts->suffix);\n+\tfree(opts->separator);\n+\tfree(opts->arrow);\n+\tfree(opts->tag);\n+}\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1526,10 +1566,11 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\tstrbuf_addstr(sb, get_revision_mark(NULL, commit));\n \t\treturn 1;\n \tcase 'd':\n-\t\tformat_decorations(sb, commit, c->auto_color);\n+\t\tformat_decorations(sb, commit, c->auto_color, NULL);\n \t\treturn 1;\n \tcase 'D':\n-\t\tformat_decorations_extended(sb, commit, c->auto_color, \"\", \", \", \"\");\n+\t\tformat_decorations(sb, commit, c->auto_color,\n+\t\t\t\t &(struct decoration_options){\"\", \"\"});\n \t\treturn 1;\n \tcase 'S':\t\t/* tag/branch like --source */\n \t\tif (!(c->pretty_ctx->rev && c->pretty_ctx->rev->sources))\n@@ -1627,6 +1668,23 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\treturn 2;\n \t}\n \n+\tif (skip_prefix(placeholder, \"(decorate\", &arg)) {\n+\t\tstruct decoration_options opts = { NULL };\n+\t\tsize_t ret = 0;\n+\n+\t\tif (*arg == ':') {\n+\t\t\targ++;\n+\t\t\tparse_decoration_options(&arg, &opts);\n+\t\t}\n+\t\tif (*arg == ')') {\n+\t\t\tformat_decorations(sb, commit, c->auto_color, &opts);\n+\t\t\tret = arg - placeholder + 1;\n+\t\t}\n+\n+\t\tfree_decoration_options(&opts);\n+\t\treturn ret;\n+\t}\n+\n \t/* For the rest we have to parse the commit header. */\n \tif (!c->commit_header_parsed) {\n \t\tmsg = c->message =\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 4cf8a77667..5ea937648a 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -576,6 +576,27 @@ test_expect_success 'clean log decoration' '\n \ttest_cmp expected actual1\n '\n \n+test_expect_success 'pretty format %decorate' '\n+\tgit checkout -b foo &&\n+\tgit commit --allow-empty -m \"new commit\" &&\n+\tgit tag bar &&\n+\tgit branch qux &&\n+\techo \" (HEAD -> foo, tag: bar, qux)\" >expect1 &&\n+\tgit log --format=\"%(decorate)\" -1 >actual1 &&\n+\ttest_cmp expect1 actual1 &&\n+\techo \"HEAD -> foo, tag: bar, qux\" >expect2 &&\n+\tgit log --format=\"%(decorate:prefix=,suffix=)\" -1 >actual2 &&\n+\ttest_cmp expect2 actual2 &&\n+\techo \"HEAD->foo bar qux\" >expect3 &&\n+\tgit log --format=\"%(decorate:prefix=,suffix=,separator= ,arrow=->,tag=)\" \\\n+\t\t-1 >actual3 &&\n+\ttest_cmp expect3 actual3 &&\n+\techo \"[HEAD,foo,bar,qux]\" >expect4 &&\n+\tgit log --format=\"%(decorate:prefix=[,suffix=],separator=%x2C,arrow=%x2C,tag=)\" \\\n+\t\t-1 >actual4 &&\n+\ttest_cmp expect4 actual4\n+'\n+\n cat >trailers <\n Acked-by: A U Thor \ndiff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh\nindex ded33a82e2..3a4eedc494 100755\n--- a/t/t4207-log-decoration-colors.sh\n+++ b/t/t4207-log-decoration-colors.sh\n@@ -55,13 +55,15 @@ 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}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B\n+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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}\\\n+${c_stash}refs/stash${c_reset}${c_commit})${c_reset} On main: Changes to A.t\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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 +80,12 @@ 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}tag: ${c_reset}${c_tag}D${c_reset}${c_commit})${c_reset} D\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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}\\\n+${c_tag}tag: ${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 +106,13 @@ 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}tag: ${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}\\\n+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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 &&\n-- \n2.41.0\n\n"},{"id":"479545","messageId":"20230715160730.4046-1-andy.koppe@gmail.com","threadId":"59990","inReplyTo":"20230715103758.3862-1-andy.koppe@gmail.com","subject":"[PATCH v2] pretty: add %(decorate[:]) format","fromName":"Andy Koppe","fromEmail":"andy.koppe@gmail.com","sentAt":"2023-07-15T16:07:29Z","receivedAt":"2023-07-15T16:07:51Z","isPatch":true,"sender":{"key":"andy.koppe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223411?v=4"},"body":"This lists ref names in the same way as the %d decoration format, but\nallows all the otherwise fixed strings printed around the ref names to\nbe customized, namely prefix, suffix, separator, the \"tag:\" annotation\nand the arrow used to show where HEAD points.\n\nExamples:\n- %(decorate) is equivalent to %d.\n- %(decorate:prefix=,suffix=) is equivalent to %D.\n- %(decorate:prefix=,suffix=,separator= ,tag=,arrow=->) produces a\n space-separated list without wrapping, tag annotations or spaces\n around the arrow.\n- %(decorate:prefix=[,suffix=],separator=%x2C,arrow=%x2C,tag=) produces\n a comma-separated list enclosed in square brackets where the arrow is\n replaced by a comma as well.\n\nAdd functions parse_decoration_option(), parse_decoration_options() and\nfree_decoration_options() to help implement the format. Test it in\nt4205-log-pretty-formats.sh and document it in pretty-formats.txt.\n\nRefactor format_decorations() to take a struct decoration_options\nargument specifying those strings, whereby NULL entries select the\ndefault. Avoid emitting color sequences for empty strings.\n\nWrap tag annotations in separate color sequences from tag names, because\notherwise tag names can end up uncolored when %w width formatting breaks\nlines between annotation and name. Amend t4207-log-decoration-colors.sh\naccordingly.\n\nSigned-off-by: Andy Koppe \n---\nCorrected mistakes in commit description.\n\n Documentation/pretty-formats.txt | 19 ++++++++-\n log-tree.c | 69 ++++++++++++++++++++------------\n log-tree.h | 17 ++++----\n pretty.c | 62 +++++++++++++++++++++++++++-\n t/t4205-log-pretty-formats.sh | 21 ++++++++++\n t/t4207-log-decoration-colors.sh | 32 +++++++++------\n 6 files changed, 171 insertions(+), 49 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 3b71334459..c08aba15af 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -222,7 +222,22 @@ The placeholders are:\n \tlinkgit:git-rev-list[1])\n '%d':: ref names, like the --decorate option of linkgit:git-log[1]\n '%D':: ref names without the \" (\", \")\" wrapping.\n-'%(describe[:options])':: human-readable name, like\n+'%(decorate[:])':: ref names with custom decorations.\n+\t\t\t The `decorate` string may be followed by a colon\n+\t\t\t and zero or more comma-separated options.\n+\t\t\t Option values may contain literal formatting codes.\n+\t\t\t These must be used for commas (`%x2C`) and closing\n+\t\t\t parentheses (`%x29`), due to their role in the option\n+\t\t\t syntax.\n++\n+** 'prefix=': Shown before the list of ref names. Defaults to \" (\".\n+** 'suffix=': Shown after the list of ref names. Defaults to \")\".\n+** 'separator=': Shown between ref names. Defaults to \", \".\n+** 'arrow=': Shown between HEAD and the branch it points to, if any.\n+\t\t Defaults to \" \\-> \".\n+** 'tag=': Shown before tag names. Defaults to \"tag: \".\n+\n+'%(describe[:])':: human-readable name, like\n \t\t\t linkgit:git-describe[1]; empty string for\n \t\t\t undescribable commits. The `describe` string\n \t\t\t may be followed by a colon and zero or more\n@@ -281,7 +296,7 @@ endif::git-rev-list[]\n '%gE':: reflog identity email (respecting .mailmap, see\n \tlinkgit:git-shortlog[1] or linkgit:git-blame[1])\n '%gs':: reflog subject\n-'%(trailers[:options])':: display the trailers of the body as\n+'%(trailers[:])':: display the trailers of the body as\n \t\t\t interpreted by\n \t\t\t linkgit:git-interpret-trailers[1]. The\n \t\t\t `trailers` string may be followed by a colon\ndiff --git a/log-tree.c b/log-tree.c\nindex f4b22a60cc..4b46884ef6 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -301,27 +301,34 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio\n \n /*\n * The caller makes sure there is no funny color before calling.\n- * format_decorations_extended makes sure the same after return.\n+ * format_decorations ensures the same after return.\n */\n-void format_decorations_extended(struct strbuf *sb,\n+void format_decorations(struct strbuf *sb,\n \t\t\tconst struct commit *commit,\n \t\t\tint use_color,\n-\t\t\tconst char *prefix,\n-\t\t\tconst char *separator,\n-\t\t\tconst char *suffix)\n+\t\t\tconst struct decoration_options *opts)\n {\n-\tconst struct name_decoration *decoration;\n-\tconst struct name_decoration *current_and_HEAD;\n-\tconst char *color_commit =\n-\t\tdiff_get_color(use_color, DIFF_COMMIT);\n-\tconst char *color_reset =\n-\t\tdecorate_get_color(use_color, DECORATION_NONE);\n+\tconst char *color_commit, *color_reset;\n+\tconst char *prefix, *suffix, *separator, *arrow, *tag;\n+\n+\tconst struct name_decoration *current_and_HEAD;\n+\tconst struct name_decoration *decoration =\n+\t\tget_name_decoration(&commit->object);\n \n-\tdecoration = get_name_decoration(&commit->object);\n \tif (!decoration)\n \t\treturn;\n \n+\tcolor_commit = diff_get_color(use_color, DIFF_COMMIT);\n+\tcolor_reset = decorate_get_color(use_color, DECORATION_NONE);\n+\n+\tprefix = (opts && opts->prefix) ? opts->prefix : \" (\";\n+\tsuffix = (opts && opts->suffix) ? opts->suffix : \")\";\n+\tseparator = (opts && opts->separator) ? opts->separator : \", \";\n+\tarrow = (opts && opts->arrow) ? opts->arrow : \" -> \";\n+\ttag = (opts && opts->tag) ? opts->tag : \"tag: \";\n+\n \tcurrent_and_HEAD = current_pointed_by_HEAD(decoration);\n+\n \twhile (decoration) {\n \t\t/*\n \t\t * When both current and HEAD are there, only\n@@ -329,20 +336,29 @@ void format_decorations_extended(struct strbuf *sb,\n \t\t * appeared, skipping the entry for current.\n \t\t */\n \t\tif (decoration != current_and_HEAD) {\n-\t\t\tstrbuf_addstr(sb, color_commit);\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\t\tstrbuf_addstr(sb, \"tag: \");\n+\t\t\tconst char *color =\n+\t\t\t\tdecorate_get_color(use_color, decoration->type);\n \n+\t\t\tif (*prefix) {\n+\t\t\t\tstrbuf_addstr(sb, color_commit);\n+\t\t\t\tstrbuf_addstr(sb, prefix);\n+\t\t\t\tstrbuf_addstr(sb, color_reset);\n+\t\t\t}\n+\n+\t\t\tif (*tag && decoration->type == DECORATION_REF_TAG) {\n+\t\t\t\tstrbuf_addstr(sb, color);\n+\t\t\t\tstrbuf_addstr(sb, tag);\n+\t\t\t\tstrbuf_addstr(sb, color_reset);\n+\t\t\t}\n+\t\t\tstrbuf_addstr(sb, color);\n \t\t\tshow_name(sb, decoration);\n \n-\t\t\tif (current_and_HEAD &&\n+\t\t\tif (*arrow && current_and_HEAD &&\n \t\t\t decoration->type == DECORATION_REF_HEAD) {\n-\t\t\t\tstrbuf_addstr(sb, \" -> \");\n+\t\t\t\tstrbuf_addstr(sb, arrow);\n \t\t\t\tstrbuf_addstr(sb, color_reset);\n-\t\t\t\tstrbuf_addstr(sb, decorate_get_color(use_color, current_and_HEAD->type));\n+\t\t\t\tstrbuf_addstr(sb, decorate_get_color(\n+\t\t\t\t\tuse_color, current_and_HEAD->type));\n \t\t\t\tshow_name(sb, current_and_HEAD);\n \t\t\t}\n \t\t\tstrbuf_addstr(sb, color_reset);\n@@ -351,9 +367,12 @@ void format_decorations_extended(struct strbuf *sb,\n \t\t}\n \t\tdecoration = decoration->next;\n \t}\n-\tstrbuf_addstr(sb, color_commit);\n-\tstrbuf_addstr(sb, suffix);\n-\tstrbuf_addstr(sb, color_reset);\n+\n+\tif (*suffix) {\n+\t\tstrbuf_addstr(sb, color_commit);\n+\t\tstrbuf_addstr(sb, suffix);\n+\t\tstrbuf_addstr(sb, color_reset);\n+\t}\n }\n \n void show_decorations(struct rev_info *opt, struct commit *commit)\n@@ -368,7 +387,7 @@ void show_decorations(struct rev_info *opt, struct commit *commit)\n \t}\n \tif (!opt->show_decorations)\n \t\treturn;\n-\tformat_decorations(&sb, commit, opt->diffopt.use_color);\n+\tformat_decorations(&sb, commit, opt->diffopt.use_color, NULL);\n \tfputs(sb.buf, opt->diffopt.file);\n \tstrbuf_release(&sb);\n }\ndiff --git a/log-tree.h b/log-tree.h\nindex e7e4641cf8..39ab06a3ca 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -13,17 +13,20 @@ struct decoration_filter {\n \tstruct string_list *exclude_ref_config_pattern;\n };\n \n+struct decoration_options {\n+\tchar *prefix;\n+\tchar *suffix;\n+\tchar *separator;\n+\tchar *arrow;\n+\tchar *tag;\n+};\n+\n int parse_decorate_color_config(const char *var, const char *slot_name, const char *value);\n int log_tree_diff_flush(struct rev_info *);\n int log_tree_commit(struct rev_info *, struct commit *);\n void show_log(struct rev_info *opt);\n-void format_decorations_extended(struct strbuf *sb, const struct commit *commit,\n-\t\t\t int use_color,\n-\t\t\t const char *prefix,\n-\t\t\t const char *separator,\n-\t\t\t const char *suffix);\n-#define format_decorations(strbuf, commit, color) \\\n-\t\t\t format_decorations_extended((strbuf), (commit), (color), \" (\", \", \", \")\")\n+void format_decorations(struct strbuf *sb, const struct commit *commit,\n+\t\t\tint use_color, const struct decoration_options *opts);\n void show_decorations(struct rev_info *opt, struct commit *commit);\n void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t const char **extra_headers_p,\ndiff --git a/pretty.c b/pretty.c\nindex 0bb938021b..a59b7f0dbc 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1373,6 +1373,46 @@ static size_t parse_describe_args(const char *start, struct strvec *args)\n \treturn arg - start;\n }\n \n+\n+static int parse_decoration_option(const char **arg,\n+\t\t\t\t const char *name,\n+\t\t\t\t char **opt)\n+{\n+\tconst char *argval;\n+\tsize_t arglen;\n+\n+\tif (match_placeholder_arg_value(*arg, name, arg, &argval, &arglen)) {\n+\t\tchar *val = xstrndup(argval, arglen);\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tstrbuf_expand(&sb, val, strbuf_expand_literal_cb, NULL);\n+\t\tfree(val);\n+\t\t*opt = strbuf_detach(&sb, NULL);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static void parse_decoration_options(const char **arg,\n+\t\t\t\t struct decoration_options *opts)\n+{\n+\twhile (parse_decoration_option(arg, \"prefix\", &opts->prefix) ||\n+\t parse_decoration_option(arg, \"suffix\", &opts->suffix) ||\n+\t parse_decoration_option(arg, \"separator\", &opts->separator) ||\n+\t parse_decoration_option(arg, \"arrow\", &opts->arrow) ||\n+\t parse_decoration_option(arg, \"tag\", &opts->tag))\n+\t\t;\n+}\n+\n+static void free_decoration_options(const struct decoration_options *opts)\n+{\n+\tfree(opts->prefix);\n+\tfree(opts->suffix);\n+\tfree(opts->separator);\n+\tfree(opts->arrow);\n+\tfree(opts->tag);\n+}\n+\n static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\t\tconst char *placeholder,\n \t\t\t\tvoid *context)\n@@ -1526,10 +1566,11 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\tstrbuf_addstr(sb, get_revision_mark(NULL, commit));\n \t\treturn 1;\n \tcase 'd':\n-\t\tformat_decorations(sb, commit, c->auto_color);\n+\t\tformat_decorations(sb, commit, c->auto_color, NULL);\n \t\treturn 1;\n \tcase 'D':\n-\t\tformat_decorations_extended(sb, commit, c->auto_color, \"\", \", \", \"\");\n+\t\tformat_decorations(sb, commit, c->auto_color,\n+\t\t\t\t &(struct decoration_options){\"\", \"\"});\n \t\treturn 1;\n \tcase 'S':\t\t/* tag/branch like --source */\n \t\tif (!(c->pretty_ctx->rev && c->pretty_ctx->rev->sources))\n@@ -1627,6 +1668,23 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\treturn 2;\n \t}\n \n+\tif (skip_prefix(placeholder, \"(decorate\", &arg)) {\n+\t\tstruct decoration_options opts = { NULL };\n+\t\tsize_t ret = 0;\n+\n+\t\tif (*arg == ':') {\n+\t\t\targ++;\n+\t\t\tparse_decoration_options(&arg, &opts);\n+\t\t}\n+\t\tif (*arg == ')') {\n+\t\t\tformat_decorations(sb, commit, c->auto_color, &opts);\n+\t\t\tret = arg - placeholder + 1;\n+\t\t}\n+\n+\t\tfree_decoration_options(&opts);\n+\t\treturn ret;\n+\t}\n+\n \t/* For the rest we have to parse the commit header. */\n \tif (!c->commit_header_parsed) {\n \t\tmsg = c->message =\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 4cf8a77667..5ea937648a 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -576,6 +576,27 @@ test_expect_success 'clean log decoration' '\n \ttest_cmp expected actual1\n '\n \n+test_expect_success 'pretty format %decorate' '\n+\tgit checkout -b foo &&\n+\tgit commit --allow-empty -m \"new commit\" &&\n+\tgit tag bar &&\n+\tgit branch qux &&\n+\techo \" (HEAD -> foo, tag: bar, qux)\" >expect1 &&\n+\tgit log --format=\"%(decorate)\" -1 >actual1 &&\n+\ttest_cmp expect1 actual1 &&\n+\techo \"HEAD -> foo, tag: bar, qux\" >expect2 &&\n+\tgit log --format=\"%(decorate:prefix=,suffix=)\" -1 >actual2 &&\n+\ttest_cmp expect2 actual2 &&\n+\techo \"HEAD->foo bar qux\" >expect3 &&\n+\tgit log --format=\"%(decorate:prefix=,suffix=,separator= ,arrow=->,tag=)\" \\\n+\t\t-1 >actual3 &&\n+\ttest_cmp expect3 actual3 &&\n+\techo \"[HEAD,foo,bar,qux]\" >expect4 &&\n+\tgit log --format=\"%(decorate:prefix=[,suffix=],separator=%x2C,arrow=%x2C,tag=)\" \\\n+\t\t-1 >actual4 &&\n+\ttest_cmp expect4 actual4\n+'\n+\n cat >trailers <\n Acked-by: A U Thor \ndiff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh\nindex ded33a82e2..3a4eedc494 100755\n--- a/t/t4207-log-decoration-colors.sh\n+++ b/t/t4207-log-decoration-colors.sh\n@@ -55,13 +55,15 @@ 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}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B\n+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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}\\\n+${c_stash}refs/stash${c_reset}${c_commit})${c_reset} On main: Changes to A.t\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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 +80,12 @@ 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}tag: ${c_reset}${c_tag}D${c_reset}${c_commit})${c_reset} D\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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}\\\n+${c_tag}tag: ${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 +106,13 @@ 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}tag: ${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}\\\n+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \\\n+${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B\n+\t${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\\\n+${c_tag}tag: ${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 &&\n-- \n2.41.0\n\n"},{"id":"479590","messageId":"xmqqmszudtih.fsf@gitster.g","threadId":"59990","inReplyTo":"20230715160730.4046-1-andy.koppe@gmail.com","subject":"Re: [PATCH v2] pretty: add %(decorate[:]) format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-17T23:10:14Z","receivedAt":"2023-07-17T23:11:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Koppe writes:\n\n> This lists ref names in the same way as the %d decoration format, but\n\nPlease replace \"This\" with \"%(descorate[:]\", i.e. a more\nconcrete form, so that people do not have to go back to the title in\norder to understand the body of the proposed log message.\n\n> -'%(describe[:options])':: human-readable name, like\n> +'%(describe[:])':: human-readable name, like\n> -'%(trailers[:options])':: display the trailers of the body as\n> +'%(trailers[:])':: display the trailers of the body as\n\nIt is a very good idea to signal that is a placeholder by\nenclosing it inside like this patch wants to do with\n%(decorate), and to make sure that other existing ones consistently\nfollow the same convention.\n\nBut the latter, being very small, can be buried in the noise.\n\nIt may be a good idea to have small \"preliminary clean-up\" patches\nthat do not add anything related to %(decorate) at the beginning of\nthe series. [PATCH 1/3] can be %(token[:]) clean-up,\n[PATCH 2/3] can be \"what is literal formatting code\" clarification,\nand [PATCH 3/3] can be the rest of this patch, for example.\n\n> +'%(decorate[:])':: ref names with custom decorations.\n> +\t\t\t The `decorate` string may be followed by a colon\n> +\t\t\t and zero or more comma-separated options.\n> +\t\t\t Option values may contain literal formatting codes.\n> +\t\t\t These must be used for commas (`%x2C`) and closing\n> +\t\t\t parentheses (`%x29`), due to their role in the option\n> +\t\t\t syntax.\n\nOK. I fear that \"literal formatting codes\" may not be understood by\nreaders without having any cross references here. Perhaps something\nlike the patch attached at the end of this message would help.\n\n> +** 'arrow=': Shown between HEAD and the branch it points to, if any.\n> +\t\t Defaults to \" \\-> \".\n\nIt feels a bit strange that this feature is limited only to \"HEAD\"\nand other symbolic refs are not annotated with an arrow, but it is\nnot the fault of this patch. We might want to see if it is worth to\nextend this to other symbolic refs but not while this patch is being\ndiscussed and polished.\n\n> diff --git a/log-tree.c b/log-tree.c\n> index f4b22a60cc..4b46884ef6 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -301,27 +301,34 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio\n> \n> /*\n> * The caller makes sure there is no funny color before calling.\n> - * format_decorations_extended makes sure the same after return.\n> + * format_decorations ensures the same after return.\n> */\n> -void format_decorations_extended(struct strbuf *sb,\n> +void format_decorations(struct strbuf *sb,\n> \t\t\tconst struct commit *commit,\n> \t\t\tint use_color,\n> -\t\t\tconst char *prefix,\n> -\t\t\tconst char *separator,\n> -\t\t\tconst char *suffix)\n> +\t\t\tconst struct decoration_options *opts)\n\nHmph, presumably the idea is to collect these parameters in a struct\nand pass it around, which would be easier to extend, and then teach\nthe hardcoded default to the callee, instead of the macro. OK.\n\nIt may have made the change easier to review if such a change that\ncan cleanly separable into a single step into a single preliminary\nclean-up patch before we start adding the customization, but let's\nread on to see if I can keep everything in my head---I'll complain\nat the end if I can't ;-).\n\n> {\n> -\tconst struct name_decoration *decoration;\n> -\tconst struct name_decoration *current_and_HEAD;\n> -\tconst char *color_commit =\n> -\t\tdiff_get_color(use_color, DIFF_COMMIT);\n> -\tconst char *color_reset =\n> -\t\tdecorate_get_color(use_color, DECORATION_NONE);\n\nGetting rid of the computation of the initialization value from the\nabove decl block, ...\n\n\n> +\tconst char *color_commit, *color_reset;\n> +\tconst char *prefix, *suffix, *separator, *arrow, *tag;\n> +\n> +\tconst struct name_decoration *current_and_HEAD;\n\n... like this, so we will return early without wasting extra cycles\nwhose result we will not use, is a very good idea. But then, ...\n\n> +\tconst struct name_decoration *decoration =\n> +\t\tget_name_decoration(&commit->object);\n> \n> -\tdecoration = get_name_decoration(&commit->object);\n> \tif (!decoration)\n> \t\treturn;\n\n... I think the original is easier to follow than the updated form\nfor the \"decoration\" variable, simply because the declaration part\nwill become absolutely free, and it becomes easier to see that the\ncomputation of \"decoration\" is the very first thing the cycles are\nspent on.\n\n> +\tcolor_commit = diff_get_color(use_color, DIFF_COMMIT);\n> +\tcolor_reset = decorate_get_color(use_color, DECORATION_NONE);\n> +\n> +\tprefix = (opts && opts->prefix) ? opts->prefix : \" (\";\n> +\tsuffix = (opts && opts->suffix) ? opts->suffix : \")\";\n> +\tseparator = (opts && opts->separator) ? opts->separator : \", \";\n\nKnowing these hardcoded values were the responsibility of the\nformat_decorations() C preprocessor macro; now it is written here.\nIt is a moral no-op change from the caller's point of view.\n\n> +\tarrow = (opts && opts->arrow) ? opts->arrow : \" -> \";\n> +\ttag = (opts && opts->tag) ? opts->tag : \"tag: \";\n\nThese two are new. That is one thing why I wondered above if it is\na good idea to separate the \"refactor to introduce\ndecoration_options structure that has three members and replace\nthree parameters to this function with it, so that we can get rid of\nthe format_decorations() macro\" into a single preliminary step.\nThen the reviewers can go on, after being convinced that such a\nmoral no-op refactoring is correct, to review the next step that\nwould presumably add these two members to the option struct and make\nthese customizable.\n\n> \tcurrent_and_HEAD = current_pointed_by_HEAD(decoration);\n> +\n> \twhile (decoration) {\n> \t\t/*\n> \t\t * When both current and HEAD are there, only\n\nUnrelated noise.\n\n> @@ -329,20 +336,29 @@ void format_decorations_extended(struct strbuf *sb,\n> \t\t * appeared, skipping the entry for current.\n> \t\t */\n> \t\tif (decoration != current_and_HEAD) {\n> -\t\t\tstrbuf_addstr(sb, color_commit);\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\t\tstrbuf_addstr(sb, \"tag: \");\n> +\t\t\tconst char *color =\n> +\t\t\t\tdecorate_get_color(use_color, decoration->type);\n> \n> +\t\t\tif (*prefix) {\n> +\t\t\t\tstrbuf_addstr(sb, color_commit);\n> +\t\t\t\tstrbuf_addstr(sb, prefix);\n> +\t\t\t\tstrbuf_addstr(sb, color_reset);\n> +\t\t\t}\n> +\n> +\t\t\tif (*tag && decoration->type == DECORATION_REF_TAG) {\n> +\t\t\t\tstrbuf_addstr(sb, color);\n> +\t\t\t\tstrbuf_addstr(sb, tag);\n> +\t\t\t\tstrbuf_addstr(sb, color_reset);\n> +\t\t\t}\n> +\t\t\tstrbuf_addstr(sb, color);\n> \t\t\tshow_name(sb, decoration);\n\nAgain, this mixes adding new things (i.e. customizeable \"tag:\"\nstring, and \"->\" we see below) and improving existing things\n(i.e. \"\" that is presumably pointless when prefix is\nan empty string is shown as an empty string). Ideally, the step to\nmove the three existing parameters to three members of the new\nstruct should be done first WITHOUT the empty-string improvement,\nthen another step should do the empty-string improvement (at which\ntime, presumably existing test script may have to be adjusted), and\nthen new features to costumize \"tag:\" and \"->\" should be added on\ntop.\n\n> -\t\t\tif (current_and_HEAD &&\n> +\t\t\tif (*arrow && current_and_HEAD &&\n> \t\t\t decoration->type == DECORATION_REF_HEAD) {\n\nBecause arrow is never allowed to be NULL, remove the above change,\nand ...\n\n> -\t\t\t\tstrbuf_addstr(sb, \" -> \");\n> +\t\t\t\tstrbuf_addstr(sb, arrow);\n\n... let the program crash to catch a future bug at runtime.\n\n> @@ -351,9 +367,12 @@ void format_decorations_extended(struct strbuf *sb,\n> \t\t}\n> \t\tdecoration = decoration->next;\n> \t}\n> -\tstrbuf_addstr(sb, color_commit);\n> -\tstrbuf_addstr(sb, suffix);\n> -\tstrbuf_addstr(sb, color_reset);\n> +\n> +\tif (*suffix) {\n> +\t\tstrbuf_addstr(sb, color_commit);\n> +\t\tstrbuf_addstr(sb, suffix);\n> +\t\tstrbuf_addstr(sb, color_reset);\n> +\t}\n\nDitto about \"improving the empty sequence is a\nseparate change from making various fields customizable\".\n\nI'll stop here. After skimming the changes to the test, I think this\nsingle patch should be split into separate steps. Perhaps the split\nshould go like this:\n\n * documentation clean-up %(token[:options]) -> %(token[:])\n plus clarification of what a \"literal formatting code\" is.\n\n * introduction of \"struct decoration_option\",\n removing the format_decorations() macro,\n renaming format_decorations_extended() to format_decorations(),\n replacing the three parameters with a single struct pointer.\n\n * improving string when the meat of the string is\n empty. This step will be the FIRST step that changes the\n externally visible behaviour, and presumably will have adjustment\n to existing tests.\n\n * making \"tag:\" and \"->\" customizable, if values are passed in the\n struct. This step does not have UI changes, and the existing\n tests should serve as a safety net to catch mistakes in this\n step.\n\n * read the %(describe[: