On 4/20/2018 2:34 PM, Elijah Newren wrote:
Hi Ben,
On Fri, Apr 20, 2018 at 10:59 AM, Ben Peart <peartben@xxxxxxxxx> wrote:
On 4/20/2018 1:02 PM, Elijah Newren wrote:
On Fri, Apr 20, 2018 at 6:36 AM, Ben Peart <Ben.Peart@xxxxxxxxxxxxx>
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.
This statement wasn't meant to be independent of the sentence that
followed it...
Yes, but that requires people to know they need to do that and then remember
to pass it on the command line every time. We've found that doesn't
typically happen, we just get someone complaining about slow merges. :)
That is why we added them as config options which change the default. That
way we can then set them on the repo and the default behavior gives them
better performance. They can still always override the config setting with
the command line 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.
Also, a link in the documentation the other way, from
Documentation/merge-strategies.txt under the entries for -Xno-renames
and -Xfind-renames should probably mention this new merge.renames
config setting (much like the -Xno-renormalize flag mentions the
merge.renomralize config option).
I'm all in favor of having more information in the documentation. I'm
of the opinion that if someone has made the effort to actually _read_
the documentation, we should be as descriptive and complete as possible.
I'll take a cut at adding the things you have pointed out would be helpful.
I agree that's the pre-existing behavior, but prior to this patch
turning off rename detection could only be done manually with every
invocation. I'm slightly concerned that users might be confused if
merge.renames was set to false somewhere -- perhaps even in a global
/etc/gitconfig that they had no knowledge of or control over -- and in
an attempt to get rename detection to work they started passing larger
and larger values for renameLimit all to no avail.
The easy fix here may just be documenting the diff.renameLimit and
merge.renameLimit options that they have no effect if rename detection
is turned off.
I can add this additional documentation as well. While some might think
it is stating the obvious, I'm sure someone will benefit from it being
explicitly called out.
Or maybe I'm just worrying too much, but we (folks at $dayjob) were
bit pretty hard by renameLimit silently being capped at a value less
than the user specified and in a way that wasn't documented anywhere.
