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>

diff --git a/builtin/blame.c b/builtin/blame.c
index 5a67c202f06abeaa90a7547d78b536f7f2b9db24..01956c80815a4f330ae27b77b4d21d263a887d1b 100644 (file)
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2050,14 +2050,8 @@ static int git_blame_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
-       switch (userdiff_config(var, value)) {
-       case 0:
-               break;
-       case -1:
+       if (userdiff_config(var, value) < 0)
                return -1;
-       default:
-               return 0;
-       }
 
        return git_default_config(var, value, cb);
 }

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 07bd984084fbbfbb826ef5b784fc68069675e73c..8ed501f220424976cc30f4a4dbf3d59f979902be 100644 (file)
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -226,14 +226,8 @@ static const char * const cat_file_usage[] = {
 
 static int git_cat_file_config(const char *var, const char *value, void *cb)
 {
-       switch (userdiff_config(var, value)) {
-       case 0:
-               break;
-       case -1:
+       if (userdiff_config(var, value) < 0)
                return -1;
-       default:
-               return 0;
-       }
 
        return git_default_config(var, value, cb);
 }

diff --git a/builtin/grep.c b/builtin/grep.c
index 5c2ae94e5576f2e8af1f8509b789a67851db2598..dc6de83ab7c8178fc14d9e27679cb0b22f9caba3 100644 (file)
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -325,11 +325,8 @@ static int grep_config(const char *var, const char *value, void *cb)
        struct grep_opt *opt = cb;
        char *color = NULL;
 
-       switch (userdiff_config(var, value)) {
-       case 0: break;
-       case -1: return -1;
-       default: return 0;
-       }
+       if (userdiff_config(var, value) < 0)
+               return -1;
 
        if (!strcmp(var, "grep.extendedregexp")) {
                if (git_config_bool(var, value))

diff --git a/diff.c b/diff.c
index 7e154265f778c645192cbf17c65b9bea2a507402..a656f5e83e406fc9a83ffaab41dd34200b99aa73 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -177,11 +177,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
-       switch (userdiff_config(var, value)) {
-               case 0: break;
-               case -1: return -1;
-               default: return 0;
-       }
+       if (userdiff_config(var, value) < 0)
+               return -1;
 
        if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
                int slot = parse_diff_color_slot(var, 11);

diff --git a/userdiff.c b/userdiff.c
index 76109da4bcb7abc26ed20508692051a40e8addd7..1e7184f7f00bc75872369761116141c087fa1c1c 100644 (file)
--- a/userdiff.c
+++ b/userdiff.c
@@ -210,14 +210,7 @@ static int parse_funcname(struct userdiff_funcname *f, const char *k,
        if (git_config_string(&f->pattern, k, v) < 0)
                return -1;
        f->cflags = cflags;
-       return 1;
-}
-
-static int parse_string(const char **d, const char *k, const char *v)
-{
-       if (git_config_string(d, k, v) < 0)
-               return -1;
-       return 1;
+       return 0;
 }
 
 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)
                *b = -1;
        else
                *b = git_config_bool(k, v);
-       return 1;
+       return 0;
 }
 
 static int parse_bool(int *b, const char *k, const char *v)
 {
        *b = git_config_bool(k, v);
-       return 1;
+       return 0;
 }
 
 int userdiff_config(const char *k, const char *v)
@@ -246,13 +239,13 @@ int userdiff_config(const char *k, const char *v)
        if ((drv = parse_driver(k, v, "binary")))
                return parse_tristate(&drv->binary, k, v);
        if ((drv = parse_driver(k, v, "command")))
-               return parse_string(&drv->external, k, v);
+               return git_config_string(&drv->external, k, v);
        if ((drv = parse_driver(k, v, "textconv")))
-               return parse_string(&drv->textconv, k, v);
+               return git_config_string(&drv->textconv, k, v);
        if ((drv = parse_driver(k, v, "cachetextconv")))
                return parse_bool(&drv->textconv_want_cache, k, v);
        if ((drv = parse_driver(k, v, "wordregex")))
-               return parse_string(&drv->word_regex, k, v);
+               return git_config_string(&drv->word_regex, k, v);
 
        return 0;
 }