GNU bug report logs - #37774
27.0.50; new :extend attribute broke visuals of all themes and other packages

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 37774 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages
Date: Fri, 06 Dec 2019 19:31:01 +0200

> Cc: 37774 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 6 Dec 2019 18:58:26 +0200
> 
> > Thanks, but is it clean enough to do such exemption for :extend?
> 
> I think so. Most importantly, it will save a lot of people the trouble 
> of adapting to the current change, limiting the necessary efforts to 
> just package authors (and Emacs core, of course).

Well, we will have to document this exemption prominently, then.

> > And if we want to do this, why do it in face-spec-recalc and not in
> > custom-set-faces itself?
> 
> Not the place to do that. custom-theme-set-faces saves the specs defined 
> by the theme (or user customization) to the face's symbol property, and 
> then leaves it to face-spec-recalc to combine and apply all the specs 
> related to the face.

Is the patch likely to change any behavior except that of
custom-theme-set-faces?  I'd like to limit it only to that use case,
so that we don't risk breaking anything else.

> I think the purpose of face-spec-recalc is clear enough. So I'd like to 
> see at least one of those "unintended consequences".

Let's try to avoid such consequences from the get-go, okay?

> >> +    (when (and theme-face-applied (not themed-extend-attr))
> >> +      (let ((extend-p (plist-get default-attrs :extend)))
> >> +        (and extend-p (face-spec-set-2 face frame '(:extend t)))))
> >                                                       ^^^^^^^^^^^^
> > I think this should be extend-p instead, because the face's default
> > spec could legitimately say ":extend nil".  Right?
> 
> But that's the default value of that attribute.

No, the default is 'unspecified', which is different from nil, when
merging with a face that specifies :extend, and when inheriting.  A
theme that says ':extend nil' should override the default spec, unlike
'unspecified'.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.