all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Drew Adams" <drew.adams@oracle.com>
To: "'Tim Cross'" <theophilusx@gmail.com>
Cc: 'Philipp Haselwarter' <philipp.haselwarter@gmx.de>, emacs-devel@gnu.org
Subject: RE: Eliminating a couple of independent face definitions
Date: Sun, 6 Feb 2011 16:59:18 -0800	[thread overview]
Message-ID: <1F1C73FF429943C6A882D1E03CC0BD23@us.oracle.com> (raw)
In-Reply-To: <1A6A06363E5F4274B2A00E2CA8A242D1@us.oracle.com>

Tim pointed out the problem of faces that are not fully defined, in particular,
for light and dark backgrounds (and for tty).  He pointed to the need for the
Emacs source code to define all faces fully so that they work, out-of-the-box,
regardless of default background etc.

And he pointed to the fact that, given such a basis, inheritance can carry the
ball forward so that newly created faces are more likely, themselves, to be
fully defined - without programmers needing to be face experts or jump through
hoops.

I agreed with Tim that it is good to fully define faces.  Not doing so makes
users customize more than they should need to.  But I also explained why
inheritance can be overkill as a solution to this problem.  Inheritance should
be used when the inheriting face is related to the inherited face - similar
use/meaning/purpose.  It should (generally) be avoided when there is no such
relation.

Inheritance does a good job of enforcing full face definition (given solid
starting points).  Can we get the same benefit some other way - a way that is
not error prone by relying on programmers to copy full definitions by hand etc.?

Sure - just use a copy instead of a pointer.  What's bad about inheriting an
unrelated fully-defined face is that customizing that face also changes the
inheriting face.  But that only happens because the new face always points to
its ancestor for its attribute values.

Why not introduce a new `defface' keyword `:copy-default'?  It would define the
new face with the same default attribute values as another face.  A copy of
those values (actually, of the attributes spec) would be made at `defface' time.
The two faces would remain independent instead of being joined at the hip.  Only
the _default_ attribute values would be used; the current values of the
reference face would have no effect at any time.

If face `barred-foo' is unrelated to font-locking and strings, then instead of
this (which is identical to the `defface' for `font-lock-doc-face):

(defface barred-foo '((t :inherit font-lock-string-face))
  "Face to use for foos that are barred."
  :group 'foobar)

You would use this:

(defface barred-foo '((t :copy-default font-lock-string-face))
  "Face to use for foos that are barred."
  :group 'foobar)

The new face `barred-foo' would have no relation to `font-lock-string-face'.  A
user could customize the latter without that change affecting the former.

The result of the `defface' would be identical to this complex definition (taken
from the definition of `font-lock-string-face'):

(defface barred-foo 
  '((((class grayscale) (background light))
     (:foreground "DimGray" :slant italic))
    (((class grayscale) (background dark))
     (:foreground "LightGray" :slant italic))
    (((class color) (min-colors 88)
     (background light)) (:foreground "VioletRed4"))
    (((class color) (min-colors 88)
     (background dark))  (:foreground "LightSalmon"))
    (((class color) (min-colors 16)
     (background light)) (:foreground "RosyBrown"))
    (((class color) (min-colors 16)
     (background dark))  (:foreground "LightSalmon"))
    (((class color) (min-colors 8))
     (:foreground "green"))
    (t (:slant italic)))
  "Face to use for foos that are barred."
  :group 'foobar)

Simple for even lazy programmers to use.  Not so error-prone for eager
programmers who might otherwise try to make such a copy by hand.  Guaranteed to
be as reasonable for all backgrounds and tty as is the tried-and-true
`font-lock-string-face'.

Anywhere you might use `:inherit' in a face definition you could use
`:copy-default'.  The same `defface' could use both `:inherit' and
`:copy-default', to inherit some attribute values from one face and copy others
(defaults) from another.

We would encourage programmers to use `:inherit' when the new face (its
use/meaning/purpose) is related to the referenced face - that is, when they want
a change in the latter to be reflected in the former.  (`font-lock-doc-face'
inherits from `font-lock-string-face' because they are related.)

We would encourage them to use `:copy-default' when the referenced face is
unrelated and all they want to do is reuse its default-attributes spec.
(`barred-foo' is about foos, not about font-locking or strings.)

I expect that the latter case is more common than the former, but I could be
wrong.

New faces are often created together, as a group in some library, and these are
often related in terms of use/purpose, so inheritance among them can make sense.
But it is less likely that there is an existing face outside of that context
whose use/meaning/purpose is related.  IOW, I see inheritance as most useful
within a library or among related libraries.

It's worth quoting Tim again here.  Having `:copy-default' in addition to
`:inherit' would improve the solution, I think.

T> I would not argue that inheritance is an ideal solution to
T> this problem,  However, I do think it can be part of the
T> solution.  Perhaps something along the lines of
T> 
T>  * Establish guidelines on how to use inheritance i.e. how to
T>    select which face to inherit from
T>  * Define a good (not too large) set of base faces.  Existing
T>    font-lock faces may be sufficient, maybe not.  Would need
T>    review.
T>  * Require all face definitions in core emacs packages to
T>    either fully define a face (i.e. definition for dark/light,
T>    tty, X mac ms etc) OR inherit from a base face (assuming
T>    all base faces are fully defined)
T>  * Add a section to the manual encouraging developers to
T>    either provide a fully defined face or inherit from a base
T>    face, but don't just define a single (usually) light
T>    background face
T> 
T> The key here is that all faces in core emacs packages would
T> end up with a fully defined face, either explicitly or via
T> inherit.




  reply	other threads:[~2011-02-07  0:59 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-02  4:05 Eliminating a couple of independent face definitions Chong Yidong
2011-02-02  4:11 ` Lennart Borgman
2011-02-02  5:02   ` Tim Cross
2011-02-02 15:15     ` Drew Adams
2011-02-02 17:17     ` Chong Yidong
2011-02-02 20:33       ` Philipp Haselwarter
2011-02-02 23:01         ` Lennart Borgman
2011-02-03 19:10         ` Drew Adams
2011-02-04  0:12           ` Tim Cross
2011-02-05 22:11             ` Drew Adams
2011-02-07  0:59               ` Drew Adams [this message]
2011-02-07  1:30                 ` Tim Cross
2011-02-07 14:09                   ` Drew Adams
2011-02-07 21:14                     ` Tim Cross
2011-02-07 22:12                       ` Drew Adams
2011-02-08  3:51                         ` Tim Cross
2011-02-08 15:26                           ` Drew Adams
2011-02-08 19:10                             ` Philipp Haselwarter
2011-02-08 13:58                 ` Davis Herring
2011-02-08 14:33                   ` Drew Adams
2011-02-08 15:34                     ` Davis Herring
2011-02-08 16:16                       ` Drew Adams
2011-02-08 17:40                         ` Lennart Borgman
2011-02-08 19:10                         ` Davis Herring
2011-02-07  1:08               ` Tim Cross
2011-02-04  0:18           ` Stephen J. Turnbull
2011-02-04  3:55             ` John Yates
2011-02-04  4:56               ` Stephen J. Turnbull
2011-02-04  4:57             ` Jambunathan K
2011-02-05 22:09             ` Drew Adams
2011-02-06  7:11               ` Stephen J. Turnbull
2011-02-04 10:26           ` Julien Danjou
2011-02-04 17:57             ` color-complement for defface (was: Eliminating a couple of independent face definitions) Ted Zlatanov
2011-02-14 18:11               ` color-complement for defface Ted Zlatanov
2011-02-20 17:44                 ` Julien Danjou
2011-03-10 19:09                   ` Ted Zlatanov
2011-03-10 19:11                     ` Ted Zlatanov
2011-02-02 21:24       ` Eliminating a couple of independent face definitions Tim Cross
2011-02-03 16:14       ` Dan Nicolaescu
2011-02-02 17:16   ` Chong Yidong
2011-02-02  9:58 ` Štěpán Němec
2011-02-02 17:05   ` Chong Yidong
2011-02-02 10:05 ` Julien Danjou

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

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

  git send-email \
    --in-reply-to=1F1C73FF429943C6A882D1E03CC0BD23@us.oracle.com \
    --to=drew.adams@oracle.com \
    --cc=emacs-devel@gnu.org \
    --cc=philipp.haselwarter@gmx.de \
    --cc=theophilusx@gmail.com \
    /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 external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.