From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Tim Cross Newsgroups: gmane.emacs.devel Subject: Re: Eliminating a couple of independent face definitions Date: Mon, 7 Feb 2011 12:30:55 +1100 Message-ID: References: <87oc6vm67v.fsf@stupidchicken.com> <87vd12z77n.fsf@stupidchicken.com> <87ipx289cu.fsf@nzebook.haselwarter.org> <1A6A06363E5F4274B2A00E2CA8A242D1@us.oracle.com> <1F1C73FF429943C6A882D1E03CC0BD23@us.oracle.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001485ebea38e58ea9049ba72eca X-Trace: dough.gmane.org 1297042273 17467 80.91.229.12 (7 Feb 2011 01:31:13 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 7 Feb 2011 01:31:13 +0000 (UTC) Cc: Philipp Haselwarter , emacs-devel@gnu.org To: Drew Adams Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Feb 07 02:31:09 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PmFwY-0003u9-4N for ged-emacs-devel@m.gmane.org; Mon, 07 Feb 2011 02:31:08 +0100 Original-Received: from localhost ([127.0.0.1]:38345 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmFwX-0005in-3Y for ged-emacs-devel@m.gmane.org; Sun, 06 Feb 2011 20:31:05 -0500 Original-Received: from [140.186.70.92] (port=35414 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmFwQ-0005hu-OT for emacs-devel@gnu.org; Sun, 06 Feb 2011 20:31:01 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmFwO-0000nd-MD for emacs-devel@gnu.org; Sun, 06 Feb 2011 20:30:58 -0500 Original-Received: from mail-iy0-f169.google.com ([209.85.210.169]:59564) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmFwO-0000nT-D3 for emacs-devel@gnu.org; Sun, 06 Feb 2011 20:30:56 -0500 Original-Received: by iyi20 with SMTP id 20so4693722iyi.0 for ; Sun, 06 Feb 2011 17:30:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=fgPSV4Y9H6ioypTFuNONQhkM0Z0Rjp98XsyL5UWuqy4=; b=geDpujxpEbvW5NNlbyduXE1ISbxXC0ocln7ub+jC9bPziZ0agVLmXurn/HxYb7ik8q AWFWaB3MqvP3JmFXHDRgbZCoZXuWH5R8XZ0Fv9pXud+0ES/5kcorVoCkmJBb2abNp7l+ ZgXV2BlcL0sDFA+21JArPE9pGV3x5dlerQR70= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=YBM0M94iUeifBP2vby8ot2AwtlAUBscs8KztgRnNq3S/WbO7HVFU2R9FewXuD1A5UR prf3TNdMLMVF2uHhKZ4Lwro+SuunvS8q0fFbv6/0WPm33ty1Z4p7LEgfIhOS8arDFFzg rLulehRazg8znKraV0oD1gvcOa/Wcga2z+v6s= Original-Received: by 10.231.59.149 with SMTP id l21mr16460164ibh.196.1297042255482; Sun, 06 Feb 2011 17:30:55 -0800 (PST) Original-Received: by 10.231.19.67 with HTTP; Sun, 6 Feb 2011 17:30:55 -0800 (PST) In-Reply-To: <1F1C73FF429943C6A882D1E03CC0BD23@us.oracle.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.210.169 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:135662 Archived-At: --001485ebea38e58ea9049ba72eca Content-Type: text/plain; charset=ISO-8859-1 On Mon, Feb 7, 2011 at 11:59 AM, Drew Adams wrote: > 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. > > Ignoring implementation issues, your suggestion appears to meet the main objective i.e. making it easier for programmers to do the right thing and have fully defined faces without having to jump through all the hoops. I would agree that it is a better solution than using inheritance with semantically unrelated parents as it does appear to provide a solution that does not compromise the strict form of preferred inheritance and a way to improve the quality of default face values. I would suggest that in addition to such enhancements to defface, we need to provide guidelines in the manual on how to use inheritance and copy and would go further and argue that for semantically related faces, inheritance *should* be used to help enhance consistency and allow users to customize a semantic 'class' in one go. Tim --001485ebea38e58ea9049ba72eca Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Mon, Feb 7, 2011 at 11:59 AM, Drew Ad= ams <drew.ada= ms@oracle.com> wrote:
Tim pointed out the problem of faces that are not fully defined, in particu= lar,
for light and dark backgrounds (and for tty). =A0He pointed to the need for= the
Emacs source code to define all faces fully so that they work, out-of-the-b= ox,
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 thro= ugh
hoops.

I agreed with Tim that it is good to fully define faces. =A0Not doing so ma= kes
users customize more than they should need to. =A0But I also explained why<= br> inheritance can be overkill as a solution to this problem. =A0Inheritance s= hould
be used when the inheriting face is related to the inherited face - similar=
use/meaning/purpose. =A0It should (generally) be avoided when there is no s= uch
relation.

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

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

Why not introduce a new `defface' keyword `:copy-default'? =A0It wo= uld define the
new face with the same default attribute values as another face. =A0A copy = of
those values (actually, of the attributes spec) would be made at `defface&#= 39; time.
The two faces would remain independent instead of being joined at the hip. = =A0Only
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 ins= tead of
this (which is identical to the `defface' for `font-lock-doc-face):

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

You would use this:

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

The new face `barred-foo' would have no relation to `font-lock-string-f= ace'. =A0A
user could customize the latter without that change affecting the former.
The result of the `defface' would be identical to this complex definiti= on (taken
from the definition of `font-lock-string-face'):

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

Simple for even lazy programmers to use. =A0Not so error-prone for eager programmers who might otherwise try to make such a copy by hand. =A0Guarant= eed 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'. =A0The same `defface' could use both `:inherit'= ; and
`:copy-default', to inherit some attribute values from one face and cop= y 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. =A0(`font-lock-doc-fa= ce'
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 b= e
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 s= ense.
But it is less likely that there is an existing face outside of that contex= t
whose use/meaning/purpose is related. =A0IOW, I see inheritance as most use= ful
within a library or among related libraries.

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

T> I would not argue that inheritance is an ideal solution to
T> this problem, =A0However, I do think it can be part of the
T> solution. =A0Perhaps something along the lines of
T>
T> =A0* Establish guidelines on how to use inheritance i.e. how to
T> =A0 =A0select which face to inherit from
T> =A0* Define a good (not too large) set of base faces. =A0Existing
T> =A0 =A0font-lock faces may be sufficient, maybe not. =A0Would need T> =A0 =A0review.
T> =A0* Require all face definitions in core emacs packages to
T> =A0 =A0either fully define a face (i.e. definition for dark/light, T> =A0 =A0tty, X mac ms etc) OR inherit from a base face (assuming
T> =A0 =A0all base faces are fully defined)
T> =A0* Add a section to the manual encouraging developers to
T> =A0 =A0either provide a fully defined face or inherit from a base
T> =A0 =A0face, but don't just define a single (usually) light
T> =A0 =A0background 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.

Ignoring implementation issues, your suggest= ion appears to meet the main objective i.e. making it easier for programmer= s to do the right thing and have fully defined faces without having to jump= through all the hoops. I would agree that it is a better solution than usi= ng inheritance with semantically unrelated parents as it does appear to pro= vide a solution that does not compromise the strict form of preferred inher= itance and a way to improve the quality of default face values.

I would suggest that in addition to such enhancements to def= face, we need to provide guidelines in the manual on how to use inheritance= and copy and would go further and argue that for semantically related face= s, inheritance *should* be used to help enhance consistency and allow users= to customize a semantic 'class' in one go.

Tim
--001485ebea38e58ea9049ba72eca--