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

On Thu, Apr 26, 2018 at 02:32:54PM +0900, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

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