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: Sun, 08 Dec 2019 17:50:13 +0200 Message-ID: <83a782es0a.fsf@gnu.org> References: <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> <83r21gdnun.fsf@gnu.org> <839f1185-fc0c-0e8b-29fd-5431fb71ab2e@yandex.ru> <83o8wkdk3n.fsf@gnu.org> <45fcbf16-c9b4-ca15-7fa2-e7ea2137218c@yandex.ru> <83immreblb.fsf@gnu.org> <52b437d6-cbee-cea9-71c8-2a39311e602c@yandex.ru> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="152122"; 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 16:51:16 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 1idyq3-000dS0-LS for geb-bug-gnu-emacs@m.gmane.org; Sun, 08 Dec 2019 16:51:15 +0100 Original-Received: from localhost ([::1]:59482 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idyq2-0005RQ-FA for geb-bug-gnu-emacs@m.gmane.org; Sun, 08 Dec 2019 10:51:14 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:48252) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1idypw-0005R2-Ds for bug-gnu-emacs@gnu.org; Sun, 08 Dec 2019 10:51:10 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1idypr-00027S-01 for bug-gnu-emacs@gnu.org; Sun, 08 Dec 2019 10:51:08 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:46126) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1idypq-000244-OC for bug-gnu-emacs@gnu.org; Sun, 08 Dec 2019 10:51:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1idypp-0001eH-NQ for bug-gnu-emacs@gnu.org; Sun, 08 Dec 2019 10:51:01 -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 15:51: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.15758202446314 (code B ref 37774); Sun, 08 Dec 2019 15:51:01 +0000 Original-Received: (at 37774) by debbugs.gnu.org; 8 Dec 2019 15:50:44 +0000 Original-Received: from localhost ([127.0.0.1]:52099 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idypX-0001dl-P4 for submit@debbugs.gnu.org; Sun, 08 Dec 2019 10:50:44 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:38391) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1idypV-0001dW-6O for 37774@debbugs.gnu.org; Sun, 08 Dec 2019 10:50:42 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:46367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1idypM-0000Xv-MS; Sun, 08 Dec 2019 10:50:33 -0500 Original-Received: from [176.228.60.248] (port=2782 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1idypK-0001gd-Hj; Sun, 08 Dec 2019 10:50:32 -0500 In-reply-to: <52b437d6-cbee-cea9-71c8-2a39311e602c@yandex.ru> (message from Dmitry Gutov on Sun, 8 Dec 2019 12:39:06 +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:173056 Archived-At: > Cc: 37774@debbugs.gnu.org, juri@linkov.net > From: Dmitry Gutov > Date: Sun, 8 Dec 2019 12:39:06 +0200 > > > If we don't have a safe solution, we will have to live with that risk, > > unfortunately. > > We haven't even started the pretest yet. If there are bugs in this patch > (unlikely, but always possible), we have time for people to see and > report them. Experience teaches up that quite a few problems, especially in subtle areas, are discovered only after the release. I guess it means that the group of people who use the pretest is not representative enough. > >>> But I don't > >>> consider myself an expert on these matters, so if you say we cannot > >>> differentiate between general face definition and what themes do, so > >>> be it.) > >> > >> What's a "general face definition"? > > > > Everything except theme definition of faces. > > Please give an example. OK, I've now reviewed all the callers of face-spec-recalc, and all of its callers' callers, and wrote a bunch of tests to make sure that we don't break anything (or at least anything important). The tests in the patch below all pass for the current code on master, and include a couple of comments where the changes to implicitly inherit :extend by themes are supposed to change the expected result. If after applying your patch all the tests still pass, both in -batch and in an interactive session, then I think we are good to go (after adding the necessary documentation and NEWS entry). Thanks. diff --git a/test/lisp/faces-tests.el b/test/lisp/faces-tests.el index f00c93cedc..7cba4b26eb 100644 --- a/test/lisp/faces-tests.el +++ b/test/lisp/faces-tests.el @@ -36,6 +36,26 @@ "" :group 'faces--test) +(defface faces--test-extend + '((t :extend t :background "blue")) + "" + :group 'faces--test) + +(defface faces--test-no-extend + '((t :extend nil :background "blue")) + "" + :group 'faces--test) + +(defface faces--test-inherit-extend + '((t :inherit (faces--test-extend faces--test2) :background "blue")) + "" + :group 'faces--test) + +(defface faces--test-inherit-no-extend + '((t :inherit (faces--test2 faces--test-no-extend) :background "blue")) + "" + :group 'faces--test) + (ert-deftest faces--test-color-at-point () (with-temp-buffer (insert (propertize "STRING" 'face '(faces--test2 faces--test1))) @@ -69,5 +89,133 @@ ;; face IDs to faces. (should (> (face-id 'faces--test1) (face-id 'tooltip)))) +(ert-deftest faces--test-extend () + (should (equal (face-attribute 'faces--test-extend :extend) t)) + (should (equal (face-attribute 'faces--test-no-extend :extend) nil)) + (should (equal (face-attribute 'faces--test1 :extend) 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend nil t) t)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend nil t) + nil)) + ) + +(ert-deftest faces--test-extend-with-themes () + ;; Make sure the diff-mode faces are not defined. + (should-not (featurep 'diff-mode)) + (defface diff-changed-face + '((t :extend t :weight bold)) + "") + (defface diff-added + '((t :background "grey")) + "") + (defface diff-file-header-face + '((t :extend nil :foreground "cyan")) + "") + (should (equal (face-attribute 'diff-changed-face :extend) t)) + (should (equal (face-attribute 'diff-added :extend) 'unspecified)) + (should (equal (face-attribute 'diff-file-header-face :extend) nil)) + (load-theme 'manoj-dark t t) + (load-theme 'tsdh-light t t) + (should (equal (face-attribute 'faces--test-inherit-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend nil t) t)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend nil t) + nil)) + (should (equal (face-attribute 'diff-changed-face :extend) t)) + (should (equal (face-attribute 'diff-added :extend) 'unspecified)) + (should (equal (face-attribute 'diff-file-header-face :extend) nil)) + (enable-theme 'manoj-dark) + (should (equal (face-attribute 'faces--test-inherit-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend nil t) t)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend nil t) + nil)) + (should (equal (face-attribute 'diff-changed-face :extend) 'unspecified)) ; should be t + (should (equal (face-attribute 'diff-added :extend) t)) + (should (equal (face-attribute 'diff-file-header-face :extend) 'unspecified)) ; should be nil + (defface faces--test-face3 + '((t :inherit diff-added :weight bold)) + "") + (should (equal (face-attribute 'faces--test-face3 :extend nil t) t)) + (disable-theme 'manoj-dark) + (should (equal (face-attribute 'faces--test-inherit-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend nil t) t)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend nil t) + nil)) + (should (equal (face-attribute 'diff-changed-face :extend) t)) + (should (equal (face-attribute 'diff-added :extend) 'unspecified)) + (should (equal (face-attribute 'diff-file-header-face :extend) nil)) + (should (equal (face-attribute 'faces--test-face3 :extend nil t) 'unspecified)) + (defface diff-indicator-changed + '((t (:weight bold :extend t))) + "") + (enable-theme 'tsdh-light) + (should (equal (face-attribute 'faces--test-inherit-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend nil t) t)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend nil t) + nil)) + (should (equal (face-attribute 'diff-changed-face :extend) t)) + (should (equal (face-attribute 'diff-added :extend) t)) + (should (equal (face-attribute 'diff-file-header-face :extend) nil)) + (should (equal (face-attribute 'diff-indicator-changed :extend) 'unspecified)) ; should be t + (should (equal (face-attribute 'faces--test-face3 :extend nil t) t)) + (frame-set-background-mode (selected-frame) 'dark) + (should (equal (face-attribute 'faces--test-inherit-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend nil t) t)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-no-extend :extend nil t) + nil)) + (should (equal (face-attribute 'diff-changed-face :extend) t)) + (should (equal (face-attribute 'diff-added :extend) t)) + (should (equal (face-attribute 'diff-file-header-face :extend) nil)) + (should (equal (face-attribute 'diff-indicator-changed :extend) 'unspecified)) ; should be t + (should (equal (face-attribute 'faces--test-face3 :extend nil t) t)) + (or noninteractive + (let ((fr (make-frame))) + (should (equal (face-attribute 'faces--test-inherit-extend :extend fr) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-extend :extend fr t) + t)) + (should (equal (face-attribute 'faces--test-inherit-no-extend + :extend fr) + 'unspecified)) + (should (equal (face-attribute 'faces--test-inherit-no-extend + :extend fr t) + nil)) + (should (equal (face-attribute 'diff-changed-face :extend fr) t)) + (should (equal (face-attribute 'diff-added :extend fr) t)) + (should (equal (face-attribute 'diff-file-header-face :extend fr) nil)) + (should (equal (face-attribute 'diff-indicator-changed :extend fr) + 'unspecified)) ; should be t + (should (equal (face-attribute 'faces--test-face3 :extend nil t) t)) + )) + (disable-theme 'tsdh-light) + (should (equal (face-attribute 'diff-indicator-changed :extend) t)) + (should (equal (face-attribute 'faces--test-face3 :extend nil t) 'unspecified)) + (or noninteractive + (let ((fr (make-frame))) + (should (equal (face-attribute 'diff-changed-face :extend fr) t)) + (should (equal (face-attribute 'diff-added :extend fr) 'unspecified)) + (should (equal (face-attribute 'diff-file-header-face :extend fr) nil)) + (should (equal (face-attribute 'diff-indicator-changed :extend fr) t)) + (should (equal (face-attribute 'faces--test-face3 :extend nil t) 'unspecified)) + )) + ) + (provide 'faces-tests) ;;; faces-tests.el ends here