bug#61396: diff mode could distinguish changed from deleted lines

[Dmitry Gutov <dmitry@gutov.dev>]

[-- Attachment #1: Type: text/plain, Size: 3321 bytes --]

On 13/09/2023 17:51, Stefan Monnier wrote:
>> Not quite ready indeed: the new option is unused (implied to be t, I guess).
> 
> Oops :-)
> This said, it's not its only problem: the name of that var sucks as well.
> 
>> Just a single piece of feedback: I get where the idea is coming from (and
>> it's good in theory), but I'm not loving the added bright spots of color
>> where there previously were just toned down lighter backgrounds.
> 
> Yeah, I'm unsure about that.  I'm also somewhat annoyed by the extra
> attention it brings to those "boring" additions and removals, but I'm
> wondering if it's really because I want them to look more dull or if
> it's just because of habit.

My view on this, is it's good to have decent contrast of foreground to 
background, more-or-less constant across the program's UI. The bright 
spots are kind of annoying because of the color calling for attention, 
but it also lowers the said contrast.

When the syntactic fontification of hunks was added (bug#33567) we went 
through a couple of rounds of toning down the existing backgrounds, so 
that they are less in-your-face, while still easy to discern. Simply 
dropping the -refine- faces on top of those would not just counter-act 
that change, but go in reverse.

> I have been bitten several times in the past when going through largish
> diffs where I overlooked important things in the added/removed parts
> because they were colored the same was as the unchanged parts of
> changed lines and so I just glossed over them.

I don't remember being bit by this myself, but it does sound like a problem.

>> If it were indicated differently somehow (though I'm not sure how), perhaps
>> I'd like it more. As it is, though, the added value (quite minor since it's
>> easy to see which hunk is "pure addition" already) doesn't seem to balance
>> out the inconvenience.
> 
> Yeah, maybe I'd prefer colors that are halfway between
> `diff-added/removed` and `diff-refine-added/removed`?
> [ Wish we had dynamically-computed face colors for that.  ]

Toning the -refine- faces down could be an option. It'll be a balance 
between making them less in-your-face and harder to notice overall 
(example: diff-refine-added-ddffdd.png).

Some other possibilities:

- In this refinement mode, toning down the "base" backgrounds instead, 
while using the current colors for -refine- faces. This is probably a 
dead end, though: the distance until white is too small, not enough to 
find a good contrast (example: diff-added-f9fff9.png). Might as well use 
white or diff-context grey, I guess.

- Like Samuel mentioned, attenuate the indicators' column. Except 
instead of inverse video just apply the refine faces? See 
diff-refine-indicators.png. Looks good to me color-wise, though the 
change in the indication method is somewhat an inconsistency.

- Use some added border around the hunk in green/red (using the color of 
diff-indicator-*). Possibly combined with the previous item. The 
drawback is the same, and in addition this might not work on the 
terminal (?). See diff-define-borders.png; these line were done using 
overline/underline so there was no way to make it thicker, but there 
must be other methods, e.g. like we do the separator line when writing 
the commit message (although that one will create a vertical offset).

[-- Attachment #2: diff-refine-added-ddffdd.png --]
[-- Type: image/png, Size: 189465 bytes --]

[-- Attachment #3: diff-added-f9fff9.png --]
[-- Type: image/png, Size: 186309 bytes --]

[-- Attachment #4: diff-refine-indicators.png --]
[-- Type: image/png, Size: 192024 bytes --]

[-- Attachment #5: diff-refine-borders.png --]
[-- Type: image/png, Size: 194002 bytes --]