From 8abc89800c09cda7910c2211ebbbbb95a3008b63 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 10 Aug 2017 14:03:58 -0400 Subject: [PATCH 1/9] trailer: put process_trailers() options into a struct We already have two options and are about to add a few more. To avoid having a huge number of boolean arguments, let's convert to an options struct which can be passed in. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/interpret-trailers.c | 13 ++++++------- trailer.c | 10 ++++++---- trailer.h | 10 +++++++++- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 175f14797b..bb0d7b937a 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -18,13 +18,12 @@ static const char * const git_interpret_trailers_usage[] = { int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) { - int in_place = 0; - int trim_empty = 0; + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; struct string_list trailers = STRING_LIST_INIT_NODUP; struct option options[] = { - OPT_BOOL(0, "in-place", &in_place, N_("edit files in place")), - OPT_BOOL(0, "trim-empty", &trim_empty, N_("trim empty trailers")), + OPT_BOOL(0, "in-place", &opts.in_place, N_("edit files in place")), + OPT_BOOL(0, "trim-empty", &opts.trim_empty, N_("trim empty trailers")), OPT_STRING_LIST(0, "trailer", &trailers, N_("trailer"), N_("trailer(s) to add")), OPT_END() @@ -36,11 +35,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) if (argc) { int i; for (i = 0; i < argc; i++) - process_trailers(argv[i], in_place, trim_empty, &trailers); + process_trailers(argv[i], &opts, &trailers); } else { - if (in_place) + if (opts.in_place) die(_("no input file given for in-place editing")); - process_trailers(NULL, in_place, trim_empty, &trailers); + process_trailers(NULL, &opts, &trailers); } string_list_clear(&trailers, 0); diff --git a/trailer.c b/trailer.c index 11f0b9fb40..9bcd54e813 100644 --- a/trailer.c +++ b/trailer.c @@ -967,7 +967,9 @@ static FILE *create_in_place_tempfile(const char *file) return outfile; } -void process_trailers(const char *file, int in_place, int trim_empty, struct string_list *trailers) +void process_trailers(const char *file, + const struct process_trailer_options *opts, + struct string_list *trailers) { LIST_HEAD(head); LIST_HEAD(arg_head); @@ -979,7 +981,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str read_input_file(&sb, file); - if (in_place) + if (opts->in_place) outfile = create_in_place_tempfile(file); /* Print the lines before the trailers */ @@ -989,14 +991,14 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str process_trailers_lists(&head, &arg_head); - print_all(outfile, &head, trim_empty); + print_all(outfile, &head, opts->trim_empty); free_all(&head); /* Print the lines after the trailers as is */ fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile); - if (in_place) + if (opts->in_place) if (rename_tempfile(&trailers_tempfile, file)) die_errno(_("could not rename temporary file to %s"), file); diff --git a/trailer.h b/trailer.h index 65cc5d79c6..9da00bedec 100644 --- a/trailer.h +++ b/trailer.h @@ -22,7 +22,15 @@ struct trailer_info { size_t trailer_nr; }; -void process_trailers(const char *file, int in_place, int trim_empty, +struct process_trailer_options { + int in_place; + int trim_empty; +}; + +#define PROCESS_TRAILER_OPTIONS_INIT {0} + +void process_trailers(const char *file, + const struct process_trailer_options *opts, struct string_list *trailers); void trailer_info_get(struct trailer_info *info, const char *str); From 56c493ed1b9c067813fb95ff7cd4f69c7c1d2e36 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 15 Aug 2017 06:23:21 -0400 Subject: [PATCH 2/9] interpret-trailers: add an option to show only the trailers In theory it's easy for any reader who wants to parse trailers to do so. But there are a lot of subtle corner cases around what counts as a trailer, when the trailer block begins and ends, etc. Since interpret-trailers already has our parsing logic, let's let callers ask it to just output the trailers. They still have to parse the "key: value" lines, but at least they can ignore all of the other corner cases. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-interpret-trailers.txt | 3 ++ builtin/interpret-trailers.c | 1 + t/t7513-interpret-trailers.sh | 39 ++++++++++++++++++++++++ trailer.c | 23 ++++++++------ trailer.h | 1 + 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 31cdeaecdf..295dffbd21 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -80,6 +80,9 @@ OPTIONS trailer to the input messages. See the description of this command. +--only-trailers:: + Output only the trailers, not any other parts of the input. + CONFIGURATION VARIABLES ----------------------- diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index bb0d7b937a..afb12c11bc 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -24,6 +24,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) struct option options[] = { OPT_BOOL(0, "in-place", &opts.in_place, N_("edit files in place")), OPT_BOOL(0, "trim-empty", &opts.trim_empty, N_("trim empty trailers")), + OPT_BOOL(0, "only-trailers", &opts.only_trailers, N_("output only the trailers")), OPT_STRING_LIST(0, "trailer", &trailers, N_("trailer"), N_("trailer(s) to add")), OPT_END() diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 0c6f91c433..90d30037b7 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -1275,4 +1275,43 @@ test_expect_success 'with cut line' ' test_cmp expected actual ' +test_expect_success 'only trailers' ' + git config trailer.sign.command "echo config-value" && + cat >expected <<-\EOF && + existing: existing-value + sign: config-value + added: added-value + EOF + git interpret-trailers \ + --trailer added:added-value \ + --only-trailers >actual <<-\EOF && + my subject + + my body + + existing: existing-value + EOF + test_cmp expected actual +' + +test_expect_success 'only-trailers omits non-trailer in middle of block' ' + git config trailer.sign.command "echo config-value" && + cat >expected <<-\EOF && + Signed-off-by: nobody + Signed-off-by: somebody + sign: config-value + EOF + git interpret-trailers --only-trailers >actual <<-\EOF && + subject + + it is important that the trailers below are signed-off-by + so that they meet the "25% trailers Git knows about" heuristic + + Signed-off-by: nobody + this is not a trailer + Signed-off-by: somebody + EOF + test_cmp expected actual +' + test_done diff --git a/trailer.c b/trailer.c index 9bcd54e813..83225c6828 100644 --- a/trailer.c +++ b/trailer.c @@ -163,13 +163,15 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val) fprintf(outfile, "%s%c %s\n", tok, separators[0], val); } -static void print_all(FILE *outfile, struct list_head *head, int trim_empty) +static void print_all(FILE *outfile, struct list_head *head, + const struct process_trailer_options *opts) { struct list_head *pos; struct trailer_item *item; list_for_each(pos, head) { item = list_entry(pos, struct trailer_item, list); - if (!trim_empty || strlen(item->value) > 0) + if ((!opts->trim_empty || strlen(item->value) > 0) && + (!opts->only_trailers || item->token)) print_tok_val(outfile, item->token, item->value); } } @@ -886,7 +888,8 @@ static int ends_with_blank_line(const char *buf, size_t len) static int process_input_file(FILE *outfile, const char *str, - struct list_head *head) + struct list_head *head, + const struct process_trailer_options *opts) { struct trailer_info info; struct strbuf tok = STRBUF_INIT; @@ -896,9 +899,10 @@ static int process_input_file(FILE *outfile, trailer_info_get(&info, str); /* Print lines before the trailers as is */ - fwrite(str, 1, info.trailer_start - str, outfile); + if (!opts->only_trailers) + fwrite(str, 1, info.trailer_start - str, outfile); - if (!info.blank_line_before_trailer) + if (!opts->only_trailers && !info.blank_line_before_trailer) fprintf(outfile, "\n"); for (i = 0; i < info.trailer_nr; i++) { @@ -913,7 +917,7 @@ static int process_input_file(FILE *outfile, add_trailer_item(head, strbuf_detach(&tok, NULL), strbuf_detach(&val, NULL)); - } else { + } else if (!opts->only_trailers) { strbuf_addstr(&val, trailer); strbuf_strip_suffix(&val, "\n"); add_trailer_item(head, @@ -985,18 +989,19 @@ void process_trailers(const char *file, outfile = create_in_place_tempfile(file); /* Print the lines before the trailers */ - trailer_end = process_input_file(outfile, sb.buf, &head); + trailer_end = process_input_file(outfile, sb.buf, &head, opts); process_command_line_args(&arg_head, trailers); process_trailers_lists(&head, &arg_head); - print_all(outfile, &head, opts->trim_empty); + print_all(outfile, &head, opts); free_all(&head); /* Print the lines after the trailers as is */ - fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile); + if (!opts->only_trailers) + fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile); if (opts->in_place) if (rename_tempfile(&trailers_tempfile, file)) diff --git a/trailer.h b/trailer.h index 9da00bedec..3cf35ced00 100644 --- a/trailer.h +++ b/trailer.h @@ -25,6 +25,7 @@ struct trailer_info { struct process_trailer_options { int in_place; int trim_empty; + int only_trailers; }; #define PROCESS_TRAILER_OPTIONS_INIT {0} From fdbdb64f49959f9c83329554080934895f02ae59 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 15 Aug 2017 06:23:25 -0400 Subject: [PATCH 3/9] interpret-trailers: add an option to show only existing trailers It can be useful to invoke interpret-trailers for the primary purpose of parsing existing trailers. But in that case, we don't want to apply existing ifMissing or ifExists rules from the config. Let's add a special mode where we avoid applying those rules. Coupled with --only-trailers, this gives us a reasonable parsing tool. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-interpret-trailers.txt | 5 +++++ builtin/interpret-trailers.c | 7 +++++++ t/t7513-interpret-trailers.sh | 16 ++++++++++++++++ trailer.c | 9 +++++---- trailer.h | 1 + 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 295dffbd21..7cc43b0e3e 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -83,6 +83,11 @@ OPTIONS --only-trailers:: Output only the trailers, not any other parts of the input. +--only-input:: + Output only trailers that exist in the input; do not add any + from the command-line or by following configured `trailer.*` + rules. + CONFIGURATION VARIABLES ----------------------- diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index afb12c11bc..2d90e0e480 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -25,6 +25,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "in-place", &opts.in_place, N_("edit files in place")), OPT_BOOL(0, "trim-empty", &opts.trim_empty, N_("trim empty trailers")), OPT_BOOL(0, "only-trailers", &opts.only_trailers, N_("output only the trailers")), + OPT_BOOL(0, "only-input", &opts.only_input, N_("do not apply config rules")), OPT_STRING_LIST(0, "trailer", &trailers, N_("trailer"), N_("trailer(s) to add")), OPT_END() @@ -33,6 +34,12 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_interpret_trailers_usage, 0); + if (opts.only_input && trailers.nr) + usage_msg_opt( + _("--trailer with --only-input does not make sense"), + git_interpret_trailers_usage, + options); + if (argc) { int i; for (i = 0; i < argc; i++) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 90d30037b7..94b6c52473 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -1314,4 +1314,20 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' ' test_cmp expected actual ' +test_expect_success 'only input' ' + git config trailer.sign.command "echo config-value" && + cat >expected <<-\EOF && + existing: existing-value + EOF + git interpret-trailers \ + --only-trailers --only-input >actual <<-\EOF && + my subject + + my body + + existing: existing-value + EOF + test_cmp expected actual +' + test_done diff --git a/trailer.c b/trailer.c index 83225c6828..847417ef89 100644 --- a/trailer.c +++ b/trailer.c @@ -976,7 +976,6 @@ void process_trailers(const char *file, struct string_list *trailers) { LIST_HEAD(head); - LIST_HEAD(arg_head); struct strbuf sb = STRBUF_INIT; int trailer_end; FILE *outfile = stdout; @@ -991,9 +990,11 @@ void process_trailers(const char *file, /* Print the lines before the trailers */ trailer_end = process_input_file(outfile, sb.buf, &head, opts); - process_command_line_args(&arg_head, trailers); - - process_trailers_lists(&head, &arg_head); + if (!opts->only_input) { + LIST_HEAD(arg_head); + process_command_line_args(&arg_head, trailers); + process_trailers_lists(&head, &arg_head); + } print_all(outfile, &head, opts); diff --git a/trailer.h b/trailer.h index 3cf35ced00..76c3b571bf 100644 --- a/trailer.h +++ b/trailer.h @@ -26,6 +26,7 @@ struct process_trailer_options { int in_place; int trim_empty; int only_trailers; + int only_input; }; #define PROCESS_TRAILER_OPTIONS_INIT {0} From 000023961a0c02d6e21dc51ea3484ff71abf1c74 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 15 Aug 2017 06:23:29 -0400 Subject: [PATCH 4/9] interpret-trailers: add an option to unfold values The point of "--only-trailers" is to give a caller an output that's easy for them to parse. Getting rid of the non-trailer material helps, but we still may see more complicated syntax like whitespace continuation. Let's add an option to unfold any continuation, giving the output as a single "key: value" line per trailer. As a bonus, this could be used even without --only-trailers to clean up unusual formatting in the incoming data. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-interpret-trailers.txt | 4 ++++ builtin/interpret-trailers.c | 1 + t/t7513-interpret-trailers.sh | 21 +++++++++++++++++ trailer.c | 29 ++++++++++++++++++++++++ trailer.h | 1 + 5 files changed, 56 insertions(+) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 7cc43b0e3e..be948e8028 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -88,6 +88,10 @@ OPTIONS from the command-line or by following configured `trailer.*` rules. +--unfold:: + Remove any whitespace-continuation in trailers, so that each + trailer appears on a line by itself with its full content. + CONFIGURATION VARIABLES ----------------------- diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 2d90e0e480..922c3bad63 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -26,6 +26,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "trim-empty", &opts.trim_empty, N_("trim empty trailers")), OPT_BOOL(0, "only-trailers", &opts.only_trailers, N_("output only the trailers")), OPT_BOOL(0, "only-input", &opts.only_input, N_("do not apply config rules")), + OPT_BOOL(0, "unfold", &opts.unfold, N_("join whitespace-continued values")), OPT_STRING_LIST(0, "trailer", &trailers, N_("trailer"), N_("trailer(s) to add")), OPT_END() diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 94b6c52473..baf2feba98 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -1330,4 +1330,25 @@ test_expect_success 'only input' ' test_cmp expected actual ' +test_expect_success 'unfold' ' + cat >expected <<-\EOF && + foo: continued across several lines + EOF + # pass through tr to make leading and trailing whitespace more obvious + tr _ " " <<-\EOF | + my subject + + my body + + foo:_ + __continued + ___across + ____several + _____lines + ___ + EOF + git interpret-trailers --only-trailers --only-input --unfold >actual && + test_cmp expected actual +' + test_done diff --git a/trailer.c b/trailer.c index 847417ef89..e63f432947 100644 --- a/trailer.c +++ b/trailer.c @@ -886,6 +886,33 @@ static int ends_with_blank_line(const char *buf, size_t len) return is_blank_line(buf + ll); } +static void unfold_value(struct strbuf *val) +{ + struct strbuf out = STRBUF_INIT; + size_t i; + + strbuf_grow(&out, val->len); + i = 0; + while (i < val->len) { + char c = val->buf[i++]; + if (c == '\n') { + /* Collapse continuation down to a single space. */ + while (i < val->len && isspace(val->buf[i])) + i++; + strbuf_addch(&out, ' '); + } else { + strbuf_addch(&out, c); + } + } + + /* Empty lines may have left us with whitespace cruft at the edges */ + strbuf_trim(&out); + + /* output goes back to val as if we modified it in-place */ + strbuf_swap(&out, val); + strbuf_release(&out); +} + static int process_input_file(FILE *outfile, const char *str, struct list_head *head, @@ -914,6 +941,8 @@ static int process_input_file(FILE *outfile, if (separator_pos >= 1) { parse_trailer(&tok, &val, NULL, trailer, separator_pos); + if (opts->unfold) + unfold_value(&val); add_trailer_item(head, strbuf_detach(&tok, NULL), strbuf_detach(&val, NULL)); diff --git a/trailer.h b/trailer.h index 76c3b571bf..194f85a102 100644 --- a/trailer.h +++ b/trailer.h @@ -27,6 +27,7 @@ struct process_trailer_options { int trim_empty; int only_trailers; int only_input; + int unfold; }; #define PROCESS_TRAILER_OPTIONS_INIT {0} From 99e09dafd7b7bcac4d8189b41dc6038bf36334f5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 15 Aug 2017 06:23:34 -0400 Subject: [PATCH 5/9] interpret-trailers: add --parse convenience option The last few commits have added command line options that can turn interpret-trailers into a parsing tool. Since they'd most often be used together, let's provide a convenient single option for callers to invoke this mode. This is implemented as a callback rather than a boolean so that its effect is applied immediately, as if those options had been specified. Later options can then override them. E.g.: git interpret-trailers --parse --no-unfold would work. Let's also update the documentation to make clear that this parsing mode behaves quite differently than the normal "add trailers to the input" mode. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-interpret-trailers.txt | 21 ++++++++++++++------- builtin/interpret-trailers.c | 12 ++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index be948e8028..1df8aabf51 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -3,24 +3,27 @@ git-interpret-trailers(1) NAME ---- -git-interpret-trailers - help add structured information into commit messages +git-interpret-trailers - add or parse structured information in commit messages SYNOPSIS -------- [verse] -'git interpret-trailers' [--in-place] [--trim-empty] [(--trailer [(=|:)])...] [...] +'git interpret-trailers' [options] [(--trailer [(=|:)])...] [...] +'git interpret-trailers' [options] [--parse] [...] DESCRIPTION ----------- -Help adding 'trailers' lines, that look similar to RFC 822 e-mail +Help parsing or adding 'trailers' lines, that look similar to RFC 822 e-mail headers, at the end of the otherwise free-form part of a commit message. This command reads some patches or commit messages from either the - arguments or the standard input if no is specified. Then -this command applies the arguments passed using the `--trailer` -option, if any, to the commit message part of each input file. The -result is emitted on the standard output. + arguments or the standard input if no is specified. If +`--parse` is specified, the output consists of the parsed trailers. + +Otherwise, the this command applies the arguments passed using the +`--trailer` option, if any, to the commit message part of each input +file. The result is emitted on the standard output. Some configuration variables control the way the `--trailer` arguments are applied to each commit message and the way any existing trailer in @@ -92,6 +95,10 @@ OPTIONS Remove any whitespace-continuation in trailers, so that each trailer appears on a line by itself with its full content. +--parse:: + A convenience alias for `--only-trailers --only-input + --unfold`. + CONFIGURATION VARIABLES ----------------------- diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 922c3bad63..555111a078 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -16,6 +16,16 @@ static const char * const git_interpret_trailers_usage[] = { NULL }; +static int parse_opt_parse(const struct option *opt, const char *arg, + int unset) +{ + struct process_trailer_options *v = opt->value; + v->only_trailers = 1; + v->only_input = 1; + v->unfold = 1; + return 0; +} + int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) { struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; @@ -27,6 +37,8 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "only-trailers", &opts.only_trailers, N_("output only the trailers")), OPT_BOOL(0, "only-input", &opts.only_input, N_("do not apply config rules")), OPT_BOOL(0, "unfold", &opts.unfold, N_("join whitespace-continued values")), + { OPTION_CALLBACK, 0, "parse", &opts, NULL, N_("set parsing options"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse }, OPT_STRING_LIST(0, "trailer", &trailers, N_("trailer"), N_("trailer(s) to add")), OPT_END() From a388b10fc17c435df32c3875225a1468edad9535 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 15 Aug 2017 06:23:56 -0400 Subject: [PATCH 6/9] pretty: move trailer formatting to trailer.c The next commit will add many features to the %(trailer) placeholder in pretty.c. We'll need to access some internal functions of trailer.c for that, so our options are either: 1. expose those functions publicly or 2. make an entry point into trailer.c to do the formatting Doing (2) ends up exposing less surface area, though do note that caveats in the docstring of the new function. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pretty.c | 13 ++----------- trailer.c | 18 ++++++++++++++++++ trailer.h | 14 ++++++++++++++ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/pretty.c b/pretty.c index 6cc812c2c7..33054e22cd 100644 --- a/pretty.c +++ b/pretty.c @@ -870,16 +870,6 @@ const char *format_subject(struct strbuf *sb, const char *msg, return msg; } -static void format_trailers(struct strbuf *sb, const char *msg) -{ - struct trailer_info info; - - trailer_info_get(&info, msg); - strbuf_add(sb, info.trailer_start, - info.trailer_end - info.trailer_start); - trailer_info_release(&info); -} - static void parse_commit_message(struct format_commit_context *c) { const char *msg = c->message + c->message_off; @@ -1273,7 +1263,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ } if (starts_with(placeholder, "(trailers)")) { - format_trailers(sb, msg + c->subject_off); + struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; + format_trailers_from_commit(sb, msg + c->subject_off, &opts); return strlen("(trailers)"); } diff --git a/trailer.c b/trailer.c index e63f432947..07580af9c0 100644 --- a/trailer.c +++ b/trailer.c @@ -1090,3 +1090,21 @@ void trailer_info_release(struct trailer_info *info) free(info->trailers[i]); free(info->trailers); } + +static void format_trailer_info(struct strbuf *out, + const struct trailer_info *info, + const struct process_trailer_options *opts) +{ + strbuf_add(out, info->trailer_start, + info->trailer_end - info->trailer_start); +} + +void format_trailers_from_commit(struct strbuf *out, const char *msg, + const struct process_trailer_options *opts) +{ + struct trailer_info info; + + trailer_info_get(&info, msg); + format_trailer_info(out, &info, opts); + trailer_info_release(&info); +} diff --git a/trailer.h b/trailer.h index 194f85a102..a172811022 100644 --- a/trailer.h +++ b/trailer.h @@ -40,4 +40,18 @@ void trailer_info_get(struct trailer_info *info, const char *str); void trailer_info_release(struct trailer_info *info); +/* + * Format the trailers from the commit msg "msg" into the strbuf "out". + * Note two caveats about "opts": + * + * - this is primarily a helper for pretty.c, and not + * all of the flags are supported. + * + * - this differs from process_trailers slightly in that we always format + * only the trailer block itself, even if the "only_trailers" option is not + * set. + */ +void format_trailers_from_commit(struct strbuf *out, const char *msg, + const struct process_trailer_options *opts); + #endif /* TRAILER_H */ From cc1735c4a3cf3377289640ce1af6b4c4523bee11 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 15 Aug 2017 06:24:39 -0400 Subject: [PATCH 7/9] t4205: refactor %(trailers) tests We currently have one test for %(trailers). In preparation for more, let's refactor a few bits: - move the commit creation to its own setup step so it can be reused by multiple tests - add a trailer with whitespace continuation (to confirm that it is left untouched) - fix the sample text which claims the placeholder is %bT. This was switched long ago to %(trailers) - replace one "cat" with an "echo" when generating the expected output. This saves a process (and sets a better pattern for future tests to follow). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4205-log-pretty-formats.sh | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 18aa1b5889..83ea85eb45 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -539,25 +539,29 @@ cat >trailers < Acked-by: A U Thor [ v2 updated patch description ] -Signed-off-by: A U Thor +Signed-off-by: A U Thor + EOF -test_expect_success 'pretty format %(trailers) shows trailers' ' +test_expect_success 'set up trailer tests' ' echo "Some contents" >trailerfile && git add trailerfile && - git commit -F - <<-EOF && + git commit -F - <<-EOF trailers: this commit message has trailers This commit is a test commit with trailers at the end. We parse this - message and display the trailers using %bT + message and display the trailers using %(trailers). $(cat trailers) EOF +' + +test_expect_success 'pretty format %(trailers) shows trailers' ' git log --no-walk --pretty="%(trailers)" >actual && - cat >expect <<-EOF && - $(cat trailers) - - EOF + { + cat trailers && + echo + } >expect && test_cmp expect actual ' From 58311c66fd316dff8f2c68a634ca0cf968227870 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 15 Aug 2017 06:25:27 -0400 Subject: [PATCH 8/9] pretty: support normalization options for %(trailers) The interpret-trailers command recently learned some options to make its output easier to parse (for a caller whose only interested in picking out the trailer values). But it's not very efficient for asking for the trailers of many commits in a single invocation. We already have "%(trailers)" to do that, but it doesn't know about unfolding or omitting non-trailers. Let's plumb those options through, so you can have the best of both. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/pretty-formats.txt | 5 ++++- pretty.c | 15 ++++++++++++--- t/t4205-log-pretty-formats.sh | 33 ++++++++++++++++++++++++++++++++ trailer.c | 32 +++++++++++++++++++++++++++++-- 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 4d6dac5770..efa67a716e 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -201,7 +201,10 @@ endif::git-rev-list[] - '%><()', '%><|()': similar to '% <()', '%<|()' respectively, but padding both sides (i.e. the text is centered) - %(trailers): display the trailers of the body as interpreted by - linkgit:git-interpret-trailers[1] + linkgit:git-interpret-trailers[1]. If the `:only` option is given, + omit non-trailer lines from the trailer block. If the `:unfold` + option is given, behave as if interpret-trailer's `--unfold` option + was given. E.g., `%(trailers:only:unfold)` to do both. NOTE: Some placeholders may depend on other options given to the revision traversal engine. For example, the `%g*` reflog options will diff --git a/pretty.c b/pretty.c index 33054e22cd..0e23fe3c05 100644 --- a/pretty.c +++ b/pretty.c @@ -1044,6 +1044,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ const struct commit *commit = c->commit; const char *msg = c->message; struct commit_list *p; + const char *arg; int ch; /* these are independent of the commit */ @@ -1262,10 +1263,18 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ return 1; } - if (starts_with(placeholder, "(trailers)")) { + if (skip_prefix(placeholder, "(trailers", &arg)) { struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT; - format_trailers_from_commit(sb, msg + c->subject_off, &opts); - return strlen("(trailers)"); + while (*arg == ':') { + if (skip_prefix(arg, ":only", &arg)) + opts.only_trailers = 1; + else if (skip_prefix(arg, ":unfold", &arg)) + opts.unfold = 1; + } + if (*arg == ')') { + format_trailers_from_commit(sb, msg + c->subject_off, &opts); + return arg - placeholder + 1; + } } return 0; /* unknown placeholder */ diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 83ea85eb45..ec5f530102 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -543,6 +543,10 @@ Signed-off-by: A U Thor EOF +unfold () { + perl -0pe 's/\n\s+/ /' +} + test_expect_success 'set up trailer tests' ' echo "Some contents" >trailerfile && git add trailerfile && @@ -565,4 +569,33 @@ test_expect_success 'pretty format %(trailers) shows trailers' ' test_cmp expect actual ' +test_expect_success '%(trailers:only) shows only "key: value" trailers' ' + git log --no-walk --pretty="%(trailers:only)" >actual && + { + grep -v patch.description expect && + test_cmp expect actual +' + +test_expect_success '%(trailers:unfold) unfolds trailers' ' + git log --no-walk --pretty="%(trailers:unfold)" >actual && + { + unfold expect && + test_cmp expect actual +' + +test_expect_success ':only and :unfold work together' ' + git log --no-walk --pretty="%(trailers:only:unfold)" >actual && + git log --no-walk --pretty="%(trailers:unfold:only)" >reverse && + test_cmp actual reverse && + { + grep -v patch.description expect && + test_cmp expect actual +' + test_done diff --git a/trailer.c b/trailer.c index 07580af9c0..6ec5505dc4 100644 --- a/trailer.c +++ b/trailer.c @@ -1095,8 +1095,36 @@ static void format_trailer_info(struct strbuf *out, const struct trailer_info *info, const struct process_trailer_options *opts) { - strbuf_add(out, info->trailer_start, - info->trailer_end - info->trailer_start); + int i; + + /* If we want the whole block untouched, we can take the fast path. */ + if (!opts->only_trailers && !opts->unfold) { + strbuf_add(out, info->trailer_start, + info->trailer_end - info->trailer_start); + return; + } + + for (i = 0; i < info->trailer_nr; i++) { + char *trailer = info->trailers[i]; + int separator_pos = find_separator(trailer, separators); + + if (separator_pos >= 1) { + struct strbuf tok = STRBUF_INIT; + struct strbuf val = STRBUF_INIT; + + parse_trailer(&tok, &val, NULL, trailer, separator_pos); + if (opts->unfold) + unfold_value(&val); + + strbuf_addf(out, "%s: %s\n", tok.buf, val.buf); + strbuf_release(&tok); + strbuf_release(&val); + + } else if (!opts->only_trailers) { + strbuf_addstr(out, trailer); + } + } + } void format_trailers_from_commit(struct strbuf *out, const char *msg, From 5a0d0c037cc187a6eee6329206b9f6a48746a054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Sun, 20 Aug 2017 05:40:09 -0400 Subject: [PATCH 9/9] doc/interpret-trailers: fix "the this" typo Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-interpret-trailers.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 1df8aabf51..2e22210734 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -21,7 +21,7 @@ This command reads some patches or commit messages from either the arguments or the standard input if no is specified. If `--parse` is specified, the output consists of the parsed trailers. -Otherwise, the this command applies the arguments passed using the +Otherwise, this command applies the arguments passed using the `--trailer` option, if any, to the commit message part of each input file. The result is emitted on the standard output.