all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Dmitry Gutov <dgutov@yandex.ru>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 37774@debbugs.gnu.org, juri@linkov.net
Subject: bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages
Date: Sat, 7 Dec 2019 03:06:05 +0200	[thread overview]
Message-ID: <ddcee80c-1a7e-8bc3-f839-ef9f8ff022c1@yandex.ru> (raw)
In-Reply-To: <831rthgy3u.fsf@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]

On 06.12.2019 19:31, Eli Zaretskii wrote:
>> Cc: 37774@debbugs.gnu.org, juri@linkov.net
>> From: Dmitry Gutov <dgutov@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.

I suppose so. Though I wouldn't consider it too important to most users, 
considering that thanks to this change in semantics, most people won't 
have to do a thing when upgrading to the new version of Emacs.

>>> 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? 

Only the behavior of other its callers (direct and indirect). :-)

Such as enable-theme, face-set-after-frame-default, 
frame-set-background-mode and face-spec-set. I'm pretty sure all of 
these should behave consistently WRT this change.

>> 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?

I'm "avoiding such consequences" by trying to make sure the change is in 
the correct function and that it makes sense within the context.

>>>> +    (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'.

This distinction is handled in the second hunk of the patch where 
themed-extend-attr is calculated. If this attribute is not themed, there 
is no difference between nil and 'unspecified' (in the default spec).

Finally, the value 'unspecified' seems impossible to get from the 
attributes list this way. plist-get will simply return nil.

That said, I think I've found one valid scenario where this patch will 
do wrong: if the themed spec includes an :inherit directive, and the 
inherited face specifies (:extend nil). The current patch would 
inevitably ignore it and override with the value from the default spec.

So here's a second try (attached).

[-- Attachment #2: inherit-face-extend-spec-2.diff --]
[-- Type: text/x-patch, Size: 1807 bytes --]

diff --git a/lisp/faces.el b/lisp/faces.el
index dc5bcca760..64cb76b4fa 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1669,7 +1669,7 @@ face-spec-recalc
   ;; `theme-face' records.
   (let ((theme-faces (get face 'theme-face))
 	(no-match-found 0)
-	face-attrs theme-face-applied)
+        default-attrs face-attrs theme-face-applied)
     (if theme-faces
 	(dolist (elt (reverse theme-faces))
 	  (setq face-attrs (face-spec-choose (cadr elt) frame no-match-found))
@@ -1677,13 +1677,19 @@ face-spec-recalc
 	    (face-spec-set-2 face frame face-attrs)
 	    (setq theme-face-applied t))))
     ;; If there was a spec applicable to FRAME, that overrides the
-    ;; defface spec entirely (rather than inheriting from it).  If
-    ;; there was no spec applicable to FRAME, apply the defface spec
-    ;; as well as any applicable X resources.
+    ;; defface spec entirely rather than inheriting from it, with the
+    ;; exception of the :extend attribute (which is inherited).
+    ;;
+    ;; If there was no spec applicable to FRAME, apply the defface
+    ;; spec as well as any applicable X resources.
+    (setq default-attrs (face-spec-choose (face-default-spec face) frame))
     (unless theme-face-applied
-      (setq face-attrs (face-spec-choose (face-default-spec face) frame))
-      (face-spec-set-2 face frame face-attrs)
+      (face-spec-set-2 face frame default-attrs)
       (make-face-x-resource-internal face frame))
+    (when (and theme-face-applied
+               (eq 'unspecified (face-attribute face :extend frame t)))
+      (let ((extend-p (plist-get default-attrs :extend)))
+        (and extend-p (face-spec-set-2 face frame '(:extend t)))))
     (setq face-attrs (face-spec-choose (get face 'face-override-spec) frame))
     (face-spec-set-2 face frame face-attrs)))
 

  reply	other threads:[~2019-12-07  1:06 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-16  7:00 bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages Andrey Orst
2019-10-16  7:53 ` Eli Zaretskii
2019-10-16  8:12   ` Andrey Orst
2019-10-16 11:05     ` Eli Zaretskii
2019-10-16 14:20       ` Stefan Kangas
2019-10-16 16:25         ` Eli Zaretskii
2019-10-16  8:59   ` Michael Heerdegen
2019-10-16  9:17     ` martin rudalics
2019-10-16 11:26     ` Eli Zaretskii
2019-10-16 11:38       ` Michael Heerdegen
2019-10-16 12:59         ` Eli Zaretskii
2019-10-16 11:10   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 11:17     ` Andrey Orst
2019-10-16 11:41       ` Eli Zaretskii
2019-10-16 12:08         ` Andrey Orst
2019-10-16 13:05           ` Eli Zaretskii
2019-10-16 11:42     ` Eli Zaretskii
2019-10-16 13:18       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 13:33         ` Andrey Orst
2019-10-16 14:21           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 17:33         ` Eli Zaretskii
2019-10-16 18:13           ` Andrey Orst
2019-10-16 18:50             ` Eli Zaretskii
2019-10-17 19:05             ` Kévin Le Gouguec
2019-10-17 20:38               ` Kévin Le Gouguec
2019-10-17 11:36           ` Michael Albinus
2019-10-17 12:38             ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-17 13:05             ` Eli Zaretskii
2019-10-17 16:18               ` Michael Albinus
2019-10-17 16:39                 ` Eli Zaretskii
2019-10-16 17:27     ` Juri Linkov
2019-10-16 18:07       ` Andrey Orst
2019-10-16 18:18         ` Juri Linkov
2019-10-16 18:54           ` Eli Zaretskii
2019-10-16 19:52             ` Juri Linkov
2019-10-16 20:06               ` Eli Zaretskii
2019-10-16 19:55             ` Eli Zaretskii
2019-10-16 20:14               ` Juri Linkov
2019-10-17  6:18                 ` Eli Zaretskii
2019-10-17  8:52                   ` Andrey Orst
2019-10-17  8:59                     ` Eli Zaretskii
2019-10-17  9:20                       ` Andrey Orst
2019-10-17 12:54                         ` Eli Zaretskii
2019-10-17 13:40                           ` Andrey Orst
2019-10-17 14:02                             ` Eli Zaretskii
2019-10-17 14:13                               ` Andrey Orst
2019-10-17 14:38                                 ` Eli Zaretskii
2019-10-17 14:02                       ` Dmitry Gutov
2019-10-17 16:20                         ` Eli Zaretskii
2019-10-17 16:46                           ` Dmitry Gutov
2019-10-17 17:20                             ` Eli Zaretskii
2019-10-17  8:25                 ` martin rudalics
2019-10-17 14:08                   ` Dmitry Gutov
2019-10-17 16:29                     ` Eli Zaretskii
2019-10-17 16:50                       ` Dmitry Gutov
2019-10-17 17:23                         ` Eli Zaretskii
2019-10-18 14:25                           ` Dmitry Gutov
2019-10-20 20:07                           ` Dmitry Gutov
2019-10-21  6:10                             ` Eli Zaretskii
2019-10-23 12:47                               ` Dmitry Gutov
2019-10-23 15:39                                 ` Eli Zaretskii
2019-10-23 16:12                                   ` Dmitry Gutov
2019-10-23 18:04                                     ` Eli Zaretskii
2019-10-23 20:28                                       ` Dmitry Gutov
2019-10-24 14:56                                         ` Eli Zaretskii
2019-10-24 17:04                                         ` Kévin Le Gouguec
2019-10-17 22:22                   ` Juri Linkov
2019-10-18  5:28                     ` Andrey Orst
2019-10-18  6:53                     ` Eli Zaretskii
2019-10-19 20:53                       ` Juri Linkov
2019-10-20  6:03                         ` Eli Zaretskii
2019-10-20 15:42                           ` Juri Linkov
2019-10-20 16:59                             ` Eli Zaretskii
2019-10-21 21:29                         ` Juri Linkov
2019-10-18  8:25                     ` martin rudalics
2019-10-18 12:41                       ` Dmitry Gutov
2019-10-18 13:04                       ` Andrey Orst
2019-10-17 13:56                 ` Dmitry Gutov
2019-10-17 16:28                   ` Eli Zaretskii
2019-10-17 13:54             ` Dmitry Gutov
2019-10-16 18:46       ` Eli Zaretskii
2019-10-16 19:46         ` Juri Linkov
2019-10-16 20:03           ` Eli Zaretskii
2019-10-16 20:23             ` Juri Linkov
2019-10-16 20:37               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-17  6:43                 ` Eli Zaretskii
2019-10-17  6:40               ` Eli Zaretskii
2019-10-16 20:29             ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-17 14:12               ` Dmitry Gutov
2019-10-16 20:23           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 11:33 ` Andrey Orst
2019-10-16 15:30   ` Eli Zaretskii
2019-10-31 16:06 ` Jonas Bernoulli
2019-10-31 16:48   ` Eli Zaretskii
2019-11-06 16:26     ` Jonas Bernoulli
2019-11-06 17:06       ` martin rudalics
2019-11-07 13:58         ` Eli Zaretskii
2019-11-07 15:41           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-11-07 17:10             ` martin rudalics
2019-11-07 21:57               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-11-08  9:20                 ` martin rudalics
2019-11-08 10:37                   ` Eli Zaretskii
2019-11-08 18:26                     ` martin rudalics
2019-11-08 19:14                       ` Eli Zaretskii
2019-11-08 11:39                   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-11-08 18:27                     ` martin rudalics
2019-12-05 16:48                   ` Kévin Le Gouguec
2019-12-05 18:02                     ` Eli Zaretskii
2019-12-05 19:05                       ` Kévin Le Gouguec
2019-12-05 19:19                         ` Eli Zaretskii
2019-12-05 18:22                     ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-12-05 18:47                       ` Eli Zaretskii
2019-11-07 13:59       ` Eli Zaretskii
2019-11-11 10:52         ` Jonas Bernoulli
2019-11-11 19:03           ` Jonas Bernoulli
2019-11-14 11:33             ` Eli Zaretskii
2019-11-14 14:14               ` Dmitry Gutov
2019-11-14 14:41                 ` Eli Zaretskii
2019-11-14 22:42                   ` Dmitry Gutov
2019-11-15  8:08                     ` Eli Zaretskii
2019-11-15 23:50                       ` Dmitry Gutov
2019-11-16  8:09                         ` Eli Zaretskii
2019-11-16  8:17                           ` Dmitry Gutov
2019-11-23 23:20             ` Juri Linkov
2019-11-24 16:14               ` Eli Zaretskii
2019-11-25 23:45                 ` Juanma Barranquero
2019-11-26 17:44                   ` Eli Zaretskii
2019-11-25  0:29               ` Dmitry Gutov
2019-11-25 16:00                 ` Eli Zaretskii
2019-11-25 23:50                   ` Dmitry Gutov
2019-11-26 17:43                     ` Eli Zaretskii
2019-12-02  0:05                       ` Dmitry Gutov
2019-12-02 16:21                         ` Eli Zaretskii
2019-12-03  0:01                           ` Dmitry Gutov
2019-12-03 16:21                             ` Eli Zaretskii
2019-12-05  1:44                               ` Dmitry Gutov
2019-12-05 15:47                                 ` Eli Zaretskii
2019-12-06 15:44                                   ` Dmitry Gutov
2019-12-06 16:18                                     ` Eli Zaretskii
2019-12-06 16:58                                       ` Dmitry Gutov
2019-12-06 17:31                                         ` Eli Zaretskii
2019-12-07  1:06                                           ` Dmitry Gutov [this message]
2019-12-07  7:53                                             ` Eli Zaretskii
2019-12-07 17:06                                               ` Dmitry Gutov
2019-12-07 17:53                                                 ` Eli Zaretskii
2019-12-07 18:55                                                   ` Dmitry Gutov
2019-12-07 19:14                                                     ` Eli Zaretskii
2019-12-08  0:42                                                       ` Dmitry Gutov
2019-12-08  3:32                                                         ` Eli Zaretskii
2019-12-08 10:39                                                           ` Dmitry Gutov
2019-12-08 15:50                                                             ` Eli Zaretskii
2019-12-08 21:20                                                               ` Dmitry Gutov
2019-12-09 12:59                                                                 ` Eli Zaretskii
2019-12-09 14:07                                                                   ` Dmitry Gutov
2019-12-09 14:42                                                                     ` Eli Zaretskii
2019-12-09 15:24                                                                       ` Dmitry Gutov
2019-12-09 15:42                                                                         ` Eli Zaretskii
2019-12-10  0:20                                                                           ` Dmitry Gutov
2019-12-10 15:57                                                                             ` Eli Zaretskii
2019-12-11 23:02                                                                               ` Juri Linkov
2019-12-12  4:36                                                                                 ` Eli Zaretskii
2019-12-12 23:44                                                                                   ` Juri Linkov
2019-12-13  7:03                                                                                     ` Eli Zaretskii
2019-11-27 21:30                     ` Juri Linkov
2019-11-27 23:34                       ` Dmitry Gutov
2019-11-28 15:19                         ` Eli Zaretskii
2019-11-28 15:16                       ` Eli Zaretskii
2019-11-30 11:35                         ` Eli Zaretskii
2019-12-02  0:07                           ` Dmitry Gutov
2019-10-31 17:29   ` Dmitry Gutov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ddcee80c-1a7e-8bc3-f839-ef9f8ff022c1@yandex.ru \
    --to=dgutov@yandex.ru \
    --cc=37774@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=juri@linkov.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.