Subject: Re: bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages

From: Eli Zaretskii <[email-masked]>
Date: Sat, 30 Nov 2019 13:35:23 +0200

> Date: Thu, 28 Nov 2019 17:16:24 +0200
> From: Eli Zaretskii <[email-masked]>
> Cc: [email-masked], [email-masked]
> 
> > >> We need to modify all the themes we provide to specify :extend for
> > >> faces where we do that by default.  It seems there's no way around
> > >> that, since the semantics of custom-theme-set-faces is clearly to
> > >> reset all face attributes to 'unspecified' before applying the face
> > >> spec, so keeping some attributes from the default face spec is out of
> > >> the question, unfortunately.  It's clear that the faces stuff was not
> > >> designed to accommodate addition of attributes easily.
> > 
> > This means manually adding :extend to all files in etc/themes?
> 
> Yes.

I've now done that.

Two comments:

  . When adding the :extend attribute to a face, we should make sure
    all of that face's definitions have the same value of it, even if
    the default definition of the face for some 'class' of displays
    doesn't need it (e.g., because it specifies only the foreground
    color).  This is so that if users customize the face, the results
    will look uniform regardless of which face attributes they
    customize.  Otherwise, if the user customizes the background color
    or :underline or some other similar attribute, the appearance will
    be different from that on other classes of terminals, and that is
    baaaad...

  . Some of the themes we have in core customize faces defined by
    unbundled packages.  I didn't change the definitions of those
    faces; it's up to the respective package developers and/or users
    to come up and ask for such changes, if it turns out the packages
    added the :extend attribute and we didn't.