From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs 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 Message-ID: References: <87o8xwrjba.fsf@bernoul.li> <834kzooo8e.fsf@gnu.org> <877e4d7yzf.fsf@bernoul.li> <83imnvg53q.fsf@gnu.org> <87zhh2ofc9.fsf@bernoul.li> <87k186nsku.fsf@bernoul.li> <87imna18nc.fsf@mail.linkov.net> <42c596c2-b5c1-9fc9-4b92-9c13b386d93d@yandex.ru> <83pnhgrlni.fsf@gnu.org> <83ftiasfdm.fsf@gnu.org> <83lfrulmva.fsf@gnu.org> <76a012f5-8cdd-75d5-322e-a453a612c655@yandex.ru> <83immxjs6q.fsf@gnu.org> <993b2f9c-6052-e791-3d3b-26d5fedd7d12@yandex.ru> <835ziuixke.fsf@gnu.org> <9c4768a5-ecce-68ce-c612-a001b2f6784d@yandex.ru> <8336dxh1ge.fsf@gnu.org> <6c68ceed-156c-a6f2-bf0f-21d7e9eb5692@yandex.ru> <831rthgy3u.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------ED5EE09A9992E952E46A366A" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="83603"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 Cc: 37774@debbugs.gnu.org, juri@linkov.net To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Dec 07 02:07:15 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1idOZ0-000Ld0-OY for geb-bug-gnu-emacs@m.gmane.org; Sat, 07 Dec 2019 02:07:14 +0100 Original-Received: from localhost ([::1]:46856 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idOYx-0006tG-Vf for geb-bug-gnu-emacs@m.gmane.org; Fri, 06 Dec 2019 20:07:11 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53815) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idOYq-0006rs-0v for bug-gnu-emacs@gnu.org; Fri, 06 Dec 2019 20:07:05 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1idOYo-0001dW-Ds for bug-gnu-emacs@gnu.org; Fri, 06 Dec 2019 20:07:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:41230) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1idOYo-0001cl-97 for bug-gnu-emacs@gnu.org; Fri, 06 Dec 2019 20:07:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1idOYo-0006fx-4F for bug-gnu-emacs@gnu.org; Fri, 06 Dec 2019 20:07:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 07 Dec 2019 01:07:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 37774 X-GNU-PR-Package: emacs Original-Received: via spool by 37774-submit@debbugs.gnu.org id=B37774.157568077825580 (code B ref 37774); Sat, 07 Dec 2019 01:07:02 +0000 Original-Received: (at 37774) by debbugs.gnu.org; 7 Dec 2019 01:06:18 +0000 Original-Received: from localhost ([127.0.0.1]:47203 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idOY6-0006eV-4e for submit@debbugs.gnu.org; Fri, 06 Dec 2019 20:06:18 -0500 Original-Received: from mail-wr1-f53.google.com ([209.85.221.53]:42072) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idOY2-0006dJ-QL for 37774@debbugs.gnu.org; Fri, 06 Dec 2019 20:06:16 -0500 Original-Received: by mail-wr1-f53.google.com with SMTP id a15so9660727wrf.9 for <37774@debbugs.gnu.org>; Fri, 06 Dec 2019 17:06:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=/wEMIgM8BZEz22G2XRD3YzfqagUzFtLZO15DVnvx2Zc=; b=u6y+Ry5M2Ogw0kc8Gzk3TMIkRsibaLvjnzt4yXPeC8lCIBicK7vvsSXuAUYhBsL5UI VlLd0xN+4wn1ynJOKwV0EuzbyRONj9qmK6neSGbV68Ee0H8T13Du8ulmW9Uc6JkBx16D KD4W/1i+bO2J1f1u6/vlo5NJXzNZSbT3ARSNf886JBTeNqDMCK8mPSFyovtGq8dDEZ+5 IrlK2L5dTIpIE65NNnthhW4lg47d+QVBGioqjb5sdYvitKUAn5uh9+k29eumS22rK3Wd HUJVxS8nHeLiC4kTkRuGmSzjitjPZeFtlIa0CMnU76tIGtlCZ30F1yQLUNSMxEuRVIAI ufBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:cc:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language; bh=/wEMIgM8BZEz22G2XRD3YzfqagUzFtLZO15DVnvx2Zc=; b=QBMAj7WOS0g9CCmtLnqUunbnF3btFIKHl9rGr58vDpoUuCyh9PyNu3IIgG/5EBPVp3 7LzpyIRROG22xr6JxUb/ugYhSJI0OEGSrJA7QmvKEGXw7CYEsTAQIV4SmGwAF7PuWEEJ xY3ekZqB8a+0jzZpjotlwkrqg6vS06ECCi3zSj9HRkWkNvZ5Uwgtou3Y4L0gvZzOCJ0l o+4yQ9mBS8eIMXpA8R6eWxmCeVQleB5nT3UDMRc+Hc+/l/nXWEqVqNN5Ucq2tugdDP6T b/rDZaBLcyONHpdxyUlBX1GeVnQgvEHIlOKKXuqkE5rJBVFv0N9Y3hYvCKeKP0njrMRD rpoA== X-Gm-Message-State: APjAAAXSCcZNTkHnP9pKZhbP9Xm5waBm26nYLYIZFDGtiXE19izXWO4V 1jk+EIgDpMRyUv/oIQiNnTQ= X-Google-Smtp-Source: APXvYqwdmIB5ku1TtgaGHxVsqVzYgd4L7ypkK2sQ6qez97SpYYxv9GhAf/Ig/qQIagUmQdg+yG/xeA== X-Received: by 2002:adf:a141:: with SMTP id r1mr18657912wrr.285.1575680768875; Fri, 06 Dec 2019 17:06:08 -0800 (PST) Original-Received: from [192.168.0.5] ([212.50.117.215]) by smtp.googlemail.com with ESMTPSA id s19sm4844877wmj.33.2019.12.06.17.06.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 06 Dec 2019 17:06:07 -0800 (PST) In-Reply-To: <831rthgy3u.fsf@gnu.org> Content-Language: en-US X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:172988 Archived-At: This is a multi-part message in MIME format. --------------ED5EE09A9992E952E46A366A Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 06.12.2019 19:31, Eli Zaretskii wrote: >> Cc: 37774@debbugs.gnu.org, juri@linkov.net >> From: Dmitry Gutov >> 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). --------------ED5EE09A9992E952E46A366A Content-Type: text/x-patch; name="inherit-face-extend-spec-2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="inherit-face-extend-spec-2.diff" 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))) --------------ED5EE09A9992E952E46A366A--