Re: [PATCH v3 2/5] interpret-trailers: add an option to show only the trailers

[Jeff King]

On Thu, Aug 10, 2017 at 11:28:52AM -0700, Stefan Beller wrote:

> > +test_expect_success 'only trailers' '
> > +   git config trailer.sign.command "echo config-value" &&
> 
> You may want to use 'test_config' here, which keeps the config
> only for one test. The subsequent tests seem to overwrite the
> config, so this is not wrong, just not the best style.

Yeah, I actually considered that but decided to keep style with the rest
of the script. I agree the whole thing could possibly switch to
test_config, but I suspect there may be some fallouts (the style of the
rest of the script seems to assume some continuity between the tests).

-Peff

Re: [PATCH v3 2/5] interpret-trailers: add an option to show only the trailers

[Stefan Beller]

On Thu, Aug 10, 2017 at 11:04 AM, Jeff King  wrote:
> 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 

Sorry for a sloppy review last round, upon reviewing
I found another nit.

>
> +test_expect_success 'only trailers' '
> +   git config trailer.sign.command "echo config-value" &&

You may want to use 'test_config' here, which keeps the config
only for one test. The subsequent tests seem to overwrite the
config, so this is not wrong, just not the best style.

[PATCH v3 2/5] interpret-trailers: add an option to show only the trailers

[Jeff King]

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 
---
 Documentation/git-interpret-trailers.txt |  3 +++
 builtin/interpret-trailers.c |  1 +
 t/t7513-interpret-trailers.sh| 39 
 trailer.c| 19 ++--
 trailer.h|  1 +
 5 files changed, 56 insertions(+), 7 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", _place, N_("edit files in 
place")),
OPT_BOOL(0, "trim-empty", _empty, N_("trim empty 
trailers")),
+   OPT_BOOL(0, "only-trailers", _trailers, N_("output 
only the trailers")),
OPT_STRING_LIST(0, "trailer", , 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 e21a0d1629..706351fc98 100644
--- a/trailer.c
+++ b/trailer.c
@@ -164,13 +164,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);
}
 }
@@ -897,9 +899,10 @@ static int process_input_file(FILE *outfile,
trailer_info_get(, str);
 
/* Print lines before the trailers as is */
-   fwrite(str, 1, info.trailer_start - str, outfile);
+   if (outfile)
+   fwrite(str, 1, info.trailer_start - str, outfile);
 
-   if (!info.blank_line_before_trailer)
+   if (outfile && !info.blank_line_before_trailer)
fprintf(outfile, "\n");
 
for (i = 0;