Re: [PATCH v3 16/20] range-diff --dual-color: work around bogus white-space warning

Hi Junio,

On Mon, 9 Jul 2018, Junio C Hamano wrote:

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.