Re: [PATCHv5 00/17] Diff machine: highlight moved lines.

From: Junio C Hamano <gitster@pobox.com>
Date: May 26, 2017, 01:20 UTC
Message-ID: <xmqq8tlkct27.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kYO8DoTDCXCk1uSFSAJA5JusD-t4BZoQfGy5n9DF34EvA@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> As you turn on/off normal coloring via "color.diff" and this only extends
> the coloring scheme, I have the impression "color" is the right section.
> Maybe color.diffStyle=[normal/enhanced] and "enhanced" means this
> feature for now?

Hmph, I thought the intent of color.diff is "is the diff command itself is colored?" In other words, color.diff=false should give you monochrome if you say "diff --word-diff", etc.

> The only option in the "diff" section related to color is diff.wsErrorHighlight
> which has a very similar purpose, so "diff.colorMoved" would fit in that
> scheme.

I didn't have "should diff output highlight whitespace errors?" in mind when I wrote the message you are responding to, but yes, that is quite similar to "should diff output show lines moved and lines deleted/added differently?".

> So with these questions, I wonder if we want to color moved lines
> as "color.diff.context" (i.e. plain white text in the normal coloring scheme)
> This would serve the intended purpose of
> dimming the attention to moved lines.

Yes, but two points.

 (1) We want to do so while making it obvious where the boundary
     between two moved blocks of text whose destination (for
     moved-deleted lines) or source (for moved-added lines) is.

 (2) My message was an impression from using the code to review a
     patch that is meant to be "move without changing other things".
     For other purposes, there may be cases where moved ones may
     want to be highlighted, not dimmed.

Show 5 quoted lines

> Regarding the last point of marking up adjacent blocks (which would
> indicate that there is a coherency issue or just moving from different
> places), we could highlight the last line of the previous block
> and first line of the next block in their "normal" colors (i.e.
> color.diff.old and color.diff.new).

Hmm.  That is an interesting thought.