Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff

Jeff King [off-list ref] wrote:

On Tue, Feb 14, 2012 at 07:23:40PM +0100, Michał Kiedrowicz wrote:

quoted

quoted

quoted

quoted

BTW GitHub is closed source, but we can check what algorithm does Trac
use for diff refinement highlighting (highlighting changed portions of
diff).

I think it's
http://trac.edgewall.org/browser/trunk/trac/versioncontrol/diff.py
(see markup intraline_changes()).

[...]
[...]

quoted

The markup_intraline_changes() function compares lines from preimage and
from postimage pairwise, requiring that number of lines matches, the same
like in your algorithm.

So using Jeff's diff-highlight we remain quite consistent with Trac
output. There's nothing we can "steal" from it.

Neat. When I originally wrote diff-highlight, I was inspired by seeing
what Trac and GitHub did, but came up with the algorithm on my own.
Learning that Trac does the multiline thing (as we started doing with
recent patches) makes me feel even better that it's a good strategy.

As an aside, I looked at what GitHub does. It turns on highlighting only
for the single-line case, and does the same prefix/suffix thing.

It's really not very complex code; most of the hassle in diff-highlight
is ignoring (but preserving) embedded colors, so that we build on top of
existing colorization. A web tool will be doing the line coloring itself
anyway, so it can be much simpler. Elsewhere, I suggested lib-ifying
diff-highlight for gitweb to use. But once you remove the embedded color
handling, there really is not that much code, and it's probably simpler
to just rewrite it.

-Peff

Sure, now it's a simple algorithm, but if we add more code, we will
have problems with making it consistent in both gitweb and
diff-highlight (which is nice-to-have IMO). Note that my patches to
gitweb already support combined diffs (in obvious cases) while
diff-highlight will fail on them badly (see for example 09bb4eb4f14c).
I haven't done it in diff-highlight because I noticed that problem
while working on patches for gitweb.

Anyway, thanks for looking at Trac/GitHub code and sharing your opinion.