Re: [PATCH v2 15/15] merge-recursive: switch directory rename detection default
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 30, 2019, 09:12 UTC
- Message-ID
- <87imw0afp4.fsf@evledraar.gmail.com>
- In-Reply-To
- <20190330003336.21940-16-newren@gmail.com>
On Sat, Mar 30 2019, Elijah Newren wrote:
I may have more, just quickly skimming this for the first time...
Show 22 quoted lines
> 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.