Re: [PATCH v3 2/3] merge: Add merge.renames config setting

[Elijah Newren <newren@gmail.com>]

On Thu, Apr 26, 2018 at 1:52 PM, Ben Peart <Ben.Peart@microsoft.com> wrote:

> +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.  If set to "copies" or "copy", Git will
> +       detect copies, as well.  Defaults to the value of diff.renames.
> +

We shouldn't allow users to force copy detection on for merges  The
diff side of the code will detect them correctly but the code in
merge-recursive will mishandle the copy pairs.  I think fixing it is
somewhere between big can of worms and
it's-a-configuration-that-doesn't-even-make-sense, but it's been a
while since I thought about it.

> diff --git a/merge-recursive.h b/merge-recursive.h
> index 80d69d1401..0c5f7eff98 100644
> --- a/merge-recursive.h
> +++ b/merge-recursive.h
> @@ -17,7 +17,8 @@ struct merge_options {
>         unsigned renormalize : 1;
>         long xdl_opts;
>         int verbosity;
> -       int detect_rename;
> +       int diff_detect_rename;
> +       int merge_detect_rename;
>         int diff_rename_limit;
>         int merge_rename_limit;
>         int rename_score;
> @@ -28,6 +29,11 @@ struct merge_options {
>         struct hashmap current_file_dir_set;
>         struct string_list df_conflict_file_set;
>  };
> +inline int merge_detect_rename(struct merge_options *o)
> +{
> +       return o->merge_detect_rename >= 0 ? o->merge_detect_rename :
> +               o->diff_detect_rename >= 0 ? o->diff_detect_rename : 1;
> +}

Why did you split o->detect_rename into two fields?  You then
recombine them in merge_detect_rename(), and after initial setup only
ever access them through that function.  Having two fields worries me
that people will accidentally introduce bugs by using one of them
instead of the merge_detect_rename() function.  Is there a reason you
decided against having the initial setup just set a single value and
then use it directly?