Comment 4 for bug 616828

Uday Reddy writes:
 > Tim Cross writes:
 >
 > > Not sure if this will help or not, but my ~tcross/vm/custom
 > > branch has been modified so that all faces are 'real' defface
 > > definitions rather than a variable just holding a face name.
 >
 > I don't think we can do that. Variables and faces are entirely
 > different beasts in elisp. So, changing one to the other will break
 > user customization.

Yes, it would mean that existing customization would change, but I'd argue this
can be justified. The current setup does not allow a user to customize the face
other than to set it to an already defined face. Making it a propper face
definition will allow much more standard and flexible customization.

Currently, vm-summary-highlight-face is a variable. It has a default of 'bold.
If the end user wants to change that default, they must use an existing face
name. In addition, they need to select a face that is defined when VM is
loaded, which is not always straight-forward as using list-faces-display or the
customizeation groups will list all faces defined at that time, including those
defined by packages which may not always be loaded when VM is started.
Furthermore, they are restricted to those faces i.e. cannot just modify a face
attribute, such as adding underline, italic, a different colour etc.

Leaving the definition as a variable holding a face name also means we now have
inconsistency in how we do things. Some faces are real faces and some faces are
jus variables holding face symbols/names. Some faces will be customizable in a
standard way and some will not. This does not lead to a good user experience.

While I agree with minimizing change and preserving backwards compatibility
where possible, I'd argu that we also need to balance this with improving VM
and making the end-user experience better. We have already made other changes
which are not backwards compatible. The change this causes is small and would
not mean complex or dificult modifications for anyones custom code. If they
have set the variable to a different face name, they would need to remove that
setting and then, if necessary, update the new face definition to meet their
tastes, which is easily done via the custom interface. Secondly, they would
need to modify any code that uses this variable. Generally, this involves
nothing more than quoting the variable to make it a symbol and possibly adding
an explicit test i.e. instead of

 (if vm-summary-highlight-face
   (....

you would do something like

 (if (facep 'vm-summary-highlight-face)
   (...

Of course, we would need to ensure this change is fully documented in the NEWS
file.

Tim

--
Tim Cross
<email address hidden>

There are two types of people in IT - those who do not manage what they
understand and those who do not understand what they manage.