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

[Ben Peart <peartben@gmail.com>]

On 4/21/2018 12:23 AM, Junio C Hamano wrote:
> Elijah Newren <newren@gmail.com> writes:
> 
>>>>> +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.
>> ...
>> Sorry, I think I wasn't being clear.  The documentation for the config
>> options for e.g. diff.renameLimit, fetch.prune, log.abbrevCommit, and
>> merge.ff all mention the equivalent command line parameters.  Your
>> patch doesn't do that for merge.renames, but I think it would be
>> helpful if it did.
> 
> Yes, and if we are adding a new configuration, we should do so in
> such a way that we do not have to come back and extend it when we
> know what the command line option does and the configuration being
> proposed is less capable already.

Between all the different command line options, config settings, merge 
strategies and the interactions between the diff and merge versions, I
was trying to keep things as simple and consistent as possible.  To that 
end 'merge.renames' was modeled after the existing 'diff.renames.'

I wonder if we can just add a
> single configuration whose value can be "never" to pretend as if
> "--Xno-renames" were given, and some similarity score like "50" to
> pretend as if "--Xfind-renames=50" were given.
> 
> That is, merge.renames does not have to a simple "yes-no to control
> the --Xno-renames option".  And it would of course be better to
> document it.
> 

With the existing differences in how these options are passed on the 
command line, I'm hesitant to add yet another pattern in the config 
settings that combines 'renames' and '--find-renames[=<n>]'.

I _have_ wondered why this all isn't configured via find-renames with 
find-renames=0 meaning renames=false (instead of mapping 0 to 32K).  I 
think that could have eliminated the need for splitting rename across 
two different settings (which is what I think you are proposing above). 
I'd then want the config setting and command line option to be the same 
syntax and behavior.

Moving the existing settings to this model and updating the config and 
command line options to be consistent without breaking backwards 
compatibility is outside the intended scope of this patch.

> I also had to wonder how "merge -s resolve" faired, if the project
> is not interested in renamed paths at all.
> 

To be clear, it isn't that we're not interested in detecting renamed 
files and paths.  We're just opposed to it taking an hour to figure that 
out!

> Thanks.
>