Jeff King <peff@xxxxxxxx> wrote: > On Tue, Feb 14, 2012 at 07:23:40PM +0100, Michał Kiedrowicz wrote: > > > > > > 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()). > > [...] > > [...] > > > 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html