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: Mon, 02 Dec 2019 18:21:13 +0200 Message-ID: <83lfrulmva.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> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="2031"; 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 Mon Dec 02 17:22:12 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 1iboSh-0000Nz-Hk for geb-bug-gnu-emacs@m.gmane.org; Mon, 02 Dec 2019 17:22:11 +0100 Original-Received: from localhost ([::1]:38160 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iboSg-0008KA-BI for geb-bug-gnu-emacs@m.gmane.org; Mon, 02 Dec 2019 11:22:10 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:55293) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iboSa-0008Jb-7Q for bug-gnu-emacs@gnu.org; Mon, 02 Dec 2019 11:22:05 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iboSY-0000UB-R4 for bug-gnu-emacs@gnu.org; Mon, 02 Dec 2019 11:22:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:32858) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iboSY-0000U7-NT for bug-gnu-emacs@gnu.org; Mon, 02 Dec 2019 11:22:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1iboSY-0000m5-Ia for bug-gnu-emacs@gnu.org; Mon, 02 Dec 2019 11:22: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: Mon, 02 Dec 2019 16:22: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.15753036932935 (code B ref 37774); Mon, 02 Dec 2019 16:22:02 +0000 Original-Received: (at 37774) by debbugs.gnu.org; 2 Dec 2019 16:21:33 +0000 Original-Received: from localhost ([127.0.0.1]:38831 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iboS5-0000lH-93 for submit@debbugs.gnu.org; Mon, 02 Dec 2019 11:21:33 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:46141) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iboS2-0000l3-NA for 37774@debbugs.gnu.org; Mon, 02 Dec 2019 11:21:32 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:39022) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1iboRx-0000G5-41; Mon, 02 Dec 2019 11:21:25 -0500 Original-Received: from [176.228.60.248] (port=4352 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1iboRv-00084K-Jw; Mon, 02 Dec 2019 11:21:25 -0500 In-reply-to: (message from Dmitry Gutov on Mon, 2 Dec 2019 02:05:10 +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:172775 Archived-At: > Cc: 37774@debbugs.gnu.org, juri@linkov.net > From: Dmitry Gutov > Date: Mon, 2 Dec 2019 02:05:10 +0200 > > feel free to change its :extend attribute. > > OK, done. But it feels pretty useless, considering any theme where this would affect something would have to repeat the 'extend t' definition anyway. That's true for themes, but what about face customization by users using the likes of set-face-background? For them, we should have the attribute even when the default face definition is empty. > Seems like it's a consequence of the implementation strategy. There were > a couple of others that had been proposed (splitting the attribute in > two, with different default values) or using a symbol property. > > Splitting into two attributes won't help here (quite the contrary, > since we'd have to deal with 2 new attributes). > > That depends on the end goal. If we were aiming to keep the previous behavior almost entirely, but avoid extending things like underline over newlines, there are two default values for the two proposed attributes that would satisfy almost everybody. And the people who would prefer not to extend the region face background over newlines would apply that in their init scripts (ditto for other faces). The context of this part was how themes customize faces, it was not about users customizing the faces directly. In the context of themes, having two attributes would not have helped, because themes will have to be changed anyway. As for the other aspect of this: the idea was that almost all faces do not need to be extended, something that no default will succeed to do silently. > Basically, on the C level each Lisp face is represented by an array of > its attributes, where each array element holds the value of the > corresponding attribute. Once this array is computed, we can (and do) > manipulate just the array, and for all practical purposes can forget > about the face's symbol. Using a symbol property would then need to > keep the symbol around at all times, which is inconvenient and would > make the code ugly. > > I don't think so. Once the symbol is gone, whatever left is just the value. So when this array is computed, the function doing that would merge the faces attributes with whatever attributes are specified using the alternative symbol property. The array is computed only once, whereas merging happens many times and in different places. So we cannot compute the value of an attribute only once, because its value depends on what other faces are being merged, on whether their :extend attribute is set. and on whether the particular merging process cares about :extend. > To be more exact, the current face attributes are also assigned to a symbol property (face-defface-spec). We can add another property: face-default-spec, which would contain attributes that should apply to the face unless explicitly overridden in face-defface-spec. It could even be set by a new defface keyword instead of plain 'put', but that's a minor concern IMO. (Option two ends here). > > This seems to be the easiest way to go around the long-established behavior that custom-theme-set-faces overwrites the face attributes (instead of trying to merge them). We could do in the other way, but it would require changes in both custom-theme-set-faces and defface, as well as some other functions I imagine, and either a whitelist of attributes that would always be retained unless overridden. Or a wholesale change to retain all attributes by default. I might like the last option personally, but it would be a major breaking change. Still, all themes could be updated to account for it while keeping compatibility with older Emacs with little trouble (which is harder to do with the current :extend situation). I don't think I understand your proposal in concrete terms, and you didn't read the code to check your proposal against the actual implementation, so this is a sure way to misunderstandings and talking past each other. I can only say that face-defface-spec and other properties of face symbols are used by custom.el on the Lisp level, whereas we were talking about what happens on the C level. On the C level, face merging creates an unnamed face with its attributes computed by the merging process, and that process currently takes the :extend attribute into account. Any proposal to use face symbol's properties instead will have to explain how those properties are communicated to the merging process. > But even if we'd overcome this annoyance, how do you specify this > property for a face like below? > > '(:inherit foo :background "green" :underline "red") > > There's no symbol to put the property on. Do we say that such > anonymous faces cannot support this attribute? Unclean. > > See above. Just add ':extend t' (or nil) to the end of the value. So you propose to have both symbol property _and_ a face attribute?