[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#33567: Syntactic fontification of diff hunks

From: Dmitry Gutov
Subject: bug#33567: Syntactic fontification of diff hunks
Date: Thu, 20 Dec 2018 00:50:15 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:64.0) Gecko/20100101 Thunderbird/64.0

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]