On Sat, Mar 30 2019, Elijah Newren wrote:
I may have more, just quickly skimming this for the first time...
> merge.renames::
> - Whether and how Git detects renames. If set to "false",
> - rename detection is disabled. If set to "true", basic rename
> - detection is enabled. Defaults to the value of diff.renames.
> + Whether Git detects renames. If set to "false", rename detection
> + is disabled. If set to "true", basic rename detection is enabled.
> + Defaults to the value of diff.renames.
> [...]
> + if (!git_config_get_string("merge.directoryrenames", &value)) {
> + if (!strcasecmp(value, "true"))
> + opt->detect_directory_renames = 2;
> + else if (!strcasecmp(value, "false"))
> + opt->detect_directory_renames = 0;
> + else if (!strcasecmp(value, "conflict"))
> + opt->detect_directory_renames = 1;
> + else {
> + error(_("Invalid value for merge.directoryRenames: %s"),
> + value);
> + opt->detect_directory_renames = 1;
> + }
> + free(value);
> + }
Instead of making your own true/false parser you can use
git_parse_maybe_bool(). See what we do for merge.ff:
builtin/merge.c-617- else if (!strcmp(k, "merge.ff")) {
builtin/merge.c:618: int boolval = git_parse_maybe_bool(v);
builtin/merge.c-619- if (0 <= boolval) {
builtin/merge.c-620- fast_forward = boolval ? FF_ALLOW : FF_NO;
builtin/merge.c-621- } else if (v && !strcmp(v, "only")) {
builtin/merge.c-622- fast_forward = FF_ONLY;
builtin/merge.c-623- } /* do not barf on values from future versions of git */
builtin/merge.c-624- return 0;
Small nit, but allows us to document "this config takse bool or ..."
without having different verions of "bool" in various places.
Also, I don't care personally, but this also violates the "if one thing
requires braces put it on all the if/else arms" in CodingGuidelines.
