]> git.ipfire.org Git - thirdparty/git.git/commit - t/t4205-log-pretty-formats.sh
pretty-format %(trailers): fix broken standalone "valueonly"
authorÆvar Arnfjörð Bjarmason <avarab@gmail.com>
Wed, 9 Dec 2020 15:52:06 +0000 (16:52 +0100)
committerJunio C Hamano <gitster@pobox.com>
Wed, 9 Dec 2020 22:16:42 +0000 (14:16 -0800)
commit8b966a0506f8b5aef7c6038fe518db45bc4a1852
tree186c3621f1f7f2554c14c101a2a97bfa06298cab
parent2762e17117fdd7ab73eb3f2970620a6f91500d01
pretty-format %(trailers): fix broken standalone "valueonly"

Fix %(trailers:valueonly) being a noop due to on overly eager
optimization in format_trailer_info() which skips custom formatting if
no custom options are given.

When "valueonly" was added in d9b936db522 (pretty: add support for
"valueonly" option in %(trailers), 2019-01-28) we forgot to add it to
the list of options that optimization checks for. See e.g. the
addition of "key" in 250bea0c165 (pretty: allow showing specific
trailers, 2019-01-28) for a similar change where this wasn't missed.

Thus the "valueonly" option in "%(trailers:valueonly)" was a noop and
the output was equivalent to that of a plain "%(trailers)". This
wasn't caught because the tests for it always combined it with other
options.

Fix the bug by adding !opts->value_only to the list. I initially
attempted to make this more future-proof by setting a flag if we got
to ":" in "%(trailers:" in format_commit_one() in pretty.c. However,
"%(trailers:" is also parsed in trailers_atom_parser() in
ref-filter.c.

There is an outstanding patch[1] unify those two, and such a fix, or
other future-proofing, such as changing "process_trailer_options"
flags into a bitfield, would conflict with that effort. Let's instead
do the bare minimum here as this aspect of trailers is being actively
worked on by another series.

Let's also test for a plain "valueonly" without any other options, as
well as "separator". All the other existing options on the pretty.c
path had tests where they were the only option provided. I'm also
keeping a sanity test for "%(trailers:)" being the same as
"%(trailers)". There's no reason to suspect it wouldn't be in the
current implementation, but let's keep it in the interest of black box
testing.

1. https://lore.kernel.org/git/pull.726.git.1599335291.gitgitgadget@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/t4205-log-pretty-formats.sh
trailer.c