unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Juri Linkov <juri@jurta.org>
To: "Drew Adams" <drew.adams@oracle.com>
Cc: 6117@debbugs.gnu.org, 'Lars Magne Ingebrigtsen' <larsi@gnus.org>,
	'Chong Yidong' <cyd@stupidchicken.com>,
	jidanni@jidanni.org
Subject: bug#6117: 24.0.50; dired-marked-face same as dired-flagged-face
Date: Wed, 17 Aug 2011 22:35:45 +0300	[thread overview]
Message-ID: <87ippvhm3g.fsf@mail.jurta.org> (raw)
In-Reply-To: <D1102E5FD3D84F7AA83527450E8C004B@us.oracle.com> (Drew Adams's message of "Wed, 10 Aug 2011 06:45:46 -0700")

> 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)
+
 \f
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; 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)
 






  reply	other threads:[~2011-08-17 19:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-05 20:31 bug#6117: 24.0.50; dired-marked-face same as dired-flagged-face jidanni
2010-05-06 13:17 ` Stefan Monnier
2010-05-06 15:20   ` Drew Adams
2011-07-13 21:28   ` Lars Magne Ingebrigtsen
2011-07-13 22:28     ` Drew Adams
2011-07-16 19:53     ` Lars Magne Ingebrigtsen
2011-08-02 19:05       ` Lars Magne Ingebrigtsen
2011-08-10  9:45         ` Juri Linkov
2011-08-10 13:45           ` Drew Adams
2011-08-17 19:35             ` Juri Linkov [this message]
2011-08-17 21:36               ` Chong Yidong
2011-08-18 11:46                 ` Juri Linkov
2011-08-24 18:09                   ` Juri Linkov
2011-08-24 18:16                     ` Drew Adams
2010-05-06 20:57 ` Juri Linkov
2010-05-07  2:43   ` Drew Adams
2010-05-07 18:16     ` Juri Linkov
2010-05-10 18:45 ` bug#6163: 24.0.50; be sure the color of the mark matches that of the filename jidanni
2011-08-02 16:00   ` Lars Magne Ingebrigtsen
2011-08-17 20:02 ` bug#6117: 24.0.50; dired-marked-face same as dired-flagged-face jidanni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ippvhm3g.fsf@mail.jurta.org \
    --to=juri@jurta.org \
    --cc=6117@debbugs.gnu.org \
    --cc=cyd@stupidchicken.com \
    --cc=drew.adams@oracle.com \
    --cc=jidanni@jidanni.org \
    --cc=larsi@gnus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).