Taylor Blau <me@xxxxxxxxxxxx> writes:
> The %(contents) atom takes a contents "field" as its argument. Since
> "trailers" is one of those fields, extend contents_atom_parser to parse
> "trailers"'s arguments when used through "%(contents)", like:
>
> %(contents:trailers:unfold,only)
>
> A caveat: trailers_atom_parser expects NULL when no arguments are given
> (see: `parse_ref_filter_atom`). To simulate this behavior without
> teaching trailers_atom_parser to accept strings with length zero,
> conditionally pass NULL to trailers_atom_parser if the arguments portion
> of the argument to %(contents) is empty.
I got an impression during the earlier rounds' review discussion
that trailers_atom_parser() will happily accept an empty string and
work correctly, so this paragraph and the conditional *arg ? NULL :
arg were both unneeded.
- arg == "" is not NULL, so we first do string_list_split() on ','
- which yields an empty list i.e. params.nr == 0
- we do not loop and leave atom->u.contents.trailer_opts untouched.
- and we set u.contents.option to C_TRAILERS
- and we clear ¶ms string list before we leave.
which is exactly the same as what happens when arg == NULL. Sooo....
>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> ref-filter.c | 6 ++++--
> t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 43ed10a5e..2c03c2bf5 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -212,8 +212,10 @@ static void contents_atom_parser(const struct ref_format *format, struct used_at
> atom->u.contents.option = C_SIG;
> else if (!strcmp(arg, "subject"))
> atom->u.contents.option = C_SUB;
> - else if (!strcmp(arg, "trailers"))
> - atom->u.contents.option = C_TRAILERS;
> + else if (skip_prefix(arg, "trailers", &arg)) {
> + skip_prefix(arg, ":", &arg);
> + trailers_atom_parser(atom, *arg ? NULL : arg);
> + }
> else if (skip_prefix(arg, "lines=", &arg)) {
Merge these two lines i.e.
} else if (skip_prefix(arg, "lines=", ...) {
> +test_expect_success '%(contents:trailers) rejects unknown trailers arguments' '
> + cat >expect <<-EOF &&
> + fatal: unknown %(trailers) argument: unsupported
> + EOF
> + test_must_fail git for-each-ref --format="%(contents:trailers:unsupported)" 2>actual &&
> + test_cmp expect actual
Funny indentation.
