Hi Junio, On Mon, 9 Jul 2018, Junio C Hamano wrote: > I also wonder if we should be feeding the context lines to ws.c > machinery in the first place though. It *is* confusing, I know. The entire "diff of diffs" concept *is* confusing. I just don't know about a better alternative. So hear me out, because there is a big misconception here: there are *two* levels of diffs. The outer one and the inner one. Context lines of the outer diffs have no problem [*1*]. The problem arises when the outer diff shows a - or + line (i.e. the line is present *either* in the old patch set or in the new patch set, but not both), *and* that line is *not* a context line of the inner diff. Let's illustrate this via an example. Let's assume that both the old patch set and the new patch set add a comment to a statement, and that the context of that statement changed between old and new patch set. Something like this would be in the old patch set: ```diff int quiet = 0; + /* This is only needed for the reflog message */ const char *branch = "HEAD"; ``` And this would be in the new patch set: ```diff int quiet = 0, try_harder = 0; + /* This is only needed for the reflog message */ const char *branch = "HEAD"; ``` So as you see, both old and new revision of the same patch add that comment, and it is just a context line that changed, which a regular reviewer would want to *not* consider a "real" change between the patch set iterations. Now, let's look at the "diff of diffs": ```diff - int quiet = 0; + int quiet = 0, try_harder = 0; + /* This is only needed for the reflog message */ const char *branch = "HEAD"; ``` Please understand that in the dual color mode: - The first line's `-` would have a red background color, the rest of that line would be uncolored (because it is a context line of the inner diff), - the second line's `+` would have a green background color, the rest would be just as uncolored as the rest of the first line, - the third line would be a context line of the outer diff, but a `+` line of the inner diff, therefore that rest of the line would be green, and - the fourth line is completely uncolored; It is a context line both of the inner and the outer diff. That's it for the diff colors. Now for the white space: The first two lines start with a `-` and a `+` respectively (outer diff marker), and then most crucially continue with a space to indicate the inner diff's context line, *and then continue with a horizontal tab*. As far as the inner diff is concerned, this *is* a context line. As far as the outer diff is concerned, this is *not* a context line. And that is the conundrum: the whitespace checker is called because the outer diff claims that the second line is a `+` line and the whitespace checker has no idea that it should treat it as a context line instead. I'll try to find some time this afternoon to study Stefan's reply, as I have a hunch that there is a deep insight hidden that helps me to figure out the proper path ahead (because I do not want to uglify the `diff.c` code the way my current iteration does, and I'd rather have a way to color the diff more intelligently myself, in a function in `range-diff.c`). Ciao, Dscho Footnote *1*: Actually, that is only half the truth. In dual color mode, if a line is a context line of the outer diff, but a - or + line of the inner diff, *we still want it colored*. And of course, ideally we still want whitespace checking for the + lines.
