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.
