Re: [PATCH v7 03/17] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)

From: Jakub Narębski <jnareb@gmail.com>
Date: Nov 18, 2016, 19:58 UTC
Message-ID: <37c2cbf2-7160-49d7-f8f1-3b65d9ecf9ec@gmail.com>
In-Reply-To: <20161108201211.25213-4-Karthik.188@gmail.com>

W dniu 08.11.2016 o 21:11, Karthik Nayak pisze:

Show 9 quoted lines

> From: Karthik Nayak <karthik.188@gmail.com>
> 
> Implement %(if:equals=<string>) wherein the if condition is only
> satisfied if the value obtained between the %(if:...) and %(then) atom
> is the same as the given '<string>'.
> 
> Similarly, implement (if:notequals=<string>) wherein the if condition
> is only satisfied if the value obtained between the %(if:...) and
> %(then) atom is differnt from the given '<string>'.

                  ^^^^^^^^

s/differnt/different/   <-- typo

Show 5 quoted lines

> 
> This is done by introducing 'if_atom_parser()' which parses the given
> %(if) atom and then stores the data in used_atom which is later passed
> on to the used_atom of the %(then) atom, so that it can do the required
> comparisons.

Nb. the syntax reminds me a bit of RPM SPEC language.

Show 20 quoted lines

> 
> Add tests and Documentation for the same.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  Documentation/git-for-each-ref.txt |  3 +++
>  ref-filter.c                       | 43 +++++++++++++++++++++++++++++++++-----
>  t/t6302-for-each-ref-filter.sh     | 18 ++++++++++++++++
>  3 files changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index fed8126..b7b8560 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -155,6 +155,9 @@ if::
>  	evaluating the string before %(then), this is useful when we
>  	use the %(HEAD) atom which prints either "*" or " " and we
>  	want to apply the 'if' condition only on the 'HEAD' ref.

So %(if) is actually %(if:notempty) ?  Just kidding.

Show 16 quoted lines

> +	Append ":equals=<string>" or ":notequals=<string>" to compare
> +	the value between the %(if:...) and %(then) atoms with the
> +	given string.
>  
>  In addition to the above, for commit and tag objects, the header
>  field names (`tree`, `parent`, `object`, `type`, and `tag`) can
> diff --git a/ref-filter.c b/ref-filter.c
> index 8392303..44481c3 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -22,6 +22,8 @@ struct align {
>  };
>  
>  struct if_then_else {
> +	const char *if_equals,
> +		*not_equals;

I guess using anonymous structs from C11 here...

Show 11 quoted lines

>  	unsigned int then_atom_seen : 1,
>  		else_atom_seen : 1,
>  		condition_satisfied : 1;
> @@ -49,6 +51,10 @@ static struct used_atom {
>  			enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
>  			unsigned int nlines;
>  		} contents;
> +		struct {
> +			const char *if_equals,
> +				*not_equals;
> +		} if_then_else;

...to avoid code duplication there is rather out of question?

Show 15 quoted lines

>  		enum { O_FULL, O_SHORT } objectname;
>  	} u;
>  } *used_atom;
> @@ -169,6 +175,19 @@ static void align_atom_parser(struct used_atom *atom, const char *arg)
>  	string_list_clear(&params, 0);
>  }
>  
> +static void if_atom_parser(struct used_atom *atom, const char *arg)
> +{
> +	if (!arg)
> +		return;
> +	else if (skip_prefix(arg, "equals=", &atom->u.if_then_else.if_equals))
> +		 ;
> +	else if (skip_prefix(arg, "notequals=", &atom->u.if_then_else.not_equals))
> +		;

Those ';' should be perfectly aligned, isn't it?
 
[...]

Show 17 quoted lines

> +test_expect_success 'check %(if:equals=<string>)' '
> +	git for-each-ref --format="%(if:equals=master)%(refname:short)%(then)Found master%(else)Not master%(end)" refs/heads/ >actual &&
> +	cat >expect <<-\EOF &&
> +	Found master
> +	Not master
> +	EOF
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'check %(if:notequals=<string>)' '
> +	git for-each-ref --format="%(if:notequals=master)%(refname:short)%(then)Not master%(else)Found master%(end)" refs/heads/ >actual &&
> +	cat >expect <<-\EOF &&
> +	Found master
> +	Not master
> +	EOF
> +	test_cmp expect actual
> +'

Nice!

-- 
Jakub Narębski