drop odd return value semantics from userdiff_config

When the userdiff_config function was introduced in be58e70 (diff: unify external diff and funcname parsing code, 2008-10-05), it used a return value convention unlike any other config callback. Like other callbacks, it used "-1" to signal error. But it returned "1" to indicate that it found something, and "0" otherwise; other callbacks simply returned "0" to indicate that no error occurred. This distinction was necessary at the time, because the userdiff namespace overlapped slightly with the color configuration namespace. So "diff.color.foo" could mean "the 'foo' slot of diff coloring" or "the 'foo' component of the "color" userdiff driver". Because the color-parsing code would die on an unknown color slot, we needed the userdiff code to indicate that it had matched the variable, letting us bypass the color-parsing code entirely. Later, in 8b8e862 (ignore unknown color configuration, 2009-12-12), the color-parsing code learned to silently ignore unknown slots. This means we no longer need to protect userdiff-matched variables from reaching the color-parsing code. We can therefore change the userdiff_config calling convention to a more normal one. This drops some code from each caller, which is nice. But more importantly, it reduces the cognitive load for readers who may wonder why userdiff_config is unlike every other config callback. There's no need to add a new test confirming that this works; t4020 already contains a test that sets diff.color.external. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 7, 2012 at 13:23 UTC 6680a0874f86600cc8b4b5327384934d83d752a8
5 files changed +12 -37
builtin/blame.c
+1 -7
@@ -2050,14 +2050,8 @@ static int git_blame_config(const char *var, const char *value, void *cb)
2050 return 0;
2051 }
2052
2053 - switch (userdiff_config(var, value)) {
2054 - case 0:
2055 - break;
2056 - case -1:
2053 + if (userdiff_config(var, value) < 0)
2054 return -1;
2058 - default:
2059 - return 0;
2060 - }
2055
2056 return git_default_config(var, value, cb);
2057 }
builtin/cat-file.c
+1 -7
@@ -226,14 +226,8 @@ static const char * const cat_file_usage[] = {
226
227 static int git_cat_file_config(const char *var, const char *value, void *cb)
228 {
229 - switch (userdiff_config(var, value)) {
230 - case 0:
231 - break;
232 - case -1:
229 + if (userdiff_config(var, value) < 0)
230 return -1;
234 - default:
235 - return 0;
236 - }
231
232 return git_default_config(var, value, cb);
233 }
builtin/grep.c
+2 -5
@@ -325,11 +325,8 @@ static int grep_config(const char *var, const char *value, void *cb)
325 struct grep_opt *opt = cb;
326 char *color = NULL;
327
328 - switch (userdiff_config(var, value)) {
329 - case 0: break;
330 - case -1: return -1;
331 - default: return 0;
332 - }
328 + if (userdiff_config(var, value) < 0)
329 + return -1;
330
331 if (!strcmp(var, "grep.extendedregexp")) {
332 if (git_config_bool(var, value))
diff.c
+2 -5
@@ -177,11 +177,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
177 return 0;
178 }
179
180 - switch (userdiff_config(var, value)) {
181 - case 0: break;
182 - case -1: return -1;
183 - default: return 0;
184 - }
180 + if (userdiff_config(var, value) < 0)
181 + return -1;
182
183 if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
184 int slot = parse_diff_color_slot(var, 11);
userdiff.c
+6 -13
@@ -210,14 +210,7 @@ static int parse_funcname(struct userdiff_funcname *f, const char *k,
210 if (git_config_string(&f->pattern, k, v) < 0)
211 return -1;
212 f->cflags = cflags;
213 - return 1;
214 -}
215 -
216 -static int parse_string(const char **d, const char *k, const char *v)
217 -{
218 - if (git_config_string(d, k, v) < 0)
219 - return -1;
220 - return 1;
213 + return 0;
214 }
215
216 static int parse_tristate(int *b, const char *k, const char *v)
@@ -226,13 +219,13 @@ static int parse_tristate(int *b, const char *k, const char *v)
219 *b = -1;
220 else
221 *b = git_config_bool(k, v);
229 - return 1;
222 + return 0;
223 }
224
225 static int parse_bool(int *b, const char *k, const char *v)
226 {
227 *b = git_config_bool(k, v);
235 - return 1;
228 + return 0;
229 }
230
231 int userdiff_config(const char *k, const char *v)
@@ -246,13 +239,13 @@ int userdiff_config(const char *k, const char *v)
239 if ((drv = parse_driver(k, v, "binary")))
240 return parse_tristate(&drv->binary, k, v);
241 if ((drv = parse_driver(k, v, "command")))
249 - return parse_string(&drv->external, k, v);
242 + return git_config_string(&drv->external, k, v);
243 if ((drv = parse_driver(k, v, "textconv")))
251 - return parse_string(&drv->textconv, k, v);
244 + return git_config_string(&drv->textconv, k, v);
245 if ((drv = parse_driver(k, v, "cachetextconv")))
246 return parse_bool(&drv->textconv_want_cache, k, v);
247 if ((drv = parse_driver(k, v, "wordregex")))
255 - return parse_string(&drv->word_regex, k, v);
248 + return git_config_string(&drv->word_regex, k, v);
249
250 return 0;
251 }