Re: [PATCH] merge-recursive: option to disable renames

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 16, 2016, 23:43 UTC
Message-ID: <xmqqoabgkyqj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CALMa68q+TgAXS0=xc1CpZ2nbzGb+TWafWCUXZhGrWPzFL5_AQQ@mail.gmail.com>

Felipe Gonçalves Assis <felipeg.assis@gmail.com> writes:

> Just a note: In git-diff, there is no "--renames". Instead, renames
> are enabled by the "--find-renames[=<n>]" option.
>
> 1. Do you think "rename-threshold=<n>" should enable renames?

I do not think it matters in practice, but if you have

	git merge-recursive --no-renames --rename-threshold=50%

in mind, yes, I think it is OK for the latter to flip "do we do the rename detection?" switch on. Afger all, that is what happens in

	git diff --no-renames --find-renames=50%

> 2. Should we have an option "find-renames" in merge?
> 3. Should git-diff have a "--renames" option?

Good questions. I think the existing command line options to merge-recursive are misdesigned. It should not have invented rename-threshold at all and instead used --find-renames=<n>.

We could declare that "--rename-threshold=<n>" that merge-recursive has is a synonym to "--find-renames=<n>" that exists for hysterical raisins, and start supporting "--find-renames=<n>" as the "kosher" version. That would align the two commands better. But I view it as a much lower priority than adding "--no-renames".

> I personally like your suggestion as it is. Just wanted to make sure
> you considered that while I work on the next patch.

I didn't actually check what longhand "-M" has in the diff land. It was a long time ago I wrote that code ;-)

For now, I would think the following is sensible:

   * Add "--no-renames" (but not "--renames") to merge-recursive;

   * Teach "--rename-threshold=<n>" in merge-recursive to enable
     rename detection if it was disabled earlier.

Optionally:

   * Teach merge-recursive to take "--find-renames=<n>" as well, and
     behave the same way as "--rename-threshold=<n>" does, after
     doing the two things above.

Thanks.