Re: [PATCH 06/15] ref-filter: move need_color_reset_at_eol into ref_format

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

Jeff King <peff@peff.net> writes:

> Calling verify_ref_format() doesn't just confirm that the
> format is sane; it actually sets some global variables that
> will be used later when formatting the refs. These logically
> should belong to the ref_format, which would make it
> possible to use multiple formats within a single program
> invocation.
>
> Let's move one such flag into the ref_format struct. There
> are still others that would need to be moved before it would
> be safe to use multiple formats, but this commit gives a
> blueprint for how that should look.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This commit is strictly optional for this series, but I wanted to give a
> sense of how the rest of the movement might look, since I was thinking
> about it. The big thing to move would be the used_atoms array, but I
> punted on that for now.

Heh, when I saw the hunk at 661,+7, used_atom[] was what immediately
came to mind.  It is OK to punt for the purpose of this patch, but
moving these statics into the ref-format structure would be a good
move in the longer term.

Thanks.

>
>  ref-filter.c | 7 +++----
>  ref-filter.h | 3 +++
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 66d234bb1..178396e1f 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -97,7 +97,6 @@ static struct used_atom {
>  	} u;
>  } *used_atom;
>  static int used_atom_cnt, need_tagged, need_symref;
> -static int need_color_reset_at_eol;
>  
>  static void color_atom_parser(struct used_atom *atom, const char *color_value)
>  {
> @@ -661,7 +660,7 @@ int verify_ref_format(struct ref_format *format)
>  {
>  	const char *cp, *sp;
>  
> -	need_color_reset_at_eol = 0;
> +	format->need_color_reset_at_eol = 0;
>  	for (cp = format->format; *cp && (sp = find_next(cp)); ) {
>  		const char *color, *ep = strchr(sp, ')');
>  		int at;
> @@ -673,7 +672,7 @@ int verify_ref_format(struct ref_format *format)
>  		cp = ep + 1;
>  
>  		if (skip_prefix(used_atom[at].name, "color:", &color))
> -			need_color_reset_at_eol = !!strcmp(color, "reset");
> +			format->need_color_reset_at_eol = !!strcmp(color, "reset");
>  	}
>  	return 0;
>  }
> @@ -2083,7 +2082,7 @@ void format_ref_array_item(struct ref_array_item *info,
>  		sp = cp + strlen(cp);
>  		append_literal(cp, sp, &state);
>  	}
> -	if (need_color_reset_at_eol) {
> +	if (format->need_color_reset_at_eol) {
>  		struct atom_value resetv;
>  		resetv.s = GIT_COLOR_RESET;
>  		append_atom(&resetv, &state);
> diff --git a/ref-filter.h b/ref-filter.h
> index 2bb58879d..9e1e89c19 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -79,6 +79,9 @@ struct ref_format {
>  	 */
>  	const char *format;
>  	int quote_style;
> +
> +	/* Internal state to ref-filter */
> +	int need_color_reset_at_eol;
>  };
>  
>  #define REF_FORMAT_INIT { NULL, 0 }