Re: [PATCHv4] diff.c: emit moved lines with a different color

[Stefan Beller <sbeller@google.com>]

On Tue, Sep 6, 2016 at 7:05 AM, Jakub Narębski <jnareb@gmail.com> wrote:
> W dniu 06.09.2016 o 09:01, Stefan Beller pisze:
>
>> ---
>>
>>  * moved new data structures into struct diff_options
>>  * color.moved=bool as well as --[no-]color-moved to {dis,en}able the new feature
>>  * color.diff.movedfrom and color.diff.movedto to control the colors
>>  * added a test
> [...]
>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 0bcb679..5daf77a 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -974,14 +974,22 @@ This does not affect linkgit:git-format-patch[1] or the
>>  'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
>>  command line with the `--color[=<when>]` option.
>>
>> +color.moved::
>> +     A boolean value, whether a diff should color moved lines
>> +     differently. The moved lines are searched for in the diff only.
>> +     Duplicated lines from somewhere in the project that are not
>> +     part of the diff are not colored as moved.
>> +     Defaults to true.
>
> [...]
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index 705a873..13b6a2a 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -234,6 +234,13 @@ ifdef::git-diff[]
>>  endif::git-diff[]
>>       It is the same as `--color=never`.
>>
>> +--[no-]color-moved::
>> +     Show moved blocks in a different color.
>> +ifdef::git-diff[]
>> +     It can be changed by the `diff.ui` and `color.diff`
>> +     configuration settings.
>> +endif::git-diff[]
>
> If not for `color.moved`, I would have thought that instead of adding
> new command line option `--color-moved` (and the fact that it is on
> by default), we could simply reuse duplication of code movement
> detection as a signal of stronger detection, namely "-M -M" (and also
> "-C -C" to handle copy detection) that git-blame uses...

Can you please elaborate on how you'd use that as a user?

The -M and -C options only operate on the file level, e.g.
these options are very good at things introduced via:

    git mv A B
    $EDIT B # only a little.

So these options make no sense when operating only on one
file or on many files that stay the same and only change very little.

The goal of my patch here is to improve cases like 11979b98
(2005-11-18, http.c: reorder to avoid compilation failure.)

In that case we just move code around, not necessarily across file
boundaries.

So that seems orthogonal to the -M/-C option as it operates on another
level. (file vs line)

In another email you asked whether this new approach works in the
word-by-word diff, which it unfortunately doesn't yet, but I would think
that it is the same problem (line vs word granularity)

So what I am asking here is, how would you imagine a better user interface
for what I am trying to do, or do you think I should adapt my goal?

Thanks,
Stefan

>
> --
> Jakub Narębski
>