[PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid)

From: Will Palmer <wmpalmer@gmail.com>
Date: Mar 28, 2011, 23:17 UTC
Message-ID: <1301354251-23380-4-git-send-email-wmpalmer@gmail.com>
In-Reply-To: <1301354251-23380-1-git-send-email-wmpalmer@gmail.com>

%C(...) had the distinction of being the only format placeholder which could trigger a die(), a side-effect of its ancestry in calling the existing color_parse_mem(...). This was good, because it gave users an explanation of what went wrong. It was also inconsistent, since every other "unknown placeholder" was interpreted as a literal.

This removes the inconsistency by interpreting %C(invalid) as a literal. Perhaps this is a step in the wrong direction.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 color.c  |   17 +++++++++++------
 color.h  |    1 +
 pretty.c |    7 +++----
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/color.c b/color.c
index 6a5a54e..9bc190b 100644
--- a/color.c
+++ b/color.c
@@ -45,6 +45,13 @@ void color_parse(const char *value, const char *var, char *dst)
 void color_parse_mem(const char *value, int value_len, const char *var,
 		char *dst)
 {
+	if (color_parse_len(value, value_len, dst))
+		return;
+	die("bad color value '%.*s' for variable '%s'", value_len, value, var);
+}
+
+int color_parse_len(const char *value, int value_len, char *dst)
+{
 	const char *ptr = value;
 	int len = value_len;
 	unsigned int attr = 0;
@@ -53,7 +60,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 
 	if (!strncasecmp(value, "reset", len)) {
 		strcpy(dst, GIT_COLOR_RESET);
-		return;
+		return 1;
 	}
 
 	/* [fg [bg]] [attr]... */
@@ -82,13 +89,13 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 				bg = val;
 				continue;
 			}
-			goto bad;
+			return 0;
 		}
 		val = parse_attr(word, wordlen);
 		if (0 <= val)
 			attr |= (1 << val);
 		else
-			goto bad;
+			return 0;
 	}
 
 	if (attr || fg >= 0 || bg >= 0) {
@@ -130,9 +137,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 		*dst++ = 'm';
 	}
 	*dst = 0;
-	return;
-bad:
-	die("bad color value '%.*s' for variable '%s'", value_len, value, var);
+	return 1;
 }
 
 int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
diff --git a/color.h b/color.h
index 170ff40..084d85d 100644
--- a/color.h
+++ b/color.h
@@ -59,6 +59,7 @@ int git_color_default_config(const char *var, const char *value, void *cb);
 
 int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
 void color_parse(const char *value, const char *var, char *dst);
+int color_parse_len(const char *value, int len, char *dst);
 void color_parse_mem(const char *value, int len, const char *var, char *dst);
 __attribute__((format (printf, 3, 4)))
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
diff --git a/pretty.c b/pretty.c
index 8549934..d5a724f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -752,11 +752,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		if (placeholder[1] == '(') {
 			const char *end = strchr(placeholder + 2, ')');
 			char color[COLOR_MAXLEN];
-			if (!end)
+			if (!end || !color_parse_len(placeholder + 2,
+						     end - (placeholder + 2),
+						     color))
 				return 0;
-			color_parse_mem(placeholder + 2,
-					end - (placeholder + 2),
-					"--pretty format", color);
 			strbuf_addstr(sb, color);
 			return end - placeholder + 1;
 		}

-- 
1.7.4.2