From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 07 Feb 2021 05:45:41 GMT
Subject: Re: [PATCH v3 3/3] ref-filter: use pretty.c logic for trailers
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:

> +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.