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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 7, 2016, 22:49 UTC
Message-ID: <xmqq7fhdoqd3.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1457265902-7949-3-git-send-email-Karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 28 quoted lines

> diff --git a/ref-filter.c b/ref-filter.c
> index 41e73f0..440e270 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;
>  	unsigned int then_atom_seen : 1,
>  		else_atom_seen : 1,
>  		condition_satisfied : 1;
> @@ -411,6 +413,14 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat
>  {
>  	struct ref_formatting_stack *new;
>  	struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1);
> +	const char *valp;
> +
> +	if (skip_prefix(atomv->s, "equals=", &valp))
> +		if_then_else->if_equals = valp;
> +	else if (skip_prefix(atomv->s, "notequals=", &valp))
> +		if_then_else->not_equals = valp;
> +	else if (atomv->s[0])
> +		die(_("format: unknown format if:%s"), atomv->s);
>  
>  	push_stack_element(&state->stack);
>  	new = state->stack;

The fact that you are pushing stack element here tells me that this "handler" is run once for each 'ref' that we prepare output for (i.e. unlike the helper functions called "parser" that are called only once while preparing used_atom[] array).

It somehow feels that this goes against the overall design you did in the earlier topic to pre-parse as much as possible when you prepare the used_atom array and avoid parsing at the runtime. Am I misreading the patch?