Re: [PATCH v1 1/2] merge: Add merge.renames config setting

[Ben Peart <peartben@gmail.com>]

On 4/20/2018 1:26 PM, Elijah Newren wrote:
> On Fri, Apr 20, 2018 at 10:02 AM, Elijah Newren <newren@gmail.com> wrote:
>> On Fri, Apr 20, 2018 at 6:36 AM, Ben Peart <Ben.Peart@microsoft.com> wrote:
>>> --- a/Documentation/merge-config.txt
>>> +++ b/Documentation/merge-config.txt
>>> @@ -37,6 +37,11 @@ merge.renameLimit::
>>>          during a merge; if not specified, defaults to the value of
>>>          diff.renameLimit.
>>>
>>> +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. This is the default.
>>
>> One can already control o->detect_rename via the -Xno-renames and
>> -Xfind-renames options.  I think the documentation should mention that
>> "false" is the same as passing -Xno-renames, and "true" is the same as
>> passing -Xfind-renames.  However, find-renames does take similarity
>> threshold as a parameter, so there's a question whether this option
>> should provide some way to do the same.  I'm not sure the answer to
>> that; it may be that we'd want a separate config option for that, and
>> we can wait to add it until someone actually wants it.
> 
> I just realized another issue, though it also affects -Xno-renames.
> Even if rename detection is turned off for the merge, it is
> unconditionally turned on for the diffstat.  In builtin/merge.c,
> function finish(), there is the code:
> 
>      if (new_head && show_diffstat) {
>          ...
>          opts.detect_rename = DIFF_DETECT_RENAME;
> 
> It seems that this option should affect that line as well.  (Do you
> have diffstat turned off by chance?  If not, you may be able to
> improve your performance even more...)
> 

Seems reasonable to me.  I'll update the patch to do that.