GNU bug report logs - #33567
Syntactic fontification of diff hunks

Previous Next

Full log

View this message in rfc822 format

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 33567 <at> debbugs.gnu.org
Subject: bug#33567: Syntactic fontification of diff hunks
Date: Fri, 21 Dec 2018 00:00:18 +0200

>>> 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.

I don't know.  We have two options for tty: highlight indicators only
or use red/green foreground without syntax highlighting.

>> 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.

By default it used the black foreground.  Only Magit uses red/green foreground.

> 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.

Maybe with conditional face definitions like

(if (boundp 'diff-font-lock-syntax)
    (defface blabla))

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.