On Sun, Feb 6, 2011 at 9:11 AM, Drew Adams <drew.adams@oracle.com> wrote:
T> One thing I would recommend doing, if possible, would be to
T> spend some time running emacs with a dark background and on a
T> tty.  This would likely reveal how bad some default face
T> definitions are and let you see how tedious it is having to
T> customize lots of individual face definitions just to get ones
T> that work.

I'm not sure what your disagreements are with what I said.  I
agree that poorly defined default face definitions are a real
problem.  I even suggested that they are likely _the_ real
problem here.

I think our point of disagreement is small - it is really a matter of degrees. Your position is one I would characterise as representing a pure or strict view of inheritance and one I see as having a strong ideological and theoretical base. Mine is possibly chracterised as being more pragmatic and less pure or strict, possibly characterised as being more liberal. 

There are also a number of assumptions and conclusions you ahve made regarding some of my points, which are incorrect. I will try to make my position with these clearer.

Given reasonable face defaults that work out of the box on
both dark and light backgrounds, I think many if not most
(perhaps all?) of the problems you cited go away.  With faces
that just work by default, regardless of the background, there
is no need to "customize lots of individual face definitions
just to get ones that work".

Agreed. If all faces were well defined there would be no problem. However, the reality is that frequently they are not. My suggestions are based on the belief that this is because defining good default face settings is not easy, takes more effort than some developers want to expend, is difficult to do right as developers don't necessarily have access to all the environments involved and it may be unreasonable to expect all developers have the necessary knowledge and experience to know/identify a good default face setting. Therefore, my focus is on what we can do to make it easier for developers to do the right thing. One solution which I think would work towards this would be to develop a set of guidelines on how to identify and select appropriate faces to inherit from and pitfalls to avoid combined with requiring that faces are either fully defined or inherit from a  core/base face which is fully defined.

I'm not arguing that faces with similar appearance should be linked via inheritance and I'm not arguing that existing fully defined faces should be converted to use inheritance. 

The main point of disagreement concerns determining appropriate faces to inherit from. Your position is that inheritance should only be used to inherit from a face with the same/similar semantics/meaning as the child face. My argument is less restrictive. If a core face exists that has the same semantic meaning as the child face, then of course that is the obvious choice. However, if there are no core faces with the same semantic meaning I would rather have the face inherit from a fully defined face with a different semantic meaning, provided that does not cause confusion within the mode used by the child face, rather than have a face that is not fully defined. 

I would go further and suggest that while you accept the use of inheritance provided the inheritance chain follows the same semantic meaning, you would prefer in general not to use inheritance even if a good candidate with the same semantic meaning does exist in the core set of fully defined faces. While I would never argue that developers should be forced to use inheritance, I would suggest this is the wrong approach. I believe inheritance should definitely be used when there is clear semantic equivalents within the core set because this will help ensure consistency in representation of semantically related items. For example, I currently have numerous definitions for 'hyperlink' type objects as well as clickable buttons. I would like objects of these types to have the same appearance in all modes. However, I like hyperlinks to be a specific colour, with underlining, bold and italic fonts. To achieve this (possibly unusual) appearance in a consistent manner, I have to customize multiple faces. However, if inheritance was used, I would only have to do this once.

If I understand your position regarding inheritance, you disagree with the concept of a child face inheriting from a semantically unrelated face because you feel doing so will lead to confusion and unexpected results because it 'muddies' the semantics of the inheritance and will cause unexpected/unwanted side effects when a core face definition is changed. I don't see it as being that straight-forward. I agree that badly chosen inheritance would cause problems - for example, having something that was not a clickable link inherit from the link face. On the other hand, core packages, such as dired, have been using inheritance from sematically unrelated faces for some time and I'm not aware it has caused problems. The dired-directory face inherits from font-lock-function-name-face. There is no 'semantic' link between these two faces, yet having this inheritance apparently works. Yes, if I change the value of font-lock-function-name-face, it will change the appearance of directory names in dired. However, in practice, this is rarely a problem. In fact, if your changing the face to make it fit better with some theme or to work better in your environment, it is likely that this change would fit with your overall objectives anyway. 

One of the reasons I think this works is because the choice is unrelated and does not confuse contexts within the mode the face is used in. If dired had a face to represent function names, then using font-lock-function-name-face for directory names would be a bad idea. However, dired does not have such a context and therefore does not cause confusion within that mode. Furthermore, I don't believe it would cause confusion with modes that use font-lock-function-name-face for function names. The two modes are distinct enough that it is clear that a face with X attributes in dired is different to a face with the same attributes in completely unrelated modes, such as emacs-lisp-mode. 

There are some other benefits to using inheritance in this manner. I think it is reasonable to suggest that all users have some underlying 'theme' they want to achieve with face settings. Some themes may seem unusual or even unpleasant, but there is usually some overriding pattern - faces are not selected randomly (I guess there may be some extreme cases where people might do this, but it isn't the 'norm'). Within any theme there is a finite set of usable faces i.e. faces that can be read, fit with the theme and are aesthetically acceptable. As we increase the number of individual face definitions, we also increase the number of faces with different names/meanings which have the same set of attributes as other unrelated faces. 

If we assume two basic approaches, your pure inheritance model and my less rigid inheritance approach, we end up with two vary different situations when it comes to the user customizing faces to obtain their desired theme. Under your approach, they will need to customize a larger number of faces to obtain their desired outcome.  However, under my approach, which is more likely to make greater use of inheritance because it is not restricted to only using semantically related faces, it is more probable that less faces will need to be defined. 

Of course, we could probably achieve a better 'middle ground' with a larger, more comprehensive set of core/base faces as this would increase the likelihood that a child face can find a sematically related core/base face definition. I have nothing against this and included it as part of my suggestion on how to improve matters and reduce the number of poorly defined faces. 
 
T> As I see it, we want to achieve four main goals. We want to
T> make it easier for developers to set appropriate defaults, we
T> want to get the best default possible for end users,
T> regardless of platform or light/dark background preferences,
T> we want to make it as easy as possible for end users to
T> customize their environment

Agree.

T> and we want to, if possible, avoid a proliferation of
T> different face definitions, especially when the definitions
T> don't really differ in anything other than name as this makes
T> customizing and theme definition more complex than it needs to
T> be.

Disagree, if by "don't really differ in anything other than
the name" you are referring only to the `defface' default
attribute values.

I am in favor of there being two faces instead of one, if
their use/meaning is different, even if their definitions
modulo the names are identical.  In particular, this
facilitates separate customization.

I was actually thinking of faces that have the same meaning, different names and different default definitions. This is a growing problem. We end up with inconsistency between modes for faces representing the same type of object. It was not meant to mean faces that happen to have the same attribute sets. For example, I have a number of modes that have the same concept of a clickable 'button'. In some modes they look different to how buttons look in other modes. I want them to all look the same and have to customize them individually. This is what I meant by the proliferation of faces.

 
It avoids coupling that is not logical and not necessary.
Inheritance (hence coupling) based on physical attributes
(appearance) can perhaps occasionally be appropriate, but in
general it is a no-no.

Yes, where possible, inheritance should be used to link semantically related faces . However, linking semantically unrelated faces is not as bad in practice as you suggest and could be used to improve the end user expereince without causing the confusion you suggest. Yes, there are dangers, but there are also benefits which I think outweigh the downside of a strict inheritance approach, especially if there are only a few core/base fonts to inherit from. 

(Names are generally indicative of meaning, but yes, there
could be two faces with different names yet identical meaning.
In that case, yes, the duplication could be eliminated.)

(There is an orthogonal reason to avoid using too many faces
at the same time: performance.  But that is a different story
altogether.)

T> It was good you pointed out that faces are not just about
T> colour.  Sometimes, I think peopoe forget this and only focus
T> on the colour aspect.  However, this is understandable as
T> colour is what distinguishes most faces from each other.
T>
T> We need to acknowledge how difficult it is to define good
T> default faces.  Many definitions fail to do this.  While your
T> suggestion of using colour compliments can help, I'm not
T> convinced it is always that easy.  One problem is that
T> different environments have different sets of colours to work
T> with.  For example, if you work under the linux console, you
T> don't have compliments for most of the colours available under
T> other environments, such as X or (I presume) Windows/Mac.  The
T> problem is further complicated because most developers won't
T> have access to many of these environments, so can't easil test
T> and design appropriate faces even if motivated to do so.

Agree.  I did not mean to raise complementing as any kind of
cure-all - far from it.  I was just mentioning that I provided
a quick-and-dirty set of default settings for dark backgrounds
for a particular library by flipping the light-background
defaults to their complements.  Obviously, any set of defaults
for either light or dark deserves to be well thought out.  And
all of the considerations you mention should be taken into
account.

The tty thing is a big limitation, in particular.  Designing a
set of faces for use on tty only is a different task from
designing a set for use with many colors and other attributes.

For a library that is intended to work with both tty and
graphic windows, either tradeoffs are called for or
essentially two modes or face schemes are needed: full and
degraded (tty).  We have constructs like `(type tty pc)' that
can help with that.

The tty face values are a little tricky because you have a much smaller set of attributes to work with and the attributes you do have have less options. However, the defface macro has the ability to handle this and while it may be a pain to do, it is doable and should be part of any fully defined face definition. I would suggest there are very few packages which are only designed to run within a GUI environment (e.g. doc-view I guess, can't think of many others). 
 
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.

Fully-defined, yes - good.  The part that I think can be
misguided is basing everything on inheritance from faces that
are not necessarily related in terms of use or meaning.

Nobody has suggested basing everything on inheritance and it has not been suggested that inheritance should be forced on anyone (though, as outlined above, there could be a strong argument for forcing inheritance for clearly semantically related faces). The suggestion is that if you don't want to fully define a face, then use inheritance RATHER than a partially defined face.
 
Basing a face on a related face is not bad.  Stretching things
to base all faces on some "basic" faces, no matter how
unrelated in terms of meaning or context, is not a great idea.

Nobody has suggested basing *all* faces on some base set. 
 
Faces, including "basic" faces, are customizable.  You don't
want a user who customizes `font-lock-doc-face' to suddenly
find that s?he has also affected a chain of unrelated faces
that inherit from it.  Customizing font-lock faces should not
affect Dired or Info or...

Except they already do! I don't see this as a 'real' problem, but more a theoretical one. Yes, changing face attributes in font-lock-* faces will change the look of dired faces, but does that really matter in practice? Maybe the dired faces should be fully defined and I would not have an issue with that. However, I'm glad they are using inheritance rather than only having partial definitions. 
.  
T> One of the reasons I like the use of inherit for face
T> definitions is mainly from an end user perspective. I would
T> rather have faces that are fully defined via inherit, even if
T> that inheritance seems odd, than have a partially defined face
T> which only works if you have a light background or are running
T> under a full gui environment.

Those are not the only two alternatives - it's not either-or.

I am in favor of full face definitions.  And I am not against
inheritance.  The important thing is for a face to inherit
only from a related face (in terms of use/meaning).

I would argue it is preferable rather than being required. 

The key is whether it will generally make sense for the
inheriting face if the inherited face gets customized.  If
not, then inheritance is likely not the best way to go.

I actually agree, except we would differ on what makes sense.
 
T> If, when I change the base face definition, it results in
T> unacceptable changes to some face which inherits from it, I
T> can customize it.

Sure you can, but now you're back to customizing all over the
place - and now across multiple libraries and multiple use
contexts, to boot.  The cure is at least as bad as the
disease.

No, its not as bad. If more faces inherited from fully defined core faces, then the faces I need to customize are only those which I find aesthetically unacceptable and does not include faces that are not fully defined and therefore are not even readable in my environment. Moreover, many of the faces I find aesthetically unacceptable are likely to have the same/similar attributes. I can change the base definition and have my changes propogate through all faces that currently have the unacceptable default. Without the use of inheritance, I have multiple faces to change. When they are not fully defined, many of those changes are needed just to make the face readable i.e. not just an aesthetic change, but a necessary one. 

Nothing wrong with inheritance.  But boiling everything down
to font-lock faces (or similar) is a bad idea.  Better for
face inheritance to reflect face usage/meaning: families
(inheritance hierarchies) of related faces, not just
everything inheriting from primordial faces `adam' and `eve'.

Why do you keep taking the extreme? Nobody has suggested that all faces inherit form some low base. This is not what I've suggested - either fully define or inherit, just don't have partial poorly defined faces that only work in a limited environment, such as a light background. 
 
T> My experience has been that this does sometimes happen, but
T> not that often and it is more preferable to fix up the few
T> faces than to spend much more time configuring lots of
T> individual faces.

I think you are again thinking of the fact that you are
needing to configure lots of faces because they were defined
incompletely or with poor defaults.  Take away that
presupposition and where is the argument?

To take away that presupposition, you need to come up with something that enables developers to do the right thing and not only use partial face definitions as the default. This is the whole point. Yes, theoretically, if all faces were fully defined, most of the problem would go away (though there are then other issues concerning consistency). The reality is that too often, faces are not fully defined. Inheritance would provide a simple way to have a fully defined face when the developer cannot or does not want to fully define the face themselves. 


T> In fact, as the number of individual face definitions has
T> increased, I actaully think we have gone the wrong direction.
T> We have too many faces which are mostly the same.

They are not the same just because they have the same
_appearance_ (by _default_ no less).  This is a key element in
the confusion, I think.  It sounds like it motivated Yidong's
search for faces to eliminate "duplication" via inheritance.

I acknowledge that. However, we have instances of faces that do represent the same semantic object, but which have different definitions in different modes i.e. clickable buttons etc. I have three different faces for representing dates (admittedly, these are add-on packages and not core ones), I have different faces for representing links etc. None of these use inheritance and in some cases, have different default values. In these instances, inheritance should be used. I then have the ability to set all like things to the same appearance and if I want, the option to change or augement them per mode. 
 
Faces are not the same as their (attribute) values.  Faces
have identity and purpose as well as values.

Agreed. We may differ on how far that identity reaches. I see dired-directory as having its own identity and its relevance is restricted to within dired mode. It inherits from a font-lock face, but I don't see that as part of its identity as such. You appear to see the face it inherits from as part of its identity as well. I'm not sure this is always so or necessary.  

This is essentially no different from having multiple user
options (or other variables) that have the same value: 42,
nil, t,...  You would not argue that we define a set of
"basic" user options and use inheritance to factor out any
"duplicate" options that have the same default value of 42 or
nil, would you?

You would not argue that we should have only one primeval user
option with a default value of 42, and inherit from it for
wildly different options with different use cases (meanings),
so that someone who customizes the "basic" option to change
the value 42 to 24 ends up changing lots and lots of
inheriting options.  Would you?  How are faces different in
this regard?

Now this is just plain silly and a very poor analogy . Faces have a lot more in common and a much closer relationship than al the user variables. Faces work together to present a theme, they are not independent as you imply. Faces that don't set an attribute inherit the value from the default face. You configure faces based on an overall theme and how they interact with each other and faces all have the same format/attributes. To suggest they are as indpendent from each other as other user variables is nonsense. 

Ignoring how poor the analogy is, the fact is that NOBODY has suggested that all faces need to inherit form a base set. This was never proposed.  

T> It seems now that every new package is adding its own face
T> definitions

Good.  That should probably happen more than it does. ;-)

If reasonable, related faces exist on which to base the new
face definitions, then by all means inheritance might well be
called for.  But a new package should _not_ avoid adding its
own face definitions, whether they are defined using
inheritance or not (i.e., pointer or copy).

Disagree. A package should only define its own face when either there is no existing face which adequately represents the meaining of the face OR there is a strong expectation that the user may want to customize the face independently. There is a definite danger of having too many faces and ending up with a consistency problem. Consider the situation where every programming mode defined its own face to represent comments and did not use inheritance. What benefit does that give us? Very little. What harm - potentially a lot as now you have to configure multiple faces to just get code comments to have a consistent appearance. 

A mode should NOT create a new face just for the sake of it and if it does create a new face, either fully define the face or inherit from a fully defined face - its that simple.  If you want to restrict inheritance to only use faces with the same semantic meaning, fine. I think it is over restrictive, but care less about that than about having a solution that would ensure faces are fully defined and providing a way to achieve this that will facilitate developers doing the right thing. 

On the contrary.  What should be avoided is reusing some face
that makes no sense in the current context.  What is the
_practical_ criterion?  Whether or not customizing the face
wrt its original context/purpose will have a negative effect
in the new context.  If not, then go for it (reuse/inherit).

Agreed - pretty much my position.  

T> and many of them are poorly defined.  It now takes much
T> longer for me to establish a consistent set of faces.

Bad - agreed.  But not a necessary consequence of having many
faces.  More likely a consequence of poor default definitions
(as you mentioned).

Yes, but encouraging the addition of new faces without good justification is bad - it is the other extreme. Too often, I use applications that don't have enough faces and I cannot change the appearance of some object that I would like to somehow stand-out. However, the other extreme is a proliferation of unnecessary faces that just make customization and theming more difficult. 

T> If we required face definitions to either be fully defined or
T> inherited from a base set that are fully defined, we would
T> likely give a much better default experience to end users.

Full face definitions are good, yes.
That is independent of the question of inheritance.

Well... indiscrimately imposing inheritance (from fully defined
faces) can ensure full definitions.  But it has a cost: it can
also introduce unnecessary and unwanted dependencies with
negative consequences.

Well, either direction has a cost. There is no avoiding that. My view is to find the point at which we get the best cost/benefit. Inheritance provides a way to reduce the burden on developers to fullly define faces and has the potential to reduce the number of faces that are only partially defined and work only in limited configurations/environments. Whether we have strict or less rigid restrictions on inheritance will affect how easily inheritance can be used in this manner. Strict inheritance will give a consistent semantic meaning in inheritance chains and potentially more predictable resutls when modifying base faces. Less rigid inheritance would allow inheritance to be used instead of fully face definitions in more situations. If badly applied it does have the potential to cause confusion, but this could be mitigated with good guidelines. New faces should only be introduced when there is a demonstrated case and not just as an automatic default action. 

T> Using inheritance also means that when we find a face
T> definition which is poor for a particular environment, we can
T> refine that base definition rather than having to find all
T> similarly poor definitions and updating them individually.

Yes and no.  "Fixing" it for one environment might do exactly
the wrong thing for another context, e.g. for one of its
descendents.  Inheritance needs to be used carefully, based on
intended use/meaning of the faces involved.

Maybe, though I don't think it is as bad in practice as you assume. Existing modes have been using inheritance in a non-semantic fashion since at least emacs 21 and this doesn't seem to have cause any major issues.

T> Using inheritance means that it is easier to make broad
T> changes to face definitions, reducing the time spent in
T> customize and lets you then tweak those specific faces which
T> don't quite work.
T>
T> So, using your example, if someone changed the base face that
T> dired+ uses, either they will find the result in dired+
T> acceptable or they can then tweak that face further.  The
T> alternative is that it would be necessary to customize both
T> faces, even when you are going to set them to the same value.

Maybe, maybe not.  Depends what kind of change you want/need.

It's true that if you want to change all occurrences of red
foreground to blue then more customization would be involved
without inheritance.  Maybe we could find a way to make such
changes easier - I don't have a problem with that.

Maybe this could be an alternative. Frequently, you do need to change colours 'across the board' - often because the faces are not fully defined and you have a different background to the assumed default and need to make the change to make the face more readable. So, perhaps this would fix the symptom. However, it does not address the basic problem, which is faces that are not fully defined.

But we should not be using _inheritance_ for that (in
general).  A Dired face should either not inherit or it should
inherit from a related face, not just some face whose _default
appearance_ looks good for Dired.  It makes no sense to base a
Dired face on a font-lock face, for instance.

So you would argue that the existing dired face definitions are wrong. However, they ahve been like that for quite some time and it appears to work.
 
Just because you might want some Dired face to have a red
foreground by default is no reason to go looking for a face
with a red foreground to inherit from.  Look instead for a
face with a related use/meaning.  If you find none, then do
not use inheritance here.  That's the guideline I'd suggest.

I emphasize the use/meaning of a given face: what it is for,
what it represents, what it does.  I think you are emphasizing
its default appearance - attribute values (color etc.).  To
me, a face or a variable `foo' is mainly about its use, not
its value.

No, I'm not focused on its appearance. The difference is that while I agree you should select the face that has a similar semantic meaning if one exists, I have no issue with using one that does not provided it does not cause confusion within the mode it is used in. I would cite dired as an example where this has been done successfully.
 
If the opposite were true, then we would name faces and
variables after their default values, not after their uses.
Yes, I've seen some Lisp code with face names like
`underlined-bold-red-foreground-on-gray-background'.
(Mea culpa: I even wrote such code at one point.)

And there's not necessarily anything wrong with that; for some
contexts it might make sense.  But in that case it makes more
sense for the "face" to be a constant, not customizable.  It
would be perverse to customize a face named `red-foreground'
to have a green foreground.
 
(And in general it is more likely for single attributes to be
reuse candidates than it is for combinations of attributes.)

T> The reality is that there are a limited set of good faces and
T> you will usually end up with the same face definition being
T> used in multiple roles.  Frequently, those roles have nothing
T> to do with each other.  This doesn't matter as the contexts
T> are completely different.  From an end user perspective, it is
T> irrelevant that compiler messages inherit from
T> font-lock-string - all the end user sees is that compiler
T> messages use a default face that is green, which may be
T> exactly the same as strings in their code, but so what.

I strongly disagree here.

If the roles/meanings/use are unrelated, then inheritance is
usually inappropriate.  It _does_ matter whether a
compiler-messages face inherits from `font-lock-string-face' -
_if_ someone customizes that parent face.

But does it really matter. I change font-lock-comment-face to orange because that is easier to read or more aesthetic given my default background colour. As a result, compiler-message face also changes to orange. Isn't it just as likely that if pink was not acceptable for comments in code that pink is also unacceptable for messages in compile-mode? Sure, maybe I might decide I'd still like pink for those messages and I can override the inheritance, but in practice, I think in most cases, this would be fine and has been my experience with the mdoes that do use inheritance in such a manner.

Of course, if there is a base face with a more closely fitting semantic meaning, it should be used in preference and you would avoid using faces like link or button that have very precise meanings that could cause confusion within the mode - especially if that mode supports links or buttons. 

If we defined a compiler-messages face to inherit from
`font-lock-string-face' _only_ because we liked the default
foreground choice of `LightSalmon', that would be misguided,
IMO.

Perhaps, if that was the only motivation. However, if you choose that font-lock face because you want to use inheritance rather than spend time fully defining compiler-message face and if the font-lock face is fully defined, then it may be a valid choice.
 
If there is no existing face that is related in terms of
use/meaning, then the compiler-messages face should not
inherit; it should just be defined with a default foreground
of `LightSalmon'.  That kind of duplication is not a curse but
a blessing.

NO, That would be a partial font definition. If I happen to use a background of LightSalmon, that face is now unreadable. Either fully define the face or inherit. 

Then a user can customize either the compiler-messages face or
`font-lock-string-face' without affecting the other.  And that
makes sense because (by hypothesis) the two are unrelated.

Even if compiler-message face uses inheritance form an unrelated face, you still have the option of customizing it to something different or even removing the inheritance if so desired. 

Tim