From: Ben Peart Date: Wed, 25 Apr 2018 15:22:33 GMT Subject: Re: [PATCH v2 0/2] add additional config settings for merge Message-ID: <365838dc-d988-b72c-ef29-20369a7f54a2@gmail.com> In-Reply-To: On 4/24/2018 8:13 PM, Junio C Hamano wrote: > Ben Peart writes: > >> diff.renameLimit:: >> The number of files to consider when performing the copy/rename >> - detection; equivalent to the 'git diff' option `-l`. >> + detection; equivalent to the 'git diff' option `-l`. This setting >> + has no effect if rename detection is turned off. > > You mean "turned off via diff.renames"? > > This is not meant as a suggestion to rewrite this paragraph > further---but if the answer is "no", then that might be an > indication that the sentence is inviting a misunderstanding. > Yes, this is referring to turned off via the config setting "diff.renames" but it could also be turned off by passing "--no-renames" on the command line. To be clear, this documentation change isn't trying to document any changes to the code or behavior - it is just an attempt to clarify what the existing behavior is. If it isn't helping, I can remove it. >> diff.renames:: >> Whether and how Git detects renames. If set to "false", >> diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt >> index 5a9ab969db..38492bcb98 100644 >> --- a/Documentation/merge-config.txt >> +++ b/Documentation/merge-config.txt >> @@ -39,7 +39,8 @@ include::fmt-merge-msg-config.txt[] >> merge.renameLimit:: >> The number of files to consider when performing rename detection >> during a merge; if not specified, defaults to the value of >> - diff.renameLimit. >> + diff.renameLimit. This setting has no effect if rename detection >> + is turned off. > > Ditto. If your design is to make the merge machinery completely > ignore diff.renames and only pay attention to merge.renames [*1*], > then it probably is a good idea to be more specific here, by saying > "... is turned off via ...", though. > >> merge.renames:: >> Whether and how Git detects renames. If set to "false", > > [Footnote] > > *1* ...which I do not think is such a good idea, by the way. I'd > personally expect merge.renames to allow overriding and falling back > to diff.renames, just like the {merge,diff}.renameLimit pair does. > It looks like I'm in the minority on whether the merge settings should inherit from the corresponding diff settings so I will submit a new patch series that does it the same way as the {merge,diff}.renameLimit pair works. I'll leave it as an exercise for someone else [1] to change any other merge settings that should behave that way. [1] https://public-inbox.org/git/20180420133632.17580-1-benpeart@microsoft.com/T/#m52a3dbd0945360bfb873fd3b553472558ef3b796