Re: [PATCH v6 7/7] ref-filter.c: parse trailers arguments with %(contents) atom

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Oct 01, 2017 at 10:25:24PM -0700, Taylor Blau wrote:

> 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`). This is because string_list_split (given
> a maxsplit of -1) returns a 1-ary string_list* containing the given
> string if the delimiter could not be found using `strchr`.
> 
> 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.
> 
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
>  ref-filter.c            |  7 ++++---
>  t/t6300-for-each-ref.sh | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 3 deletions(-)

This patch seems to fail a bunch of tests in t6300.

> diff --git a/ref-filter.c b/ref-filter.c
> index 43ed10a5e..6c26b4733 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -212,9 +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, "lines=", &arg)) {
> +	else if (skip_prefix(arg, "trailers", &arg)) {
> +		skip_prefix(arg, ":", &arg);
> +		trailers_atom_parser(format, atom, *arg ? NULL : arg);

I think your logic is flipped. You want "*arg ? arg : NULL";

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux