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: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
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, 6 Dec 2019 17:44:33 +0200

[Message part 1 (text/plain, inline)]

On 05.12.2019 17:47, Eli Zaretskii wrote:
>> Cc: 37774 <at> debbugs.gnu.org, juri <at> linkov.net
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Thu, 5 Dec 2019 03:44:22 +0200
>>
>> The new definition for diff-added would look like:
>>
>>     (defface diff-added
>>       '((default
>>          :inherit diff-changed)
>>         (((class color) (min-colors 257) (background light))
>>          :background "#eeffee")
>>         (((class color) (min-colors 88) (background light))
>>          :background "#ddffdd")
>>         (((class color) (min-colors 88) (background dark))
>>          :background "#335533")
>>         (((class color))
>>          :foreground "green"))
>>       "`diff-mode' face used to highlight added lines.")
>>
>>     (put 'diff-added 'face-transient-spec '((t :extend t)))
> 
> OK, and how will this work to countermand the problem with themes?
> 
> custom-theme-set-faces calls face-spec-set, which calls
> face-spec-recalc, which starts by resetting all face attributes to
> 'unspecified'.  And the last 2 functions are general-purpose, not
> specific to themes.

Well, the idea was to use a different structure to store the "transient" 
attributes. That could be an extra symbol property, or an additional 
structure for storing faces attributes, in addition to 
default-frame-alist. But looking at the code now, it seems fairly clunky 
and crossing abstraction levels.

It's great that you mentioned face-spec-recalc. It looks just like the 
place to change, since both defface and theme definitions and 
customizations go through it.

We can implement in there a new kind of "face spec" along the lines of 
my previous description, or simply special-case the :extend attribute, 
and take it from the default spec. The latter option is implemented in 
the attached patch, which seems to work in my limited testing.

[inherit-face-extend-spec.diff (text/x-patch, attachment)]

Previous Next

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