{"thread":{"id":"61124","subject":"[PATCH 0/5] Unify trailer formatting functions","startedAt":"2024-03-15T06:55:09Z","lastAt":"2024-07-02T09:37:34Z","messageCount":16,"participants":["Linus Arver via GitGitGadget","Junio C Hamano","Kristoffer Haugsbakk","Linus Arver","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"490670","messageId":"pull.1694.git.1710485706.gitgitgadget@gmail.com","threadId":"61124","inReplyTo":null,"subject":"[PATCH 0/5] Unify trailer formatting functions","fromName":"Linus Arver via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-15T06:55:00Z","receivedAt":"2024-03-15T06:55:09Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"This series is based on the initial series [1], notably the v4 version of\npatches 10-16 as suggested by Christian [2]. This version addresses the\nreview comments for those patches, namely the avoidance of (temporary) test\nbreakages.\n\nThe central idea is to unify the functions format_trailer_info() and\nformat_trailers() together (preferring the name of the latter), so that both\nthe interpret-trailers built-in and format_trailers_from_commit() can call\nthe same format_trailers() function, while maintaining feature parity.\n\n[1]\nhttps://lore.kernel.org/git/pull.1632.v4.git.1707196348.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/CAP8UFD08F0V13X0+CJ1uhMPzPWVMs2okGVMJch0DkQg5M3BWLA@mail.gmail.com/\n\nLinus Arver (5):\n format_trailer_info(): use trailer_item objects\n format_trailer_info(): drop redundant unfold_value()\n format_trailer_info(): append newline for non-trailer lines\n trailer: begin formatting unification\n trailer: finish formatting unification\n\n trailer.c | 81 ++++++++++++++++++++-----------------------------------\n trailer.h | 13 +++------\n 2 files changed, 32 insertions(+), 62 deletions(-)\n\n\nbase-commit: 4f9b731bdeccffa1b13e5edf4bc0428b8d49704e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1694%2Flistx%2Ftrailer-api-part-2-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1694/listx/trailer-api-part-2-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1694\n-- \ngitgitgadget\n"},{"id":"490671","messageId":"e3da8eb7f587e05f20645550d6987f0a01ed18b6.1710485706.git.gitgitgadget@gmail.com","threadId":"61124","inReplyTo":"pull.1694.git.1710485706.gitgitgadget@gmail.com","subject":"[PATCH 1/5] format_trailer_info(): use trailer_item objects","fromName":"Linus Arver via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-15T06:55:01Z","receivedAt":"2024-03-15T06:55:10Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"From: Linus Arver \n\nThis is another preparatory refactor to unify the trailer formatters.\n\nMake format_trailer_info() operate on trailer_item objects, not the raw\nstring array.\n\nWe will continue to make improvements, culminating in the renaming of\nformat_trailer_info() to format_trailers(), at which point the\nunification of these formatters will be complete.\n\nIn order to avoid breaking t4205 and t6300, flip *_success to *_failure\nin the affected test cases. Add a temporary\n\"test_trailer_option_expect_failure\" wrapper which we will use along\nwith \"test_expect_failure\" in the next commit to avoid breaking tests.\nWhen the dust settles with the refactors a few more commits later, we\nwill drop the use of *_failure to make the tests truly pass again.\n\nWhen the preparatory refactors are complete,\nwe'll be able to drop the use of *_failure that we introduce here.\n\nSigned-off-by: Linus Arver \n---\n t/t4205-log-pretty-formats.sh | 12 ++++++------\n t/t6300-for-each-ref.sh | 16 ++++++++++++++--\n trailer.c | 21 ++++++++++-----------\n 3 files changed, 30 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex e3d655e6b8b..339e0c892ef 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -675,7 +675,7 @@ test_expect_success '%(trailers:only=no,only=true) shows only \"key: value\" trail\n \ttest_cmp expect actual\n '\n \n-test_expect_success '%(trailers:unfold) unfolds trailers' '\n+test_expect_failure '%(trailers:unfold) unfolds trailers' '\n \tgit log --no-walk --pretty=\"%(trailers:unfold)\" >actual &&\n \t{\n \t\tunfold actual &&\n \t{\n \t\techo \"Acked-by: A U Thor \" &&\n@@ -752,7 +752,7 @@ test_expect_success '%(trailers:key) without value is error' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '%(trailers:keyonly) shows only keys' '\n+test_expect_failure '%(trailers:keyonly) shows only keys' '\n \tgit log --no-walk --pretty=\"format:%(trailers:keyonly)\" >actual &&\n \ttest_write_lines \\\n \t\t\"Signed-off-by\" \\\n@@ -774,7 +774,7 @@ test_expect_success '%(trailers:key=foo,valueonly) shows only value' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '%(trailers:valueonly) shows only values' '\n+test_expect_failure '%(trailers:valueonly) shows only values' '\n \tgit log --no-walk --pretty=\"format:%(trailers:valueonly)\" >actual &&\n \ttest_write_lines \\\n \t\t\"A U Thor \" \\\n@@ -813,7 +813,7 @@ test_expect_success 'pretty format %(trailers:separator=X,unfold) changes separa\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'pretty format %(trailers:key_value_separator) changes key-value separator' '\n+test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-value separator' '\n \tgit log --no-walk --pretty=format:\"X%(trailers:key_value_separator=%x00)X\" >actual &&\n \t(\n \t\tprintf \"XSigned-off-by\\0A U Thor \\n\" &&\n@@ -824,7 +824,7 @@ test_expect_success 'pretty format %(trailers:key_value_separator) changes key-v\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '\n+test_expect_failure 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '\n \tgit log --no-walk --pretty=format:\"X%(trailers:key_value_separator=%x00,unfold)X\" >actual &&\n \t(\n \t\tprintf \"XSigned-off-by\\0A U Thor \\n\" &&\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex eb6c8204e8b..2688dcc7b9e 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -1446,7 +1446,19 @@ test_trailer_option () {\n \t'\n }\n \n-test_trailer_option '%(trailers:unfold) unfolds trailers' \\\n+# Just like test_trailer_option, but expect failure instead of success.\n+test_trailer_option_expect_failure () {\n+\ttitle=$1 option=$2\n+\tcat >expect\n+\ttest_expect_failure \"$title\" '\n+\t\tgit for-each-ref --format=\"%($option)\" refs/heads/main >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\tgit for-each-ref --format=\"%(contents:$option)\" refs/heads/main >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+}\n+\n+test_trailer_option_expect_failure '%(trailers:unfold) unfolds trailers' \\\n \t'trailers:unfold' <<-EOF\n \t$(unfold \n \t$(grep patch.description len;\n-\tsize_t i;\n-\n-\tfor (i = 0; i < info->trailer_nr; i++) {\n-\t\tchar *trailer = info->trailers[i];\n-\t\tssize_t separator_pos = find_separator(trailer, separators);\n+\tstruct list_head *pos;\n+\tstruct trailer_item *item;\n \n-\t\tif (separator_pos >= 1) {\n+\tlist_for_each(pos, trailers) {\n+\t\titem = list_entry(pos, struct trailer_item, list);\n+\t\tif (item->token) {\n \t\t\tstruct strbuf tok = STRBUF_INIT;\n \t\t\tstruct strbuf val = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&tok, item->token);\n+\t\t\tstrbuf_addstr(&val, item->value);\n \n-\t\t\tparse_trailer(&tok, &val, NULL, trailer, separator_pos);\n \t\t\tif (!opts->filter || opts->filter(&tok, opts->filter_data)) {\n \t\t\t\tif (opts->unfold)\n \t\t\t\t\tunfold_value(&val);\n@@ -1126,13 +1126,12 @@ static void format_trailer_info(const struct process_trailer_options *opts,\n \t\t\tif (opts->separator && out->len != origlen) {\n \t\t\t\tstrbuf_addbuf(out, opts->separator);\n \t\t\t}\n-\t\t\tstrbuf_addstr(out, trailer);\n+\t\t\tstrbuf_addstr(out, item->value);\n \t\t\tif (opts->separator) {\n \t\t\t\tstrbuf_rtrim(out);\n \t\t\t}\n \t\t}\n \t}\n-\n }\n \n void format_trailers_from_commit(const struct process_trailer_options *opts,\n@@ -1151,7 +1150,7 @@ void format_trailers_from_commit(const struct process_trailer_options *opts,\n \t\tstrbuf_add(out, msg + info.trailer_block_start,\n \t\t\t info.trailer_block_end - info.trailer_block_start);\n \t} else\n-\t\tformat_trailer_info(opts, &info, out);\n+\t\tformat_trailer_info(opts, &trailer_objects, out);\n \n \tfree_trailers(&trailer_objects);\n \ttrailer_info_release(&info);\n-- \ngitgitgadget\n\n"},{"id":"490672","messageId":"5f9e444197468d065264b85c8cf7434df0e08070.1710485706.git.gitgitgadget@gmail.com","threadId":"61124","inReplyTo":"pull.1694.git.1710485706.gitgitgadget@gmail.com","subject":"[PATCH 2/5] format_trailer_info(): drop redundant unfold_value()","fromName":"Linus Arver via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-15T06:55:02Z","receivedAt":"2024-03-15T06:55:11Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"From: Linus Arver \n\nThis is another preparatory refactor to unify the trailer formatters.\n\nIn the last patch we made format_trailer_info() use trailer_item objects\ninstead of the \"trailers\" string array. This means that the call to\nunfold_value() here is redundant because the trailer_item objects are\nalready unfolded in parse_trailers() which is a dependency of our\ncaller, format_trailers_from_commit().\n\nRemove the redundant call.\n\nSigned-off-by: Linus Arver \n---\n trailer.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/trailer.c b/trailer.c\nindex a74f05db55c..2c0dd8ac829 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1101,9 +1101,6 @@ static void format_trailer_info(const struct process_trailer_options *opts,\n \t\t\tstrbuf_addstr(&val, item->value);\n \n \t\t\tif (!opts->filter || opts->filter(&tok, opts->filter_data)) {\n-\t\t\t\tif (opts->unfold)\n-\t\t\t\t\tunfold_value(&val);\n-\n \t\t\t\tif (opts->separator && out->len != origlen)\n \t\t\t\t\tstrbuf_addbuf(out, opts->separator);\n \t\t\t\tif (!opts->value_only)\n-- \ngitgitgadget\n\n"},{"id":"490673","messageId":"259a4edcfa5138053255dbade411ef151617bb1b.1710485706.git.gitgitgadget@gmail.com","threadId":"61124","inReplyTo":"pull.1694.git.1710485706.gitgitgadget@gmail.com","subject":"[PATCH 3/5] format_trailer_info(): append newline for non-trailer lines","fromName":"Linus Arver via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-15T06:55:03Z","receivedAt":"2024-03-15T06:55:13Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"From: Linus Arver \n\nThis wraps up the preparatory refactors to unify the trailer formatters.\n\nTwo patches ago we made format_trailer_info() use trailer_item objects\ninstead of the \"trailers\" string array. The strings in the array\ninclude trailing newlines, because the string array is split up with\n\n trailer_lines = strbuf_split_buf(str + trailer_block_start,\n end_of_log_message - trailer_block_start,\n '\\n',\n 0);\n\nin trailer_info_get() and strbuf_split_buf() includes the terminator (in\nthis case the newline character '\\n') for each split-up substring.\n\nAnd before we made the transition to use trailer_item objects for it,\nformat_trailer_info() called parse_trailer() (which trims newlines) for\ntrailer lines but did _not_ call parse_trailer() for non-trailer lines.\nSo for trailer lines it had to add back the trimmed newline like this\n\n if (!opts->separator)\n strbuf_addch(out, '\\n');\n\nBut for non-trailer lines it didn't have to add back the newline because\nit could just reuse same string in the \"trailers\" string array (which\nagain, already included the trailing newline).\n\nNow that format_trailer_info() uses trailer_item objects for all cases,\nit can't rely on \"trailers\" string array anymore. And so it must be\ntaught to add a newline back when printing non-trailer lines, just like\nit already does for trailer lines. Do so now.\n\nThe test suite can pass again without the need to hide failures\nwith *_failure, so flip the affected test cases back to *_success. Now,\nformat_trailer_info() is in better shape to supersede format_trailers(),\nwhich we'll do in the next commit.\n\nSigned-off-by: Linus Arver \n---\n t/t4205-log-pretty-formats.sh | 12 ++++++------\n t/t6300-for-each-ref.sh | 16 ++--------------\n trailer.c | 5 +++--\n 3 files changed, 11 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 339e0c892ef..e3d655e6b8b 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -675,7 +675,7 @@ test_expect_success '%(trailers:only=no,only=true) shows only \"key: value\" trail\n \ttest_cmp expect actual\n '\n \n-test_expect_failure '%(trailers:unfold) unfolds trailers' '\n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n \tgit log --no-walk --pretty=\"%(trailers:unfold)\" >actual &&\n \t{\n \t\tunfold actual &&\n \t{\n \t\techo \"Acked-by: A U Thor \" &&\n@@ -752,7 +752,7 @@ test_expect_success '%(trailers:key) without value is error' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure '%(trailers:keyonly) shows only keys' '\n+test_expect_success '%(trailers:keyonly) shows only keys' '\n \tgit log --no-walk --pretty=\"format:%(trailers:keyonly)\" >actual &&\n \ttest_write_lines \\\n \t\t\"Signed-off-by\" \\\n@@ -774,7 +774,7 @@ test_expect_success '%(trailers:key=foo,valueonly) shows only value' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure '%(trailers:valueonly) shows only values' '\n+test_expect_success '%(trailers:valueonly) shows only values' '\n \tgit log --no-walk --pretty=\"format:%(trailers:valueonly)\" >actual &&\n \ttest_write_lines \\\n \t\t\"A U Thor \" \\\n@@ -813,7 +813,7 @@ test_expect_success 'pretty format %(trailers:separator=X,unfold) changes separa\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-value separator' '\n+test_expect_success 'pretty format %(trailers:key_value_separator) changes key-value separator' '\n \tgit log --no-walk --pretty=format:\"X%(trailers:key_value_separator=%x00)X\" >actual &&\n \t(\n \t\tprintf \"XSigned-off-by\\0A U Thor \\n\" &&\n@@ -824,7 +824,7 @@ test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-v\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '\n+test_expect_success 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '\n \tgit log --no-walk --pretty=format:\"X%(trailers:key_value_separator=%x00,unfold)X\" >actual &&\n \t(\n \t\tprintf \"XSigned-off-by\\0A U Thor \\n\" &&\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 2688dcc7b9e..eb6c8204e8b 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -1446,19 +1446,7 @@ test_trailer_option () {\n \t'\n }\n \n-# Just like test_trailer_option, but expect failure instead of success.\n-test_trailer_option_expect_failure () {\n-\ttitle=$1 option=$2\n-\tcat >expect\n-\ttest_expect_failure \"$title\" '\n-\t\tgit for-each-ref --format=\"%($option)\" refs/heads/main >actual &&\n-\t\ttest_cmp expect actual &&\n-\t\tgit for-each-ref --format=\"%(contents:$option)\" refs/heads/main >actual &&\n-\t\ttest_cmp expect actual\n-\t'\n-}\n-\n-test_trailer_option_expect_failure '%(trailers:unfold) unfolds trailers' \\\n+test_trailer_option '%(trailers:unfold) unfolds trailers' \\\n \t'trailers:unfold' <<-EOF\n \t$(unfold \n \t$(grep patch.description separator);\n \t\t\t}\n \t\t\tstrbuf_addstr(out, item->value);\n-\t\t\tif (opts->separator) {\n+\t\t\tif (opts->separator)\n \t\t\t\tstrbuf_rtrim(out);\n-\t\t\t}\n+\t\t\telse\n+\t\t\t\tstrbuf_addch(out, '\\n');\n \t\t}\n \t}\n }\n-- \ngitgitgadget\n\n"},{"id":"490674","messageId":"f8b102a5b8c898100d24e50d197ef0a24b17a247.1710485706.git.gitgitgadget@gmail.com","threadId":"61124","inReplyTo":"pull.1694.git.1710485706.gitgitgadget@gmail.com","subject":"[PATCH 4/5] trailer: begin formatting unification","fromName":"Linus Arver via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-15T06:55:04Z","receivedAt":"2024-03-15T06:55:13Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"From: Linus Arver \n\nNow that the preparatory refactors are over, we can replace the call to\nformat_trailers() in interpret-trailers with format_trailer_info(). This\nunifies the trailer formatting machinery\n\nIn order to avoid breakages in t7502 and t7513, we have to steal the\nfeatures present in format_trailers(). Namely, we have to teach\nformat_trailer_info() as follows:\n\n (1) make it aware of opts->trim_empty, and\n\n (2) make it avoid hardcoding \": \" as the separator and space (which\n can result in double-printing these characters).\n\nFor (2), make it only print the separator and space if we cannot find\nany recognized separator somewhere in the key (yes, keys may have a\ntrailing separator in it --- we will eventually fix this design but not\nnow). Do so by copying the code out of print_tok_val(), and deleting the\nsame function.\n\nHelped-by: Junio C Hamano \nHelped-by: Christian Couder \nSigned-off-by: Linus Arver \n---\n builtin/interpret-trailers.c | 2 +-\n trailer.c | 54 ++++++++++++------------------------\n trailer.h | 2 +-\n 3 files changed, 19 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 11f4ce9e4a2..f57af0db37b 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -171,7 +171,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n \t}\n \n \t/* Print trailer block. */\n-\tformat_trailers(opts, &head, &trailer_block);\n+\tformat_trailer_info(opts, &head, &trailer_block);\n \tfree_trailers(&head);\n \tfwrite(trailer_block.buf, 1, trailer_block.len, outfile);\n \tstrbuf_release(&trailer_block);\ndiff --git a/trailer.c b/trailer.c\nindex fe8b0819d55..43d5baef9ce 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -144,38 +144,6 @@ static char last_non_space_char(const char *s)\n \treturn '\\0';\n }\n \n-static void print_tok_val(struct strbuf *out, const char *tok, const char *val)\n-{\n-\tchar c;\n-\n-\tif (!tok) {\n-\t\tstrbuf_addf(out, \"%s\\n\", val);\n-\t\treturn;\n-\t}\n-\n-\tc = last_non_space_char(tok);\n-\tif (!c)\n-\t\treturn;\n-\tif (strchr(separators, c))\n-\t\tstrbuf_addf(out, \"%s%s\\n\", tok, val);\n-\telse\n-\t\tstrbuf_addf(out, \"%s%c %s\\n\", tok, separators[0], val);\n-}\n-\n-void format_trailers(const struct process_trailer_options *opts,\n-\t\t struct list_head *trailers,\n-\t\t struct strbuf *out)\n-{\n-\tstruct list_head *pos;\n-\tstruct trailer_item *item;\n-\tlist_for_each(pos, trailers) {\n-\t\titem = list_entry(pos, struct trailer_item, list);\n-\t\tif ((!opts->trim_empty || strlen(item->value) > 0) &&\n-\t\t (!opts->only_trailers || item->token))\n-\t\t\tprint_tok_val(out, item->token, item->value);\n-\t}\n-}\n-\n static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)\n {\n \tstruct trailer_item *new_item = xcalloc(1, sizeof(*new_item));\n@@ -1084,9 +1052,9 @@ void trailer_info_release(struct trailer_info *info)\n \tfree(info->trailers);\n }\n \n-static void format_trailer_info(const struct process_trailer_options *opts,\n-\t\t\t\tstruct list_head *trailers,\n-\t\t\t\tstruct strbuf *out)\n+void format_trailer_info(const struct process_trailer_options *opts,\n+\t\t\t struct list_head *trailers,\n+\t\t\t struct strbuf *out)\n {\n \tsize_t origlen = out->len;\n \tstruct list_head *pos;\n@@ -1100,6 +1068,15 @@ static void format_trailer_info(const struct process_trailer_options *opts,\n \t\t\tstrbuf_addstr(&tok, item->token);\n \t\t\tstrbuf_addstr(&val, item->value);\n \n+\t\t\t/*\n+\t\t\t * Skip key/value pairs where the value was empty. This\n+\t\t\t * can happen from trailers specified without a\n+\t\t\t * separator, like `--trailer \"Reviewed-by\"` (no\n+\t\t\t * corresponding value).\n+\t\t\t */\n+\t\t\tif (opts->trim_empty && !strlen(item->value))\n+\t\t\t\tcontinue;\n+\n \t\t\tif (!opts->filter || opts->filter(&tok, opts->filter_data)) {\n \t\t\t\tif (opts->separator && out->len != origlen)\n \t\t\t\t\tstrbuf_addbuf(out, opts->separator);\n@@ -1108,8 +1085,11 @@ static void format_trailer_info(const struct process_trailer_options *opts,\n \t\t\t\tif (!opts->key_only && !opts->value_only) {\n \t\t\t\t\tif (opts->key_value_separator)\n \t\t\t\t\t\tstrbuf_addbuf(out, opts->key_value_separator);\n-\t\t\t\t\telse\n-\t\t\t\t\t\tstrbuf_addstr(out, \": \");\n+\t\t\t\t\telse {\n+\t\t\t\t\t\tchar c = last_non_space_char(tok.buf);\n+\t\t\t\t\t\tif (c && !strchr(separators, c))\n+\t\t\t\t\t\t\tstrbuf_addf(out, \"%c \", separators[0]);\n+\t\t\t\t\t}\n \t\t\t\t}\n \t\t\t\tif (!opts->key_only)\n \t\t\t\t\tstrbuf_addbuf(out, &val);\ndiff --git a/trailer.h b/trailer.h\nindex 1d106b6dd40..3c13006a4c1 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -101,7 +101,7 @@ void trailer_info_get(const struct process_trailer_options *,\n void trailer_info_release(struct trailer_info *info);\n \n void trailer_config_init(void);\n-void format_trailers(const struct process_trailer_options *,\n+void format_trailer_info(const struct process_trailer_options *,\n \t\t struct list_head *trailers,\n \t\t struct strbuf *out);\n void free_trailers(struct list_head *);\n-- \ngitgitgadget\n\n"},{"id":"490675","messageId":"cd3c4b455f168d840b9d1214709ab5b152a76c17.1710485706.git.gitgitgadget@gmail.com","threadId":"61124","inReplyTo":"pull.1694.git.1710485706.gitgitgadget@gmail.com","subject":"[PATCH 5/5] trailer: finish formatting unification","fromName":"Linus Arver via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-15T06:55:05Z","receivedAt":"2024-03-15T06:55:15Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"From: Linus Arver \n\nRename format_trailer_info() to format_trailers(). Finally, both\ninterpret-trailers and format_trailers_from_commit() can call\n\"format_trailers()\"!\n\nUpdate the comment in to remove the (now obsolete) caveats\nabout format_trailers_from_commit(). Those caveats come from\na388b10fc1 (pretty: move trailer formatting to trailer.c, 2017-08-15)\nwhere it says:\n\n pretty: move trailer formatting to trailer.c\n\n The next commit will add many features to the %(trailer)\n placeholder in pretty.c. We'll need to access some internal\n functions of trailer.c for that, so our options are either:\n\n 1. expose those functions publicly\n\n or\n\n 2. make an entry point into trailer.c to do the formatting\n\n Doing (2) ends up exposing less surface area, though do note\n that caveats in the docstring of the new function.\n\nwhich suggests format_trailers_from_commit() started out from pretty.c\nand did not have access to all of the trailer implementation internals,\nand was never intended to replace (unify) the formatting machinery in\ntrailer.c. The refactors leading up to this commit (as well as\nadditional refactors that will follow) expose additional functions\npublicly, and is therefore choosing option (1) as described in\na388b10fc1.\n\nSigned-off-by: Linus Arver \n---\n builtin/interpret-trailers.c | 2 +-\n trailer.c | 8 ++++----\n trailer.h | 15 ++++-----------\n 3 files changed, 9 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex f57af0db37b..11f4ce9e4a2 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -171,7 +171,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,\n \t}\n \n \t/* Print trailer block. */\n-\tformat_trailer_info(opts, &head, &trailer_block);\n+\tformat_trailers(opts, &head, &trailer_block);\n \tfree_trailers(&head);\n \tfwrite(trailer_block.buf, 1, trailer_block.len, outfile);\n \tstrbuf_release(&trailer_block);\ndiff --git a/trailer.c b/trailer.c\nindex 43d5baef9ce..3e4dab9c065 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1052,9 +1052,9 @@ void trailer_info_release(struct trailer_info *info)\n \tfree(info->trailers);\n }\n \n-void format_trailer_info(const struct process_trailer_options *opts,\n-\t\t\t struct list_head *trailers,\n-\t\t\t struct strbuf *out)\n+void format_trailers(const struct process_trailer_options *opts,\n+\t\t struct list_head *trailers,\n+\t\t struct strbuf *out)\n {\n \tsize_t origlen = out->len;\n \tstruct list_head *pos;\n@@ -1128,7 +1128,7 @@ void format_trailers_from_commit(const struct process_trailer_options *opts,\n \t\tstrbuf_add(out, msg + info.trailer_block_start,\n \t\t\t info.trailer_block_end - info.trailer_block_start);\n \t} else\n-\t\tformat_trailer_info(opts, &trailer_objects, out);\n+\t\tformat_trailers(opts, &trailer_objects, out);\n \n \tfree_trailers(&trailer_objects);\n \ttrailer_info_release(&info);\ndiff --git a/trailer.h b/trailer.h\nindex 3c13006a4c1..9f42aa75994 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -101,23 +101,16 @@ void trailer_info_get(const struct process_trailer_options *,\n void trailer_info_release(struct trailer_info *info);\n \n void trailer_config_init(void);\n-void format_trailer_info(const struct process_trailer_options *,\n+void format_trailers(const struct process_trailer_options *,\n \t\t struct list_head *trailers,\n \t\t struct strbuf *out);\n void free_trailers(struct list_head *);\n \n /*\n- * Format the trailers from the commit msg \"msg\" into the strbuf \"out\".\n- * Note two caveats about \"opts\":\n- *\n- * - this is primarily a helper for pretty.c, and not\n- * all of the flags are supported.\n- *\n- * - this differs from process_trailers slightly in that we always format\n- * only the trailer block itself, even if the \"only_trailers\" option is not\n- * set.\n+ * Convenience function to format the trailers from the commit msg \"msg\" into\n+ * the strbuf \"out\". Reuses format_trailers() internally.\n */\n-void format_trailers_from_commit(const struct process_trailer_options *opts,\n+void format_trailers_from_commit(const struct process_trailer_options *,\n \t\t\t\t const char *msg,\n \t\t\t\t struct strbuf *out);\n \n-- \ngitgitgadget\n"},{"id":"490700","messageId":"xmqqjzm3qumx.fsf@gitster.g","threadId":"61124","inReplyTo":"pull.1694.git.1710485706.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-15T17:20:38Z","receivedAt":"2024-03-15T17:20:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Not about the series, but about the way it was sent.\n\nThe messages in this series have exactly the same kind of breakages\nin the recipient names/addresses we recently saw:\n\n https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/\n\nHuman-readable names with a SP inside [square bracket] pair\nappended, and one of the addresses had that square bracket applied\ninside pair and breaking MTAs (I manually fixed\nkhaugsbakk's address before sending this response, so replying to\nthis messages should be OK).\n\nWhat are you and Aryan's pull.1675.v3 did differently from other\nseries sent via GGG to trigger this, I have to wonder? Without\nknowing it, it would be hard to avoid seeing these broken addresses\nagain.\n\nThanks.\n"},{"id":"490701","messageId":"xmqqfrwrquka.fsf@gitster.g","threadId":"61124","inReplyTo":"259a4edcfa5138053255dbade411ef151617bb1b.1710485706.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] format_trailer_info(): append newline for non-trailer lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-15T17:22:13Z","receivedAt":"2024-03-15T17:22:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Linus Arver via GitGitGadget\" writes:\n\n> From: Linus Arver \n>\n> This wraps up the preparatory refactors to unify the trailer formatters.\n> ...\n> The test suite can pass again without the need to hide failures\n> with *_failure, so flip the affected test cases back to *_success. Now,\n> format_trailer_info() is in better shape to supersede format_trailers(),\n> which we'll do in the next commit.\n\nNicely done. Queued.\n\n>\n> Signed-off-by: Linus Arver \n> ---\n> t/t4205-log-pretty-formats.sh | 12 ++++++------\n> t/t6300-for-each-ref.sh | 16 ++--------------\n> trailer.c | 5 +++--\n> 3 files changed, 11 insertions(+), 22 deletions(-)\n>\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index 339e0c892ef..e3d655e6b8b 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -675,7 +675,7 @@ test_expect_success '%(trailers:only=no,only=true) shows only \"key: value\" trail\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure '%(trailers:unfold) unfolds trailers' '\n> +test_expect_success '%(trailers:unfold) unfolds trailers' '\n> \tgit log --no-walk --pretty=\"%(trailers:unfold)\" >actual &&\n> \t{\n> \t\tunfold @@ -737,7 +737,7 @@ test_expect_success '%(trailers:key=foo,unfold) properly unfolds' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '\n> +test_expect_success 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '\n> \tgit log --no-walk --pretty=\"format:%(trailers:key=Acked-by,only=no)\" >actual &&\n> \t{\n> \t\techo \"Acked-by: A U Thor \" &&\n> @@ -752,7 +752,7 @@ test_expect_success '%(trailers:key) without value is error' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure '%(trailers:keyonly) shows only keys' '\n> +test_expect_success '%(trailers:keyonly) shows only keys' '\n> \tgit log --no-walk --pretty=\"format:%(trailers:keyonly)\" >actual &&\n> \ttest_write_lines \\\n> \t\t\"Signed-off-by\" \\\n> @@ -774,7 +774,7 @@ test_expect_success '%(trailers:key=foo,valueonly) shows only value' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure '%(trailers:valueonly) shows only values' '\n> +test_expect_success '%(trailers:valueonly) shows only values' '\n> \tgit log --no-walk --pretty=\"format:%(trailers:valueonly)\" >actual &&\n> \ttest_write_lines \\\n> \t\t\"A U Thor \" \\\n> @@ -813,7 +813,7 @@ test_expect_success 'pretty format %(trailers:separator=X,unfold) changes separa\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-value separator' '\n> +test_expect_success 'pretty format %(trailers:key_value_separator) changes key-value separator' '\n> \tgit log --no-walk --pretty=format:\"X%(trailers:key_value_separator=%x00)X\" >actual &&\n> \t(\n> \t\tprintf \"XSigned-off-by\\0A U Thor \\n\" &&\n> @@ -824,7 +824,7 @@ test_expect_failure 'pretty format %(trailers:key_value_separator) changes key-v\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '\n> +test_expect_success 'pretty format %(trailers:key_value_separator,unfold) changes key-value separator' '\n> \tgit log --no-walk --pretty=format:\"X%(trailers:key_value_separator=%x00,unfold)X\" >actual &&\n> \t(\n> \t\tprintf \"XSigned-off-by\\0A U Thor \\n\" &&\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 2688dcc7b9e..eb6c8204e8b 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -1446,19 +1446,7 @@ test_trailer_option () {\n> \t'\n> }\n> \n> -# Just like test_trailer_option, but expect failure instead of success.\n> -test_trailer_option_expect_failure () {\n> -\ttitle=$1 option=$2\n> -\tcat >expect\n> -\ttest_expect_failure \"$title\" '\n> -\t\tgit for-each-ref --format=\"%($option)\" refs/heads/main >actual &&\n> -\t\ttest_cmp expect actual &&\n> -\t\tgit for-each-ref --format=\"%(contents:$option)\" refs/heads/main >actual &&\n> -\t\ttest_cmp expect actual\n> -\t'\n> -}\n> -\n> -test_trailer_option_expect_failure '%(trailers:unfold) unfolds trailers' \\\n> +test_trailer_option '%(trailers:unfold) unfolds trailers' \\\n> \t'trailers:unfold' <<-EOF\n> \t$(unfold \n> @@ -1542,7 +1530,7 @@ test_trailer_option '%(trailers:key=foo,unfold) properly unfolds' \\\n> \n> \tEOF\n> \n> -test_trailer_option_expect_failure '%(trailers:key=foo,only=no) also includes nontrailer lines' \\\n> +test_trailer_option '%(trailers:key=foo,only=no) also includes nontrailer lines' \\\n> \t'trailers:key=Signed-off-by,only=no' <<-EOF\n> \tSigned-off-by: A U Thor \n> \t$(grep patch.description diff --git a/trailer.c b/trailer.c\n> index 2c0dd8ac829..fe8b0819d55 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -1124,9 +1124,10 @@ static void format_trailer_info(const struct process_trailer_options *opts,\n> \t\t\t\tstrbuf_addbuf(out, opts->separator);\n> \t\t\t}\n> \t\t\tstrbuf_addstr(out, item->value);\n> -\t\t\tif (opts->separator) {\n> +\t\t\tif (opts->separator)\n> \t\t\t\tstrbuf_rtrim(out);\n> -\t\t\t}\n> +\t\t\telse\n> +\t\t\t\tstrbuf_addch(out, '\\n');\n> \t\t}\n> \t}\n> }\n"},{"id":"490703","messageId":"75ff994e-f086-4490-a442-dcaa6e2230df@app.fastmail.com","threadId":"61124","inReplyTo":"xmqqjzm3qumx.fsf@gitster.g","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-03-15T18:26:13Z","receivedAt":"2024-03-15T18:26:45Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Fri, Mar 15, 2024, at 18:20, Junio C Hamano wrote:\n> Not about the series, but about the way it was sent.\n>\n> The messages in this series have exactly the same kind of breakages\n> in the recipient names/addresses we recently saw:\n>\n> https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/\n>\n> Human-readable names with a SP inside [square bracket] pair\n> appended, and one of the addresses had that square bracket applied\n> inside pair and breaking MTAs (I manually fixed\n> khaugsbakk's address before sending this response, so replying to\n> this messages should be OK).\n>\n> What are you and Aryan's pull.1675.v3 did differently from other\n> series sent via GGG to trigger this, I have to wonder? Without\n> knowing it, it would be hard to avoid seeing these broken addresses\n> again.\n>\n> Thanks.\n\nI’m the only common CC on these two. Maybe dropping me for the possible\nnext round would fix it. Would be interesting to see.\n\nCheers\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"490705","messageId":"xmqqzfuzpayk.fsf@gitster.g","threadId":"61124","inReplyTo":"75ff994e-f086-4490-a442-dcaa6e2230df@app.fastmail.com","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-15T19:10:59Z","receivedAt":"2024-03-15T19:11:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> I’m the only common CC on these two. Maybe dropping me for the possible\n> next round would fix it. Would be interesting to see.\n\nBut there is nothing strange in your name or address. There may be\n\"something\" that confuses GGG about how they spelled your name and\naddress, and if the next user does that same \"something\" for some\nother recipient, it wouldn't be you but that other recipient with\nthe same issue, I am afraid.\n\n"},{"id":"490737","messageId":"owly4jd741ph.fsf@fine.c.googlers.com","threadId":"61124","inReplyTo":"xmqqjzm3qumx.fsf@gitster.g","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2024-03-15T21:36:26Z","receivedAt":"2024-03-15T21:36:28Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"Junio C Hamano writes:\n\n> Not about the series, but about the way it was sent.\n>\n> The messages in this series have exactly the same kind of breakages\n> in the recipient names/addresses we recently saw:\n>\n> https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/\n>\n> Human-readable names with a SP inside [square bracket] pair\n> appended, and one of the addresses had that square bracket applied\n> inside pair and breaking MTAs (I manually fixed\n> khaugsbakk's address before sending this response, so replying to\n> this messages should be OK).\n\nUGH, I'm so sorry about that.\n\n> What are you and Aryan's pull.1675.v3 did differently from other\n> series sent via GGG to trigger this, I have to wonder?\n\nI realize now that it's because I copy/pasted the \"Cc: ...\" lines in the PR\ndescription from\nhttps://github.com/gitgitgadget/git/pull/1632#issue-2068188239, such\nthat when I pasted those in for the PR description for this series at\nhttps://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it\ncarried over the email addresses as Markdown-formatted hyperlinks.\nCurrently it reads\n\n Cc: Christian Couder [chriscool@tuxfamily.org](mailto:chriscool@tuxfamily.org)\n Cc: Junio C Hamano [gitster@pobox.com](mailto:gitster@pobox.com)\n Cc: Emily Shaffer [nasamuffin@google.com](mailto:nasamuffin@google.com)\n cc: Josh Steadmon [steadmon@google.com](mailto:steadmon@google.com)\n cc: Randall S. Becker [rsbecker@nexbridge.com](mailto:rsbecker@nexbridge.com)\n cc: Christian Couder [christian.couder@gmail.com](mailto:christian.couder@gmail.com)\n cc: \"Kristoffer Haugsbakk\" [code@khaugsbakk.name](mailto:code@khaugsbakk.name)\n cc: \"Kristoffer Haugsbakk\" \n\nwhen I click on \"edit\", where the last line must be from your manual fix\nwhich GGG picked up. I've cleaned up the PR description manually now,\nand for this message I'm also attempting to clean up those square\nbrackets.\n"},{"id":"490739","messageId":"xmqqv85np3wp.fsf@gitster.g","threadId":"61124","inReplyTo":"owly4jd741ph.fsf@fine.c.googlers.com","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-15T21:43:18Z","receivedAt":"2024-03-15T21:43:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Arver writes:\n\n> I realize now that it's because I copy/pasted the \"Cc: ...\" lines in the PR\n> description from\n> https://github.com/gitgitgadget/git/pull/1632#issue-2068188239, such\n> that when I pasted those in for the PR description for this series at\n> https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it\n> carried over the email addresses as Markdown-formatted hyperlinks.\n> Currently it reads\n>\n> Cc: Christian Couder [chriscool@tuxfamily.org](mailto:chriscool@tuxfamily.org)\n> Cc: Junio C Hamano [gitster@pobox.com](mailto:gitster@pobox.com)\n> Cc: Emily Shaffer [nasamuffin@google.com](mailto:nasamuffin@google.com)\n> cc: Josh Steadmon [steadmon@google.com](mailto:steadmon@google.com)\n> cc: Randall S. Becker [rsbecker@nexbridge.com](mailto:rsbecker@nexbridge.com)\n> cc: Christian Couder [christian.couder@gmail.com](mailto:christian.couder@gmail.com)\n> cc: \"Kristoffer Haugsbakk\" [code@khaugsbakk.name](mailto:code@khaugsbakk.name)\n> cc: \"Kristoffer Haugsbakk\" \n>\n> when I click on \"edit\", where the last line must be from your manual fix\n> which GGG picked up. I've cleaned up the PR description manually now,\n> and for this message I'm also attempting to clean up those square\n> brackets.\n\nThe last time I asked the author with the same question, we\nunfortunately did not get a clear answer on what should be avoided\n(probably the author was happy enough with the message resulted from\nthe updated pull request and forgot that it is more important to\nhelp others from having the same issue). Now we have an example we\ncan point at when a similar issue arises.\n\nThanks.\n"},{"id":"491612","messageId":"xmqqplvg7j1y.fsf@gitster.g","threadId":"61124","inReplyTo":"pull.1694.git.1710485706.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-26T21:57:29Z","receivedAt":"2024-03-26T21:57:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Linus Arver via GitGitGadget\" writes:\n\n> This series is based on the initial series [1], notably the v4 version of\n> patches 10-16 as suggested by Christian [2]. This version addresses the\n> review comments for those patches, namely the avoidance of (temporary) test\n> breakages.\n\nIt has been 10 days but we haven't seen any reviews on this one. It\ncould be that it did not get to intended mailboxes due to the header\ncorruption (I manually fixed in this message), but without reviews\nwe cannot move forward.\n\n"},{"id":"492012","messageId":"owlyjzlg623v.fsf@fine.c.googlers.com","threadId":"61124","inReplyTo":"xmqqplvg7j1y.fsf@gitster.g","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2024-04-02T00:27:00Z","receivedAt":"2024-04-02T00:27:02Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"\nJunio C Hamano writes:\n\n> \"Linus Arver via GitGitGadget\" writes:\n>\n>> This series is based on the initial series [1], notably the v4 version of\n>> patches 10-16 as suggested by Christian [2]. This version addresses the\n>> review comments for those patches, namely the avoidance of (temporary) test\n>> breakages.\n>\n> It has been 10 days but we haven't seen any reviews on this one. It\n> could be that it did not get to intended mailboxes due to the header\n> corruption (I manually fixed in this message), but without reviews\n> we cannot move forward.\n\nAdding a couple more folks to the CC list according to the\ncontrib/contacts/git-contacts script. Thanks!\n"},{"id":"492965","messageId":"xmqqedb6l4pw.fsf@gitster.g","threadId":"61124","inReplyTo":"owlyjzlg623v.fsf@fine.c.googlers.com","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-15T21:02:19Z","receivedAt":"2024-04-15T21:02:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Arver writes:\n\n> Junio C Hamano writes:\n>\n>> \"Linus Arver via GitGitGadget\" writes:\n>>\n>>> This series is based on the initial series [1], notably the v4 version of\n>>> patches 10-16 as suggested by Christian [2]. This version addresses the\n>>> review comments for those patches, namely the avoidance of (temporary) test\n>>> breakages.\n>>\n>> It has been 10 days but we haven't seen any reviews on this one. It\n>> could be that it did not get to intended mailboxes due to the header\n>> corruption (I manually fixed in this message), but without reviews\n>> we cannot move forward.\n>\n> Adding a couple more folks to the CC list according to the\n> contrib/contacts/git-contacts script. Thanks!\n\nAs we saw no responses after two weeks, I took some time to scan\nthese patches myself for the final time. I didn't see anything\ngravely wrong in it and am tempted to merge it down to 'next'.\n\nUnless I hear objections in the next few days, that is.\n\nThanks.\n\n\n"},{"id":"497966","messageId":"efdda91a-1118-ea54-d28f-dd6fa6df67c5@gmx.de","threadId":"61124","inReplyTo":"owly4jd741ph.fsf@fine.c.googlers.com","subject":"Re: [PATCH 0/5] Unify trailer formatting functions","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-07-02T09:37:13Z","receivedAt":"2024-07-02T09:37:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 Mar 2024, Linus Arver wrote:\n\n> Junio C Hamano writes:\n>\n> > Not about the series, but about the way it was sent.\n> >\n> > The messages in this series have exactly the same kind of breakages\n> > in the recipient names/addresses we recently saw:\n> >\n> > https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/\n> >\n> > Human-readable names with a SP inside [square bracket] pair\n> > appended, and one of the addresses had that square bracket applied\n> > inside pair and breaking MTAs (I manually fixed\n> > khaugsbakk's address before sending this response, so replying to\n> > this messages should be OK).\n>\n> UGH, I'm so sorry about that.\n>\n> > What are you and Aryan's pull.1675.v3 did differently from other\n> > series sent via GGG to trigger this, I have to wonder?\n>\n> I realize now that it's because I copy/pasted the \"Cc: ...\" lines in the PR\n> description from\n> https://github.com/gitgitgadget/git/pull/1632#issue-2068188239, such\n> that when I pasted those in for the PR description for this series at\n> https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it\n> carried over the email addresses as Markdown-formatted hyperlinks.\n> Currently it reads\n>\n> Cc: Christian Couder [chriscool@tuxfamily.org](mailto:chriscool@tuxfamily.org)\n> Cc: Junio C Hamano [gitster@pobox.com](mailto:gitster@pobox.com)\n> Cc: Emily Shaffer [nasamuffin@google.com](mailto:nasamuffin@google.com)\n> cc: Josh Steadmon [steadmon@google.com](mailto:steadmon@google.com)\n> cc: Randall S. Becker [rsbecker@nexbridge.com](mailto:rsbecker@nexbridge.com)\n> cc: Christian Couder [christian.couder@gmail.com](mailto:christian.couder@gmail.com)\n> cc: \"Kristoffer Haugsbakk\" [code@khaugsbakk.name](mailto:code@khaugsbakk.name)\n> cc: \"Kristoffer Haugsbakk\" \n>\n> when I click on \"edit\", where the last line must be from your manual fix\n> which GGG picked up. I've cleaned up the PR description manually now,\n> and for this message I'm also attempting to clean up those square\n> brackets.\n\nI would love to let myself be nerdsniped into working on this, alas,\nI cannot afford that before I learn the trick to stretch time.\n\nSo I did the next-best thing and jotted down pointers for any volunteer\nwho wants to work on this:\nhttps://github.com/gitgitgadget/gitgitgadget/issues/1645#issuecomment-2202545542\n\nCiao,\nJohannes\n"}]}