Re: [PATCH v8 02/11] ref-filter: introduce ref_formatting_state

[Junio C Hamano <gitster@pobox.com>]

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

> Introduce a ref_formatting_state which will eventually hold the values
> of modifier atoms. Implement this within ref-filter.
>
> 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>
> ---
>  ref-filter.c | 49 +++++++++++++++++++++++++++++++++++++------------
>  ref-filter.h |  4 ++++
>  2 files changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index febdc45..c4c7064 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1190,9 +1190,10 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
>  	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
>  }
>  
> -static void print_value(struct atom_value *v, int quote_style, struct strbuf *output)
> +static void print_value(struct atom_value *v, struct ref_formatting_state *state,
> +			struct strbuf *output)
>  {

I expect that the state would eventually become a stack of states
(i.e. the topmost one passed around, each having a pointer to the
previous level) in order to implement that "divert" mechanism for
(possibly nested) if ... end construct.

With that in mind, I suspect that state->output should be "where the
current level would output to", i.e. no need to pass state and
output around separately.

> +static void apply_formatting_state(struct ref_formatting_state *state, struct strbuf *value,
> +				   struct strbuf *format)
> +{

The name "format" feels quite misleading; the readers would expect
that you would use it in "strbuf_addf(format, value)", but that is
not what is going on here.

> @@ -1275,12 +1299,13 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
>  		if (color_parse("reset", color) < 0)
>  			die("BUG: couldn't parse 'reset' as a color");
>  		resetv.s = color;
> -		print_value(&resetv, quote_style, &output);
> +		print_value(&resetv, &state, &value);
> +		apply_formatting_state(&state, &value, &final_buf);
>  	}
> -	for (i = 0; i < output.len; i++)
> -		printf("%c", output.buf[i]);
> +	for (i = 0; i < final_buf.len; i++)
> +		printf("%c", final_buf.buf[i]);
>  	putchar('\n');
> -	strbuf_release(&output);
> +	strbuf_release(&final_buf);
>  }
>  
>  /*  If no sorting option is given, use refname to sort as default */
> diff --git a/ref-filter.h b/ref-filter.h
> index 6bf27d8..b64677f 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -16,6 +16,10 @@
>  #define FILTER_REFS_INCLUDE_BROKEN 0x1
>  #define FILTER_REFS_ALL 0x2
>  
> +struct ref_formatting_state {
> +	int quote_style;
> +};
> +
>  struct atom_value {
>  	const char *s;
>  	unsigned long ul; /* used for sorting when not FIELD_STR */