Re: [PATCH 3/5] interpret-trailers: add an option to show only existing trailers

[Jonathan Tan <jonathantanmy@google.com>]

On Wed, 9 Aug 2017 08:24:40 -0400
Jeff King <peff@peff.net> wrote:

> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
> index e5b0718ef6..525fd53e5b 100755
> --- a/t/t7513-interpret-trailers.sh
> +++ b/t/t7513-interpret-trailers.sh
> @@ -1312,4 +1312,19 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'only existing' '
> +	cat >expected <<-\EOF &&
> +		existing: existing-value
> +	EOF
> +	git interpret-trailers \
> +		--only-trailers --only-existing >actual <<-\EOF &&
> +		my subject
> +
> +		my body
> +
> +		existing: existing-value
> +	EOF
> +	test_cmp expected actual

Would it be worth asserting that the "sign" trailer is configured here
and would normally appear? Maybe through a grep on the output of "git
config".

> +'
> +
>  test_done
> diff --git a/trailer.c b/trailer.c
> index a4ff99f98a..88f6efe523 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -991,9 +991,10 @@ void process_trailers(const char *file, struct process_trailer_options *opts,
>  	trailer_end = process_input_file(opts->only_trailers ? NULL : outfile,
>  					 sb.buf, &head);
>  
> -	process_command_line_args(&arg_head, trailers);
> -
> -	process_trailers_lists(&head, &arg_head);
> +	if (!opts->only_existing) {
> +		process_command_line_args(&arg_head, trailers);
> +		process_trailers_lists(&head, &arg_head);
> +	}

This is a bit confusing, but it is correct, since
process_command_line_args() processes both configured trailers and
command-line trailers.

Having said that, it might be worth declaring LIST_HEAD(arg_head) inside
the if block now.