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

From: Ben Peart <peartben@gmail.com>
To: "Eckhard Maaß" <eckhard.s.maass@googlemail.com>,
	"Elijah Newren" <newren@gmail.com>
Cc: Ben Peart <Ben.Peart@microsoft.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	"peff@peff.net" <peff@peff.net>,
	"gitster@pobox.com" <gitster@pobox.com>,
	"pclouds@gmail.com" <pclouds@gmail.com>,
	"vmiklos@frugalware.org" <vmiklos@frugalware.org>,
	Kevin Willford <kewillf@microsoft.com>,
	"Johannes.Schindelin@gmx.de" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v1 1/2] merge: Add merge.renames config setting
Date: Mon, 23 Apr 2018 09:15:09 -0400	[thread overview]
Message-ID: <0eea1726-d511-6818-aa29-add6c13900da@gmail.com> (raw)
In-Reply-To: <20180422120718.GA29956@esm>

On 4/22/2018 8:07 AM, Eckhard Maaß wrote:
> On Fri, Apr 20, 2018 at 11:34:25AM -0700, Elijah Newren wrote:
>> 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.
> 
> I wonder here what the relation to the diff.* options should be in this
> regard anyway. There is already diff.renames. Naively, I would assume
> that these options are in sync, that is you control the behavior of both
> the normal diff family like git show and git merge. The reasoning, at
> least for me, is to keep consistency between the outcome of rename
> detection while merging and a later simple "git show MERGE_BASE..HEAD".
> I would expect those to give me the same style of rename detection.
> 
> Hence, I would like to use diff.renames and maybe enhance this option to
> also carry the score in backward compatible way (or introduce a second
> configuration option?). Is this idea going in a good direction? If yes,
> I will try to submit a patch for this.

It's a fair question.  If you look at all the options in 
Documentation/merge-config.txt, you will see many merge specific 
settings.  I think the ability to control these settings separately is 
pretty well established.

In commit 2a2ac926547 when merge.renamelimit was added, it was decided 
to have separate settings for merge and diff to give users the ability 
to control that behavior.  In this particular case, it will default to 
the value of diff.renamelimit when it isn't set.  That isn't consistent 
with the other merge settings.

Changing that behavior across the rest of the merge settings is outside 
the scope of this patch.  I don't have a strong opinion as to whether 
that is a good or bad thing.

> 
> Ah, by the way: for people that have not touched diff.renames there will
> be no visible change in how Git behaves - the default for diff.renames
> is a rename with 50% score with is the same for merge. So it will only
> change if one has tweaked diff.renames already. But I wonder if one does
> that and expect the merge to use a different rename detection anyway.
> 
> Greetings,
> Eckhard
> 