Re: [PATCHv3] diff.c: emit moved lines with a different color
On Wed, Sep 7, 2016 at 10:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 21 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> as we do not want to see dashes ('moved-old'), I think I'l go with
>> "movedfrom" and "movedto".
>
> OK. They would be color.diff.movedFrom and color.diff.movedTo in
> the doc, and "movedfrom" and "movedto" in the code (as the caller
> already downcased them for you to strcmp()).
>
>> When going through the second pass and actually emitting colored lines
>> we only find matches in the hash map if the current line AND the previous line
>> match as we lookup by hash code, i.e. if we have a moved line, but the
>> previous line
>> changed we do not find it in the hashmap and we don't color it, such
>> that the reviewer
>> can spot a permutation.
>
> Hmph. Does this have impact on a line that was at the beginning or
> the end of a file that got moved to the beginning or the end of a
> file (four permutations, of 9 if you throw in "middle"), because
> some cases it does not have a "previous" line?I spotted that problem as well. We need to adapt the algorithm a bit more:
If the previous line is of the same kind (i.e. starting with + or -),
then we have to take it into account,
otherwise (i.e. previous line is different, such as header, hunk header,
or different sign)
then ignore previous line, as the previous line is already
having a different color.That works for the very first line of a file as well.
Stefan Beller <sbeller@google.com>