Re: [PATCH v4 02/10] ref-filter: make the 'color' use ref_formatting_state

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

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

> Make color which was considered as an atom, to use
> ref_formatting_state and act as a pseudo atom. This allows
> interchangeability between 'align' and 'color'.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> 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>
> ---

I think 1/10 and 2/10 are the other way around.  Preferably, these
would be three patches, in this order:

 (1) prepare the "formatting state" and pass it around; no other
     code change.

 (2) have "color" use that facility.

 (3) add a new feature using that facility.

The first two may want to be combined into a single patch, if it
does not make the patch too noisy.

>  ref-filter.c | 16 ++++++++++++----
>  ref-filter.h |  1 +
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 3c90ffc..fd13a23 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -663,7 +663,8 @@ static void populate_value(struct ref_formatting_state *state,
>  
>  			if (color_parse(name + 6, color) < 0)
>  				die(_("unable to parse format"));
> -			v->s = xstrdup(color);
> +			state->color = xstrdup(color);
> +			v->pseudo_atom = 1;
>  			continue;
>  		} else if (!strcmp(name, "flag")) {
>  			char buf[256], *cp = buf;
> @@ -1217,6 +1218,11 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
>  static void ref_formatting(struct ref_formatting_state *state, struct atom_value *v,
>  			   struct strbuf *value)
>  {
> +	if (state->color) {
> +		strbuf_addstr(value, state->color);
> +		free((void *)state->color);
> +		state->color = NULL;
> +	}
>  	if (state->pad_to_right) {
>  		if (!is_utf8(v->s))
>  			strbuf_addf(value, "%-*s", state->pad_to_right, v->s);
> @@ -1224,8 +1230,9 @@ static void ref_formatting(struct ref_formatting_state *state, struct atom_value
>  			int len = strlen(v->s) - utf8_strwidth(v->s);
>  			strbuf_addf(value, "%-*s", state->pad_to_right + len, v->s);
>  		}
> -	} else
> -		strbuf_addf(value, "%s", v->s);
> +		return;
> +	}
> +	strbuf_addf(value, "%s", v->s);
>  }
>  
>  static void print_value(struct ref_formatting_state *state, struct atom_value *v)
> @@ -1326,7 +1333,8 @@ 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;
> +		resetv.s = "";
> +		state.color = xstrdup(color);
>  		print_value(&state, &resetv);
>  	}
>  	putchar('\n');
> diff --git a/ref-filter.h b/ref-filter.h
> index ea2d0e6..bacbb23 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -31,6 +31,7 @@ struct ref_sorting {
>  struct ref_formatting_state {
>  	unsigned int pad_to_right; /*pad atoms to the right*/
>  	int quote_style;
> +	const char *color;
>  };
>  
>  struct ref_array_item {