Re: [PATCH v3 3/3] ref-filter: use pretty.c logic for trailers

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 7, 2021, 05:45 UTC
Message-ID: <xmqqpn1c8m7u.fsf@gitster.c.googlers.com>
In-Reply-To: <47d89f872314cad6dc6010ff3c8ade43a70bc540.1612602945.git.gitgitgadget@gmail.com>

"Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 15 quoted lines

> +test_trailer_option() {
> +	title="$1"
> +	option="$2"
> +	expect="$3"
> +	test_expect_success "$title" '
> +		printf "$expect\n" >expect &&
> +		git for-each-ref --format="%($option)" refs/heads/main >actual &&
> +		test_cmp expect actual &&
> +		git for-each-ref --format="%(contents:$option)" refs/heads/main >actual &&
> +		test_cmp expect actual
> +	'
> +}
> +
> +test_trailer_option '%(trailers:key=foo) shows that trailer' \
> +	'trailers:key=Signed-off-by' 'Signed-off-by: A U Thor <author@example.com>\n'

This is *not* an issue about the test script and its helper function, but I just noticed that --format="%(trailers:key=<key>)" is expected to write the matching trailers *AND* an empty line, and I wonder if that is a sensible thing to expect.

The "--pretty" side does not give such an extra blank line after the output, though.

 $ git show -s --pretty=format:"%(trailers:key=Signed-off-by:)" \
   js/range-diff-wo-dotdot
 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
 Signed-off-by: Junio C Hamano <gitster@pobox.com>
 $ git show -s --pretty=format:"%(trailers:key=None:)" \
   js/range-diff-wo-dotdot
 $ exit

Unlike the above, when there is no matching trailer lines, the "for-each-ref" in this series shows zero lines, and when there is one matching trailer line, it gives that single line plus an empty line, two lines in total. The inconsistency is a bit disturbing.

Is the extra blank line given on purpose? I don't see why we would want it. Or is it a bug we did not catch during the previous two rounds of reviews?

Thanks.