bug#33567: Syntactic fontification of diff hunks

[Dmitry Gutov]

On 19.12.2018 23:49, Juri Linkov wrote:
> > Anyway, to be clear, and other considerations aside, this works:
> > [...]
> >   (defface diff-added
> > @@ -315,9 +313,7 @@ diff-added
> >       (((class color) (min-colors 88) (background light))
> >        :background "#ddffdd")
> >       (((class color) (min-colors 88) (background dark))
> > -     :background "#335533")
> > -    (((class color))
> > -     :foreground "green"))
> > +     :background "#335533"))
> >     "`diff-mode' face used to highlight added lines.")
> > [...]
> >   (defface diff-indicator-added
> >     '((default :inherit diff-added)
> >       (((class color) (min-colors 88))
> > -     :foreground "#22aa22"))
> > +     :foreground "#22aa22")
> > +    (((class color))
> > +     :foreground "green"))
> >     "`diff-mode' face used to highlight indicator of added lines (+, >)."
> >     :version "22.1")
> >   (defvar diff-indicator-added-face 'diff-indicator-added)
>
> This looks good.

Should I install it? Nobody has commented on my earlier stated concerns, 
but maybe we should just push it and see how it plays out.

> For the same reason we have the face font-lock-comment-delimiter-face
> separate from font-lock-comment-face to use colors only on the former,
> but not on the latter on tty with 8 colors to make easier to read comments.

Yeah, it's totally fine to use separate faces. And I was happy to see 
diff-indicator-* were already defined and in use.

My concerns were different, though:

1. Is it okay to use the black foreground inside diff hunks even when 
diff-font-lock-syntax is nil? It's an incompatible change.

2. Even if we change the default in diff-added and diff-removed, some 
themes might have their foregrounds customized, so those users won't 
notice the change. It will trickle down to the themes eventually, I 
think, but it's unclear how the theme authors will choose to deal with 
this change while keeping compatibility with previous Emacs releases.