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
