Re: [PATCH v3 18/24] merge-recursive: consolidate unnecessary fields in merge_options

[Junio C Hamano <gitster@pobox.com>]

Elijah Newren <newren@gmail.com> writes:

>  static inline int merge_detect_rename(struct merge_options *opt)
>  {
> -	return opt->merge_detect_rename >= 0 ? opt->merge_detect_rename :
> -		opt->diff_detect_rename >= 0 ? opt->diff_detect_rename : 1;
> +	return (opt->detect_renames != -1) ? opt->detect_renames : 1;
>  }

Every time I see "is it not negative?" (or more generally "is it in
this range?") converted to "is it not this exact value?", it makes
me feel uneasy.

> -	opts.rename_limit = opt->merge_rename_limit >= 0 ? opt->merge_rename_limit :
> -			    opt->diff_rename_limit >= 0 ? opt->diff_rename_limit :
> -			    1000;
> +	opts.rename_limit = (opt->rename_limit != -1) ? opt->rename_limit : 1000;

Likewise.  I have no objection to merging two rename-limit to a
single field (and two detect-renames to a single field).

> @@ -3732,14 +3729,14 @@ static void merge_recursive_config(struct merge_options *opt)
>  {
>  	char *value = NULL;
>  	git_config_get_int("merge.verbosity", &opt->verbosity);
> -	git_config_get_int("diff.renamelimit", &opt->diff_rename_limit);
> -	git_config_get_int("merge.renamelimit", &opt->merge_rename_limit);
> +	git_config_get_int("diff.renamelimit", &opt->rename_limit);
> +	git_config_get_int("merge.renamelimit", &opt->rename_limit);

Hmph.  If merge.renameLimit is there, that would overwrite whatever
we get by reading from diff.renameLimit, so the two fields with
runtime precedence order can easily be replaced by these two calls.

Nice.

If you have "[diff] renamelimit = -2" in your $GIT_DIR/config, would
we change behaviour due to the earlier conversion that has nothing
to do with the theme of this step (i.e. consolidate two variables
into one)?

> @@ -3765,11 +3762,9 @@ void init_merge_options(struct merge_options *opt,
>  	opt->repo = repo;
>  	opt->verbosity = 2;
>  	opt->buffer_output = 1;
> -	opt->diff_rename_limit = -1;
> -	opt->merge_rename_limit = -1;
> +	opt->rename_limit = -1;
>  	opt->renormalize = 0;
> -	opt->diff_detect_rename = -1;
> -	opt->merge_detect_rename = -1;
> +	opt->detect_renames = -1;
>  	opt->detect_directory_renames = MERGE_DIRECTORY_RENAMES_CONFLICT;
>  	merge_recursive_config(opt);
>  	merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
> @@ -3821,16 +3816,16 @@ int parse_merge_opt(struct merge_options *opt, const char *s)
>  	else if (!strcmp(s, "no-renormalize"))
>  		opt->renormalize = 0;
>  	else if (!strcmp(s, "no-renames"))
> -		opt->merge_detect_rename = 0;
> +		opt->detect_renames = 0;
>  	else if (!strcmp(s, "find-renames")) {
> -		opt->merge_detect_rename = 1;
> +		opt->detect_renames = 1;
>  		opt->rename_score = 0;
>  	}
>  	else if (skip_prefix(s, "find-renames=", &arg) ||
>  		 skip_prefix(s, "rename-threshold=", &arg)) {
>  		if ((opt->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)
>  			return -1;
> -		opt->merge_detect_rename = 1;
> +		opt->detect_renames = 1;
>  	}
>  	/*
>  	 * Please update $__git_merge_strategy_options in
> diff --git a/merge-recursive.h b/merge-recursive.h
> index 0fdae904dd..f4bdfbc897 100644
> --- a/merge-recursive.h
> +++ b/merge-recursive.h
> @@ -27,10 +27,8 @@ struct merge_options {
>  		MERGE_DIRECTORY_RENAMES_CONFLICT = 1,
>  		MERGE_DIRECTORY_RENAMES_TRUE = 2
>  	} detect_directory_renames;
> -	int diff_detect_rename;
> -	int merge_detect_rename;
> -	int diff_rename_limit;
> -	int merge_rename_limit;
> +	int detect_renames;
> +	int rename_limit;
>  	int rename_score;
>  	int needed_rename_limit;
>  	int show_rename_progress;