From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages Date: Sat, 07 Dec 2019 09:53:40 +0200 Message-ID: <83o8wkfu63.fsf@gnu.org> 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> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="199140"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 37774@debbugs.gnu.org, juri@linkov.net To: Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Dec 08 04:29:27 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 1idnGA-000pet-7s for geb-bug-gnu-emacs@m.gmane.org; Sun, 08 Dec 2019 04:29:26 +0100 Original-Received: from localhost ([::1]:55272 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idnG8-0006ls-El for geb-bug-gnu-emacs@m.gmane.org; Sat, 07 Dec 2019 22:29:24 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36940) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idnFo-0006la-UC for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 22:29:06 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1idnFm-0004m6-Q0 for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 22:29:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:44250) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1idnFm-0004ll-6N for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 22:29:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1idnFm-0004sE-3S for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 22:29:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 08 Dec 2019 03:29: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.157577571918705 (code B ref 37774); Sun, 08 Dec 2019 03:29:02 +0000 Original-Received: (at 37774) by debbugs.gnu.org; 8 Dec 2019 03:28:39 +0000 Original-Received: from localhost ([127.0.0.1]:50223 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idnFP-0004rc-8R for submit@debbugs.gnu.org; Sat, 07 Dec 2019 22:28:39 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:55223) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idnFM-0004rP-Il for 37774@debbugs.gnu.org; Sat, 07 Dec 2019 22:28:38 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:59252) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1idUuU-0005Ph-TT; Sat, 07 Dec 2019 02:53:51 -0500 Original-Received: from [176.228.60.248] (port=1456 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1idUuU-0002yf-8Q; Sat, 07 Dec 2019 02:53:50 -0500 In-reply-to: (message from Dmitry Gutov on Sat, 7 Dec 2019 03:06:05 +0200) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] 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:173015 Archived-At: > Cc: 37774@debbugs.gnu.org, juri@linkov.net > From: Dmitry Gutov > Date: Sat, 7 Dec 2019 03:06:05 +0200 > > > Well, we will have to document this exemption prominently, then. > > I suppose so. Though I wouldn't consider it too important to most users, It's important to developers of Lisp programs that manipulate faces. > > 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. I meant to avoid such consequences by making sure those other callers can never trigger this new processing of :extend. Can we please do that? That will go a long way towards my agreement to have this code in Emacs 27. > >>>> + (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). The use case I had in mind is this: . the theme doesn't specify :extend . the default spec for a face specifies ':extend nil' In this case, after applying the theme, the face should have ':extend nil', implicitly "inherited" from the default spec. Do you agree? > Finally, the value 'unspecified' seems impossible to get from the > attributes list this way. plist-get will simply return nil. Exactly. And when a face does specify a nil value for :extend, then plist will return the list '(:extend nil), which is non-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. Once again, I think this part: > + (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))))) ^^^^^^^^^^^^ isn't right, because it seems to say that when the default face says ':extend nil', the face after applying a theme will have ':extend t', which isn't TRT, IMO. Thanks.