Re: [PATCH/RFC] ignore unknown color configuration

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH/RFC] ignore unknown color configuration

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:55

Jeff King [off-list ref] writes:
quoted hunk
diff --git a/builtin-branch.c b/builtin-branch.c
index 05e876e..c87e63b 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -65,7 +65,7 @@ static int parse_branch_color_slot(const char *var, int ofs)
 		return BRANCH_COLOR_LOCAL;
 	if (!strcasecmp(var+ofs, "current"))
 		return BRANCH_COLOR_CURRENT;
-	die("bad config variable '%s'", var);
+	return -1;
 }
 
 static int git_branch_config(const char *var, const char *value, void *cb)
@@ -76,6 +76,8 @@ static int git_branch_config(const char *var, const char *value, void *cb)
 	}
 	if (!prefixcmp(var, "color.branch.")) {
 		int slot = parse_branch_color_slot(var, 13);
+		if (slot < 0)
+			return 0;
 		if (!value)
 			return config_error_nonbool(var);
 		color_parse(value, var, branch_colors[slot]);
diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..326cd63 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -890,7 +890,7 @@ static int parse_status_slot(const char *var, int offset)
 		return WT_STATUS_NOBRANCH;
 	if (!strcasecmp(var+offset, "unmerged"))
 		return WT_STATUS_UNMERGED;
-	die("bad config variable '%s'", var);
+	return -1;
 }
 
 static int git_status_config(const char *k, const char *v, void *cb)
@@ -910,6 +910,8 @@ static int git_status_config(const char *k, const char *v, void *cb)
 	}
 	if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
 		int slot = parse_status_slot(k, 13);
+		if (slot < 0)
+			return -1;
Shouldn't this return 0, to say "we handled it (by ignoring), don't
worry", instead of saying "hey it's error" by returning -1?  That's what
is done on the "diff" side below...
quoted hunk
diff --git a/diff.c b/diff.c
index d952686..08bbd3e 100644
--- a/diff.c
+++ b/diff.c
@@ -122,6 +122,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 
 	if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
 		int slot = parse_diff_color_slot(var, 11);
+		if (slot < 0)
+			return 0;
 		if (!value)
 			return config_error_nonbool(var);
 		color_parse(value, var, diff_colors[slot]);

Re: [PATCH/RFC] ignore unknown color configuration

From: Jeff King <hidden>
Date: 2016-06-15 22:47:55

On Tue, Dec 15, 2009 at 05:29:18PM -0800, Junio C Hamano wrote:
quoted
 	if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
 		int slot = parse_status_slot(k, 13);
+		if (slot < 0)
+			return -1;
Shouldn't this return 0, to say "we handled it (by ignoring), don't
worry", instead of saying "hey it's error" by returning -1?  That's what
is done on the "diff" side below...
Yes, thank you for catching it. It should definitely be "return 0" (as
the other two cases are). It was a simple think-o on my part.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help