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

[Elijah Newren <newren@gmail.com>]

On Tue, Apr 24, 2018 at 1:31 PM, Ben Peart <peartben@gmail.com> wrote:
> On 4/24/2018 2:59 PM, Elijah Newren wrote:
>> On Tue, Apr 24, 2018 at 10:11 AM, Ben Peart <Ben.Peart@microsoft.com>
>> wrote:
>>>
>>> diff --git a/builtin/merge.c b/builtin/merge.c
>>> index 8746c5e3e8..3be52cd316 100644
>>> --- a/builtin/merge.c
>>> +++ b/builtin/merge.c
>>> @@ -424,6 +424,7 @@ static void finish(struct commit *head_commit,
>>>                  opts.output_format |=
>>>                          DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
>>>                  opts.detect_rename = DIFF_DETECT_RENAME;
>>> +               git_config_get_bool("merge.renames",
>>> &opts.detect_rename);
>>>                  diff_setup_done(&opts);
>>>                  diff_tree_oid(head, new_head, "", &opts);
>>>                  diffcore_std(&opts);
>>
>>
>> Shouldn't this also be turned off if either (a) merge.renames is unset
>> and diff.renames is false, or (b) the user specifies -Xno-renames?
>>
>
> This makes me think that I should probably remove the line that overrides
> the detect_rename setting with the merge config setting.  As I look at the
> code, none of the other merge options are reflected in the diffstat;
> instead, all the settings are pretty much hard coded.  Perhaps I shouldn't
> rock that boat.

Actually, stat_graph_width respects the diff.statGraphWidth config
option, even though it's slightly hidden due to the magic value of -1,
and being handled from diff.c.

However, trying to get this suggestion of mine hooked up, particularly
with -Xno-renames and -Xfind-renames (the latter because it might need
to override a merge.renames or diff.renames config setting), might be
slightly tricky because the -X options are only passed down to a
single merge strategy but this code is outside of the merge
strategies.  So making it a separate patch, or even a separate patch
series may make sense.  I'm still interested in this change if you
aren't, but I'm fine with it not being part of your series if you
don't want to tackle it.