Stefan Beller <sbel...@google.com> writes:

>> * js/range-diff (2018-07-25) 21 commits

> I think the current coloring is good enough to ship, but it still has
> errors around corners, for example introduction of new files,
> having lines in the inner diff as:
>
>      diff --git a/Makefile b/Makefile
>      --- a/Makefile
>      +++ b/Makefile
>
> will be colored white/red/green (in that order), but in the outer diff
> these are all "context", but as these specific context lines happen
> to start with +/- we color them.
> If we want to be perfect, we rather need to parse&understand
> the inner diff on a more detailed level, but I would argue to leave
> that to a later stage for another volunteer to step in and cleanup.

I think the primary part of coloring i.e. "white is common, green is
added, red is removed" together with "bold is new, dimmed is old" is
quite usable and not broken.  

The non-coloring part, like patch matching and driving diff over a
pair of "git show" output, looked reasonably solid when I read it
(even though I admit I did not audit for leaks).

The only thing I think we would be better off without is the
coloring of whitespace errors (at least in the current shape).  I
cannot shake the feeling that temporarily setting core.whitespace to
nothing (i.e. we do not detect any whitespace errors and hence we do
not show any) while running diff-of-diff may be a workaround that is
less damaging to the code base than piling band-aid on the codepath
that is shared with plain diff (not diff-of-diff).

Reply via email to