color_parse: do not mention variable name in error message

Originally the color-parsing function was used only for
config variables. It made sense to pass the variable name so
that the die() message could be something like:

  $ git -c color.branch.plain=bogus branch
  fatal: bad color value 'bogus' for variable 'color.branch.plain'

These days we call it in other contexts, and the resulting
error messages are a little confusing:

  $ git log --pretty='%C(bogus)'
  fatal: bad color value 'bogus' for variable '--pretty format'

  $ git config --get-color foo.bar bogus
  fatal: bad color value 'bogus' for variable 'command line'

This patch teaches color_parse to complain only about the
value, and then return an error code. Config callers can
then propagate that up to the config parser, which mentions
the variable name. Other callers can provide a custom
message. After this patch these three cases now look like:

  $ git -c color.branch.plain=bogus branch
  error: invalid color value: bogus
  fatal: unable to parse 'color.branch.plain' from command-line config

  $ git log --pretty='%C(bogus)'
  error: invalid color value: bogus
  fatal: unable to parse --pretty format

  $ git config --get-color foo.bar bogus
  error: invalid color value: bogus
  fatal: unable to parse default color value

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: peff

builtin/branch.c

@@ -93,8 +93,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)
 			return 0;
 		if (!value)
 			return config_error_nonbool(var);
-		color_parse(value, var, branch_colors[slot]);
-		return 0;
+		return color_parse(value, branch_colors[slot]);
 	}
 	return git_color_default_config(var, value, cb);
 }

builtin/clean.c

@@ -116,8 +116,7 @@ static int git_clean_config(const char *var, const char *value, void *cb)
 			return 0;
 		if (!value)
 			return config_error_nonbool(var);
-		color_parse(value, var, clean_colors[slot]);
-		return 0;
+		return color_parse(value, clean_colors[slot]);
 	}
 
 	if (!strcmp(var, "clean.requireforce")) {

builtin/commit.c

@@ -1295,8 +1295,7 @@ static int git_status_config(const char *k, const char *v, void *cb)
 			return 0;
 		if (!v)
 			return config_error_nonbool(k);
-		color_parse(v, k, s->color_palette[slot]);
-		return 0;
+		return color_parse(v, s->color_palette[slot]);
 	}
 	if (!strcmp(k, "status.relativepaths")) {
 		s->relative_paths = git_config_bool(k, v);

builtin/config.c

@@ -296,7 +296,8 @@ static int git_get_color_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, get_color_slot)) {
 		if (!value)
 			config_error_nonbool(var);
-		color_parse(value, var, parsed_color);
+		if (color_parse(value, parsed_color) < 0)
+			return -1;
 		get_color_found = 1;
 	}
 	return 0;
@@ -309,8 +310,10 @@ static void get_color(const char *def_color)
 	git_config_with_options(git_get_color_config, NULL,
 				&given_config_source, respect_includes);
 
-	if (!get_color_found && def_color)
-		color_parse(def_color, "command line", parsed_color);
+	if (!get_color_found && def_color) {
+		if (color_parse(def_color, parsed_color) < 0)
+			die(_("unable to parse default color value"));
+	}
 
 	fputs(parsed_color, stdout);
 }

builtin/for-each-ref.c

@@ -673,7 +673,8 @@ static void populate_value(struct refinfo *ref)
 		} else if (starts_with(name, "color:")) {
 			char color[COLOR_MAXLEN] = "";
 
-			color_parse(name + 6, "--format", color);
+			if (color_parse(name + 6, color) < 0)
+				die(_("unable to parse format"));
 			v->s = xstrdup(color);
 			continue;
 		} else if (!strcmp(name, "flag")) {
@@ -1007,7 +1008,8 @@ static void show_ref(struct refinfo *info, const char *format, int quote_style)
 		struct atom_value resetv;
 		char color[COLOR_MAXLEN] = "";
 
-		color_parse("reset", "--format", color);
+		if (color_parse("reset", color) < 0)
+			die("BUG: couldn't parse 'reset' as a color");
 		resetv.s = color;
 		print_value(&resetv, quote_style);
 	}

color.c

@@ -60,13 +60,12 @@ static int parse_attr(const char *name, int len)
 	return -1;
 }
 
-void color_parse(const char *value, const char *var, char *dst)
+int color_parse(const char *value, char *dst)
 {
-	color_parse_mem(value, strlen(value), var, dst);
+	return color_parse_mem(value, strlen(value), dst);
 }
 
-void color_parse_mem(const char *value, int value_len, const char *var,
-		char *dst)
+int color_parse_mem(const char *value, int value_len, char *dst)
 {
 	const char *ptr = value;
 	int len = value_len;
@@ -76,7 +75,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 0;
 	}
 
 	/* [fg [bg]] [attr]... */
@@ -153,9 +152,9 @@ void color_parse_mem(const char *value, int value_len, const char *var,
 		*dst++ = 'm';
 	}
 	*dst = 0;
-	return;
+	return 0;
 bad:
-	die("bad color value '%.*s' for variable '%s'", value_len, value, var);
+	return error(_("invalid color value: %.*s"), value_len, value);
 }
 
 int git_config_colorbool(const char *var, const char *value)

color.h

@@ -77,8 +77,8 @@ int git_color_default_config(const char *var, const char *value, void *cb);
 
 int git_config_colorbool(const char *var, const char *value);
 int want_color(int var);
-void color_parse(const char *value, const char *var, char *dst);
-void color_parse_mem(const char *value, int len, const char *var, char *dst);
+int color_parse(const char *value, char *dst);
+int color_parse_mem(const char *value, int len, char *dst);
 __attribute__((format (printf, 3, 4)))
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
 __attribute__((format (printf, 3, 4)))

diff.c

@@ -248,8 +248,7 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 			return 0;
 		if (!value)
 			return config_error_nonbool(var);
-		color_parse(value, var, diff_colors[slot]);
-		return 0;
+		return color_parse(value, diff_colors[slot]);
 	}
 
 	/* like GNU diff's --suppress-blank-empty option  */

grep.c

@@ -111,7 +111,7 @@ int grep_config(const char *var, const char *value, void *cb)
 	if (color) {
 		if (!value)
 			return config_error_nonbool(var);
-		color_parse(value, var, color);
+		return color_parse(value, color);
 	}
 	return 0;
 }

log-tree.c

@@ -73,8 +73,7 @@ int parse_decorate_color_config(const char *var, const char *slot_name, const ch
 		return 0;
 	if (!value)
 		return config_error_nonbool(var);
-	color_parse(value, var, decoration_colors[slot]);
-	return 0;
+	return color_parse(value, decoration_colors[slot]);
 }
 
 /*

pretty.c

@@ -979,9 +979,8 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
 				return end - placeholder + 1;
 			begin += 5;
 		}
-		color_parse_mem(begin,
-				end - begin,
-				"--pretty format", color);
+		if (color_parse_mem(begin, end - begin, color) < 0)
+			die(_("unable to parse --pretty format"));
 		strbuf_addstr(sb, color);
 		return end - placeholder + 1;
 	}