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

From: Ben Peart <peartben@gmail.com>
Date: Apr 23, 2018, 13:15 UTC
Message-ID: <0eea1726-d511-6818-aa29-add6c13900da@gmail.com>
In-Reply-To: <20180422120718.GA29956@esm>

On 4/22/2018 8:07 AM, Eckhard Maaß wrote:

Show 19 quoted lines

> 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.

Show 10 quoted lines

> 
> 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
>