{"thread":{"id":"46548","subject":"[PATCH 0/5] make interpret-trailers useful for parsing","startedAt":"2017-08-09T12:21:54Z","lastAt":"2017-08-17T18:19:44Z","messageCount":68,"participants":["Jeff King","Junio C Hamano","Stefan Beller","Jonathan Tan","Jacob Keller","Christian Couder","Ramsay Jones","Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"325942","messageId":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","threadId":"46548","inReplyTo":null,"subject":"[PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T12:21:47Z","receivedAt":"2017-08-09T12:21:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Parsing trailers out of a commit message is _mostly_ easy, but there\narea a lot of funny corner cases (e.g., heuristics for how many\nnon-trailers must be present before a final paragraph isn't a trailer\nblock anymore). The code in trailer.c already knows about these corner\ncases, but there's no way to access it from the command line.\n\nThis series teaches interpret-trailers to parse and output just the\ntrailers. So now you can do:\n\n $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n git interpret-trailers --parse\n Signed-off-by: Hartmut Henkel \n Helped-by: Stefan Beller \n Signed-off-by: Ralf Thielow \n Acked-by: Matthias Rüster \n\nI considered a few different approaches before deciding on\nwhat's here:\n\n 1. The output format is actually the normal \"key: value\" trailers. I\n considered something more structured like JSON. But the \"key:\n value\" format is quite easy to parse, once it has been normalized\n (finding the trailers, unfolding whitespace continuation, etc).\n\n 2. This series introduces several orthogonal options which can be used\n together to achieve my goal, when there could just be a \"parse\"\n mode. Since interpret-trailers is plumbing, I reasoned that the\n individual options might still be useful apart from each other (for\n instance, you could re-normalize existing trailers while writing\n your new ones from a commit hook). I did add a \"--parse\" for\n convenience and to help point users in the right direction.\n\n For the same reason (and so we could build on other orthogonal\n features like --in-place and --trim-empty), I decided against\n having a separate command like \"git parse-trailers\".\n\n [1/5]: trailer: put process_trailers() options into a struct\n [2/5]: interpret-trailers: add an option to show only the trailers\n [3/5]: interpret-trailers: add an option to show only existing trailers\n [4/5]: interpret-trailers: add an option to normalize output\n [5/5]: interpret-trailers: add --parse convenience option\n\n Documentation/git-interpret-trailers.txt | 17 ++++++++\n builtin/interpret-trailers.c | 34 ++++++++++++---\n t/t7513-interpret-trailers.sh | 73 ++++++++++++++++++++++++++++++++\n trailer.c | 65 ++++++++++++++++++++++------\n trailer.h | 12 +++++-\n 5 files changed, 180 insertions(+), 21 deletions(-)\n\n-Peff\n"},{"id":"325943","messageId":"20170809122232.lfvznzh66m2a5i6o@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","subject":"[PATCH 1/5] trailer: put process_trailers() options into a struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T12:22:32Z","receivedAt":"2017-08-09T12:22:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already have two options and are about to add a few more.\nTo avoid having a huge number of boolean arguments, let's\nconvert to an options struct which can be passed in.\n\nSigned-off-by: Jeff King \n---\n builtin/interpret-trailers.c | 13 ++++++-------\n trailer.c | 9 +++++----\n trailer.h | 9 ++++++++-\n 3 files changed, 19 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 175f14797b..bb0d7b937a 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -18,13 +18,12 @@ static const char * const git_interpret_trailers_usage[] = {\n \n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n-\tint in_place = 0;\n-\tint trim_empty = 0;\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n \tstruct string_list trailers = STRING_LIST_INIT_NODUP;\n \n \tstruct option options[] = {\n-\t\tOPT_BOOL(0, \"in-place\", &in_place, N_(\"edit files in place\")),\n-\t\tOPT_BOOL(0, \"trim-empty\", &trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n+\t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -36,11 +35,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\n-\t\t\tprocess_trailers(argv[i], in_place, trim_empty, &trailers);\n+\t\t\tprocess_trailers(argv[i], &opts, &trailers);\n \t} else {\n-\t\tif (in_place)\n+\t\tif (opts.in_place)\n \t\t\tdie(_(\"no input file given for in-place editing\"));\n-\t\tprocess_trailers(NULL, in_place, trim_empty, &trailers);\n+\t\tprocess_trailers(NULL, &opts, &trailers);\n \t}\n \n \tstring_list_clear(&trailers, 0);\ndiff --git a/trailer.c b/trailer.c\nindex 751b56c009..0a0c2c264d 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -968,7 +968,8 @@ static FILE *create_in_place_tempfile(const char *file)\n \treturn outfile;\n }\n \n-void process_trailers(const char *file, int in_place, int trim_empty, struct string_list *trailers)\n+void process_trailers(const char *file, struct process_trailer_options *opts,\n+\t\t struct string_list *trailers)\n {\n \tLIST_HEAD(head);\n \tLIST_HEAD(arg_head);\n@@ -980,7 +981,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tread_input_file(&sb, file);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n@@ -990,14 +991,14 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, trim_empty);\n+\tprint_all(outfile, &head, opts->trim_empty);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n \tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\n \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n \ndiff --git a/trailer.h b/trailer.h\nindex 65cc5d79c6..b33c5aa57d 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -22,7 +22,14 @@ struct trailer_info {\n \tsize_t trailer_nr;\n };\n \n-void process_trailers(const char *file, int in_place, int trim_empty,\n+struct process_trailer_options {\n+\tint in_place;\n+\tint trim_empty;\n+};\n+\n+#define PROCESS_TRAILER_OPTIONS_INIT {0}\n+\n+void process_trailers(const char *file, struct process_trailer_options *opts,\n \t\t struct string_list *trailers);\n \n void trailer_info_get(struct trailer_info *info, const char *str);\n-- \n2.14.0.609.gd2d1f7ddf\n\n"},{"id":"325944","messageId":"20170809122402.7y4g5g5y2kkrky5e@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","subject":"[PATCH 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T12:24:03Z","receivedAt":"2017-08-09T12:24:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In theory it's easy for any reader who wants to parse\ntrailers to do so. But there are a lot of subtle corner\ncases around what counts as a trailer, when the trailer\nblock begins and ends, etc. Since interpret-trailers already\nhas our parsing logic, let's let callers ask it to just\noutput the trailers.\n\nThey still have to parse the \"key: value\" lines, but at\nleast they can ignore all of the other corner cases.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 3 +++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 37 ++++++++++++++++++++++++++++++++\n trailer.c | 19 ++++++++++------\n trailer.h | 1 +\n 5 files changed, 54 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 31cdeaecdf..295dffbd21 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -80,6 +80,9 @@ OPTIONS\n \ttrailer to the input messages. See the description of this\n \tcommand.\n \n+--only-trailers::\n+\tOutput only the trailers, not any other parts of the input.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex bb0d7b937a..afb12c11bc 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -24,6 +24,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 0c6f91c433..e5b0718ef6 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1275,4 +1275,41 @@ test_expect_success 'with cut line' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only trailers' '\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\t\tsign: A U Thor \n+\t\tadded: added-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--trailer added:added-value \\\n+\t\t--only-trailers >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'only-trailers omits non-trailer in middle of block' '\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody \n+\t\tSigned-off-by: somebody \n+\t\tsign: A U Thor \n+\tEOF\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\tthis is not a trailer\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 0a0c2c264d..a4ff99f98a 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -164,13 +164,15 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)\n \t\tfprintf(outfile, \"%s%c %s\\n\", tok, separators[0], val);\n }\n \n-static void print_all(FILE *outfile, struct list_head *head, int trim_empty)\n+static void print_all(FILE *outfile, struct list_head *head,\n+\t\t struct process_trailer_options *opts)\n {\n \tstruct list_head *pos;\n \tstruct trailer_item *item;\n \tlist_for_each(pos, head) {\n \t\titem = list_entry(pos, struct trailer_item, list);\n-\t\tif (!trim_empty || strlen(item->value) > 0)\n+\t\tif ((!opts->trim_empty || strlen(item->value) > 0) &&\n+\t\t (!opts->only_trailers || item->token))\n \t\t\tprint_tok_val(outfile, item->token, item->value);\n \t}\n }\n@@ -897,9 +899,10 @@ static int process_input_file(FILE *outfile,\n \ttrailer_info_get(&info, str);\n \n \t/* Print lines before the trailers as is */\n-\tfwrite(str, 1, info.trailer_start - str, outfile);\n+\tif (outfile)\n+\t\tfwrite(str, 1, info.trailer_start - str, outfile);\n \n-\tif (!info.blank_line_before_trailer)\n+\tif (outfile && !info.blank_line_before_trailer)\n \t\tfprintf(outfile, \"\\n\");\n \n \tfor (i = 0; i < info.trailer_nr; i++) {\n@@ -985,18 +988,20 @@ void process_trailers(const char *file, struct process_trailer_options *opts,\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n-\ttrailer_end = process_input_file(outfile, sb.buf, &head);\n+\ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n+\t\t\t\t\t sb.buf, &head);\n \n \tprocess_command_line_args(&arg_head, trailers);\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, opts->trim_empty);\n+\tprint_all(outfile, &head, opts);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n-\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n+\tif (!opts->only_trailers)\n+\t\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n \tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\ndiff --git a/trailer.h b/trailer.h\nindex b33c5aa57d..4270849d80 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -25,6 +25,7 @@ struct trailer_info {\n struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n+\tint only_trailers;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.609.gd2d1f7ddf\n\n"},{"id":"325945","messageId":"20170809122439.fscozhyvxcx2oq2n@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","subject":"[PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T12:24:40Z","receivedAt":"2017-08-09T12:24:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It can be useful to invoke interpret-trailers for the\nprimary purpose of parsing existing trailers. But in that\ncase, we don't want to apply existing ifMissing or ifExists\nrules from the config. Let's add a special mode where we\navoid applying those rules. Coupled with --only-trailers,\nthis gives us a reasonable parsing tool.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 5 +++++\n builtin/interpret-trailers.c | 7 +++++++\n t/t7513-interpret-trailers.sh | 15 +++++++++++++++\n trailer.c | 7 ++++---\n trailer.h | 1 +\n 5 files changed, 32 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 295dffbd21..b2a8fad248 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -83,6 +83,11 @@ OPTIONS\n --only-trailers::\n \tOutput only the trailers, not any other parts of the input.\n \n+--only-existing::\n+\tOutput only trailers that exist in the input; do not add any\n+\tfrom the command-line or by following configured `trailer.*`\n+\trules.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex afb12c11bc..a49f94ba34 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -25,6 +25,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n+\t\tOPT_BOOL(0, \"only-existing\", &opts.only_existing, N_(\"output only existing trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -33,6 +34,12 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t git_interpret_trailers_usage, 0);\n \n+\tif (opts.only_existing && trailers.nr)\n+\t\tusage_msg_opt(\n+\t\t\t_(\"--trailer with --only-existing does not make sense\"),\n+\t\t\tgit_interpret_trailers_usage,\n+\t\t\toptions);\n+\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex e5b0718ef6..525fd53e5b 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1312,4 +1312,19 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only existing' '\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--only-trailers --only-existing >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex a4ff99f98a..88f6efe523 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -991,9 +991,10 @@ void process_trailers(const char *file, struct process_trailer_options *opts,\n \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n \t\t\t\t\t sb.buf, &head);\n \n-\tprocess_command_line_args(&arg_head, trailers);\n-\n-\tprocess_trailers_lists(&head, &arg_head);\n+\tif (!opts->only_existing) {\n+\t\tprocess_command_line_args(&arg_head, trailers);\n+\t\tprocess_trailers_lists(&head, &arg_head);\n+\t}\n \n \tprint_all(outfile, &head, opts);\n \ndiff --git a/trailer.h b/trailer.h\nindex 4270849d80..6356b890ba 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -26,6 +26,7 @@ struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n \tint only_trailers;\n+\tint only_existing;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.609.gd2d1f7ddf\n\n"},{"id":"325946","messageId":"20170809122549.pk5dy74jd2hgt735@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","subject":"[PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T12:25:49Z","receivedAt":"2017-08-09T12:25:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of \"--only-trailers\" is to give a caller an output\nthat's easy for them to parse. Getting rid of the\nnon-trailer material helps, but we still may see more\ncomplicated syntax like whitespace continuation. Let's add\nan option to normalize the output into one \"key: value\" line\nper trailer.\n\nAs a bonus, this could be used even without --only-trailers\nto clean up unusual formatting in the incoming data.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 5 +++++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 21 ++++++++++++++++++++\n trailer.c | 34 +++++++++++++++++++++++++++++++-\n trailer.h | 1 +\n 5 files changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex b2a8fad248..6d867e8ab3 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -88,6 +88,11 @@ OPTIONS\n \tfrom the command-line or by following configured `trailer.*`\n \trules.\n \n+--normalize::\n+\tNormalize trailer output so that trailers appear exactly one per\n+\tline, with any existing whitespace continuation folded into a\n+\tsingle line.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex a49f94ba34..ed2d893b4f 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -26,6 +26,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-existing\", &opts.only_existing, N_(\"output only existing trailers\")),\n+\t\tOPT_BOOL(0, \"normalize\", &opts.normalize, N_(\"normalize trailer formatting\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 525fd53e5b..6ad1feb1c5 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1327,4 +1327,25 @@ test_expect_success 'only existing' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'normalize' '\n+\tcat >expected <<-\\EOF &&\n+\t\tfoo: continued across several lines\n+\tEOF\n+\t# pass through tr to make leading and trailing whitespace more obvious\n+\ttr _ \" \" <<-\\EOF |\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\tfoo:_\n+\t\t__continued\n+\t\t___across\n+\t\t____several\n+\t\t_____lines\n+\t\t___\n+\tEOF\n+\tgit interpret-trailers --only-trailers --only-existing --normalize >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 88f6efe523..575aa6dcbf 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -887,7 +887,35 @@ static int ends_with_blank_line(const char *buf, size_t len)\n \treturn is_blank_line(buf + ll);\n }\n \n+static void normalize_value(struct strbuf *val)\n+{\n+\tstruct strbuf out = STRBUF_INIT;\n+\tsize_t i;\n+\n+\tstrbuf_grow(&out, val->len);\n+\ti = 0;\n+\twhile (i < val->len) {\n+\t\tchar c = val->buf[i++];\n+\t\tif (c == '\\n') {\n+\t\t\t/* Collapse continuation down to a single space. */\n+\t\t\twhile (i < val->len && isspace(val->buf[i]))\n+\t\t\t\ti++;\n+\t\t\tstrbuf_addch(&out, ' ');\n+\t\t} else {\n+\t\t\tstrbuf_addch(&out, c);\n+\t\t}\n+\t}\n+\n+\t/* Empty lines may have left us with whitespace cruft at the edges */\n+\tstrbuf_trim(&out);\n+\n+\t/* output goes back to val as if we modified it in-place */\n+\tstrbuf_swap(&out, val);\n+\tstrbuf_release(&out);\n+}\n+\n static int process_input_file(FILE *outfile,\n+\t\t\t int normalize,\n \t\t\t const char *str,\n \t\t\t struct list_head *head)\n {\n@@ -914,12 +942,16 @@ static int process_input_file(FILE *outfile,\n \t\tif (separator_pos >= 1) {\n \t\t\tparse_trailer(&tok, &val, NULL, trailer,\n \t\t\t\t separator_pos);\n+\t\t\tif (normalize)\n+\t\t\t\tnormalize_value(&val);\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t strbuf_detach(&tok, NULL),\n \t\t\t\t\t strbuf_detach(&val, NULL));\n \t\t} else {\n \t\t\tstrbuf_addstr(&val, trailer);\n \t\t\tstrbuf_strip_suffix(&val, \"\\n\");\n+\t\t\tif (normalize)\n+\t\t\t\tnormalize_value(&val);\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t NULL,\n \t\t\t\t\t strbuf_detach(&val, NULL));\n@@ -989,7 +1021,7 @@ void process_trailers(const char *file, struct process_trailer_options *opts,\n \n \t/* Print the lines before the trailers */\n \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n-\t\t\t\t\t sb.buf, &head);\n+\t\t\t\t\t opts->normalize, sb.buf, &head);\n \n \tif (!opts->only_existing) {\n \t\tprocess_command_line_args(&arg_head, trailers);\ndiff --git a/trailer.h b/trailer.h\nindex 6356b890ba..a5a9c08b5f 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -27,6 +27,7 @@ struct process_trailer_options {\n \tint trim_empty;\n \tint only_trailers;\n \tint only_existing;\n+\tint normalize;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.609.gd2d1f7ddf\n\n"},{"id":"325947","messageId":"20170809122642.uwhv2epmgmr7wrk5@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","subject":"[PATCH 5/5] interpret-trailers: add --parse convenience option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T12:26:42Z","receivedAt":"2017-08-09T12:26:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The last few commits have added command line options that\ncan turn interpret-trailers into a parsing tool. Since\nthey'd most often be used together, let's provide a\nconvenient single option for callers to invoke this mode.\n\nThis is implemented as a callback rather than a boolean so\nthat its effect is applied immediately, as if those options\nhad been specified. Later options can then override them.\nE.g.:\n\n git interpret-trailers --parse --no-normalize\n\nwould work.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 4 ++++\n builtin/interpret-trailers.c | 12 ++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 6d867e8ab3..ab2d5c7696 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -93,6 +93,10 @@ OPTIONS\n \tline, with any existing whitespace continuation folded into a\n \tsingle line.\n \n+--parse::\n+\tA convenience alias for `--only-trailers --only-existing\n+\t--normalize`.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex ed2d893b4f..70ca855aa6 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -16,6 +16,16 @@ static const char * const git_interpret_trailers_usage[] = {\n \tNULL\n };\n \n+static int parse_opt_parse(const struct option *opt, const char *arg,\n+\t\t\t int unset)\n+{\n+\tstruct process_trailer_options *v = opt->value;\n+\tv->only_trailers = 1;\n+\tv->only_existing = 1;\n+\tv->normalize = 1;\n+\treturn 0;\n+}\n+\n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n \tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n@@ -27,6 +37,8 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-existing\", &opts.only_existing, N_(\"output only existing trailers\")),\n \t\tOPT_BOOL(0, \"normalize\", &opts.normalize, N_(\"normalize trailer formatting\")),\n+\t\t{ OPTION_CALLBACK, 0, \"parse\", &opts, NULL, N_(\"set parsing options\"),\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse },\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n-- \n2.14.0.609.gd2d1f7ddf\n"},{"id":"325965","messageId":"xmqq60dw7j5u.fsf@gitster.mtv.corp.google.com","threadId":"46548","inReplyTo":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-09T17:19:57Z","receivedAt":"2017-08-09T17:20:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Parsing trailers out of a commit message is _mostly_ easy, but there\n> area a lot of funny corner cases (e.g., heuristics for how many\n> non-trailers must be present before a final paragraph isn't a trailer\n> block anymore). The code in trailer.c already knows about these corner\n> cases, but there's no way to access it from the command line.\n>\n> This series teaches interpret-trailers to parse and output just the\n> trailers. So now you can do:\n>\n> $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n> git interpret-trailers --parse\n> Signed-off-by: Hartmut Henkel \n> Helped-by: Stefan Beller \n> Signed-off-by: Ralf Thielow \n> Acked-by: Matthias Rüster \n\nThank-you, thank-you, thank-you.\n\nThe above example made me wonder if we also want a format specifier\nto do the above without piping, but it turns out that we already\nhave \"log --format=%(trailers)\", so we are good ;-)\n\n> I considered a few different approaches before deciding on\n> what's here:\n>\n> 1. The output format is actually the normal \"key: value\" trailers. I\n> considered something more structured like JSON. But the \"key:\n> value\" format is quite easy to parse, once it has been normalized\n> (finding the trailers, unfolding whitespace continuation, etc).\n>\n> 2. This series introduces several orthogonal options which can be used\n> together to achieve my goal, when there could just be a \"parse\"\n> mode. Since interpret-trailers is plumbing, I reasoned that the\n> individual options might still be useful apart from each other (for\n> instance, you could re-normalize existing trailers while writing\n> your new ones from a commit hook). I did add a \"--parse\" for\n> convenience and to help point users in the right direction.\n>\n> For the same reason (and so we could build on other orthogonal\n> features like --in-place and --trim-empty), I decided against\n> having a separate command like \"git parse-trailers\".\n>\n> [1/5]: trailer: put process_trailers() options into a struct\n> [2/5]: interpret-trailers: add an option to show only the trailers\n> [3/5]: interpret-trailers: add an option to show only existing trailers\n> [4/5]: interpret-trailers: add an option to normalize output\n> [5/5]: interpret-trailers: add --parse convenience option\n>\n> Documentation/git-interpret-trailers.txt | 17 ++++++++\n> builtin/interpret-trailers.c | 34 ++++++++++++---\n> t/t7513-interpret-trailers.sh | 73 ++++++++++++++++++++++++++++++++\n> trailer.c | 65 ++++++++++++++++++++++------\n> trailer.h | 12 +++++-\n> 5 files changed, 180 insertions(+), 21 deletions(-)\n>\n> -Peff\n"},{"id":"325973","messageId":"CAGZ79kZxtq9Fe1usBgyNSpL1qAXiE-cVvBeWZf1_bP8bjMLwWw@mail.gmail.com","threadId":"46548","inReplyTo":"20170809122402.7y4g5g5y2kkrky5e@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-09T17:52:09Z","receivedAt":"2017-08-09T17:52:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 9, 2017 at 5:24 AM, Jeff King wrote:\n> In theory it's easy for any reader who wants to parse\n> trailers to do so. But there are a lot of subtle corner\n> cases around what counts as a trailer, when the trailer\n> block begins and ends, etc. Since interpret-trailers already\n> has our parsing logic, let's let callers ask it to just\n> output the trailers.\n>\n> They still have to parse the \"key: value\" lines, but at\n> least they can ignore all of the other corner cases.\n>\n> Signed-off-by: Jeff King \n> ---\n...\n> +test_expect_success 'only-trailers omits non-trailer in middle of block' '\n> + cat >expected <<-\\EOF &&\n> + Signed-off-by: nobody \n> + Signed-off-by: somebody \n> + sign: A U Thor \n> + EOF\n> + git interpret-trailers --only-trailers >actual <<-\\EOF &&\n> + subject\n> +\n> + it is important that the trailers below are signed-off-by\n> + so that they meet the \"25% trailers Git knows about\" heuristic\n> +\n> + Signed-off-by: nobody \n> + this is not a trailer\n\nPlease see 60ef86a162 (trailer: support values folded to multiple lines,\n2016-10-21), maybe we also want to test for\n\nVeryLongTrailerKey: long text with spaces and breaking\n the line.\n\nFor those parsing the trailer do we unbreak the line?\nSuch that one line equals one trailer? or is the user of the parsed\noutput expected to take care of this themselves?\n"},{"id":"325974","messageId":"CAGZ79kY072zvfV3QOA8-fr9sB26QqprxJLV46DrP6RUG4_inGw@mail.gmail.com","threadId":"46548","inReplyTo":"CAGZ79kZxtq9Fe1usBgyNSpL1qAXiE-cVvBeWZf1_bP8bjMLwWw@mail.gmail.com","subject":"Re: [PATCH 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-09T17:55:10Z","receivedAt":"2017-08-09T17:55:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 9, 2017 at 10:52 AM, Stefan Beller wrote:\n> On Wed, Aug 9, 2017 at 5:24 AM, Jeff King wrote:\n>> In theory it's easy for any reader who wants to parse\n>> trailers to do so. But there are a lot of subtle corner\n>> cases around what counts as a trailer, when the trailer\n>> block begins and ends, etc. Since interpret-trailers already\n>> has our parsing logic, let's let callers ask it to just\n>> output the trailers.\n>>\n>> They still have to parse the \"key: value\" lines, but at\n>> least they can ignore all of the other corner cases.\n>>\n>> Signed-off-by: Jeff King \n>> ---\n> ...\n>> +test_expect_success 'only-trailers omits non-trailer in middle of block' '\n>> + cat >expected <<-\\EOF &&\n>> + Signed-off-by: nobody \n>> + Signed-off-by: somebody \n>> + sign: A U Thor \n>> + EOF\n>> + git interpret-trailers --only-trailers >actual <<-\\EOF &&\n>> + subject\n>> +\n>> + it is important that the trailers below are signed-off-by\n>> + so that they meet the \"25% trailers Git knows about\" heuristic\n>> +\n>> + Signed-off-by: nobody \n>> + this is not a trailer\n>\n> Please see 60ef86a162 (trailer: support values folded to multiple lines,\n> 2016-10-21), maybe we also want to test for\n>\n> VeryLongTrailerKey: long text with spaces and breaking\n> the line.\n>\n> For those parsing the trailer do we unbreak the line?\n> Such that one line equals one trailer? or is the user of the parsed\n> output expected to take care of this themselves?\\\n\nNevermind, 4/5 solves that problem.\n"},{"id":"325978","messageId":"CAGZ79kYqq_EhRxckM4iV=99r59Y7_y94j+-65xXLUF4y2vNUUw@mail.gmail.com","threadId":"46548","inReplyTo":"20170809122439.fscozhyvxcx2oq2n@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-09T18:18:19Z","receivedAt":"2017-08-09T18:18:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 9, 2017 at 5:24 AM, Jeff King wrote:\n> It can be useful to invoke interpret-trailers for the\n> primary purpose of parsing existing trailers. But in that\n> case, we don't want to apply existing ifMissing or ifExists\n> rules from the config. Let's add a special mode where we\n> avoid applying those rules. Coupled with --only-trailers,\n> this gives us a reasonable parsing tool.\n\nI have the impression that the name is slightly misleading\nbecause 'only' just reduces the set. it does not enhance it.\n(Do we have a configuration that says \"remove this trailer\nanytime\"?)\n\nSo maybe this is rather worded as 'exact-trailers' ?\n"},{"id":"325979","messageId":"CAGZ79kZ8_bx3M6vac9MDLJjZCS5vqosPhO51dmGjC=3Fiz3O6Q@mail.gmail.com","threadId":"46548","inReplyTo":"20170809122642.uwhv2epmgmr7wrk5@sigill.intra.peff.net","subject":"Re: [PATCH 5/5] interpret-trailers: add --parse convenience option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-09T18:20:12Z","receivedAt":"2017-08-09T18:20:19Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 9, 2017 at 5:26 AM, Jeff King wrote:\n> The last few commits have added command line options that\n> can turn interpret-trailers into a parsing tool. Since\n> they'd most often be used together, let's provide a\n> convenient single option for callers to invoke this mode.\n>\n> This is implemented as a callback rather than a boolean so\n> that its effect is applied immediately, as if those options\n> had been specified. Later options can then override them.\n> E.g.:\n>\n> git interpret-trailers --parse --no-normalize\n>\n> would work.\n>\n> Signed-off-by: Jeff King \n> ---\n> Documentation/git-interpret-trailers.txt | 4 ++++\n> builtin/interpret-trailers.c | 12 ++++++++++++\n> 2 files changed, 16 insertions(+)\n>\n> diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\n> index 6d867e8ab3..ab2d5c7696 100644\n> --- a/Documentation/git-interpret-trailers.txt\n> +++ b/Documentation/git-interpret-trailers.txt\n> @@ -93,6 +93,10 @@ OPTIONS\n> line, with any existing whitespace continuation folded into a\n> single line.\n>\n> +--parse::\n> + A convenience alias for `--only-trailers --only-existing\n> + --normalize`.\n\nSomewhere in this series, we'd want to not just describe each\nof the new knobs, but reword the initial description, too?\n\n git-interpret-trailers - help add structured information\n into commit messages\n\nMaybe a s/add/handle/ s/into// is enough?\n"},{"id":"325980","messageId":"20170809113527.292c1776@twelve2.svl.corp.google.com","threadId":"46548","inReplyTo":"20170809122402.7y4g5g5y2kkrky5e@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-08-09T18:35:27Z","receivedAt":"2017-08-09T18:35:43Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 9 Aug 2017 08:24:03 -0400\nJeff King wrote:\n\n> diff --git a/trailer.c b/trailer.c\n> index 0a0c2c264d..a4ff99f98a 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -164,13 +164,15 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)\n> \t\tfprintf(outfile, \"%s%c %s\\n\", tok, separators[0], val);\n> }\n> \n> -static void print_all(FILE *outfile, struct list_head *head, int trim_empty)\n> +static void print_all(FILE *outfile, struct list_head *head,\n> +\t\t struct process_trailer_options *opts)\n\nThis can be const, I think. (Same thing for patch 1.)\n\n> {\n> \tstruct list_head *pos;\n> \tstruct trailer_item *item;\n> \tlist_for_each(pos, head) {\n> \t\titem = list_entry(pos, struct trailer_item, list);\n> -\t\tif (!trim_empty || strlen(item->value) > 0)\n> +\t\tif ((!opts->trim_empty || strlen(item->value) > 0) &&\n> +\t\t (!opts->only_trailers || item->token))\n> \t\t\tprint_tok_val(outfile, item->token, item->value);\n> \t}\n> }\n> @@ -897,9 +899,10 @@ static int process_input_file(FILE *outfile,\n> \ttrailer_info_get(&info, str);\n> \n> \t/* Print lines before the trailers as is */\n> -\tfwrite(str, 1, info.trailer_start - str, outfile);\n> +\tif (outfile)\n\nAny reason why you expect outfile to possibly be NULL?\n\n> +\t\tfwrite(str, 1, info.trailer_start - str, outfile);\n> \n> -\tif (!info.blank_line_before_trailer)\n> +\tif (outfile && !info.blank_line_before_trailer)\n\nSame comment here.\n\n> \t\tfprintf(outfile, \"\\n\");\n> \n> \tfor (i = 0; i < info.trailer_nr; i++) {\n"},{"id":"325981","messageId":"20170809113820.50cfb247@twelve2.svl.corp.google.com","threadId":"46548","inReplyTo":"20170809122439.fscozhyvxcx2oq2n@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-08-09T18:38:20Z","receivedAt":"2017-08-09T18:38:35Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 9 Aug 2017 08:24:40 -0400\nJeff King wrote:\n\n> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\n> index e5b0718ef6..525fd53e5b 100755\n> --- a/t/t7513-interpret-trailers.sh\n> +++ b/t/t7513-interpret-trailers.sh\n> @@ -1312,4 +1312,19 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success 'only existing' '\n> +\tcat >expected <<-\\EOF &&\n> +\t\texisting: existing-value\n> +\tEOF\n> +\tgit interpret-trailers \\\n> +\t\t--only-trailers --only-existing >actual <<-\\EOF &&\n> +\t\tmy subject\n> +\n> +\t\tmy body\n> +\n> +\t\texisting: existing-value\n> +\tEOF\n> +\ttest_cmp expected actual\n\nWould it be worth asserting that the \"sign\" trailer is configured here\nand would normally appear? Maybe through a grep on the output of \"git\nconfig\".\n\n> +'\n> +\n> test_done\n> diff --git a/trailer.c b/trailer.c\n> index a4ff99f98a..88f6efe523 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -991,9 +991,10 @@ void process_trailers(const char *file, struct process_trailer_options *opts,\n> \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n> \t\t\t\t\t sb.buf, &head);\n> \n> -\tprocess_command_line_args(&arg_head, trailers);\n> -\n> -\tprocess_trailers_lists(&head, &arg_head);\n> +\tif (!opts->only_existing) {\n> +\t\tprocess_command_line_args(&arg_head, trailers);\n> +\t\tprocess_trailers_lists(&head, &arg_head);\n> +\t}\n\nThis is a bit confusing, but it is correct, since\nprocess_command_line_args() processes both configured trailers and\ncommand-line trailers.\n\nHaving said that, it might be worth declaring LIST_HEAD(arg_head) inside\nthe if block now.\n"},{"id":"326006","messageId":"CA+P7+xoifkJyH34Q0NJdE_=UzWK1SA+2gwyXrHpF7Sv2PBHATQ@mail.gmail.com","threadId":"46548","inReplyTo":"xmqq60dw7j5u.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-08-10T07:04:49Z","receivedAt":"2017-08-10T07:05:16Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Aug 9, 2017 at 10:19 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> Parsing trailers out of a commit message is _mostly_ easy, but there\n>> area a lot of funny corner cases (e.g., heuristics for how many\n>> non-trailers must be present before a final paragraph isn't a trailer\n>> block anymore). The code in trailer.c already knows about these corner\n>> cases, but there's no way to access it from the command line.\n>>\n>> This series teaches interpret-trailers to parse and output just the\n>> trailers. So now you can do:\n>>\n>> $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n>> git interpret-trailers --parse\n>> Signed-off-by: Hartmut Henkel \n>> Helped-by: Stefan Beller \n>> Signed-off-by: Ralf Thielow \n>> Acked-by: Matthias Rüster \n>\n> Thank-you, thank-you, thank-you.\n>\n> The above example made me wonder if we also want a format specifier\n> to do the above without piping, but it turns out that we already\n> have \"log --format=%(trailers)\", so we are good ;-)\n>\n\nI was going to say, I thought we had a way to get trailers for a\ncommit via the pretty format, since that is what i used in the past.\n\nThanks,\nJake\n"},{"id":"326009","messageId":"20170810072822.rj6y6zcqhyfz4yi7@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CA+P7+xoifkJyH34Q0NJdE_=UzWK1SA+2gwyXrHpF7Sv2PBHATQ@mail.gmail.com","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T07:28:22Z","receivedAt":"2017-08-10T07:28:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2017 at 12:04:49AM -0700, Jacob Keller wrote:\n\n> >> $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n> >> git interpret-trailers --parse\n> >> Signed-off-by: Hartmut Henkel \n> >> Helped-by: Stefan Beller \n> >> Signed-off-by: Ralf Thielow \n> >> Acked-by: Matthias Rüster \n> >\n> > Thank-you, thank-you, thank-you.\n> >\n> > The above example made me wonder if we also want a format specifier\n> > to do the above without piping, but it turns out that we already\n> > have \"log --format=%(trailers)\", so we are good ;-)\n> \n> I was going to say, I thought we had a way to get trailers for a\n> commit via the pretty format, since that is what i used in the past.\n\nI do like that you could get the trailers for many commits in a single\ninvocation. That doesn't matter for my current use-case, but obviously\npiping through O(n) interpret-trailers invocations is a bad idea.\n\nBut there are a few difficulties with using %(trailers) for this, as it\nshows everything between the start/end points of the trailer block. In\nparticular:\n\n 1. You don't get any kind of normalization, so you're on your own for\n parsing things like whitespace continuation, extra spaces before\n separators, etc.\n\n 2. It prints non-trailers that fall inside the block. For instance:\n\n $ git commit --allow-empty -F - <<-\\EOF\n subject\n\n body\n\n Signed-off-by: me\n this is not a trailer\n Signed-off-by: you\n EOF\n \n $ git log -1 --format=%B | git interpret-trailers --parse\n Signed-off-by: me\n Signed-off-by: you\n\n $ git log -1 --format='%(trailers)'\n Signed-off-by: me\n this is not a trailer\n Signed-off-by: you\n\nFor (1) I think many callers would prefer to see the original\nformatting. Maybe we'd need a %(trailers:normalize) or something.\n\nI'm tempted to call (2) a bug, but I guess it's unclear whether callers\nwould want to see the whole block, or if they really want just the\nindividual trailers.\n\n-Peff\n"},{"id":"326010","messageId":"20170810073239.ny64rqmwb3yqbpb6@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAGZ79kYqq_EhRxckM4iV=99r59Y7_y94j+-65xXLUF4y2vNUUw@mail.gmail.com","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T07:32:40Z","receivedAt":"2017-08-10T07:32:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 09, 2017 at 11:18:19AM -0700, Stefan Beller wrote:\n\n> On Wed, Aug 9, 2017 at 5:24 AM, Jeff King wrote:\n> > It can be useful to invoke interpret-trailers for the\n> > primary purpose of parsing existing trailers. But in that\n> > case, we don't want to apply existing ifMissing or ifExists\n> > rules from the config. Let's add a special mode where we\n> > avoid applying those rules. Coupled with --only-trailers,\n> > this gives us a reasonable parsing tool.\n> \n> I have the impression that the name is slightly misleading\n> because 'only' just reduces the set. it does not enhance it.\n> (Do we have a configuration that says \"remove this trailer\n> anytime\"?)\n\nNo, I think you can only add trailers via ifExists or ifMissing.\nI actually called this --no-config originally, because to me it meant\n\"do not apply config\". But the processing applies also to --trailer\narguments no the command line, which is how I ended up with\n--only-existing.\n\n> So maybe this is rather worded as 'exact-trailers' ?\n\nI'm not fond of that, as it's vague about which exact trailers we're\ntalking about. I also thought of something like --verbatim, but I'd\nworry that would seem to conflict with --normalize.\n\nI dunno. All of the names seem not quite descriptive enough to me.\n\n-Peff\n"},{"id":"326011","messageId":"20170810073622.ojm2ezqodkcflj5y@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809113820.50cfb247@twelve2.svl.corp.google.com","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T07:36:22Z","receivedAt":"2017-08-10T07:36:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 09, 2017 at 11:38:20AM -0700, Jonathan Tan wrote:\n\n> > diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\n> > index e5b0718ef6..525fd53e5b 100755\n> > --- a/t/t7513-interpret-trailers.sh\n> > +++ b/t/t7513-interpret-trailers.sh\n> > @@ -1312,4 +1312,19 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n> > \ttest_cmp expected actual\n> > '\n> > \n> > +test_expect_success 'only existing' '\n> > +\tcat >expected <<-\\EOF &&\n> > +\t\texisting: existing-value\n> > +\tEOF\n> > +\tgit interpret-trailers \\\n> > +\t\t--only-trailers --only-existing >actual <<-\\EOF &&\n> > +\t\tmy subject\n> > +\n> > +\t\tmy body\n> > +\n> > +\t\texisting: existing-value\n> > +\tEOF\n> > +\ttest_cmp expected actual\n> \n> Would it be worth asserting that the \"sign\" trailer is configured here\n> and would normally appear? Maybe through a grep on the output of \"git\n> config\".\n\nI'd much rather re-add it than assert it with grep hackery. Note that\nits presence is implied in all of the follow-on tests, too (so I had\nsort of assumed that its presence in the --only-trailers test would\nimply that it was carried through to the others). We can be more\nexplicit, though, I guess.\n\n> > diff --git a/trailer.c b/trailer.c\n> > index a4ff99f98a..88f6efe523 100644\n> > --- a/trailer.c\n> > +++ b/trailer.c\n> > @@ -991,9 +991,10 @@ void process_trailers(const char *file, struct process_trailer_options *opts,\n> > \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n> > \t\t\t\t\t sb.buf, &head);\n> > \n> > -\tprocess_command_line_args(&arg_head, trailers);\n> > -\n> > -\tprocess_trailers_lists(&head, &arg_head);\n> > +\tif (!opts->only_existing) {\n> > +\t\tprocess_command_line_args(&arg_head, trailers);\n> > +\t\tprocess_trailers_lists(&head, &arg_head);\n> > +\t}\n> \n> This is a bit confusing, but it is correct, since\n> process_command_line_args() processes both configured trailers and\n> command-line trailers.\n\nYes, it confused me, too. That combination is why \"--trailer\" is\ndisallowed with --only-existing (which otherwise could work together). I\ndidn't think it was worth refactoring for a case that I don't think\nanybody would care about.\n\n> Having said that, it might be worth declaring LIST_HEAD(arg_head) inside\n> the if block now.\n\nAgreed.\n\n-Peff\n"},{"id":"326012","messageId":"20170810074038.3q5sclod7ag7xnti@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809113527.292c1776@twelve2.svl.corp.google.com","subject":"Re: [PATCH 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T07:40:38Z","receivedAt":"2017-08-10T07:40:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 09, 2017 at 11:35:27AM -0700, Jonathan Tan wrote:\n\n> > -static void print_all(FILE *outfile, struct list_head *head, int trim_empty)\n> > +static void print_all(FILE *outfile, struct list_head *head,\n> > +\t\t struct process_trailer_options *opts)\n> \n> This can be const, I think. (Same thing for patch 1.)\n\nOK. We often leave these kinds of option structs as non-const because\nthey can sometimes grow to carry state between functions (e.g.,\ndiff_opt). But it's certainly const-able now, so we can let somebody\nundo it later if they want.\n\n> > @@ -897,9 +899,10 @@ static int process_input_file(FILE *outfile,\n> > \ttrailer_info_get(&info, str);\n> > \n> > \t/* Print lines before the trailers as is */\n> > -\tfwrite(str, 1, info.trailer_start - str, outfile);\n> > +\tif (outfile)\n> \n> Any reason why you expect outfile to possibly be NULL?\n> \n> > +\t\tfwrite(str, 1, info.trailer_start - str, outfile);\n> > \n> > -\tif (!info.blank_line_before_trailer)\n> > +\tif (outfile && !info.blank_line_before_trailer)\n> \n> Same comment here.\n\nBecause of this hunk from later in the file where we pass in NULL:\n\n /* Print the lines before the trailers */\n- trailer_end = process_input_file(outfile, sb.buf, &head);\n+ trailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n+ sb.buf, &head);\n\n-Peff\n"},{"id":"326013","messageId":"20170810075903.2f4jsaog4ttvaf4p@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAGZ79kZ8_bx3M6vac9MDLJjZCS5vqosPhO51dmGjC=3Fiz3O6Q@mail.gmail.com","subject":"Re: [PATCH 5/5] interpret-trailers: add --parse convenience option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T07:59:03Z","receivedAt":"2017-08-10T07:59:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 09, 2017 at 11:20:12AM -0700, Stefan Beller wrote:\n\n> > +--parse::\n> > + A convenience alias for `--only-trailers --only-existing\n> > + --normalize`.\n> \n> Somewhere in this series, we'd want to not just describe each\n> of the new knobs, but reword the initial description, too?\n> \n> git-interpret-trailers - help add structured information\n> into commit messages\n> \n> Maybe a s/add/handle/ s/into// is enough?\n\nThanks, my initial attempt actually included that, when I thought I was\ngoing to have a unique \"parsing mode\". But then as it grew into\nindividual options, I dropped it. And forgot how modification-centric\nthe rest of the manpage description is. I'll try to include something in\nthe final commit, which is where the parsing mode is all tied together.\n\n-Peff\n"},{"id":"326014","messageId":"20170810080246.njjd5zkphytzmlda@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170809122147.g44nwaitzctbadzm@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T08:02:46Z","receivedAt":"2017-08-10T08:02:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 09, 2017 at 08:21:47AM -0400, Jeff King wrote:\n\n> This series teaches interpret-trailers to parse and output just the\n> trailers. So now you can do:\n> \n> $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n> git interpret-trailers --parse\n> Signed-off-by: Hartmut Henkel \n> Helped-by: Stefan Beller \n> Signed-off-by: Ralf Thielow \n> Acked-by: Matthias Rüster \n\nAnd here's a v2 that addresses all of the comments except one: Stefan\nsuggested that --only-existing wasn't a great name. I agree, but I like\neverything else less.\n\nSummary:\n\n - opts arguments are now const\n - tests that depend on the value of trailer.sign.command now set it\n explicitly\n - the arg_head variable is now moved into the conditional block whewre\n it's used\n - the interpret-trailers manpage has been updated in patch 5 to make\n it clear that \"add\" and \"parse\" are the two major modes\n\n [1/5]: trailer: put process_trailers() options into a struct\n [2/5]: interpret-trailers: add an option to show only the trailers\n [3/5]: interpret-trailers: add an option to show only existing trailers\n [4/5]: interpret-trailers: add an option to normalize output\n [5/5]: interpret-trailers: add --parse convenience option\n\n Documentation/git-interpret-trailers.txt | 34 +++++++++++---\n builtin/interpret-trailers.c | 34 +++++++++++---\n t/t7513-interpret-trailers.sh | 76 ++++++++++++++++++++++++++++++++\n trailer.c | 68 ++++++++++++++++++++++------\n trailer.h | 13 +++++-\n 5 files changed, 196 insertions(+), 29 deletions(-)\n\n-Peff\n"},{"id":"326015","messageId":"20170810080315.bkadgxstmxuqe6n6@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810080246.njjd5zkphytzmlda@sigill.intra.peff.net","subject":"[PATCH 1/5] trailer: put process_trailers() options into a struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T08:03:15Z","receivedAt":"2017-08-10T08:03:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already have two options and are about to add a few more.\nTo avoid having a huge number of boolean arguments, let's\nconvert to an options struct which can be passed in.\n\nSigned-off-by: Jeff King \n---\n builtin/interpret-trailers.c | 13 ++++++-------\n trailer.c | 10 ++++++----\n trailer.h | 10 +++++++++-\n 3 files changed, 21 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 175f14797b..bb0d7b937a 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -18,13 +18,12 @@ static const char * const git_interpret_trailers_usage[] = {\n \n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n-\tint in_place = 0;\n-\tint trim_empty = 0;\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n \tstruct string_list trailers = STRING_LIST_INIT_NODUP;\n \n \tstruct option options[] = {\n-\t\tOPT_BOOL(0, \"in-place\", &in_place, N_(\"edit files in place\")),\n-\t\tOPT_BOOL(0, \"trim-empty\", &trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n+\t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -36,11 +35,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\n-\t\t\tprocess_trailers(argv[i], in_place, trim_empty, &trailers);\n+\t\t\tprocess_trailers(argv[i], &opts, &trailers);\n \t} else {\n-\t\tif (in_place)\n+\t\tif (opts.in_place)\n \t\t\tdie(_(\"no input file given for in-place editing\"));\n-\t\tprocess_trailers(NULL, in_place, trim_empty, &trailers);\n+\t\tprocess_trailers(NULL, &opts, &trailers);\n \t}\n \n \tstring_list_clear(&trailers, 0);\ndiff --git a/trailer.c b/trailer.c\nindex 751b56c009..e21a0d1629 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -968,7 +968,9 @@ static FILE *create_in_place_tempfile(const char *file)\n \treturn outfile;\n }\n \n-void process_trailers(const char *file, int in_place, int trim_empty, struct string_list *trailers)\n+void process_trailers(const char *file,\n+\t\t const struct process_trailer_options *opts,\n+\t\t struct string_list *trailers)\n {\n \tLIST_HEAD(head);\n \tLIST_HEAD(arg_head);\n@@ -980,7 +982,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tread_input_file(&sb, file);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n@@ -990,14 +992,14 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, trim_empty);\n+\tprint_all(outfile, &head, opts->trim_empty);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n \tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\n \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n \ndiff --git a/trailer.h b/trailer.h\nindex 65cc5d79c6..9da00bedec 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -22,7 +22,15 @@ struct trailer_info {\n \tsize_t trailer_nr;\n };\n \n-void process_trailers(const char *file, int in_place, int trim_empty,\n+struct process_trailer_options {\n+\tint in_place;\n+\tint trim_empty;\n+};\n+\n+#define PROCESS_TRAILER_OPTIONS_INIT {0}\n+\n+void process_trailers(const char *file,\n+\t\t const struct process_trailer_options *opts,\n \t\t struct string_list *trailers);\n \n void trailer_info_get(struct trailer_info *info, const char *str);\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326016","messageId":"20170810080319.64ftt7yzcsmizvlm@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810080246.njjd5zkphytzmlda@sigill.intra.peff.net","subject":"[PATCH 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T08:03:19Z","receivedAt":"2017-08-10T08:03:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In theory it's easy for any reader who wants to parse\ntrailers to do so. But there are a lot of subtle corner\ncases around what counts as a trailer, when the trailer\nblock begins and ends, etc. Since interpret-trailers already\nhas our parsing logic, let's let callers ask it to just\noutput the trailers.\n\nThey still have to parse the \"key: value\" lines, but at\nleast they can ignore all of the other corner cases.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 3 +++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 39 ++++++++++++++++++++++++++++++++\n trailer.c | 19 ++++++++++------\n trailer.h | 1 +\n 5 files changed, 56 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 31cdeaecdf..295dffbd21 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -80,6 +80,9 @@ OPTIONS\n \ttrailer to the input messages. See the description of this\n \tcommand.\n \n+--only-trailers::\n+\tOutput only the trailers, not any other parts of the input.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex bb0d7b937a..afb12c11bc 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -24,6 +24,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 0c6f91c433..90d30037b7 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1275,4 +1275,43 @@ test_expect_success 'with cut line' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only trailers' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\t\tsign: config-value\n+\t\tadded: added-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--trailer added:added-value \\\n+\t\t--only-trailers >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'only-trailers omits non-trailer in middle of block' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody \n+\t\tSigned-off-by: somebody \n+\t\tsign: config-value\n+\tEOF\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\tthis is not a trailer\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex e21a0d1629..706351fc98 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -164,13 +164,15 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)\n \t\tfprintf(outfile, \"%s%c %s\\n\", tok, separators[0], val);\n }\n \n-static void print_all(FILE *outfile, struct list_head *head, int trim_empty)\n+static void print_all(FILE *outfile, struct list_head *head,\n+\t\t const struct process_trailer_options *opts)\n {\n \tstruct list_head *pos;\n \tstruct trailer_item *item;\n \tlist_for_each(pos, head) {\n \t\titem = list_entry(pos, struct trailer_item, list);\n-\t\tif (!trim_empty || strlen(item->value) > 0)\n+\t\tif ((!opts->trim_empty || strlen(item->value) > 0) &&\n+\t\t (!opts->only_trailers || item->token))\n \t\t\tprint_tok_val(outfile, item->token, item->value);\n \t}\n }\n@@ -897,9 +899,10 @@ static int process_input_file(FILE *outfile,\n \ttrailer_info_get(&info, str);\n \n \t/* Print lines before the trailers as is */\n-\tfwrite(str, 1, info.trailer_start - str, outfile);\n+\tif (outfile)\n+\t\tfwrite(str, 1, info.trailer_start - str, outfile);\n \n-\tif (!info.blank_line_before_trailer)\n+\tif (outfile && !info.blank_line_before_trailer)\n \t\tfprintf(outfile, \"\\n\");\n \n \tfor (i = 0; i < info.trailer_nr; i++) {\n@@ -986,18 +989,20 @@ void process_trailers(const char *file,\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n-\ttrailer_end = process_input_file(outfile, sb.buf, &head);\n+\ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n+\t\t\t\t\t sb.buf, &head);\n \n \tprocess_command_line_args(&arg_head, trailers);\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, opts->trim_empty);\n+\tprint_all(outfile, &head, opts);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n-\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n+\tif (!opts->only_trailers)\n+\t\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n \tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\ndiff --git a/trailer.h b/trailer.h\nindex 9da00bedec..3cf35ced00 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -25,6 +25,7 @@ struct trailer_info {\n struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n+\tint only_trailers;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326017","messageId":"20170810080322.3jlbzjmmxg2jidbg@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810080246.njjd5zkphytzmlda@sigill.intra.peff.net","subject":"[PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T08:03:22Z","receivedAt":"2017-08-10T08:03:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It can be useful to invoke interpret-trailers for the\nprimary purpose of parsing existing trailers. But in that\ncase, we don't want to apply existing ifMissing or ifExists\nrules from the config. Let's add a special mode where we\navoid applying those rules. Coupled with --only-trailers,\nthis gives us a reasonable parsing tool.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 5 +++++\n builtin/interpret-trailers.c | 7 +++++++\n t/t7513-interpret-trailers.sh | 16 ++++++++++++++++\n trailer.c | 9 +++++----\n trailer.h | 1 +\n 5 files changed, 34 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 295dffbd21..b2a8fad248 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -83,6 +83,11 @@ OPTIONS\n --only-trailers::\n \tOutput only the trailers, not any other parts of the input.\n \n+--only-existing::\n+\tOutput only trailers that exist in the input; do not add any\n+\tfrom the command-line or by following configured `trailer.*`\n+\trules.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex afb12c11bc..a49f94ba34 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -25,6 +25,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n+\t\tOPT_BOOL(0, \"only-existing\", &opts.only_existing, N_(\"output only existing trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -33,6 +34,12 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t git_interpret_trailers_usage, 0);\n \n+\tif (opts.only_existing && trailers.nr)\n+\t\tusage_msg_opt(\n+\t\t\t_(\"--trailer with --only-existing does not make sense\"),\n+\t\t\tgit_interpret_trailers_usage,\n+\t\t\toptions);\n+\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 90d30037b7..97f4ac0fb3 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1314,4 +1314,20 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only existing' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--only-trailers --only-existing >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 706351fc98..3abac3db37 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -976,7 +976,6 @@ void process_trailers(const char *file,\n \t\t struct string_list *trailers)\n {\n \tLIST_HEAD(head);\n-\tLIST_HEAD(arg_head);\n \tstruct strbuf sb = STRBUF_INIT;\n \tint trailer_end;\n \tFILE *outfile = stdout;\n@@ -992,9 +991,11 @@ void process_trailers(const char *file,\n \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n \t\t\t\t\t sb.buf, &head);\n \n-\tprocess_command_line_args(&arg_head, trailers);\n-\n-\tprocess_trailers_lists(&head, &arg_head);\n+\tif (!opts->only_existing) {\n+\t\tLIST_HEAD(arg_head);\n+\t\tprocess_command_line_args(&arg_head, trailers);\n+\t\tprocess_trailers_lists(&head, &arg_head);\n+\t}\n \n \tprint_all(outfile, &head, opts);\n \ndiff --git a/trailer.h b/trailer.h\nindex 3cf35ced00..96dcbe801a 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -26,6 +26,7 @@ struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n \tint only_trailers;\n+\tint only_existing;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326018","messageId":"20170810080325.tehbbgajm4cgn2ku@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810080246.njjd5zkphytzmlda@sigill.intra.peff.net","subject":"[PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T08:03:25Z","receivedAt":"2017-08-10T08:03:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of \"--only-trailers\" is to give a caller an output\nthat's easy for them to parse. Getting rid of the\nnon-trailer material helps, but we still may see more\ncomplicated syntax like whitespace continuation. Let's add\nan option to normalize the output into one \"key: value\" line\nper trailer.\n\nAs a bonus, this could be used even without --only-trailers\nto clean up unusual formatting in the incoming data.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 5 +++++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 21 ++++++++++++++++++++\n trailer.c | 34 +++++++++++++++++++++++++++++++-\n trailer.h | 1 +\n 5 files changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex b2a8fad248..6537faf887 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -88,6 +88,11 @@ OPTIONS\n \tfrom the command-line or by following configured `trailer.*`\n \trules.\n \n+--normalize::\n+\tNormalize trailer output so that trailers appear exactly one per\n+\tline, with any existing whitespace continuation folded into a\n+\tsingle line.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex a49f94ba34..ed2d893b4f 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -26,6 +26,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-existing\", &opts.only_existing, N_(\"output only existing trailers\")),\n+\t\tOPT_BOOL(0, \"normalize\", &opts.normalize, N_(\"normalize trailer formatting\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 97f4ac0fb3..ef34ee4e49 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1330,4 +1330,25 @@ test_expect_success 'only existing' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'normalize' '\n+\tcat >expected <<-\\EOF &&\n+\t\tfoo: continued across several lines\n+\tEOF\n+\t# pass through tr to make leading and trailing whitespace more obvious\n+\ttr _ \" \" <<-\\EOF |\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\tfoo:_\n+\t\t__continued\n+\t\t___across\n+\t\t____several\n+\t\t_____lines\n+\t\t___\n+\tEOF\n+\tgit interpret-trailers --only-trailers --only-existing --normalize >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 3abac3db37..1ccf9996b1 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -887,7 +887,35 @@ static int ends_with_blank_line(const char *buf, size_t len)\n \treturn is_blank_line(buf + ll);\n }\n \n+static void normalize_value(struct strbuf *val)\n+{\n+\tstruct strbuf out = STRBUF_INIT;\n+\tsize_t i;\n+\n+\tstrbuf_grow(&out, val->len);\n+\ti = 0;\n+\twhile (i < val->len) {\n+\t\tchar c = val->buf[i++];\n+\t\tif (c == '\\n') {\n+\t\t\t/* Collapse continuation down to a single space. */\n+\t\t\twhile (i < val->len && isspace(val->buf[i]))\n+\t\t\t\ti++;\n+\t\t\tstrbuf_addch(&out, ' ');\n+\t\t} else {\n+\t\t\tstrbuf_addch(&out, c);\n+\t\t}\n+\t}\n+\n+\t/* Empty lines may have left us with whitespace cruft at the edges */\n+\tstrbuf_trim(&out);\n+\n+\t/* output goes back to val as if we modified it in-place */\n+\tstrbuf_swap(&out, val);\n+\tstrbuf_release(&out);\n+}\n+\n static int process_input_file(FILE *outfile,\n+\t\t\t int normalize,\n \t\t\t const char *str,\n \t\t\t struct list_head *head)\n {\n@@ -914,12 +942,16 @@ static int process_input_file(FILE *outfile,\n \t\tif (separator_pos >= 1) {\n \t\t\tparse_trailer(&tok, &val, NULL, trailer,\n \t\t\t\t separator_pos);\n+\t\t\tif (normalize)\n+\t\t\t\tnormalize_value(&val);\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t strbuf_detach(&tok, NULL),\n \t\t\t\t\t strbuf_detach(&val, NULL));\n \t\t} else {\n \t\t\tstrbuf_addstr(&val, trailer);\n \t\t\tstrbuf_strip_suffix(&val, \"\\n\");\n+\t\t\tif (normalize)\n+\t\t\t\tnormalize_value(&val);\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t NULL,\n \t\t\t\t\t strbuf_detach(&val, NULL));\n@@ -989,7 +1021,7 @@ void process_trailers(const char *file,\n \n \t/* Print the lines before the trailers */\n \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n-\t\t\t\t\t sb.buf, &head);\n+\t\t\t\t\t opts->normalize, sb.buf, &head);\n \n \tif (!opts->only_existing) {\n \t\tLIST_HEAD(arg_head);\ndiff --git a/trailer.h b/trailer.h\nindex 96dcbe801a..0eaf2e1bdf 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -27,6 +27,7 @@ struct process_trailer_options {\n \tint trim_empty;\n \tint only_trailers;\n \tint only_existing;\n+\tint normalize;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326019","messageId":"20170810080328.mzxgstxl5zma67nu@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810080246.njjd5zkphytzmlda@sigill.intra.peff.net","subject":"[PATCH 5/5] interpret-trailers: add --parse convenience option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T08:03:28Z","receivedAt":"2017-08-10T08:03:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The last few commits have added command line options that\ncan turn interpret-trailers into a parsing tool. Since\nthey'd most often be used together, let's provide a\nconvenient single option for callers to invoke this mode.\n\nThis is implemented as a callback rather than a boolean so\nthat its effect is applied immediately, as if those options\nhad been specified. Later options can then override them.\nE.g.:\n\n git interpret-trailers --parse --no-normalize\n\nwould work.\n\nLet's also update the documentation to make clear that this\nparsing mode behaves quite differently than the normal\n\"add trailers to the input\" mode.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 21 ++++++++++++++-------\n builtin/interpret-trailers.c | 12 ++++++++++++\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 6537faf887..863cb4ec5d 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -3,24 +3,27 @@ git-interpret-trailers(1)\n \n NAME\n ----\n-git-interpret-trailers - help add structured information into commit messages\n+git-interpret-trailers - add or parse structured information in commit messages\n \n SYNOPSIS\n --------\n [verse]\n-'git interpret-trailers' [--in-place] [--trim-empty] [(--trailer [(=|:)])...] [...]\n+'git interpret-trailers' [options] [(--trailer [(=|:)])...] [...]\n+'git interpret-trailers' [options] [--parse] [...]\n \n DESCRIPTION\n -----------\n-Help adding 'trailers' lines, that look similar to RFC 822 e-mail\n+Help parsing or adding 'trailers' lines, that look similar to RFC 822 e-mail\n headers, at the end of the otherwise free-form part of a commit\n message.\n \n This command reads some patches or commit messages from either the\n- arguments or the standard input if no is specified. Then\n-this command applies the arguments passed using the `--trailer`\n-option, if any, to the commit message part of each input file. The\n-result is emitted on the standard output.\n+ arguments or the standard input if no is specified. If\n+`--parse` is specified, the output consists of the parsed trailers.\n+\n+Otherwise, the this command applies the arguments passed using the\n+`--trailer` option, if any, to the commit message part of each input\n+file. The result is emitted on the standard output.\n \n Some configuration variables control the way the `--trailer` arguments\n are applied to each commit message and the way any existing trailer in\n@@ -93,6 +96,10 @@ OPTIONS\n \tline, with any existing whitespace continuation folded into a\n \tsingle line.\n \n+--parse::\n+\tA convenience alias for `--only-trailers --only-existing\n+\t--normalize`.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex ed2d893b4f..70ca855aa6 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -16,6 +16,16 @@ static const char * const git_interpret_trailers_usage[] = {\n \tNULL\n };\n \n+static int parse_opt_parse(const struct option *opt, const char *arg,\n+\t\t\t int unset)\n+{\n+\tstruct process_trailer_options *v = opt->value;\n+\tv->only_trailers = 1;\n+\tv->only_existing = 1;\n+\tv->normalize = 1;\n+\treturn 0;\n+}\n+\n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n \tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n@@ -27,6 +37,8 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-existing\", &opts.only_existing, N_(\"output only existing trailers\")),\n \t\tOPT_BOOL(0, \"normalize\", &opts.normalize, N_(\"normalize trailer formatting\")),\n+\t\t{ OPTION_CALLBACK, 0, \"parse\", &opts, NULL, N_(\"set parsing options\"),\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse },\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n-- \n2.14.0.614.g0beb26d5e9\n"},{"id":"326036","messageId":"CAGZ79kYHPhaZLSsBq2vM=D+_QMP9MY=xYjRwfG5q36neFyNH3w@mail.gmail.com","threadId":"46548","inReplyTo":"20170810073239.ny64rqmwb3yqbpb6@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-10T17:27:19Z","receivedAt":"2017-08-10T17:27:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 10, 2017 at 12:32 AM, Jeff King wrote:\n> On Wed, Aug 09, 2017 at 11:18:19AM -0700, Stefan Beller wrote:\n>\n>> On Wed, Aug 9, 2017 at 5:24 AM, Jeff King wrote:\n>> > It can be useful to invoke interpret-trailers for the\n>> > primary purpose of parsing existing trailers. But in that\n>> > case, we don't want to apply existing ifMissing or ifExists\n>> > rules from the config. Let's add a special mode where we\n>> > avoid applying those rules. Coupled with --only-trailers,\n>> > this gives us a reasonable parsing tool.\n>>\n>> I have the impression that the name is slightly misleading\n>> because 'only' just reduces the set. it does not enhance it.\n>> (Do we have a configuration that says \"remove this trailer\n>> anytime\"?)\n>\n> No, I think you can only add trailers via ifExists or ifMissing.\n> I actually called this --no-config originally, because to me it meant\n> \"do not apply config\". But the processing applies also to --trailer\n> arguments no the command line, which is how I ended up with\n> --only-existing.\n>\n>> So maybe this is rather worded as 'exact-trailers' ?\n>\n> I'm not fond of that, as it's vague about which exact trailers we're\n> talking about. I also thought of something like --verbatim, but I'd\n> worry that would seem to conflict with --normalize.\n>\n> I dunno. All of the names seem not quite descriptive enough to me.\n\nI meant 'exact' as in 'exactly from the patch/commit, no external\ninfluence such as config', so maybe '--from-patch' or '--from-commit'\n(which says the same as --no-config just the other way round.\nHaving --no- in config options as the standard is a UX disaster\nIMHO as then we have to forbid the --no-no-X or reintroduce X\nand flip the default)\n\nMaybe --genuine ?\n\n>\n> -Peff\n"},{"id":"326038","messageId":"20170810173328.xfrswan6tijwkaex@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAGZ79kYHPhaZLSsBq2vM=D+_QMP9MY=xYjRwfG5q36neFyNH3w@mail.gmail.com","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T17:33:28Z","receivedAt":"2017-08-10T17:33:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2017 at 10:27:19AM -0700, Stefan Beller wrote:\n\n> > I'm not fond of that, as it's vague about which exact trailers we're\n> > talking about. I also thought of something like --verbatim, but I'd\n> > worry that would seem to conflict with --normalize.\n> >\n> > I dunno. All of the names seem not quite descriptive enough to me.\n> \n> I meant 'exact' as in 'exactly from the patch/commit, no external\n> influence such as config', so maybe '--from-patch' or '--from-commit'\n> (which says the same as --no-config just the other way round.\n> Having --no- in config options as the standard is a UX disaster\n> IMHO as then we have to forbid the --no-no-X or reintroduce X\n> and flip the default)\n\nYes, that was definitely the other reason I didn't want to call it\n\"--no-config\". :)\n\nIt's not always from a patch or commit. The most accurate along those\nlines is \"--from-input\". \n\n> Maybe --genuine ?\n\nBut in the greater context I think that's vague again; we don't know\nwhich part of the command's operation is \"genuine\".\n\nPerhaps \"--exact-input\" hits all of those. Or maybe \"--only-input\" to\nmatch the other \"--only\".\n\nI think I like that last one the best. It makes it clear that we are\nlooking just at the input, and not anything else. Which is exactly what\nthe feature does.\n\n-Peff\n"},{"id":"326041","messageId":"CAGZ79kaAK-kKTw37S6OtvecmiXyS4XJF9BH=LDXi67cUxA_JHg@mail.gmail.com","threadId":"46548","inReplyTo":"20170810173328.xfrswan6tijwkaex@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-10T17:38:45Z","receivedAt":"2017-08-10T17:38:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 10, 2017 at 10:33 AM, Jeff King wrote:\n> On Thu, Aug 10, 2017 at 10:27:19AM -0700, Stefan Beller wrote:\n>\n>> > I'm not fond of that, as it's vague about which exact trailers we're\n>> > talking about. I also thought of something like --verbatim, but I'd\n>> > worry that would seem to conflict with --normalize.\n>> >\n>> > I dunno. All of the names seem not quite descriptive enough to me.\n>>\n>> I meant 'exact' as in 'exactly from the patch/commit, no external\n>> influence such as config', so maybe '--from-patch' or '--from-commit'\n>> (which says the same as --no-config just the other way round.\n>> Having --no- in config options as the standard is a UX disaster\n>> IMHO as then we have to forbid the --no-no-X or reintroduce X\n>> and flip the default)\n>\n> Yes, that was definitely the other reason I didn't want to call it\n> \"--no-config\". :)\n>\n> It's not always from a patch or commit. The most accurate along those\n> lines is \"--from-input\".\n>\n>> Maybe --genuine ?\n>\n> But in the greater context I think that's vague again; we don't know\n> which part of the command's operation is \"genuine\".\n\nThe input of course. ;) --genuine-input.\n\n>\n> Perhaps \"--exact-input\" hits all of those. Or maybe \"--only-input\" to\n> match the other \"--only\".\n>\n> I think I like that last one the best. It makes it clear that we are\n> looking just at the input, and not anything else. Which is exactly what\n> the feature does.\n\nMakes sense to me,\n\nThanks,\nStefan\n"},{"id":"326043","messageId":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810080246.njjd5zkphytzmlda@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:03:26Z","receivedAt":"2017-08-10T18:03:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2017 at 04:02:46AM -0400, Jeff King wrote:\n\n> On Wed, Aug 09, 2017 at 08:21:47AM -0400, Jeff King wrote:\n> \n> > This series teaches interpret-trailers to parse and output just the\n> > trailers. So now you can do:\n> > \n> > $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n> > git interpret-trailers --parse\n> > Signed-off-by: Hartmut Henkel \n> > Helped-by: Stefan Beller \n> > Signed-off-by: Ralf Thielow \n> > Acked-by: Matthias Rüster \n> \n> And here's a v2 that addresses all of the comments except one: Stefan\n> suggested that --only-existing wasn't a great name. I agree, but I like\n> everything else less.\n\nHere's a v3 that takes care of that (renaming it to --only-input).\n\nIt's otherwise the same as v2, but since the name-change ripples through\nthe remaining patches, I wanted to get v3 in front of people sooner\nrather than later.\n\n [1/5]: trailer: put process_trailers() options into a struct\n [2/5]: interpret-trailers: add an option to show only the trailers\n [3/5]: interpret-trailers: add an option to show only existing trailers\n [4/5]: interpret-trailers: add an option to normalize output\n [5/5]: interpret-trailers: add --parse convenience option\n\n Documentation/git-interpret-trailers.txt | 34 +++++++++++---\n builtin/interpret-trailers.c | 34 +++++++++++---\n t/t7513-interpret-trailers.sh | 76 ++++++++++++++++++++++++++++++++\n trailer.c | 68 ++++++++++++++++++++++------\n trailer.h | 13 +++++-\n 5 files changed, 196 insertions(+), 29 deletions(-)\n\n"},{"id":"326044","messageId":"20170810180358.xym3cj2p6qgtrpmg@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","subject":"[PATCH v3 1/5] trailer: put process_trailers() options into a struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:03:58Z","receivedAt":"2017-08-10T18:04:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already have two options and are about to add a few more.\nTo avoid having a huge number of boolean arguments, let's\nconvert to an options struct which can be passed in.\n\nSigned-off-by: Jeff King \n---\n builtin/interpret-trailers.c | 13 ++++++-------\n trailer.c | 10 ++++++----\n trailer.h | 10 +++++++++-\n 3 files changed, 21 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 175f14797b..bb0d7b937a 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -18,13 +18,12 @@ static const char * const git_interpret_trailers_usage[] = {\n \n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n-\tint in_place = 0;\n-\tint trim_empty = 0;\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n \tstruct string_list trailers = STRING_LIST_INIT_NODUP;\n \n \tstruct option options[] = {\n-\t\tOPT_BOOL(0, \"in-place\", &in_place, N_(\"edit files in place\")),\n-\t\tOPT_BOOL(0, \"trim-empty\", &trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n+\t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -36,11 +35,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\n-\t\t\tprocess_trailers(argv[i], in_place, trim_empty, &trailers);\n+\t\t\tprocess_trailers(argv[i], &opts, &trailers);\n \t} else {\n-\t\tif (in_place)\n+\t\tif (opts.in_place)\n \t\t\tdie(_(\"no input file given for in-place editing\"));\n-\t\tprocess_trailers(NULL, in_place, trim_empty, &trailers);\n+\t\tprocess_trailers(NULL, &opts, &trailers);\n \t}\n \n \tstring_list_clear(&trailers, 0);\ndiff --git a/trailer.c b/trailer.c\nindex 751b56c009..e21a0d1629 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -968,7 +968,9 @@ static FILE *create_in_place_tempfile(const char *file)\n \treturn outfile;\n }\n \n-void process_trailers(const char *file, int in_place, int trim_empty, struct string_list *trailers)\n+void process_trailers(const char *file,\n+\t\t const struct process_trailer_options *opts,\n+\t\t struct string_list *trailers)\n {\n \tLIST_HEAD(head);\n \tLIST_HEAD(arg_head);\n@@ -980,7 +982,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tread_input_file(&sb, file);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n@@ -990,14 +992,14 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, trim_empty);\n+\tprint_all(outfile, &head, opts->trim_empty);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n \tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\n \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n \ndiff --git a/trailer.h b/trailer.h\nindex 65cc5d79c6..9da00bedec 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -22,7 +22,15 @@ struct trailer_info {\n \tsize_t trailer_nr;\n };\n \n-void process_trailers(const char *file, int in_place, int trim_empty,\n+struct process_trailer_options {\n+\tint in_place;\n+\tint trim_empty;\n+};\n+\n+#define PROCESS_TRAILER_OPTIONS_INIT {0}\n+\n+void process_trailers(const char *file,\n+\t\t const struct process_trailer_options *opts,\n \t\t struct string_list *trailers);\n \n void trailer_info_get(struct trailer_info *info, const char *str);\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326045","messageId":"20170810180404.yg3jdzhquregh4ta@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","subject":"[PATCH v3 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:04:04Z","receivedAt":"2017-08-10T18:04:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In theory it's easy for any reader who wants to parse\ntrailers to do so. But there are a lot of subtle corner\ncases around what counts as a trailer, when the trailer\nblock begins and ends, etc. Since interpret-trailers already\nhas our parsing logic, let's let callers ask it to just\noutput the trailers.\n\nThey still have to parse the \"key: value\" lines, but at\nleast they can ignore all of the other corner cases.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 3 +++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 39 ++++++++++++++++++++++++++++++++\n trailer.c | 19 ++++++++++------\n trailer.h | 1 +\n 5 files changed, 56 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 31cdeaecdf..295dffbd21 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -80,6 +80,9 @@ OPTIONS\n \ttrailer to the input messages. See the description of this\n \tcommand.\n \n+--only-trailers::\n+\tOutput only the trailers, not any other parts of the input.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex bb0d7b937a..afb12c11bc 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -24,6 +24,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 0c6f91c433..90d30037b7 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1275,4 +1275,43 @@ test_expect_success 'with cut line' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only trailers' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\t\tsign: config-value\n+\t\tadded: added-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--trailer added:added-value \\\n+\t\t--only-trailers >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'only-trailers omits non-trailer in middle of block' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody \n+\t\tSigned-off-by: somebody \n+\t\tsign: config-value\n+\tEOF\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\tthis is not a trailer\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex e21a0d1629..706351fc98 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -164,13 +164,15 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)\n \t\tfprintf(outfile, \"%s%c %s\\n\", tok, separators[0], val);\n }\n \n-static void print_all(FILE *outfile, struct list_head *head, int trim_empty)\n+static void print_all(FILE *outfile, struct list_head *head,\n+\t\t const struct process_trailer_options *opts)\n {\n \tstruct list_head *pos;\n \tstruct trailer_item *item;\n \tlist_for_each(pos, head) {\n \t\titem = list_entry(pos, struct trailer_item, list);\n-\t\tif (!trim_empty || strlen(item->value) > 0)\n+\t\tif ((!opts->trim_empty || strlen(item->value) > 0) &&\n+\t\t (!opts->only_trailers || item->token))\n \t\t\tprint_tok_val(outfile, item->token, item->value);\n \t}\n }\n@@ -897,9 +899,10 @@ static int process_input_file(FILE *outfile,\n \ttrailer_info_get(&info, str);\n \n \t/* Print lines before the trailers as is */\n-\tfwrite(str, 1, info.trailer_start - str, outfile);\n+\tif (outfile)\n+\t\tfwrite(str, 1, info.trailer_start - str, outfile);\n \n-\tif (!info.blank_line_before_trailer)\n+\tif (outfile && !info.blank_line_before_trailer)\n \t\tfprintf(outfile, \"\\n\");\n \n \tfor (i = 0; i < info.trailer_nr; i++) {\n@@ -986,18 +989,20 @@ void process_trailers(const char *file,\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n-\ttrailer_end = process_input_file(outfile, sb.buf, &head);\n+\ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n+\t\t\t\t\t sb.buf, &head);\n \n \tprocess_command_line_args(&arg_head, trailers);\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, opts->trim_empty);\n+\tprint_all(outfile, &head, opts);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n-\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n+\tif (!opts->only_trailers)\n+\t\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n \tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\ndiff --git a/trailer.h b/trailer.h\nindex 9da00bedec..3cf35ced00 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -25,6 +25,7 @@ struct trailer_info {\n struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n+\tint only_trailers;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326046","messageId":"20170810180407.kryp545rplazxheq@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","subject":"[PATCH v3 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:04:07Z","receivedAt":"2017-08-10T18:04:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It can be useful to invoke interpret-trailers for the\nprimary purpose of parsing existing trailers. But in that\ncase, we don't want to apply existing ifMissing or ifExists\nrules from the config. Let's add a special mode where we\navoid applying those rules. Coupled with --only-trailers,\nthis gives us a reasonable parsing tool.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 5 +++++\n builtin/interpret-trailers.c | 7 +++++++\n t/t7513-interpret-trailers.sh | 16 ++++++++++++++++\n trailer.c | 9 +++++----\n trailer.h | 1 +\n 5 files changed, 34 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 295dffbd21..7cc43b0e3e 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -83,6 +83,11 @@ OPTIONS\n --only-trailers::\n \tOutput only the trailers, not any other parts of the input.\n \n+--only-input::\n+\tOutput only trailers that exist in the input; do not add any\n+\tfrom the command-line or by following configured `trailer.*`\n+\trules.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex afb12c11bc..2d90e0e480 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -25,6 +25,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n+\t\tOPT_BOOL(0, \"only-input\", &opts.only_input, N_(\"do not apply config rules\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -33,6 +34,12 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t git_interpret_trailers_usage, 0);\n \n+\tif (opts.only_input && trailers.nr)\n+\t\tusage_msg_opt(\n+\t\t\t_(\"--trailer with --only-input does not make sense\"),\n+\t\t\tgit_interpret_trailers_usage,\n+\t\t\toptions);\n+\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 90d30037b7..94b6c52473 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1314,4 +1314,20 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only input' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--only-trailers --only-input >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 706351fc98..2b5269183a 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -976,7 +976,6 @@ void process_trailers(const char *file,\n \t\t struct string_list *trailers)\n {\n \tLIST_HEAD(head);\n-\tLIST_HEAD(arg_head);\n \tstruct strbuf sb = STRBUF_INIT;\n \tint trailer_end;\n \tFILE *outfile = stdout;\n@@ -992,9 +991,11 @@ void process_trailers(const char *file,\n \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n \t\t\t\t\t sb.buf, &head);\n \n-\tprocess_command_line_args(&arg_head, trailers);\n-\n-\tprocess_trailers_lists(&head, &arg_head);\n+\tif (!opts->only_input) {\n+\t\tLIST_HEAD(arg_head);\n+\t\tprocess_command_line_args(&arg_head, trailers);\n+\t\tprocess_trailers_lists(&head, &arg_head);\n+\t}\n \n \tprint_all(outfile, &head, opts);\n \ndiff --git a/trailer.h b/trailer.h\nindex 3cf35ced00..76c3b571bf 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -26,6 +26,7 @@ struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n \tint only_trailers;\n+\tint only_input;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326047","messageId":"20170810180410.6spwzqq6bkzeyqax@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","subject":"[PATCH v3 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:04:11Z","receivedAt":"2017-08-10T18:04:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of \"--only-trailers\" is to give a caller an output\nthat's easy for them to parse. Getting rid of the\nnon-trailer material helps, but we still may see more\ncomplicated syntax like whitespace continuation. Let's add\nan option to normalize the output into one \"key: value\" line\nper trailer.\n\nAs a bonus, this could be used even without --only-trailers\nto clean up unusual formatting in the incoming data.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 5 +++++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 21 ++++++++++++++++++++\n trailer.c | 34 +++++++++++++++++++++++++++++++-\n trailer.h | 1 +\n 5 files changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 7cc43b0e3e..598b74efd7 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -88,6 +88,11 @@ OPTIONS\n \tfrom the command-line or by following configured `trailer.*`\n \trules.\n \n+--normalize::\n+\tNormalize trailer output so that trailers appear exactly one per\n+\tline, with any existing whitespace continuation folded into a\n+\tsingle line.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 2d90e0e480..b7592bedd9 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -26,6 +26,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-input\", &opts.only_input, N_(\"do not apply config rules\")),\n+\t\tOPT_BOOL(0, \"normalize\", &opts.normalize, N_(\"normalize trailer formatting\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 94b6c52473..37c4f37882 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1330,4 +1330,25 @@ test_expect_success 'only input' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'normalize' '\n+\tcat >expected <<-\\EOF &&\n+\t\tfoo: continued across several lines\n+\tEOF\n+\t# pass through tr to make leading and trailing whitespace more obvious\n+\ttr _ \" \" <<-\\EOF |\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\tfoo:_\n+\t\t__continued\n+\t\t___across\n+\t\t____several\n+\t\t_____lines\n+\t\t___\n+\tEOF\n+\tgit interpret-trailers --only-trailers --only-input --normalize >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 2b5269183a..07ef082284 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -887,7 +887,35 @@ static int ends_with_blank_line(const char *buf, size_t len)\n \treturn is_blank_line(buf + ll);\n }\n \n+static void normalize_value(struct strbuf *val)\n+{\n+\tstruct strbuf out = STRBUF_INIT;\n+\tsize_t i;\n+\n+\tstrbuf_grow(&out, val->len);\n+\ti = 0;\n+\twhile (i < val->len) {\n+\t\tchar c = val->buf[i++];\n+\t\tif (c == '\\n') {\n+\t\t\t/* Collapse continuation down to a single space. */\n+\t\t\twhile (i < val->len && isspace(val->buf[i]))\n+\t\t\t\ti++;\n+\t\t\tstrbuf_addch(&out, ' ');\n+\t\t} else {\n+\t\t\tstrbuf_addch(&out, c);\n+\t\t}\n+\t}\n+\n+\t/* Empty lines may have left us with whitespace cruft at the edges */\n+\tstrbuf_trim(&out);\n+\n+\t/* output goes back to val as if we modified it in-place */\n+\tstrbuf_swap(&out, val);\n+\tstrbuf_release(&out);\n+}\n+\n static int process_input_file(FILE *outfile,\n+\t\t\t int normalize,\n \t\t\t const char *str,\n \t\t\t struct list_head *head)\n {\n@@ -914,12 +942,16 @@ static int process_input_file(FILE *outfile,\n \t\tif (separator_pos >= 1) {\n \t\t\tparse_trailer(&tok, &val, NULL, trailer,\n \t\t\t\t separator_pos);\n+\t\t\tif (normalize)\n+\t\t\t\tnormalize_value(&val);\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t strbuf_detach(&tok, NULL),\n \t\t\t\t\t strbuf_detach(&val, NULL));\n \t\t} else {\n \t\t\tstrbuf_addstr(&val, trailer);\n \t\t\tstrbuf_strip_suffix(&val, \"\\n\");\n+\t\t\tif (normalize)\n+\t\t\t\tnormalize_value(&val);\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t NULL,\n \t\t\t\t\t strbuf_detach(&val, NULL));\n@@ -989,7 +1021,7 @@ void process_trailers(const char *file,\n \n \t/* Print the lines before the trailers */\n \ttrailer_end = process_input_file(opts->only_trailers ? NULL : outfile,\n-\t\t\t\t\t sb.buf, &head);\n+\t\t\t\t\t opts->normalize, sb.buf, &head);\n \n \tif (!opts->only_input) {\n \t\tLIST_HEAD(arg_head);\ndiff --git a/trailer.h b/trailer.h\nindex 76c3b571bf..55bdec8320 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -27,6 +27,7 @@ struct process_trailer_options {\n \tint trim_empty;\n \tint only_trailers;\n \tint only_input;\n+\tint normalize;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.0.614.g0beb26d5e9\n\n"},{"id":"326048","messageId":"20170810180414.figto5eri4ebi632@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","subject":"[PATCH v3 5/5] interpret-trailers: add --parse convenience option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:04:14Z","receivedAt":"2017-08-10T18:04:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The last few commits have added command line options that\ncan turn interpret-trailers into a parsing tool. Since\nthey'd most often be used together, let's provide a\nconvenient single option for callers to invoke this mode.\n\nThis is implemented as a callback rather than a boolean so\nthat its effect is applied immediately, as if those options\nhad been specified. Later options can then override them.\nE.g.:\n\n git interpret-trailers --parse --no-normalize\n\nwould work.\n\nLet's also update the documentation to make clear that this\nparsing mode behaves quite differently than the normal\n\"add trailers to the input\" mode.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 21 ++++++++++++++-------\n builtin/interpret-trailers.c | 12 ++++++++++++\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 598b74efd7..54c8b081c6 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -3,24 +3,27 @@ git-interpret-trailers(1)\n \n NAME\n ----\n-git-interpret-trailers - help add structured information into commit messages\n+git-interpret-trailers - add or parse structured information in commit messages\n \n SYNOPSIS\n --------\n [verse]\n-'git interpret-trailers' [--in-place] [--trim-empty] [(--trailer [(=|:)])...] [...]\n+'git interpret-trailers' [options] [(--trailer [(=|:)])...] [...]\n+'git interpret-trailers' [options] [--parse] [...]\n \n DESCRIPTION\n -----------\n-Help adding 'trailers' lines, that look similar to RFC 822 e-mail\n+Help parsing or adding 'trailers' lines, that look similar to RFC 822 e-mail\n headers, at the end of the otherwise free-form part of a commit\n message.\n \n This command reads some patches or commit messages from either the\n- arguments or the standard input if no is specified. Then\n-this command applies the arguments passed using the `--trailer`\n-option, if any, to the commit message part of each input file. The\n-result is emitted on the standard output.\n+ arguments or the standard input if no is specified. If\n+`--parse` is specified, the output consists of the parsed trailers.\n+\n+Otherwise, the this command applies the arguments passed using the\n+`--trailer` option, if any, to the commit message part of each input\n+file. The result is emitted on the standard output.\n \n Some configuration variables control the way the `--trailer` arguments\n are applied to each commit message and the way any existing trailer in\n@@ -93,6 +96,10 @@ OPTIONS\n \tline, with any existing whitespace continuation folded into a\n \tsingle line.\n \n+--parse::\n+\tA convenience alias for `--only-trailers --only-input\n+\t--normalize`.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex b7592bedd9..010c181492 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -16,6 +16,16 @@ static const char * const git_interpret_trailers_usage[] = {\n \tNULL\n };\n \n+static int parse_opt_parse(const struct option *opt, const char *arg,\n+\t\t\t int unset)\n+{\n+\tstruct process_trailer_options *v = opt->value;\n+\tv->only_trailers = 1;\n+\tv->only_input = 1;\n+\tv->normalize = 1;\n+\treturn 0;\n+}\n+\n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n \tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n@@ -27,6 +37,8 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-input\", &opts.only_input, N_(\"do not apply config rules\")),\n \t\tOPT_BOOL(0, \"normalize\", &opts.normalize, N_(\"normalize trailer formatting\")),\n+\t\t{ OPTION_CALLBACK, 0, \"parse\", &opts, NULL, N_(\"set parsing options\"),\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse },\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n-- \n2.14.0.614.g0beb26d5e9\n"},{"id":"326052","messageId":"CAGZ79kZY1ifks_TAniR+diHOKWVQpRVFeS=XEBGW+7LT=Js5RQ@mail.gmail.com","threadId":"46548","inReplyTo":"20170810180404.yg3jdzhquregh4ta@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-10T18:28:52Z","receivedAt":"2017-08-10T18:29:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 10, 2017 at 11:04 AM, Jeff King wrote:\n> In theory it's easy for any reader who wants to parse\n> trailers to do so. But there are a lot of subtle corner\n> cases around what counts as a trailer, when the trailer\n> block begins and ends, etc. Since interpret-trailers already\n> has our parsing logic, let's let callers ask it to just\n> output the trailers.\n>\n> They still have to parse the \"key: value\" lines, but at\n> least they can ignore all of the other corner cases.\n>\n> Signed-off-by: Jeff King \n\nSorry for a sloppy review last round, upon reviewing\nI found another nit.\n\n>\n> +test_expect_success 'only trailers' '\n> + git config trailer.sign.command \"echo config-value\" &&\n\nYou may want to use 'test_config' here, which keeps the config\nonly for one test. The subsequent tests seem to overwrite the\nconfig, so this is not wrong, just not the best style.\n"},{"id":"326053","messageId":"20170810183107.fykw6w3ypxnxzcy4@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAGZ79kZY1ifks_TAniR+diHOKWVQpRVFeS=XEBGW+7LT=Js5RQ@mail.gmail.com","subject":"Re: [PATCH v3 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:31:07Z","receivedAt":"2017-08-10T18:31:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2017 at 11:28:52AM -0700, Stefan Beller wrote:\n\n> > +test_expect_success 'only trailers' '\n> > + git config trailer.sign.command \"echo config-value\" &&\n> \n> You may want to use 'test_config' here, which keeps the config\n> only for one test. The subsequent tests seem to overwrite the\n> config, so this is not wrong, just not the best style.\n\nYeah, I actually considered that but decided to keep style with the rest\nof the script. I agree the whole thing could possibly switch to\ntest_config, but I suspect there may be some fallouts (the style of the\nrest of the script seems to assume some continuity between the tests).\n\n-Peff\n"},{"id":"326057","messageId":"CAGZ79kabhz-FWNyjB6KjF4qpGfSqONBNVBcVd=+J=5XT+emz-A@mail.gmail.com","threadId":"46548","inReplyTo":"20170810080325.tehbbgajm4cgn2ku@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-10T18:35:00Z","receivedAt":"2017-08-10T18:35:07Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 10, 2017 at 1:03 AM, Jeff King wrote:\n> The point of \"--only-trailers\" is to give a caller an output\n> that's easy for them to parse. Getting rid of the\n> non-trailer material helps, but we still may see more\n> complicated syntax like whitespace continuation. Let's add\n> an option to normalize the output into one \"key: value\" line\n> per trailer.\n>\n> As a bonus, this could be used even without --only-trailers\n> to clean up unusual formatting in the incoming data.\n\nThis is useful for the parsing part, but for the writing part we'd\nrather want to have the opposite thing, such as\n'--line-break=rfc822'. But this doesn't have to be part of this\nseries. With this in mind, I do not quite understand the latter\nuse case how you would use normalized trailers without\n--only-trailers?\n"},{"id":"326058","messageId":"CAGZ79kaNrqvqr3OVcwhxFEMoS2sK8wvfVE_pmxXTWYVB-OBDXQ@mail.gmail.com","threadId":"46548","inReplyTo":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-10T18:35:48Z","receivedAt":"2017-08-10T18:35:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 10, 2017 at 11:03 AM, Jeff King wrote:\n> On Thu, Aug 10, 2017 at 04:02:46AM -0400, Jeff King wrote:\n>\n>> On Wed, Aug 09, 2017 at 08:21:47AM -0400, Jeff King wrote:\n>>\n>> > This series teaches interpret-trailers to parse and output just the\n>> > trailers. So now you can do:\n>> >\n>> > $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n>> > git interpret-trailers --parse\n>> > Signed-off-by: Hartmut Henkel \n>> > Helped-by: Stefan Beller \n>> > Signed-off-by: Ralf Thielow \n>> > Acked-by: Matthias Rüster \n>>\n>> And here's a v2 that addresses all of the comments except one: Stefan\n>> suggested that --only-existing wasn't a great name. I agree, but I like\n>> everything else less.\n>\n> Here's a v3 that takes care of that (renaming it to --only-input).\n>\n> It's otherwise the same as v2, but since the name-change ripples through\n> the remaining patches, I wanted to get v3 in front of people sooner\n> rather than later.\n>\n\nLooks good,\n\nThanks,\nStefan\n"},{"id":"326059","messageId":"20170810183713.z75bwk5eeavi6z22@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAGZ79kabhz-FWNyjB6KjF4qpGfSqONBNVBcVd=+J=5XT+emz-A@mail.gmail.com","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T18:37:13Z","receivedAt":"2017-08-10T18:37:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:\n\n> On Thu, Aug 10, 2017 at 1:03 AM, Jeff King wrote:\n> > The point of \"--only-trailers\" is to give a caller an output\n> > that's easy for them to parse. Getting rid of the\n> > non-trailer material helps, but we still may see more\n> > complicated syntax like whitespace continuation. Let's add\n> > an option to normalize the output into one \"key: value\" line\n> > per trailer.\n> >\n> > As a bonus, this could be used even without --only-trailers\n> > to clean up unusual formatting in the incoming data.\n> \n> This is useful for the parsing part, but for the writing part we'd\n> rather want to have the opposite thing, such as\n> '--line-break=rfc822'. But this doesn't have to be part of this\n> series. With this in mind, I do not quite understand the latter\n> use case how you would use normalized trailers without\n> --only-trailers?\n\nIf you prefer the normalized form (and the input was line-broken in a\nway that you don't like), then this would convert to your preferred\nform. I agree that you could potentially want the opposite (folding long\nlines). Perhaps something like --wrap=72.\n\n-Peff\n"},{"id":"326060","messageId":"xmqqd1834645.fsf@gitster.mtv.corp.google.com","threadId":"46548","inReplyTo":"20170810072822.rj6y6zcqhyfz4yi7@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-10T18:42:18Z","receivedAt":"2017-08-10T18:42:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> > The above example made me wonder if we also want a format specifier\n>> > to do the above without piping, but it turns out that we already\n>> > have \"log --format=%(trailers)\", so we are good ;-)\n>> \n>> I was going to say, I thought we had a way to get trailers for a\n>> commit via the pretty format, since that is what i used in the past.\n>\n> I do like that you could get the trailers for many commits in a single\n> invocation. That doesn't matter for my current use-case, but obviously\n> piping through O(n) interpret-trailers invocations is a bad idea.\n> But there are a few difficulties with using %(trailers) for this,...\n\nI think it is clear to you, but it may not be clear to others, that\nI did not mean to say \"because 'log --format' already knows about\nit, this change to interpret-trailers is unnecessary\".\n\n> For (1) I think many callers would prefer to see the original\n> formatting. Maybe we'd need a %(trailers:normalize) or something.\n\nThanks; that is exactly the line of thought I had in the back of my\nhead without even realizing when I brought up %(trailers) format\nelement.\n"},{"id":"326061","messageId":"xmqq8tir462l.fsf@gitster.mtv.corp.google.com","threadId":"46548","inReplyTo":"20170810173328.xfrswan6tijwkaex@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-10T18:43:14Z","receivedAt":"2017-08-10T18:43:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Perhaps \"--exact-input\" hits all of those. Or maybe \"--only-input\" to\n> match the other \"--only\".\n>\n> I think I like that last one the best. It makes it clear that we are\n> looking just at the input, and not anything else. Which is exactly what\n> the feature does.\n\nSounds good. Thanks.\n"},{"id":"326064","messageId":"xmqq4ltf45sk.fsf@gitster.mtv.corp.google.com","threadId":"46548","inReplyTo":"20170810183107.fykw6w3ypxnxzcy4@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/5] interpret-trailers: add an option to show only the trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-10T18:49:15Z","receivedAt":"2017-08-10T18:49:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Aug 10, 2017 at 11:28:52AM -0700, Stefan Beller wrote:\n>\n>> > +test_expect_success 'only trailers' '\n>> > + git config trailer.sign.command \"echo config-value\" &&\n>> \n>> You may want to use 'test_config' here, which keeps the config\n>> only for one test. The subsequent tests seem to overwrite the\n>> config, so this is not wrong, just not the best style.\n>\n> Yeah, I actually considered that but decided to keep style with the rest\n> of the script. I agree the whole thing could possibly switch to\n> test_config, but I suspect there may be some fallouts (the style of the\n> rest of the script seems to assume some continuity between the tests).\n\nI agree with your judgment here. As you said in the \"use tool to\nenforce consistent style\" thread, sometimes humans need to apply\nbetter taste than mechanical tooling could, and I view this one of a\ngood example. \n\nEven though this is not a C coding-style thing, but the essense of\nthe problem is the same. That is one of the reasons why I earlier\nsaid that we may see more style-only critique to patches if we\nintroduce a new tool without setting the expectation of what we want\nto get out of such a tool straight.\n"},{"id":"326077","messageId":"CAP8UFD2kWW41NDHVKjzdL2O2=QAeEtzKEPdJW3zR-zoeTiJPgw@mail.gmail.com","threadId":"46548","inReplyTo":"20170810183713.z75bwk5eeavi6z22@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-08-10T19:39:21Z","receivedAt":"2017-08-10T19:39:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Aug 10, 2017 at 8:37 PM, Jeff King wrote:\n> On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:\n>\n>> On Thu, Aug 10, 2017 at 1:03 AM, Jeff King wrote:\n>> > The point of \"--only-trailers\" is to give a caller an output\n>> > that's easy for them to parse. Getting rid of the\n>> > non-trailer material helps, but we still may see more\n>> > complicated syntax like whitespace continuation. Let's add\n>> > an option to normalize the output into one \"key: value\" line\n>> > per trailer.\n>> >\n>> > As a bonus, this could be used even without --only-trailers\n>> > to clean up unusual formatting in the incoming data.\n>>\n>> This is useful for the parsing part, but for the writing part we'd\n>> rather want to have the opposite thing, such as\n>> '--line-break=rfc822'. But this doesn't have to be part of this\n>> series. With this in mind, I do not quite understand the latter\n>> use case how you would use normalized trailers without\n>> --only-trailers?\n>\n> If you prefer the normalized form (and the input was line-broken in a\n> way that you don't like), then this would convert to your preferred\n> form. I agree that you could potentially want the opposite (folding long\n> lines). Perhaps something like --wrap=72.\n\nRelated to this, I wonder if people might want to \"normalize\" in\ndifferent ways later. If that happens, we might regret having called\nthis option \"--normalize\" instead of \"--one-per-line\" for example.\n"},{"id":"326078","messageId":"20170810194251.r76pumddcchlke7c@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAP8UFD2kWW41NDHVKjzdL2O2=QAeEtzKEPdJW3zR-zoeTiJPgw@mail.gmail.com","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T19:42:52Z","receivedAt":"2017-08-10T19:42:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2017 at 09:39:21PM +0200, Christian Couder wrote:\n\n> > If you prefer the normalized form (and the input was line-broken in a\n> > way that you don't like), then this would convert to your preferred\n> > form. I agree that you could potentially want the opposite (folding long\n> > lines). Perhaps something like --wrap=72.\n> \n> Related to this, I wonder if people might want to \"normalize\" in\n> different ways later. If that happens, we might regret having called\n> this option \"--normalize\" instead of \"--one-per-line\" for example.\n\nMy assumption was that it would be OK to add other normalization later\nif it brings us closer to the \"key: value\" form as a standard, and it\ncould fall under \"--normalize\", since that's what callers would want.\nAnd that's why I didn't want to call it something like --one-per-line.\n\nBut if you are arguing that there can be many \"standards\" to normalize\nto, I agree that's a possibility. I think we have an out by extending to\n\"--normalize=whatever-form\" in the future.\n\n-Peff\n"},{"id":"326079","messageId":"xmqqr2wj2opv.fsf@gitster.mtv.corp.google.com","threadId":"46548","inReplyTo":"CAGZ79kaNrqvqr3OVcwhxFEMoS2sK8wvfVE_pmxXTWYVB-OBDXQ@mail.gmail.com","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-10T19:43:24Z","receivedAt":"2017-08-10T19:43:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Thu, Aug 10, 2017 at 11:03 AM, Jeff King wrote:\n>> On Thu, Aug 10, 2017 at 04:02:46AM -0400, Jeff King wrote:\n>>\n>>> On Wed, Aug 09, 2017 at 08:21:47AM -0400, Jeff King wrote:\n>>>\n>>> > This series teaches interpret-trailers to parse and output just the\n>>> > trailers. So now you can do:\n>>> >\n>>> > $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |\n>>> > git interpret-trailers --parse\n>>> > Signed-off-by: Hartmut Henkel \n>>> > Helped-by: Stefan Beller \n>>> > Signed-off-by: Ralf Thielow \n>>> > Acked-by: Matthias Rüster \n>>>\n>>> And here's a v2 that addresses all of the comments except one: Stefan\n>>> suggested that --only-existing wasn't a great name. I agree, but I like\n>>> everything else less.\n>>\n>> Here's a v3 that takes care of that (renaming it to --only-input).\n>>\n>> It's otherwise the same as v2, but since the name-change ripples through\n>> the remaining patches, I wanted to get v3 in front of people sooner\n>> rather than later.\n>>\n>\n> Looks good,\n\nYeah, looks good to me too. Thanks for the \"--only-input\"\ndiscussion.\n"},{"id":"326081","messageId":"CAGZ79kYeNb3sN4b8xZvYAjQOXWKP9oi+dNsJmS8fyz+iUH9p8A@mail.gmail.com","threadId":"46548","inReplyTo":"CAP8UFD2kWW41NDHVKjzdL2O2=QAeEtzKEPdJW3zR-zoeTiJPgw@mail.gmail.com","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-10T19:44:44Z","receivedAt":"2017-08-10T19:44:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 10, 2017 at 12:39 PM, Christian Couder\n wrote:\n> On Thu, Aug 10, 2017 at 8:37 PM, Jeff King wrote:\n>> On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:\n>>\n>>> On Thu, Aug 10, 2017 at 1:03 AM, Jeff King wrote:\n>>> > The point of \"--only-trailers\" is to give a caller an output\n>>> > that's easy for them to parse. Getting rid of the\n>>> > non-trailer material helps, but we still may see more\n>>> > complicated syntax like whitespace continuation. Let's add\n>>> > an option to normalize the output into one \"key: value\" line\n>>> > per trailer.\n>>> >\n>>> > As a bonus, this could be used even without --only-trailers\n>>> > to clean up unusual formatting in the incoming data.\n>>>\n>>> This is useful for the parsing part, but for the writing part we'd\n>>> rather want to have the opposite thing, such as\n>>> '--line-break=rfc822'. But this doesn't have to be part of this\n>>> series. With this in mind, I do not quite understand the latter\n>>> use case how you would use normalized trailers without\n>>> --only-trailers?\n>>\n>> If you prefer the normalized form (and the input was line-broken in a\n>> way that you don't like), then this would convert to your preferred\n>> form. I agree that you could potentially want the opposite (folding long\n>> lines). Perhaps something like --wrap=72.\n>\n> Related to this, I wonder if people might want to \"normalize\" in\n> different ways later. If that happens, we might regret having called\n> this option \"--normalize\" instead of \"--one-per-line\" for example.\n\nWhat is normal?\n\nMaybe --style=[one-line, wrapped:72, rfc, json, xml, ...]\nthen?\n\nIf you have --one-per-line, this may be orthogonal to e.g. json\n(as json can be crammed into one line IIUC), but when given the\nselection you cannot combine multiple styles.\n\nScratch that, we actually want to combine these styles with each\nother.\n"},{"id":"326089","messageId":"CAP8UFD0A59f70i4qT7BBxcbK7Dx4dEoNsWHEmYaqp=MqshpHgw@mail.gmail.com","threadId":"46548","inReplyTo":"20170810194251.r76pumddcchlke7c@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-08-10T20:26:24Z","receivedAt":"2017-08-10T20:26:34Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Aug 10, 2017 at 9:42 PM, Jeff King wrote:\n> On Thu, Aug 10, 2017 at 09:39:21PM +0200, Christian Couder wrote:\n>\n>> > If you prefer the normalized form (and the input was line-broken in a\n>> > way that you don't like), then this would convert to your preferred\n>> > form. I agree that you could potentially want the opposite (folding long\n>> > lines). Perhaps something like --wrap=72.\n>>\n>> Related to this, I wonder if people might want to \"normalize\" in\n>> different ways later. If that happens, we might regret having called\n>> this option \"--normalize\" instead of \"--one-per-line\" for example.\n>\n> My assumption was that it would be OK to add other normalization later\n> if it brings us closer to the \"key: value\" form as a standard, and it\n> could fall under \"--normalize\", since that's what callers would want.\n> And that's why I didn't want to call it something like --one-per-line.\n>\n> But if you are arguing that there can be many \"standards\" to normalize\n> to, I agree that's a possibility. I think we have an out by extending to\n> \"--normalize=whatever-form\" in the future.\n\nIf we take `git log` as an example, we now have \"--oneline\" which is a\nshorthand for \"--pretty=oneline --abbrev-commit\".\nAnd the default for \"--pretty\" is called \"medium\".\n\nSo instead of your suggestion, we could call this option \"--oneline\"\nnow, and if other normalizations are later required we could then\ncreate \"--pretty=whatever\" and say that \"--oneline\" is a shorthand for\n\"--pretty=oneline\".\n"},{"id":"326098","messageId":"CAP8UFD08oyY9eK-0Ydm2qh2etvc2_QGc0ujNswGUgUumwhy4+w@mail.gmail.com","threadId":"46548","inReplyTo":"CAGZ79kYeNb3sN4b8xZvYAjQOXWKP9oi+dNsJmS8fyz+iUH9p8A@mail.gmail.com","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-08-10T21:06:40Z","receivedAt":"2017-08-10T21:06:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Aug 10, 2017 at 9:44 PM, Stefan Beller wrote:\n> On Thu, Aug 10, 2017 at 12:39 PM, Christian Couder\n> wrote:\n>> On Thu, Aug 10, 2017 at 8:37 PM, Jeff King wrote:\n>>> On Thu, Aug 10, 2017 at 11:35:00AM -0700, Stefan Beller wrote:\n>>>\n>>>> On Thu, Aug 10, 2017 at 1:03 AM, Jeff King wrote:\n>>>> > The point of \"--only-trailers\" is to give a caller an output\n>>>> > that's easy for them to parse. Getting rid of the\n>>>> > non-trailer material helps, but we still may see more\n>>>> > complicated syntax like whitespace continuation. Let's add\n>>>> > an option to normalize the output into one \"key: value\" line\n>>>> > per trailer.\n>>>> >\n>>>> > As a bonus, this could be used even without --only-trailers\n>>>> > to clean up unusual formatting in the incoming data.\n>>>>\n>>>> This is useful for the parsing part, but for the writing part we'd\n>>>> rather want to have the opposite thing, such as\n>>>> '--line-break=rfc822'. But this doesn't have to be part of this\n>>>> series. With this in mind, I do not quite understand the latter\n>>>> use case how you would use normalized trailers without\n>>>> --only-trailers?\n>>>\n>>> If you prefer the normalized form (and the input was line-broken in a\n>>> way that you don't like), then this would convert to your preferred\n>>> form. I agree that you could potentially want the opposite (folding long\n>>> lines). Perhaps something like --wrap=72.\n>>\n>> Related to this, I wonder if people might want to \"normalize\" in\n>> different ways later. If that happens, we might regret having called\n>> this option \"--normalize\" instead of \"--one-per-line\" for example.\n>\n> What is normal?\n>\n> Maybe --style=[one-line, wrapped:72, rfc, json, xml, ...]\n> then?\n\nYeah, we could go there right now (using perhaps \"--pretty\" or\n\"--format\" instead of \"--style\", so that we are more consistent with\nother commands), but on the other hand we don't know yet if the other\nformats will really be needed.\n\n> If you have --one-per-line, this may be orthogonal to e.g. json\n> (as json can be crammed into one line IIUC), but when given the\n> selection you cannot combine multiple styles.\n>\n> Scratch that, we actually want to combine these styles with each\n> other.\n\nYeah, that's another possibility for the future. People might want a\n--json option that can be used both with and without --oneline. But as\nthe future is difficult to predict, let's try to make it easy for us\nin both cases.\n\nAnd I think starting with just \"--oneline\" would be easier to deal\nwith later than \"--normalize\" (or \"--style\" or \"--pretty\" or\n\"--format\") especially in the latter case.\n"},{"id":"326099","messageId":"20170810211039.okpwglvcmkngzri2@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAP8UFD08oyY9eK-0Ydm2qh2etvc2_QGc0ujNswGUgUumwhy4+w@mail.gmail.com","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T21:10:39Z","receivedAt":"2017-08-10T21:10:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2017 at 11:06:40PM +0200, Christian Couder wrote:\n\n> >> Related to this, I wonder if people might want to \"normalize\" in\n> >> different ways later. If that happens, we might regret having called\n> >> this option \"--normalize\" instead of \"--one-per-line\" for example.\n> >\n> > What is normal?\n> >\n> > Maybe --style=[one-line, wrapped:72, rfc, json, xml, ...]\n> > then?\n> \n> Yeah, we could go there right now (using perhaps \"--pretty\" or\n> \"--format\" instead of \"--style\", so that we are more consistent with\n> other commands), but on the other hand we don't know yet if the other\n> formats will really be needed.\n\nBut some of those things are not 1:1 mappings with normalization. For\ninstance, --json presumably implies --only-trailers. Or are we proposing\nto break the whole commit message down into components and output it all\nas json?\n\n-Peff\n"},{"id":"326116","messageId":"98b14604-33f1-6d91-8c9e-ddfa0c5cb0e7@ramsayjones.plus.com","threadId":"46548","inReplyTo":"20170810211039.okpwglvcmkngzri2@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-08-10T23:02:49Z","receivedAt":"2017-08-10T23:02:59Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 10/08/17 22:10, Jeff King wrote:\n> On Thu, Aug 10, 2017 at 11:06:40PM +0200, Christian Couder wrote:\n> \n>>>> Related to this, I wonder if people might want to \"normalize\" in\n>>>> different ways later. If that happens, we might regret having called\n>>>> this option \"--normalize\" instead of \"--one-per-line\" for example.\n>>>\n>>> What is normal?\n>>>\n>>> Maybe --style=[one-line, wrapped:72, rfc, json, xml, ...]\n>>> then?\n>>\n>> Yeah, we could go there right now (using perhaps \"--pretty\" or\n>> \"--format\" instead of \"--style\", so that we are more consistent with\n>> other commands), but on the other hand we don't know yet if the other\n>> formats will really be needed.\n> \n> But some of those things are not 1:1 mappings with normalization. For\n> instance, --json presumably implies --only-trailers. Or are we proposing\n> to break the whole commit message down into components and output it all\n> as json?\n\nHmm, to me the operation wasn't so much a normalization, rather\nit was an --unfold (or maybe --un-fold ;-D). I suppose going in\nthe other direction could be --fold=72, or some such.\n\n[blue is my favourite colour ... :-P ]\n\nATB,\nRamsay Jones\n\n"},{"id":"326118","messageId":"20170810231038.4s2btsotoq76atuu@sigill.intra.peff.net","threadId":"46548","inReplyTo":"98b14604-33f1-6d91-8c9e-ddfa0c5cb0e7@ramsayjones.plus.com","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-10T23:10:39Z","receivedAt":"2017-08-10T23:10:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 11, 2017 at 12:02:49AM +0100, Ramsay Jones wrote:\n\n> > But some of those things are not 1:1 mappings with normalization. For\n> > instance, --json presumably implies --only-trailers. Or are we proposing\n> > to break the whole commit message down into components and output it all\n> > as json?\n> \n> Hmm, to me the operation wasn't so much a normalization, rather\n> it was an --unfold (or maybe --un-fold ;-D). I suppose going in\n> the other direction could be --fold=72, or some such.\n\nBut I really don't want callers to think of it as \"unfold\". I want it to\nbe \"turn this into something I can parse simply\". Hence if we were to\nfind another case where the output is irregular, I'd feel comfortable\ncalling that a bug and fixing it (one that I suspect but haven't tested\nis that alternate separators probably should all be converted to\ncolons).\n\n> [blue is my favourite colour ... :-P ]\n\nYes, I'm feeling that, too. :-/\n\n-Peff\n"},{"id":"326124","messageId":"b41ac83a-7b45-923c-71d0-039a45eaec2d@ramsayjones.plus.com","threadId":"46548","inReplyTo":"20170810231038.4s2btsotoq76atuu@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-08-10T23:36:51Z","receivedAt":"2017-08-10T23:36:59Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 11/08/17 00:10, Jeff King wrote:\n> On Fri, Aug 11, 2017 at 12:02:49AM +0100, Ramsay Jones wrote:\n> \n>>> But some of those things are not 1:1 mappings with normalization. For\n>>> instance, --json presumably implies --only-trailers. Or are we proposing\n>>> to break the whole commit message down into components and output it all\n>>> as json?\n>>\n>> Hmm, to me the operation wasn't so much a normalization, rather\n>> it was an --unfold (or maybe --un-fold ;-D). I suppose going in\n>> the other direction could be --fold=72, or some such.\n> \n> But I really don't want callers to think of it as \"unfold\". I want it to\n> be \"turn this into something I can parse simply\". Hence if we were to\n> find another case where the output is irregular, I'd feel comfortable\n> calling that a bug and fixing it (one that I suspect but haven't tested\n> is that alternate separators probably should all be converted to\n> colons).\n\nAh, yes, good point.\n\nATB,\nRamsay Jones\n\n\n"},{"id":"326129","messageId":"CAP8UFD3RWByUBZf=E_EBCW2Zk7voogfB+Qpsx4U2YNq0T0Hqag@mail.gmail.com","threadId":"46548","inReplyTo":"20170810231038.4s2btsotoq76atuu@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-08-11T07:02:24Z","receivedAt":"2017-08-11T07:02:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Aug 11, 2017 at 1:10 AM, Jeff King wrote:\n> On Fri, Aug 11, 2017 at 12:02:49AM +0100, Ramsay Jones wrote:\n>\n>> > But some of those things are not 1:1 mappings with normalization. For\n>> > instance, --json presumably implies --only-trailers. Or are we proposing\n>> > to break the whole commit message down into components and output it all\n>> > as json?\n\nWell who knows what people might want/need?\nAnyway in `git log` --oneline is not a direct mapping with\n--pretty=oneline as it also means --abbrev-commit, and this is not a\nbig problem.\n\n>> Hmm, to me the operation wasn't so much a normalization, rather\n>> it was an --unfold (or maybe --un-fold ;-D). I suppose going in\n>> the other direction could be --fold=72, or some such.\n\nYeah, we could call that --no-fold or --no-wrap if we expect to need\n--fold=72 or --wrap=72.\nAt least it is more descriptive than --normalize and if we later\nintroduce --pretty or --format we can say that it is a shorthand for\n--pretty=nofold or --pretty=unfolded.\n\n> But I really don't want callers to think of it as \"unfold\". I want it to\n> be \"turn this into something I can parse simply\". Hence if we were to\n> find another case where the output is irregular, I'd feel comfortable\n> calling that a bug and fixing it (one that I suspect but haven't tested\n> is that alternate separators probably should all be converted to\n> colons).\n\nThough \"fixing\" this after a release has been made might introduce a\nregression for people who would already use the option on trailers\nwith different separators. That's also why I don't like --normalize.\nWe just don't know if we will need to \"fix\" it a lot to make sure\nscripts using it will work in all cases.\n\nIf we use --no-fold or --oneline instead, we don't promise too much\nfrom this option, so users cannot say that they expect it to work for\nthem in all cases.\n"},{"id":"326138","messageId":"20170811090615.ds3qlgudhdyxggpv@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAP8UFD3RWByUBZf=E_EBCW2Zk7voogfB+Qpsx4U2YNq0T0Hqag@mail.gmail.com","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-11T09:06:16Z","receivedAt":"2017-08-11T09:06:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 11, 2017 at 09:02:24AM +0200, Christian Couder wrote:\n\n> > But I really don't want callers to think of it as \"unfold\". I want it to\n> > be \"turn this into something I can parse simply\". Hence if we were to\n> > find another case where the output is irregular, I'd feel comfortable\n> > calling that a bug and fixing it (one that I suspect but haven't tested\n> > is that alternate separators probably should all be converted to\n> > colons).\n> \n> Though \"fixing\" this after a release has been made might introduce a\n> regression for people who would already use the option on trailers\n> with different separators. That's also why I don't like --normalize.\n> We just don't know if we will need to \"fix\" it a lot to make sure\n> scripts using it will work in all cases.\n> \n> If we use --no-fold or --oneline instead, we don't promise too much\n> from this option, so users cannot say that they expect it to work for\n> them in all cases.\n\nBut promising a normalized form is exactly what I want from the option.\n\nThat said, I'm OK to promise that via \"--parse\", and call this --unfold,\nif you really feel strongly.\n\n-Peff\n"},{"id":"326161","messageId":"CAP8UFD0ngR7gpu=r3ba9JLK-Bnz+uQEt2T9HXNKw=XKKUhBesw@mail.gmail.com","threadId":"46548","inReplyTo":"20170811090615.ds3qlgudhdyxggpv@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] interpret-trailers: add an option to normalize output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-08-11T19:02:43Z","receivedAt":"2017-08-11T19:02:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Aug 11, 2017 at 11:06 AM, Jeff King wrote:\n> On Fri, Aug 11, 2017 at 09:02:24AM +0200, Christian Couder wrote:\n>\n>> > But I really don't want callers to think of it as \"unfold\". I want it to\n>> > be \"turn this into something I can parse simply\". Hence if we were to\n>> > find another case where the output is irregular, I'd feel comfortable\n>> > calling that a bug and fixing it (one that I suspect but haven't tested\n>> > is that alternate separators probably should all be converted to\n>> > colons).\n>>\n>> Though \"fixing\" this after a release has been made might introduce a\n>> regression for people who would already use the option on trailers\n>> with different separators. That's also why I don't like --normalize.\n>> We just don't know if we will need to \"fix\" it a lot to make sure\n>> scripts using it will work in all cases.\n>>\n>> If we use --no-fold or --oneline instead, we don't promise too much\n>> from this option, so users cannot say that they expect it to work for\n>> them in all cases.\n>\n> But promising a normalized form is exactly what I want from the option.\n>\n> That said, I'm OK to promise that via \"--parse\", and call this --unfold,\n> if you really feel strongly.\n\nYeah, I think promising these kind of things via an higher level\noption that is a shorthand for a mix of other basic options is much\nbetter especially if it's clearly documented that the option mix could\nchange in case of bugs or improvements.\n\nThis way people who want something stable, know that they should use\ntheir own mix of basic options. And those who are ok with something\nnot as stable as long as it evolves towards a specific goal, know that\nthey should use the higher level option.\n"},{"id":"326247","messageId":"CA+P7+xoLNzVE4ogw+TEij=DtZM4648Jz3JvxdMBQnUb9t+Ez5A@mail.gmail.com","threadId":"46548","inReplyTo":"xmqqd1834645.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/5] make interpret-trailers useful for parsing","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-08-13T19:03:45Z","receivedAt":"2017-08-13T19:04:12Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Aug 10, 2017 at 11:42 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>>> > The above example made me wonder if we also want a format specifier\n>>> > to do the above without piping, but it turns out that we already\n>>> > have \"log --format=%(trailers)\", so we are good ;-)\n>>>\n>>> I was going to say, I thought we had a way to get trailers for a\n>>> commit via the pretty format, since that is what i used in the past.\n>>\n>> I do like that you could get the trailers for many commits in a single\n>> invocation. That doesn't matter for my current use-case, but obviously\n>> piping through O(n) interpret-trailers invocations is a bad idea.\n>> But there are a few difficulties with using %(trailers) for this,...\n>\n> I think it is clear to you, but it may not be clear to others, that\n> I did not mean to say \"because 'log --format' already knows about\n> it, this change to interpret-trailers is unnecessary\".\n>\n>> For (1) I think many callers would prefer to see the original\n>> formatting. Maybe we'd need a %(trailers:normalize) or something.\n>\n> Thanks; that is exactly the line of thought I had in the back of my\n> head without even realizing when I brought up %(trailers) format\n> element.\n\nI'll add that I also think this patch series is good, it's useful to\nhave a separate command.\n\nThanks,\nJake\n"},{"id":"326370","messageId":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170810180326.4kkmawywvdbuzwnp@sigill.intra.peff.net","subject":"[PATCH v4 0/8] trailer parsing via interpret-trailers and %(trailers)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:22:54Z","receivedAt":"2017-08-15T10:23:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here's a fourth version of my series to make the output of\ninterpret-trailers useful for callers who just want to parse existing\ntrailers.\n\nThe big change here is that I added similar features to pretty.c's\n\"%(trailers)\", meaning you can now efficiently get the same output for\nmultiple commits with a single invocation. Like:\n\n $ git rev-list --no-walk --format='%(trailers:only:unfold)' \\\n 3ac53e0d13fa7483cce90eb6a1cfcdcbda5b8e35\n commit 3ac53e0d13fa7483cce90eb6a1cfcdcbda5b8e35\n Signed-off-by: H. Peter Anvin \n Signed-off-by: Junio C Hamano \n\nThat doesn't look very exciting, but with just \"%(trailers)\" you would\nsee that the block also contains a \"cherry-picked from\" line. There\naren't good examples of unfolding in git.git because we don't tend to\nfold. Nor does linux.git (though they do have a lot of non-trailer lines\nin square brackets). The tests show off some samples of both.\n\nI left %(trailers) exactly as it is: it shows the trailer block\nverbatim, including funny syntax, etc. As soon as you use one of :only\nor :unfold, we have to parse further, and as a side effect we normalize\na few bits (like whitespace around separators), because we round-trip\nthrough the parser. I think that's fine and what callers would want. But\nit did make me wonder if I should just have a single \"parse\" option that\ndoes all that normalizing. Or alternatively, have %(trailers) always do\nthat round-trip normalizing. That's how interpret-trailers behaves,\nafter all(since it always parses and reconstructs the trailer block). I\nfelt better taking the conservative route, though, and leaving\n%(trailers) alone.\n\nThere are a few other changes from v3:\n\n - the --normalize option is now --unfold (and I matched it with\n \":unfold\" for the placeholder)\n\n - the process_input_file function now takes the\n process_trailer_options to check \"only_trailers\" instead of taking a\n hint from a NULL outfile. This is hopefully a bit more obvious.\n\n - when only_trailers is set, I don't even bother adding non-trailers\n from the middle of the trailer-block to the list (we know we'd never\n output them, and they can't affect the trailer rules). This is just\n a micro-optimization I noticed while writing the %(trailers) helper.\n\n - Similarly, we no longer unfold non-trailers. So if you have:\n\n key: value\n more value\n this is not a trailer\n this is also not a trailer\n\n we would unfold \"key: value more value\", but not the other two\n lines. Because without a separator, that's not really folding. I'm\n not sure it actually matters. It's hard to have non-trailers in the\n block in the first place. But I think the new behavior is the right\n thing if it ever does come up.\n\n [1/8]: trailer: put process_trailers() options into a struct\n [2/8]: interpret-trailers: add an option to show only the trailers\n [3/8]: interpret-trailers: add an option to show only existing trailers\n [4/8]: interpret-trailers: add an option to unfold values\n [5/8]: interpret-trailers: add --parse convenience option\n [6/8]: pretty: move trailer formatting to trailer.c\n [7/8]: t4205: refactor %(trailer) tests\n [8/8]: pretty: support normalization options for %(trailers)\n\n Documentation/git-interpret-trailers.txt | 33 +++++++--\n Documentation/pretty-formats.txt | 5 +-\n builtin/interpret-trailers.c | 34 +++++++--\n pretty.c | 26 +++----\n t/t4205-log-pretty-formats.sh | 51 ++++++++++++--\n t/t7513-interpret-trailers.sh | 76 ++++++++++++++++++++\n trailer.c | 115 ++++++++++++++++++++++++++-----\n trailer.h | 27 +++++++-\n 8 files changed, 315 insertions(+), 52 deletions(-)\n\n-Peff\n"},{"id":"326372","messageId":"20170815102317.wrdnj2tlg3pqg7ze@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 1/8] trailer: put process_trailers() options into a struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:23:17Z","receivedAt":"2017-08-15T10:23:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already have two options and are about to add a few more.\nTo avoid having a huge number of boolean arguments, let's\nconvert to an options struct which can be passed in.\n\nSigned-off-by: Jeff King \n---\n builtin/interpret-trailers.c | 13 ++++++-------\n trailer.c | 10 ++++++----\n trailer.h | 10 +++++++++-\n 3 files changed, 21 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 175f14797b..bb0d7b937a 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -18,13 +18,12 @@ static const char * const git_interpret_trailers_usage[] = {\n \n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n-\tint in_place = 0;\n-\tint trim_empty = 0;\n+\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n \tstruct string_list trailers = STRING_LIST_INIT_NODUP;\n \n \tstruct option options[] = {\n-\t\tOPT_BOOL(0, \"in-place\", &in_place, N_(\"edit files in place\")),\n-\t\tOPT_BOOL(0, \"trim-empty\", &trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n+\t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -36,11 +35,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\n-\t\t\tprocess_trailers(argv[i], in_place, trim_empty, &trailers);\n+\t\t\tprocess_trailers(argv[i], &opts, &trailers);\n \t} else {\n-\t\tif (in_place)\n+\t\tif (opts.in_place)\n \t\t\tdie(_(\"no input file given for in-place editing\"));\n-\t\tprocess_trailers(NULL, in_place, trim_empty, &trailers);\n+\t\tprocess_trailers(NULL, &opts, &trailers);\n \t}\n \n \tstring_list_clear(&trailers, 0);\ndiff --git a/trailer.c b/trailer.c\nindex 751b56c009..e21a0d1629 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -968,7 +968,9 @@ static FILE *create_in_place_tempfile(const char *file)\n \treturn outfile;\n }\n \n-void process_trailers(const char *file, int in_place, int trim_empty, struct string_list *trailers)\n+void process_trailers(const char *file,\n+\t\t const struct process_trailer_options *opts,\n+\t\t struct string_list *trailers)\n {\n \tLIST_HEAD(head);\n \tLIST_HEAD(arg_head);\n@@ -980,7 +982,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tread_input_file(&sb, file);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n@@ -990,14 +992,14 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, trim_empty);\n+\tprint_all(outfile, &head, opts->trim_empty);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n \tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n-\tif (in_place)\n+\tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\n \t\t\tdie_errno(_(\"could not rename temporary file to %s\"), file);\n \ndiff --git a/trailer.h b/trailer.h\nindex 65cc5d79c6..9da00bedec 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -22,7 +22,15 @@ struct trailer_info {\n \tsize_t trailer_nr;\n };\n \n-void process_trailers(const char *file, int in_place, int trim_empty,\n+struct process_trailer_options {\n+\tint in_place;\n+\tint trim_empty;\n+};\n+\n+#define PROCESS_TRAILER_OPTIONS_INIT {0}\n+\n+void process_trailers(const char *file,\n+\t\t const struct process_trailer_options *opts,\n \t\t struct string_list *trailers);\n \n void trailer_info_get(struct trailer_info *info, const char *str);\n-- \n2.14.1.352.ge5efb0d3f3\n\n"},{"id":"326373","messageId":"20170815102321.xpa3ywfizykyq4tz@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 2/8] interpret-trailers: add an option to show only the trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:23:21Z","receivedAt":"2017-08-15T10:23:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In theory it's easy for any reader who wants to parse\ntrailers to do so. But there are a lot of subtle corner\ncases around what counts as a trailer, when the trailer\nblock begins and ends, etc. Since interpret-trailers already\nhas our parsing logic, let's let callers ask it to just\noutput the trailers.\n\nThey still have to parse the \"key: value\" lines, but at\nleast they can ignore all of the other corner cases.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 3 +++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 39 ++++++++++++++++++++++++++++++++\n trailer.c | 23 +++++++++++--------\n trailer.h | 1 +\n 5 files changed, 58 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 31cdeaecdf..295dffbd21 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -80,6 +80,9 @@ OPTIONS\n \ttrailer to the input messages. See the description of this\n \tcommand.\n \n+--only-trailers::\n+\tOutput only the trailers, not any other parts of the input.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex bb0d7b937a..afb12c11bc 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -24,6 +24,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n+\t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 0c6f91c433..90d30037b7 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1275,4 +1275,43 @@ test_expect_success 'with cut line' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only trailers' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\t\tsign: config-value\n+\t\tadded: added-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--trailer added:added-value \\\n+\t\t--only-trailers >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'only-trailers omits non-trailer in middle of block' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody \n+\t\tSigned-off-by: somebody \n+\t\tsign: config-value\n+\tEOF\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\tthis is not a trailer\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex e21a0d1629..22146002e9 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -164,13 +164,15 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)\n \t\tfprintf(outfile, \"%s%c %s\\n\", tok, separators[0], val);\n }\n \n-static void print_all(FILE *outfile, struct list_head *head, int trim_empty)\n+static void print_all(FILE *outfile, struct list_head *head,\n+\t\t const struct process_trailer_options *opts)\n {\n \tstruct list_head *pos;\n \tstruct trailer_item *item;\n \tlist_for_each(pos, head) {\n \t\titem = list_entry(pos, struct trailer_item, list);\n-\t\tif (!trim_empty || strlen(item->value) > 0)\n+\t\tif ((!opts->trim_empty || strlen(item->value) > 0) &&\n+\t\t (!opts->only_trailers || item->token))\n \t\t\tprint_tok_val(outfile, item->token, item->value);\n \t}\n }\n@@ -887,7 +889,8 @@ static int ends_with_blank_line(const char *buf, size_t len)\n \n static int process_input_file(FILE *outfile,\n \t\t\t const char *str,\n-\t\t\t struct list_head *head)\n+\t\t\t struct list_head *head,\n+\t\t\t const struct process_trailer_options *opts)\n {\n \tstruct trailer_info info;\n \tstruct strbuf tok = STRBUF_INIT;\n@@ -897,9 +900,10 @@ static int process_input_file(FILE *outfile,\n \ttrailer_info_get(&info, str);\n \n \t/* Print lines before the trailers as is */\n-\tfwrite(str, 1, info.trailer_start - str, outfile);\n+\tif (!opts->only_trailers)\n+\t\tfwrite(str, 1, info.trailer_start - str, outfile);\n \n-\tif (!info.blank_line_before_trailer)\n+\tif (!opts->only_trailers && !info.blank_line_before_trailer)\n \t\tfprintf(outfile, \"\\n\");\n \n \tfor (i = 0; i < info.trailer_nr; i++) {\n@@ -914,7 +918,7 @@ static int process_input_file(FILE *outfile,\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t strbuf_detach(&tok, NULL),\n \t\t\t\t\t strbuf_detach(&val, NULL));\n-\t\t} else {\n+\t\t} else if (!opts->only_trailers) {\n \t\t\tstrbuf_addstr(&val, trailer);\n \t\t\tstrbuf_strip_suffix(&val, \"\\n\");\n \t\t\tadd_trailer_item(head,\n@@ -986,18 +990,19 @@ void process_trailers(const char *file,\n \t\toutfile = create_in_place_tempfile(file);\n \n \t/* Print the lines before the trailers */\n-\ttrailer_end = process_input_file(outfile, sb.buf, &head);\n+\ttrailer_end = process_input_file(outfile, sb.buf, &head, opts);\n \n \tprocess_command_line_args(&arg_head, trailers);\n \n \tprocess_trailers_lists(&head, &arg_head);\n \n-\tprint_all(outfile, &head, opts->trim_empty);\n+\tprint_all(outfile, &head, opts);\n \n \tfree_all(&head);\n \n \t/* Print the lines after the trailers as is */\n-\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n+\tif (!opts->only_trailers)\n+\t\tfwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);\n \n \tif (opts->in_place)\n \t\tif (rename_tempfile(&trailers_tempfile, file))\ndiff --git a/trailer.h b/trailer.h\nindex 9da00bedec..3cf35ced00 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -25,6 +25,7 @@ struct trailer_info {\n struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n+\tint only_trailers;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.1.352.ge5efb0d3f3\n\n"},{"id":"326374","messageId":"20170815102325.yddy6csk2v6pbliw@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 3/8] interpret-trailers: add an option to show only existing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:23:25Z","receivedAt":"2017-08-15T10:23:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It can be useful to invoke interpret-trailers for the\nprimary purpose of parsing existing trailers. But in that\ncase, we don't want to apply existing ifMissing or ifExists\nrules from the config. Let's add a special mode where we\navoid applying those rules. Coupled with --only-trailers,\nthis gives us a reasonable parsing tool.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 5 +++++\n builtin/interpret-trailers.c | 7 +++++++\n t/t7513-interpret-trailers.sh | 16 ++++++++++++++++\n trailer.c | 9 +++++----\n trailer.h | 1 +\n 5 files changed, 34 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 295dffbd21..7cc43b0e3e 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -83,6 +83,11 @@ OPTIONS\n --only-trailers::\n \tOutput only the trailers, not any other parts of the input.\n \n+--only-input::\n+\tOutput only trailers that exist in the input; do not add any\n+\tfrom the command-line or by following configured `trailer.*`\n+\trules.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex afb12c11bc..2d90e0e480 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -25,6 +25,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"in-place\", &opts.in_place, N_(\"edit files in place\")),\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n+\t\tOPT_BOOL(0, \"only-input\", &opts.only_input, N_(\"do not apply config rules\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n@@ -33,6 +34,12 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t git_interpret_trailers_usage, 0);\n \n+\tif (opts.only_input && trailers.nr)\n+\t\tusage_msg_opt(\n+\t\t\t_(\"--trailer with --only-input does not make sense\"),\n+\t\t\tgit_interpret_trailers_usage,\n+\t\t\toptions);\n+\n \tif (argc) {\n \t\tint i;\n \t\tfor (i = 0; i < argc; i++)\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 90d30037b7..94b6c52473 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1314,4 +1314,20 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'only input' '\n+\tgit config trailer.sign.command \"echo config-value\" &&\n+\tcat >expected <<-\\EOF &&\n+\t\texisting: existing-value\n+\tEOF\n+\tgit interpret-trailers \\\n+\t\t--only-trailers --only-input >actual <<-\\EOF &&\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\texisting: existing-value\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 22146002e9..f97fcef3a5 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -977,7 +977,6 @@ void process_trailers(const char *file,\n \t\t struct string_list *trailers)\n {\n \tLIST_HEAD(head);\n-\tLIST_HEAD(arg_head);\n \tstruct strbuf sb = STRBUF_INIT;\n \tint trailer_end;\n \tFILE *outfile = stdout;\n@@ -992,9 +991,11 @@ void process_trailers(const char *file,\n \t/* Print the lines before the trailers */\n \ttrailer_end = process_input_file(outfile, sb.buf, &head, opts);\n \n-\tprocess_command_line_args(&arg_head, trailers);\n-\n-\tprocess_trailers_lists(&head, &arg_head);\n+\tif (!opts->only_input) {\n+\t\tLIST_HEAD(arg_head);\n+\t\tprocess_command_line_args(&arg_head, trailers);\n+\t\tprocess_trailers_lists(&head, &arg_head);\n+\t}\n \n \tprint_all(outfile, &head, opts);\n \ndiff --git a/trailer.h b/trailer.h\nindex 3cf35ced00..76c3b571bf 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -26,6 +26,7 @@ struct process_trailer_options {\n \tint in_place;\n \tint trim_empty;\n \tint only_trailers;\n+\tint only_input;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.1.352.ge5efb0d3f3\n\n"},{"id":"326375","messageId":"20170815102329.y576hw2gyb2hhj27@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 4/8] interpret-trailers: add an option to unfold values","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:23:29Z","receivedAt":"2017-08-15T10:23:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of \"--only-trailers\" is to give a caller an output\nthat's easy for them to parse. Getting rid of the\nnon-trailer material helps, but we still may see more\ncomplicated syntax like whitespace continuation. Let's add\nan option to unfold any continuation, giving the output as a\nsingle \"key: value\" line per trailer.\n\nAs a bonus, this could be used even without --only-trailers\nto clean up unusual formatting in the incoming data.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 4 ++++\n builtin/interpret-trailers.c | 1 +\n t/t7513-interpret-trailers.sh | 21 +++++++++++++++++++++\n trailer.c | 29 +++++++++++++++++++++++++++++\n trailer.h | 1 +\n 5 files changed, 56 insertions(+)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 7cc43b0e3e..be948e8028 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -88,6 +88,10 @@ OPTIONS\n \tfrom the command-line or by following configured `trailer.*`\n \trules.\n \n+--unfold::\n+\tRemove any whitespace-continuation in trailers, so that each\n+\ttrailer appears on a line by itself with its full content.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 2d90e0e480..922c3bad63 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -26,6 +26,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"trim-empty\", &opts.trim_empty, N_(\"trim empty trailers\")),\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-input\", &opts.only_input, N_(\"do not apply config rules\")),\n+\t\tOPT_BOOL(0, \"unfold\", &opts.unfold, N_(\"join whitespace-continued values\")),\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 94b6c52473..baf2feba98 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1330,4 +1330,25 @@ test_expect_success 'only input' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'unfold' '\n+\tcat >expected <<-\\EOF &&\n+\t\tfoo: continued across several lines\n+\tEOF\n+\t# pass through tr to make leading and trailing whitespace more obvious\n+\ttr _ \" \" <<-\\EOF |\n+\t\tmy subject\n+\n+\t\tmy body\n+\n+\t\tfoo:_\n+\t\t__continued\n+\t\t___across\n+\t\t____several\n+\t\t_____lines\n+\t\t___\n+\tEOF\n+\tgit interpret-trailers --only-trailers --only-input --unfold >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex f97fcef3a5..ed4fedc087 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -887,6 +887,33 @@ static int ends_with_blank_line(const char *buf, size_t len)\n \treturn is_blank_line(buf + ll);\n }\n \n+static void unfold_value(struct strbuf *val)\n+{\n+\tstruct strbuf out = STRBUF_INIT;\n+\tsize_t i;\n+\n+\tstrbuf_grow(&out, val->len);\n+\ti = 0;\n+\twhile (i < val->len) {\n+\t\tchar c = val->buf[i++];\n+\t\tif (c == '\\n') {\n+\t\t\t/* Collapse continuation down to a single space. */\n+\t\t\twhile (i < val->len && isspace(val->buf[i]))\n+\t\t\t\ti++;\n+\t\t\tstrbuf_addch(&out, ' ');\n+\t\t} else {\n+\t\t\tstrbuf_addch(&out, c);\n+\t\t}\n+\t}\n+\n+\t/* Empty lines may have left us with whitespace cruft at the edges */\n+\tstrbuf_trim(&out);\n+\n+\t/* output goes back to val as if we modified it in-place */\n+\tstrbuf_swap(&out, val);\n+\tstrbuf_release(&out);\n+}\n+\n static int process_input_file(FILE *outfile,\n \t\t\t const char *str,\n \t\t\t struct list_head *head,\n@@ -915,6 +942,8 @@ static int process_input_file(FILE *outfile,\n \t\tif (separator_pos >= 1) {\n \t\t\tparse_trailer(&tok, &val, NULL, trailer,\n \t\t\t\t separator_pos);\n+\t\t\tif (opts->unfold)\n+\t\t\t\tunfold_value(&val);\n \t\t\tadd_trailer_item(head,\n \t\t\t\t\t strbuf_detach(&tok, NULL),\n \t\t\t\t\t strbuf_detach(&val, NULL));\ndiff --git a/trailer.h b/trailer.h\nindex 76c3b571bf..194f85a102 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -27,6 +27,7 @@ struct process_trailer_options {\n \tint trim_empty;\n \tint only_trailers;\n \tint only_input;\n+\tint unfold;\n };\n \n #define PROCESS_TRAILER_OPTIONS_INIT {0}\n-- \n2.14.1.352.ge5efb0d3f3\n\n"},{"id":"326376","messageId":"20170815102334.qc4w7akl44bti44x@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 5/8] interpret-trailers: add --parse convenience option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:23:34Z","receivedAt":"2017-08-15T10:23:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The last few commits have added command line options that\ncan turn interpret-trailers into a parsing tool. Since\nthey'd most often be used together, let's provide a\nconvenient single option for callers to invoke this mode.\n\nThis is implemented as a callback rather than a boolean so\nthat its effect is applied immediately, as if those options\nhad been specified. Later options can then override them.\nE.g.:\n\n git interpret-trailers --parse --no-unfold\n\nwould work.\n\nLet's also update the documentation to make clear that this\nparsing mode behaves quite differently than the normal\n\"add trailers to the input\" mode.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-interpret-trailers.txt | 21 ++++++++++++++-------\n builtin/interpret-trailers.c | 12 ++++++++++++\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex be948e8028..1df8aabf51 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -3,24 +3,27 @@ git-interpret-trailers(1)\n \n NAME\n ----\n-git-interpret-trailers - help add structured information into commit messages\n+git-interpret-trailers - add or parse structured information in commit messages\n \n SYNOPSIS\n --------\n [verse]\n-'git interpret-trailers' [--in-place] [--trim-empty] [(--trailer [(=|:)])...] [...]\n+'git interpret-trailers' [options] [(--trailer [(=|:)])...] [...]\n+'git interpret-trailers' [options] [--parse] [...]\n \n DESCRIPTION\n -----------\n-Help adding 'trailers' lines, that look similar to RFC 822 e-mail\n+Help parsing or adding 'trailers' lines, that look similar to RFC 822 e-mail\n headers, at the end of the otherwise free-form part of a commit\n message.\n \n This command reads some patches or commit messages from either the\n- arguments or the standard input if no is specified. Then\n-this command applies the arguments passed using the `--trailer`\n-option, if any, to the commit message part of each input file. The\n-result is emitted on the standard output.\n+ arguments or the standard input if no is specified. If\n+`--parse` is specified, the output consists of the parsed trailers.\n+\n+Otherwise, the this command applies the arguments passed using the\n+`--trailer` option, if any, to the commit message part of each input\n+file. The result is emitted on the standard output.\n \n Some configuration variables control the way the `--trailer` arguments\n are applied to each commit message and the way any existing trailer in\n@@ -92,6 +95,10 @@ OPTIONS\n \tRemove any whitespace-continuation in trailers, so that each\n \ttrailer appears on a line by itself with its full content.\n \n+--parse::\n+\tA convenience alias for `--only-trailers --only-input\n+\t--unfold`.\n+\n CONFIGURATION VARIABLES\n -----------------------\n \ndiff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c\nindex 922c3bad63..555111a078 100644\n--- a/builtin/interpret-trailers.c\n+++ b/builtin/interpret-trailers.c\n@@ -16,6 +16,16 @@ static const char * const git_interpret_trailers_usage[] = {\n \tNULL\n };\n \n+static int parse_opt_parse(const struct option *opt, const char *arg,\n+\t\t\t int unset)\n+{\n+\tstruct process_trailer_options *v = opt->value;\n+\tv->only_trailers = 1;\n+\tv->only_input = 1;\n+\tv->unfold = 1;\n+\treturn 0;\n+}\n+\n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n {\n \tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n@@ -27,6 +37,8 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"only-trailers\", &opts.only_trailers, N_(\"output only the trailers\")),\n \t\tOPT_BOOL(0, \"only-input\", &opts.only_input, N_(\"do not apply config rules\")),\n \t\tOPT_BOOL(0, \"unfold\", &opts.unfold, N_(\"join whitespace-continued values\")),\n+\t\t{ OPTION_CALLBACK, 0, \"parse\", &opts, NULL, N_(\"set parsing options\"),\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse },\n \t\tOPT_STRING_LIST(0, \"trailer\", &trailers, N_(\"trailer\"),\n \t\t\t\tN_(\"trailer(s) to add\")),\n \t\tOPT_END()\n-- \n2.14.1.352.ge5efb0d3f3\n\n"},{"id":"326377","messageId":"20170815102355.zf4lvkntbwxacgca@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 6/8] pretty: move trailer formatting to trailer.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:23:56Z","receivedAt":"2017-08-15T10:24:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The next commit will add many features to the %(trailer)\nplaceholder in pretty.c. We'll need to access some internal\nfunctions of trailer.c for that, so our options are either:\n\n 1. expose those functions publicly\n\nor\n\n 2. make an entry point into trailer.c to do the formatting\n\nDoing (2) ends up exposing less surface area, though do note\nthat caveats in the docstring of the new function.\n\nSigned-off-by: Jeff King \n---\n pretty.c | 13 ++-----------\n trailer.c | 18 ++++++++++++++++++\n trailer.h | 14 ++++++++++++++\n 3 files changed, 34 insertions(+), 11 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 39cad5112b..99b309bb15 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -871,16 +871,6 @@ const char *format_subject(struct strbuf *sb, const char *msg,\n \treturn msg;\n }\n \n-static void format_trailers(struct strbuf *sb, const char *msg)\n-{\n-\tstruct trailer_info info;\n-\n-\ttrailer_info_get(&info, msg);\n-\tstrbuf_add(sb, info.trailer_start,\n-\t\t info.trailer_end - info.trailer_start);\n-\ttrailer_info_release(&info);\n-}\n-\n static void parse_commit_message(struct format_commit_context *c)\n {\n \tconst char *msg = c->message + c->message_off;\n@@ -1293,7 +1283,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t}\n \n \tif (starts_with(placeholder, \"(trailers)\")) {\n-\t\tformat_trailers(sb, msg + c->subject_off);\n+\t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n+\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\n \t\treturn strlen(\"(trailers)\");\n \t}\n \ndiff --git a/trailer.c b/trailer.c\nindex ed4fedc087..2d2b997ccc 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1091,3 +1091,21 @@ void trailer_info_release(struct trailer_info *info)\n \t\tfree(info->trailers[i]);\n \tfree(info->trailers);\n }\n+\n+static void format_trailer_info(struct strbuf *out,\n+\t\t\t\tconst struct trailer_info *info,\n+\t\t\t\tconst struct process_trailer_options *opts)\n+{\n+\tstrbuf_add(out, info->trailer_start,\n+\t\t info->trailer_end - info->trailer_start);\n+}\n+\n+void format_trailers_from_commit(struct strbuf *out, const char *msg,\n+\t\t\t\t const struct process_trailer_options *opts)\n+{\n+\tstruct trailer_info info;\n+\n+\ttrailer_info_get(&info, msg);\n+\tformat_trailer_info(out, &info, opts);\n+\ttrailer_info_release(&info);\n+}\ndiff --git a/trailer.h b/trailer.h\nindex 194f85a102..a172811022 100644\n--- a/trailer.h\n+++ b/trailer.h\n@@ -40,4 +40,18 @@ void trailer_info_get(struct trailer_info *info, const char *str);\n \n void trailer_info_release(struct trailer_info *info);\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+ */\n+void format_trailers_from_commit(struct strbuf *out, const char *msg,\n+\t\t\t\t const struct process_trailer_options *opts);\n+\n #endif /* TRAILER_H */\n-- \n2.14.1.352.ge5efb0d3f3\n\n"},{"id":"326378","messageId":"20170815102439.gillxuv56pcn3p6z@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 7/8] t4205: refactor %(trailers) tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:24:39Z","receivedAt":"2017-08-15T10:24:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We currently have one test for %(trailers). In preparation\nfor more, let's refactor a few bits:\n\n - move the commit creation to its own setup step so it can\n be reused by multiple tests\n\n - add a trailer with whitespace continuation (to confirm\n that it is left untouched)\n\n - fix the sample text which claims the placeholder is %bT.\n This was switched long ago to %(trailers)\n\n - replace one \"cat\" with an \"echo\" when generating the\n expected output. This saves a process (and sets a better\n pattern for future tests to follow).\n\nSigned-off-by: Jeff King \n---\n t/t4205-log-pretty-formats.sh | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 18aa1b5889..83ea85eb45 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -539,25 +539,29 @@ cat >trailers <\n Acked-by: A U Thor \n [ v2 updated patch description ]\n-Signed-off-by: A U Thor \n+Signed-off-by: A U Thor\n+ \n EOF\n \n-test_expect_success 'pretty format %(trailers) shows trailers' '\n+test_expect_success 'set up trailer tests' '\n \techo \"Some contents\" >trailerfile &&\n \tgit add trailerfile &&\n-\tgit commit -F - <<-EOF &&\n+\tgit commit -F - <<-EOF\n \ttrailers: this commit message has trailers\n \n \tThis commit is a test commit with trailers at the end. We parse this\n-\tmessage and display the trailers using %bT\n+\tmessage and display the trailers using %(trailers).\n \n \t$(cat trailers)\n \tEOF\n-\tgit log --no-walk --pretty=\"%(trailers)\" >actual &&\n-\tcat >expect <<-EOF &&\n-\t$(cat trailers)\n+'\n \n-\tEOF\n+test_expect_success 'pretty format %(trailers) shows trailers' '\n+\tgit log --no-walk --pretty=\"%(trailers)\" >actual &&\n+\t{\n+\t\tcat trailers &&\n+\t\techo\n+\t} >expect &&\n \ttest_cmp expect actual\n '\n \n-- \n2.14.1.352.ge5efb0d3f3\n\n"},{"id":"326379","messageId":"20170815102527.5rblspnpao67srtq@sigill.intra.peff.net","threadId":"46548","inReplyTo":"20170815102254.knccmhgralfijwli@sigill.intra.peff.net","subject":"[PATCH v4 8/8] pretty: support normalization options for %(trailers)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-15T10:25:27Z","receivedAt":"2017-08-15T10:25:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The interpret-trailers command recently learned some options\nto make its output easier to parse (for a caller whose only\ninterested in picking out the trailer values). But it's not\nvery efficient for asking for the trailers of many commits\nin a single invocation.\n\nWe already have \"%(trailers)\" to do that, but it doesn't\nknow about unfolding or omitting non-trailers. Let's plumb\nthose options through, so you can have the best of both.\n\nSigned-off-by: Jeff King \n---\n Documentation/pretty-formats.txt | 5 ++++-\n pretty.c | 15 ++++++++++++---\n t/t4205-log-pretty-formats.sh | 33 +++++++++++++++++++++++++++++++++\n trailer.c | 32 ++++++++++++++++++++++++++++++--\n 4 files changed, 79 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 973d19606b..d433d50f81 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -205,7 +205,10 @@ endif::git-rev-list[]\n - '%><()', '%><|()': similar to '% <()', '%<|()'\n respectively, but padding both sides (i.e. the text is centered)\n - %(trailers): display the trailers of the body as interpreted by\n- linkgit:git-interpret-trailers[1]\n+ linkgit:git-interpret-trailers[1]. If the `:only` option is given,\n+ omit non-trailer lines from the trailer block. If the `:unfold`\n+ option is given, behave as if interpret-trailer's `--unfold` option\n+ was given. E.g., `%(trailers:only:unfold)` to do both.\n \n NOTE: Some placeholders may depend on other options given to the\n revision traversal engine. For example, the `%g*` reflog options will\ndiff --git a/pretty.c b/pretty.c\nindex 99b309bb15..94eab5c89e 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1064,6 +1064,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \tconst struct commit *commit = c->commit;\n \tconst char *msg = c->message;\n \tstruct commit_list *p;\n+\tconst char *arg;\n \tint ch;\n \n \t/* these are independent of the commit */\n@@ -1282,10 +1283,18 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\treturn 1;\n \t}\n \n-\tif (starts_with(placeholder, \"(trailers)\")) {\n+\tif (skip_prefix(placeholder, \"(trailers\", &arg)) {\n \t\tstruct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n-\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\n-\t\treturn strlen(\"(trailers)\");\n+\t\twhile (*arg == ':') {\n+\t\t\tif (skip_prefix(arg, \":only\", &arg))\n+\t\t\t\topts.only_trailers = 1;\n+\t\t\telse if (skip_prefix(arg, \":unfold\", &arg))\n+\t\t\t\topts.unfold = 1;\n+\t\t}\n+\t\tif (*arg == ')') {\n+\t\t\tformat_trailers_from_commit(sb, msg + c->subject_off, &opts);\n+\t\t\treturn arg - placeholder + 1;\n+\t\t}\n \t}\n \n \treturn 0;\t/* unknown placeholder */\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 83ea85eb45..ec5f530102 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -543,6 +543,10 @@ Signed-off-by: A U Thor\n \n EOF\n \n+unfold () {\n+\tperl -0pe 's/\\n\\s+/ /'\n+}\n+\n test_expect_success 'set up trailer tests' '\n \techo \"Some contents\" >trailerfile &&\n \tgit add trailerfile &&\n@@ -565,4 +569,33 @@ test_expect_success 'pretty format %(trailers) shows trailers' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '%(trailers:only) shows only \"key: value\" trailers' '\n+\tgit log --no-walk --pretty=\"%(trailers:only)\" >actual &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%(trailers:unfold) unfolds trailers' '\n+\tgit log --no-walk --pretty=\"%(trailers:unfold)\" >actual &&\n+\t{\n+\t\tunfold expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success ':only and :unfold work together' '\n+\tgit log --no-walk --pretty=\"%(trailers:only:unfold)\" >actual &&\n+\tgit log --no-walk --pretty=\"%(trailers:unfold:only)\" >reverse &&\n+\ttest_cmp actual reverse &&\n+\t{\n+\t\tgrep -v patch.description expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/trailer.c b/trailer.c\nindex 2d2b997ccc..5220d498e5 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -1096,8 +1096,36 @@ static void format_trailer_info(struct strbuf *out,\n \t\t\t\tconst struct trailer_info *info,\n \t\t\t\tconst struct process_trailer_options *opts)\n {\n-\tstrbuf_add(out, info->trailer_start,\n-\t\t info->trailer_end - info->trailer_start);\n+\tint i;\n+\n+\t/* If we want the whole block untouched, we can take the fast path. */\n+\tif (!opts->only_trailers && !opts->unfold) {\n+\t\tstrbuf_add(out, info->trailer_start,\n+\t\t\t info->trailer_end - info->trailer_start);\n+\t\treturn;\n+\t}\n+\n+\tfor (i = 0; i < info->trailer_nr; i++) {\n+\t\tchar *trailer = info->trailers[i];\n+\t\tint separator_pos = find_separator(trailer, separators);\n+\n+\t\tif (separator_pos >= 1) {\n+\t\t\tstruct strbuf tok = STRBUF_INIT;\n+\t\t\tstruct strbuf val = STRBUF_INIT;\n+\n+\t\t\tparse_trailer(&tok, &val, NULL, trailer, separator_pos);\n+\t\t\tif (opts->unfold)\n+\t\t\t\tunfold_value(&val);\n+\n+\t\t\tstrbuf_addf(out, \"%s: %s\\n\", tok.buf, val.buf);\n+\t\t\tstrbuf_release(&tok);\n+\t\t\tstrbuf_release(&val);\n+\n+\t\t} else if (!opts->only_trailers) {\n+\t\t\tstrbuf_addstr(out, trailer);\n+\t\t}\n+\t}\n+\n }\n \n void format_trailers_from_commit(struct strbuf *out, const char *msg,\n-- \n2.14.1.352.ge5efb0d3f3\n"},{"id":"326381","messageId":"CAN0heSrUz2jU2ijeSjYRhvyXA8qpw=JoOhaK-4Fuyv4_1L6Gtw@mail.gmail.com","threadId":"46548","inReplyTo":"20170815102334.qc4w7akl44bti44x@sigill.intra.peff.net","subject":"Re: [PATCH v4 5/8] interpret-trailers: add --parse convenience option","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2017-08-15T11:26:53Z","receivedAt":"2017-08-15T11:27:02Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 15 August 2017 at 12:23, Jeff King wrote:\n> SYNOPSIS\n> --------\n> [verse]\n> -'git interpret-trailers' [--in-place] [--trim-empty] [(--trailer [(=|:)])...] [...]\n> +'git interpret-trailers' [options] [(--trailer [(=|:)])...] [...]\n> +'git interpret-trailers' [options] [--parse] [...]\n>\n> DESCRIPTION\n> -----------\n> -Help adding 'trailers' lines, that look similar to RFC 822 e-mail\n> +Help parsing or adding 'trailers' lines, that look similar to RFC 822 e-mail\n> headers, at the end of the otherwise free-form part of a commit\n> message.\n>\n> This command reads some patches or commit messages from either the\n> - arguments or the standard input if no is specified. Then\n> -this command applies the arguments passed using the `--trailer`\n> -option, if any, to the commit message part of each input file. The\n> -result is emitted on the standard output.\n> + arguments or the standard input if no is specified. If\n> +`--parse` is specified, the output consists of the parsed trailers.\n> +\n> +Otherwise, the this command applies the arguments passed using the\n> +`--trailer` option, if any, to the commit message part of each input\n> +file. The result is emitted on the standard output.\n\n\"the this\"\n\nI think I get why you use --parse above (and in the synopsis), although\nit kind of feels like it should be --only-input or perhaps \"--only-input\n(or --parse)\".\n\n--only-input is sort of not covered by the \"--parse\"-part above, and it\nis sort of not covered in the \"Otherwise\", since --only-input and\n--trailer are incompatible. So it is sort of lost. :-) Probably doesn't\nmatter much. (I'm just thinking out loud without constructive ideas.)\nThose who care about such details can continue reading..\n\nMartin\n"},{"id":"326492","messageId":"20170816082022.5use3im2bq27wik4@sigill.intra.peff.net","threadId":"46548","inReplyTo":"CAN0heSrUz2jU2ijeSjYRhvyXA8qpw=JoOhaK-4Fuyv4_1L6Gtw@mail.gmail.com","subject":"Re: [PATCH v4 5/8] interpret-trailers: add --parse convenience option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-16T08:20:22Z","receivedAt":"2017-08-16T08:20:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 15, 2017 at 01:26:53PM +0200, Martin Ågren wrote:\n\n> > This command reads some patches or commit messages from either the\n> > - arguments or the standard input if no is specified. Then\n> > -this command applies the arguments passed using the `--trailer`\n> > -option, if any, to the commit message part of each input file. The\n> > -result is emitted on the standard output.\n> > + arguments or the standard input if no is specified. If\n> > +`--parse` is specified, the output consists of the parsed trailers.\n> > +\n> > +Otherwise, the this command applies the arguments passed using the\n> > +`--trailer` option, if any, to the commit message part of each input\n> > +file. The result is emitted on the standard output.\n> \n> \"the this\"\n\nThanks.\n\n> I think I get why you use --parse above (and in the synopsis), although\n> it kind of feels like it should be --only-input or perhaps \"--only-input\n> (or --parse)\".\n\nI really wanted to point people to --parse as the go-to option for the\nparsing mode for the sake of simplicity (in fact I initially considered\nnot even exposing them at all). And I hoped that if they jumped to the\ndefinition of --parse, that would lead them to the other options.\n\nI dunno. I agree it is does not read particularly well. Probably the\nwhole description section could be rewritten to cover the newly dual\nnature of the command a bit better. But I didn't want to disrupt the\nexisting flow too much.\n\n-Peff\n"},{"id":"326622","messageId":"CAN0heSqTu9F6Coi7qydO6OKd_ojiriHMuvqjj3C9ag6M_48YOQ@mail.gmail.com","threadId":"46548","inReplyTo":"20170816082022.5use3im2bq27wik4@sigill.intra.peff.net","subject":"Re: [PATCH v4 5/8] interpret-trailers: add --parse convenience option","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2017-08-17T18:19:39Z","receivedAt":"2017-08-17T18:19:44Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 16 August 2017 at 10:20, Jeff King wrote:\n> On Tue, Aug 15, 2017 at 01:26:53PM +0200, Martin Ågren wrote:\n>\n>> > This command reads some patches or commit messages from either the\n>> > - arguments or the standard input if no is specified. Then\n>> > -this command applies the arguments passed using the `--trailer`\n>> > -option, if any, to the commit message part of each input file. The\n>> > -result is emitted on the standard output.\n>> > + arguments or the standard input if no is specified. If\n>> > +`--parse` is specified, the output consists of the parsed trailers.\n>> > +\n>> > +Otherwise, the this command applies the arguments passed using the\n>> > +`--trailer` option, if any, to the commit message part of each input\n>> > +file. The result is emitted on the standard output.\n>>\n>> \"the this\"\n>\n> Thanks.\n>\n>> I think I get why you use --parse above (and in the synopsis), although\n>> it kind of feels like it should be --only-input or perhaps \"--only-input\n>> (or --parse)\".\n>\n> I really wanted to point people to --parse as the go-to option for the\n> parsing mode for the sake of simplicity (in fact I initially considered\n> not even exposing them at all). And I hoped that if they jumped to the\n> definition of --parse, that would lead them to the other options.\n>\n> I dunno. I agree it is does not read particularly well. Probably the\n> whole description section could be rewritten to cover the newly dual\n> nature of the command a bit better. But I didn't want to disrupt the\n> existing flow too much.\n\nCertainly. It's not like I can offer a \"better\" way. After thinking\nabout this some more, I think this is a good example of \"less is more\".\nIt's possibly less precise or complete in some sense, but it's probably\nmuch more useful and readable.\n"}]}