Subject: bug#6117: 24.0.50; dired-marked-face same as dired-flagged-face

From: Juri Linkov <[email-masked]>
Date: Wed, 17 Aug 2011 22:35:45 +0300

> It is simply a bad idea to inherit from a font-lock face here.

Yes, basically the inheritance is a convenient tool to allow the user
to modify a set of faces by editing one parent face, but only if
these faces are semantically related.

Font-lock faces semantically correspond to the entities existing in
programming languages like comments, strings, keywords, function-names,
variable-names, constants.

So to create a reasonable face hierarchy, we should introduce faces with
a universal color code like traffic lights where the red color denotes
a dangerous activity or errors, the yellow/amber color denotes a caution,
and the green color denotes a successful activity.

In the patch below, I've removed the regression and restored traditional
pre-24 colors in compilation and dired, and also changed `dired-flagged'
to inherit from the `error' face, and `dired-marked' from `warning'.

There are more places that need these new faces to indicate
error/warning/success, e.g. battery.el and other packages
unrelated to font-lock that currently abuse font-lock faces.
They could be modified later.

=== modified file 'lisp/dired.el'
--- lisp/dired.el	2011-08-10 08:40:46 +0000
+++ lisp/dired.el	2011-08-17 19:31:50 +0000
@@ -342,7 +342,7 @@ (defvar dired-mark-face 'dired-mark
   "Face name used for dired marks.")
 
 (defface dired-marked
-  '((t (:inherit font-lock-warning-face)))
+  '((t (:inherit warning)))
   "Face used for marked files."
   :group 'dired-faces
   :version "22.1")
@@ -350,7 +350,7 @@ (defvar dired-marked-face 'dired-marked
   "Face name used for marked files.")
 
 (defface dired-flagged
-  '((t (:inherit font-lock-variable-name-face)))
+  '((t (:inherit error)))
   "Face used for files flagged for deletion."
   :group 'dired-faces
   :version "22.1")

=== modified file 'lisp/progmodes/compile.el'
--- lisp/progmodes/compile.el	2011-07-25 08:23:29 +0000
+++ lisp/progmodes/compile.el	2011-08-17 19:32:45 +0000
@@ -647,19 +647,19 @@ (defvar compilation-environment nil
 (defvar compile-history nil)
 
 (defface compilation-error
-  '((t :inherit font-lock-warning-face))
+  '((t :inherit error))
   "Face used to highlight compiler errors."
   :group 'compilation
   :version "22.1")
 
 (defface compilation-warning
-  '((t :inherit font-lock-variable-name-face))
+  '((t :inherit warning))
   "Face used to highlight compiler warnings."
   :group 'compilation
   :version "22.1")
 
 (defface compilation-info
-  '((t :inherit font-lock-type-face))
+  '((t :inherit success))
   "Face used to highlight compiler information."
   :group 'compilation
   :version "22.1")

=== modified file 'lisp/faces.el'
--- lisp/faces.el	2011-08-10 18:17:21 +0000
+++ lisp/faces.el	2011-08-17 19:31:45 +0000
@@ -2427,6 +2427,39 @@ (defface glyphless-char
 It is used for characters of no fonts too."
   :version "24.1"
   :group 'basic-faces)
+
+(defface error
+  '((((class color) (min-colors 88) (background light)) (:foreground "Red1" :weight bold))
+    (((class color) (min-colors 88) (background dark)) (:foreground "Pink" :weight bold))
+    (((class color) (min-colors 16) (background light)) (:foreground "Red1" :weight bold))
+    (((class color) (min-colors 16) (background dark)) (:foreground "Pink" :weight bold))
+    (((class color) (min-colors 8)) (:foreground "red"))
+    (t (:inverse-video t :weight bold)))
+  "Face used to highlight errors."
+  :version "24.1"
+  :group 'basic-faces)
+
+(defface warning
+  '((((class color) (min-colors 16)) (:foreground "Orange" :weight bold))
+    (((class color)) (:foreground "cyan" :weight bold))
+    (t (:weight bold)))
+  "Face used to highlight warnings."
+  :version "24.1"
+  :group 'basic-faces)
+
+(defface success
+  '((((class color) (min-colors 16) (background light))
+     (:foreground "Green3" :weight bold))
+    (((class color) (min-colors 88) (background dark))
+     (:foreground "Green1" :weight bold))
+    (((class color) (min-colors 16) (background dark))
+     (:foreground "Green" :weight bold))
+    (((class color)) (:foreground "green" :weight bold))
+    (t (:weight bold)))
+  "Face used to indicate success."
+  :version "24.1"
+  :group 'basic-faces)
+
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Manipulating font names.

=== modified file 'lisp/font-lock.el'
--- lisp/font-lock.el	2011-07-14 13:50:25 +0000
+++ lisp/font-lock.el	2011-08-17 19:32:27 +0000
@@ -1959,12 +1959,7 @@ (defface font-lock-constant-face
   :group 'font-lock-faces)
 
 (defface font-lock-warning-face
-  '((((class color) (min-colors 88) (background light)) (:foreground "Red1" :weight bold))
-    (((class color) (min-colors 88) (background dark)) (:foreground "Pink" :weight bold))
-    (((class color) (min-colors 16) (background light)) (:foreground "Red1" :weight bold))
-    (((class color) (min-colors 16) (background dark)) (:foreground "Pink" :weight bold))
-    (((class color) (min-colors 8)) (:foreground "red"))
-    (t (:inverse-video t :weight bold)))
+  '((t :inherit error))
   "Font Lock mode face used to highlight warnings."
   :group 'font-lock-faces)