merge-recursive: consolidate unnecessary fields in merge_options

We provided users with the ability to state whether they wanted rename detection, and to put a limit on how much CPU would be spent. Both of these fields had multiple configuration parameters for setting them, with one being a fallback and the other being an override. However, instead of implementing the logic for how to combine the multiple source locations into the appropriate setting at config loading time, we loaded and tracked both values and then made the code combine them every time it wanted to check the overall value. This had a few minor drawbacks: * it seems more complicated than necessary * it runs the risk of people using the independent settings in the future and breaking the intent of how the options are used together * it makes merge_options more complicated than necessary for other potential users of the API Fix these problems by moving the logic for combining the pairs of options into a single value; make it apply at time-of-config-loading instead of each-time-of-use. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Aug 17, 2019 at 11:41 UTC 8599ab4574ce3a8b2fc894c7cfdac8fd61450b7b
2 files changed +13 -20
merge-recursive.c
+11 -16
@@ -385,8 +385,7 @@ static int add_cacheinfo(struct merge_options *opt,
385
386 static inline int merge_detect_rename(struct merge_options *opt)
387 {
388 - return opt->merge_detect_rename >= 0 ? opt->merge_detect_rename :
389 - opt->diff_detect_rename >= 0 ? opt->diff_detect_rename : 1;
388 + return (opt->detect_renames >= 0) ? opt->detect_renames : 1;
389 }
390
391 static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
@@ -1883,9 +1882,7 @@ static struct diff_queue_struct *get_diffpairs(struct merge_options *opt,
1882 */
1883 if (opts.detect_rename > DIFF_DETECT_RENAME)
1884 opts.detect_rename = DIFF_DETECT_RENAME;
1886 - opts.rename_limit = opt->merge_rename_limit >= 0 ? opt->merge_rename_limit :
1887 - opt->diff_rename_limit >= 0 ? opt->diff_rename_limit :
1888 - 1000;
1885 + opts.rename_limit = (opt->rename_limit >= 0) ? opt->rename_limit : 1000;
1886 opts.rename_score = opt->rename_score;
1887 opts.show_rename_progress = opt->show_rename_progress;
1888 opts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -3727,14 +3724,14 @@ static void merge_recursive_config(struct merge_options *opt)
3724 {
3725 char *value = NULL;
3726 git_config_get_int("merge.verbosity", &opt->verbosity);
3730 - git_config_get_int("diff.renamelimit", &opt->diff_rename_limit);
3731 - git_config_get_int("merge.renamelimit", &opt->merge_rename_limit);
3727 + git_config_get_int("diff.renamelimit", &opt->rename_limit);
3728 + git_config_get_int("merge.renamelimit", &opt->rename_limit);
3729 if (!git_config_get_string("diff.renames", &value)) {
3733 - opt->diff_detect_rename = git_config_rename("diff.renames", value);
3730 + opt->detect_renames = git_config_rename("diff.renames", value);
3731 free(value);
3732 }
3733 if (!git_config_get_string("merge.renames", &value)) {
3737 - opt->merge_detect_rename = git_config_rename("merge.renames", value);
3734 + opt->detect_renames = git_config_rename("merge.renames", value);
3735 free(value);
3736 }
3737 if (!git_config_get_string("merge.directoryrenames", &value)) {
@@ -3760,11 +3757,9 @@ void init_merge_options(struct merge_options *opt,
3757 opt->repo = repo;
3758 opt->verbosity = 2;
3759 opt->buffer_output = 1;
3763 - opt->diff_rename_limit = -1;
3764 - opt->merge_rename_limit = -1;
3760 + opt->rename_limit = -1;
3761 opt->renormalize = 0;
3766 - opt->diff_detect_rename = -1;
3767 - opt->merge_detect_rename = -1;
3762 + opt->detect_renames = -1;
3763 opt->detect_directory_renames = MERGE_DIRECTORY_RENAMES_CONFLICT;
3764 merge_recursive_config(opt);
3765 merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
@@ -3816,16 +3811,16 @@ int parse_merge_opt(struct merge_options *opt, const char *s)
3811 else if (!strcmp(s, "no-renormalize"))
3812 opt->renormalize = 0;
3813 else if (!strcmp(s, "no-renames"))
3819 - opt->merge_detect_rename = 0;
3814 + opt->detect_renames = 0;
3815 else if (!strcmp(s, "find-renames")) {
3821 - opt->merge_detect_rename = 1;
3816 + opt->detect_renames = 1;
3817 opt->rename_score = 0;
3818 }
3819 else if (skip_prefix(s, "find-renames=", &arg) ||
3820 skip_prefix(s, "rename-threshold=", &arg)) {
3821 if ((opt->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)
3822 return -1;
3828 - opt->merge_detect_rename = 1;
3823 + opt->detect_renames = 1;
3824 }
3825 /*
3826 * Please update $__git_merge_strategy_options in
merge-recursive.h
+2 -4
@@ -27,10 +27,8 @@ struct merge_options {
27 MERGE_DIRECTORY_RENAMES_CONFLICT = 1,
28 MERGE_DIRECTORY_RENAMES_TRUE = 2
29 } detect_directory_renames;
30 - int diff_detect_rename;
31 - int merge_detect_rename;
32 - int diff_rename_limit;
33 - int merge_rename_limit;
30 + int detect_renames;
31 + int rename_limit;
32 int rename_score;
33 int needed_rename_limit;
34 int show_rename_progress;