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

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

Taylor Blau <me@ttaylorr.com> 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.  

Here is the difference between what has been queued and the result
of applying these latest patches I found.

diff --git a/builtin/config.c b/builtin/config.c
index 69e7270356..ec5c11293b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -72,7 +72,8 @@ static struct option builtin_config_options[];
 static int option_parse_type(const struct option *opt, const char *arg,
 			     int unset)
 {
-	int new_type, *to_type;
+	int new_type;
+	int *to_type;
 
 	if (unset) {
 		*((int *) opt->value) = 0;
@@ -95,13 +96,11 @@ static int option_parse_type(const struct option *opt, const char *arg,
 			new_type = TYPE_PATH;
 		else if (!strcmp(arg, "expiry-date"))
 			new_type = TYPE_EXPIRY_DATE;
-		else if (!strcmp(arg, "color"))
-			new_type = TYPE_COLOR;
 		else
 			die(_("unrecognized --type argument, %s"), arg);
 	}
 
-	to_type = opt->value;
+	*to_type = opt->value;
 	if (*to_type && *to_type != new_type) {
 		/*
 		 * Complain when there is a new type not equal to the old type.