Subject: Re: [PATCH 5/5] builtin/config: introduce `color` type specifier

From: Taylor Blau <[email-masked]>
Date: Wed, 25 Apr 2018 23:01:15 -0700

On Thu, Apr 26, 2018 at 02:32:54PM +0900, Junio C Hamano wrote:
> Taylor Blau <[email-masked]> writes:
>
> > diff --git a/builtin/config.c b/builtin/config.c
> > index d7acf912cd..ec5c11293b 100644
> > --- a/builtin/config.c
> > +++ b/builtin/config.c
> > @@ -61,6 +61,7 @@ static int show_origin;
> >  #define TYPE_BOOL_OR_INT	3
> >  #define TYPE_PATH		4
> >  #define TYPE_EXPIRY_DATE	5
> > +#define TYPE_COLOR		6
> >
> >  #define OPT_CALLBACK_VALUE(s, l, v, h, i) \
> >  	{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
> > @@ -231,6 +232,11 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
> >  			if (git_config_expiry_date(&t, key_, value_) < 0)
> >  				return -1;
> >  			strbuf_addf(buf, "%"PRItime, t);
> > +		} else if (type == TYPE_COLOR) {
> > +			char v[COLOR_MAXLEN];
> > +			if (git_config_color(v, key_, value_) < 0)
> > +				return -1;
> > +			strbuf_addstr(buf, v);
> >  		} else if (value_) {
> >  			strbuf_addstr(buf, value_);
> >  		} else {
> > @@ -376,6 +382,20 @@ static char *normalize_value(const char *key, const char *value)
> >  		else
> >  			return xstrdup(v ? "true" : "false");
> >  	}
> > +	if (type == TYPE_COLOR) {
> > +		char v[COLOR_MAXLEN];
> > +		if (git_config_color(v, key, value))
> > +			die("cannot parse color '%s'", value);
> > +
> > +		/*
> > +		 * The contents of `v` now contain an ANSI escape
> > +		 * sequence, not suitable for including within a
> > +		 * configuration file. Treat the above as a
> > +		 * "sanity-check", and return the given value, which we
> > +		 * know is representable as valid color code.
> > +		 */
> > +		return xstrdup(value);
> > +	}
> >
> >  	die("BUG: cannot normalize type %d", type);
> >  }
>
> Hmmm, option_parse_type() introduced in [PATCH 2/5] used to learn
> to parse "color" in this step, but it no longer does.

Oof, again. I dropped this hunk on the floor when integrating. I put it
back in v2.

Thanks,
Taylor