On Fri, Feb 10, 2012 at 10:47:13PM +0100, Michał Kiedrowicz wrote: > contrib/diff-highlight/diff-highlight | 96 ++++++++++++++++++++++----------- > 1 files changed, 65 insertions(+), 31 deletions(-) Thanks for sending. I looked at a whole bunch of patches, and I was pleasantly surprised to find how infrequently we hit false positives in practice. In fact, the only things that looked worse with your patch were places where your patch happened to turn on highlighting for lines where the existing heuristics already were a little ugly (i.e., the problem was not your patch, but that the existing heuristic is sometimes non-optimal). I ended up pulling your changes out into a few distinct commits. That made it easier for me to review and understand what was going on (and hopefully ditto for other reviewers, or people who end up bisecting or reading the log later). I'll post that series in a moment. > After looking at outputs I noticed that it can also ignore lines with > prefixes/suffixes that consist only of punctuation (asterisk, semicolon, dot, > etc), because otherwise whole line is highlighted except for terminating > punctuation. I missed this note when I applied the patch and started looking at the outputs, and ended up having a similar thought. However, I don't know that it buys much in practice, and it's nice to be fairly agnostic about content. I did leave that open to easy tweaking in my series, though. [1/5]: diff-highlight: make perl strict and warnings fatal [2/5]: diff-highlight: don't highlight whole lines [3/5]: diff-highlight: refactor to prepare for multi-line hunks [4/5]: diff-highlight: match multi-line hunks [5/5]: diff-highlight: document some non-optimal cases -Peff -- 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