| Message ID | patch-01.11-fb7346cd296-20210324T014604Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 20210215154427.32693-1-avarab@gmail.com | expand |
On Wed, Mar 24, 2021 at 02:48:43AM +0100, Ævar Arnfjörð Bjarmason wrote: > Since 6680a0874f (drop odd return value semantics from > userdiff_config, 2012-02-07) we have not cared about the return values > of parse_tristate() or git_config_bool() v.s. falling through in > userdiff_config(), so let's do so in those cases to make the code > easier to read. > > Having a wrapper function for git_config_bool() dates back to > d9bae1a178 (diff: cache textconv output, 2010-04-01) and > 122aa6f9c0 (diff: introduce diff.<driver>.binary, 2008-10-05), both of > which predated the change in 6680a0874f which made their return values > redundant. I think this cleanup makes sense. > int userdiff_config(const char *k, const char *v) > @@ -312,16 +305,17 @@ int userdiff_config(const char *k, const char *v) > return parse_funcname(&drv->funcname, k, v, 0); > if (!strcmp(type, "xfuncname")) > return parse_funcname(&drv->funcname, k, v, REG_EXTENDED); > - if (!strcmp(type, "binary")) > - return parse_tristate(&drv->binary, k, v); > if (!strcmp(type, "command")) > return git_config_string(&drv->external, k, v); > if (!strcmp(type, "textconv")) > return git_config_string(&drv->textconv, k, v); > - if (!strcmp(type, "cachetextconv")) > - return parse_bool(&drv->textconv_want_cache, k, v); > if (!strcmp(type, "wordregex")) > return git_config_string(&drv->word_regex, k, v); > + /* Don't care about the parse errors for these, fallthrough */ > + if (!strcmp(type, "cachetextconv")) > + drv->textconv_want_cache = git_config_bool(k, v); > + if (!strcmp(type, "binary")) > + parse_tristate(&drv->binary, k, v); The original returned early, which short-circuited the rest of the strcmp(). that probably doesn't matter much in practice (after all, an unrecognized value is inherently O(n)). But perhaps: if (...) assign...; else if (...) assign...; would make the comment unnecessary. I don't feel strongly either way, though. -Peff
diff --git a/userdiff.c b/userdiff.c index 3f81a2261c5..c147bcbb173 100644 --- a/userdiff.c +++ b/userdiff.c @@ -275,19 +275,12 @@ static int parse_funcname(struct userdiff_funcname *f, const char *k, return 0; } -static int parse_tristate(int *b, const char *k, const char *v) +static void parse_tristate(int *b, const char *k, const char *v) { if (v && !strcasecmp(v, "auto")) *b = -1; else *b = git_config_bool(k, v); - return 0; -} - -static int parse_bool(int *b, const char *k, const char *v) -{ - *b = git_config_bool(k, v); - return 0; } int userdiff_config(const char *k, const char *v) @@ -312,16 +305,17 @@ int userdiff_config(const char *k, const char *v) return parse_funcname(&drv->funcname, k, v, 0); if (!strcmp(type, "xfuncname")) return parse_funcname(&drv->funcname, k, v, REG_EXTENDED); - if (!strcmp(type, "binary")) - return parse_tristate(&drv->binary, k, v); if (!strcmp(type, "command")) return git_config_string(&drv->external, k, v); if (!strcmp(type, "textconv")) return git_config_string(&drv->textconv, k, v); - if (!strcmp(type, "cachetextconv")) - return parse_bool(&drv->textconv_want_cache, k, v); if (!strcmp(type, "wordregex")) return git_config_string(&drv->word_regex, k, v); + /* Don't care about the parse errors for these, fallthrough */ + if (!strcmp(type, "cachetextconv")) + drv->textconv_want_cache = git_config_bool(k, v); + if (!strcmp(type, "binary")) + parse_tristate(&drv->binary, k, v); return 0; }
Since 6680a0874f (drop odd return value semantics from userdiff_config, 2012-02-07) we have not cared about the return values of parse_tristate() or git_config_bool() v.s. falling through in userdiff_config(), so let's do so in those cases to make the code easier to read. Having a wrapper function for git_config_bool() dates back to d9bae1a178 (diff: cache textconv output, 2010-04-01) and 122aa6f9c0 (diff: introduce diff.<driver>.binary, 2008-10-05), both of which predated the change in 6680a0874f which made their return values redundant. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- userdiff.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)