"Hariom Verma via GitGitGadget" [off-list ref] 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 [off-list ref]\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 [off-list ref]
Signed-off-by: Junio C Hamano [off-list ref]
$ 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.
Hi,
On Sun, Feb 7, 2021 at 11:15 AM Junio C Hamano [off-list ref] wrote:
"Hariom Verma via GitGitGadget" [off-list ref] writes:
quoted
+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 [off-list ref]\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 [off-list ref]
Signed-off-by: Junio C Hamano [off-list ref]
$ 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?
I don't think that "extra blank line" is due to this patch series.
Wait. Let me see.
Since "for-each-ref"'s original code does not support
"trailers:key=<KEY>", Let's check original code for "trailers:only":
$ git for-each-ref refs/heads/master --format="%(trailers:only)"
Signed-off-by: Junio C Hamano <gitster@pobox.com>
$ exit
I see. The original code also gives an extra blank line.
Now, let's check for this patch series:
$ ./bin-wrappers/git for-each-ref refs/heads/master
--format="%(trailers:key=Signed-off-by)"
Signed-off-by: Junio C Hamano <gitster@pobox.com>
$ ./bin-wrappers/git for-each-ref refs/heads/master
--format="%(trailers:key=None)"
$ exit
when there is no matching trailer lines, the "for-each-ref" in this
series shows one empty line, and when there is one matching trailer
line, it gives that single line plus an empty line, two lines in
total. Seems consistent to me.
So this isn't about the patch series. Question still remains the same.
Why extra blank line?
Let's dig a bit.
Ah. I guess I found the reason. It's due to `putchar('\n');` in
`show_ref_array_item() [1]`. It puts a new line after each ref item.
Do you want me to include a patch to get rid of this "extra blank
line" for trailers in "for-each-ref"?
Thanks,
Hariom.
[1]: https://github.com/git/git/blob/fb7fa4a1fd273f22efcafdd13c7f897814fd1eb9/ref-filter.c#L2435