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 19:06:08 +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> <83o8wkfu63.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------D7B21DABB797416A74327DD1" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="66649"; 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 Sun Dec 08 03:54:40 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 1idmiV-000HD9-K8 for geb-bug-gnu-emacs@m.gmane.org; Sun, 08 Dec 2019 03:54:39 +0100 Original-Received: from localhost ([::1]:54994 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idmiU-0006XP-G1 for geb-bug-gnu-emacs@m.gmane.org; Sat, 07 Dec 2019 21:54:38 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:42286) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idmhx-0006Sn-7t for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 21:54:06 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1idmhv-00023S-3V for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 21:54:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:44090) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1idmhu-00022u-0y for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 21:54:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1idmhu-0008NV-15 for bug-gnu-emacs@gnu.org; Sat, 07 Dec 2019 21:54: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: Sun, 08 Dec 2019 02:54:01 +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.157577361432123 (code B ref 37774); Sun, 08 Dec 2019 02:54:01 +0000 Original-Received: (at 37774) by debbugs.gnu.org; 8 Dec 2019 02:53:34 +0000 Original-Received: from localhost ([127.0.0.1]:50056 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idmhR-0008M1-R7 for submit@debbugs.gnu.org; Sat, 07 Dec 2019 21:53:34 -0500 Original-Received: from mail-wm1-f50.google.com ([209.85.128.50]:51562) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idmhQ-0008Lg-At for 37774@debbugs.gnu.org; Sat, 07 Dec 2019 21:53:32 -0500 Original-Received: by mail-wm1-f50.google.com with SMTP id g206so11898818wme.1 for <37774@debbugs.gnu.org>; Sat, 07 Dec 2019 18:53:32 -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=HGe1JyZ9E3oZeItTHyV9F/0Z8+hFhhWzSsn9lre0vbk=; b=t1EEI7EQn8PacJB6fjPG2CmpE6224CGIy8eks1o5cMiWwMJFDMpuDE80W+4WUnXo53 71nNeLA685nbnSbA5n/vFy3jE3vYF3nAQ4abb+Q3q0QAkwmeaeoN/YFUBjTl0S35/R4R a7kPf48v+M4KyzFyZkIKHl8Z9f/AcoVuH7hnCcZW6lENSmxKkqBU1czHmxEUZHMybmW6 3JQB86hq1e9UsqyO0+UwgErvZcx0minjlaPL+vgIEsBIyo03HRUOSzw0ZcyH50E+LKyR I+cX1v884XEC350Q2aK1P6NVK6MFA8Tm45p/lRmbXXvGf9iG2IVEaePdms3B0C0IQ2eg W6QA== 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=HGe1JyZ9E3oZeItTHyV9F/0Z8+hFhhWzSsn9lre0vbk=; b=gzKrfoBa4kPPyJKvhiBIpcKkd44gzAhu1WM9lYOWWOhNkHAcjFzIUOBmjnmsNneFmD Jxf4YmBuQyXJ5D9y/9F6eaHoETUywYmin3ON+2A5KNCUJS8jkBjgBY2XrT5X8RgWJz4M Y+cIXEprFvPGfFu78ECoRLqdVRiIOJ4gN2pdg0mbVuziYBgqJHDrsJ7lRnDNNZzS6TlD xbU9HeXxhwBv1jUAdFtT3jCtZuGZ2PCL1FdbI/t1lm/hlsSAsCJwVygrdpomahTYNzEZ uDAMIbfW0MqwYajOzM+Z64Wf1ftecdWrzACjIb0VohzcLkJVldQBJNZC/Qmp6kiRX+05 qF5w== X-Gm-Message-State: APjAAAUmHyRHZqqrQDbxOv4LYppt/6Dzz9UUYHWa362Do14jkeBxAR8a lhQ9BFGp9CS9WBtgyU/YX6OPgCPo X-Google-Smtp-Source: APXvYqyRJyCFYcCHLToYdioZzXlnGTI3WVQocJPJ+gjH51CuiG9a5+Y/TKw4KpS6+vcXerGxF2VaKA== X-Received: by 2002:a1c:2dc8:: with SMTP id t191mr16263629wmt.75.1575738371484; Sat, 07 Dec 2019 09:06:11 -0800 (PST) Original-Received: from [192.168.0.5] ([212.50.117.215]) by smtp.googlemail.com with ESMTPSA id u10sm7139896wmd.1.2019.12.07.09.06.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 07 Dec 2019 09:06:09 -0800 (PST) In-Reply-To: <83o8wkfu63.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:173002 Archived-At: This is a multi-part message in MIME format. --------------D7B21DABB797416A74327DD1 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 07.12.2019 9:53, Eli Zaretskii wrote: >>> 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. Yes, sure. >> 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. Eli, what you're asking for would be actively harmful. To make an analogy, when we're changing a core Emacs behavior, we shouldn't try to make it work only on Tuesdays. Even if the original bug reporter observed the problem on a Tuesday. Can we please focus on the more pressing question: whether the proposed patch actually works, and does that reliably, or are there scenarios/configurations where it would do something unexpected. >> 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? Ok, I think I understand the distinction: it's for when a character has several faces specified for it. Sure. It's easy to fix anyway. >> 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. plist-member, you mean. >> 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. Updated patch attached. --------------D7B21DABB797416A74327DD1 Content-Type: text/x-patch; name="inherit-face-extend-spec-3.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="inherit-face-extend-spec-3.diff" diff --git a/lisp/faces.el b/lisp/faces.el index dc5bcca760..0f31628f5f 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,20 @@ 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 ((tail (plist-member default-attrs :extend))) + (and tail (face-spec-set-2 face frame + (list :extend (cadr tail)))))) (setq face-attrs (face-spec-choose (get face 'face-override-spec) frame)) (face-spec-set-2 face frame face-attrs))) --------------D7B21DABB797416A74327DD1--