unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Dmitry Gutov <dgutov@yandex.ru>
Cc: 37774@debbugs.gnu.org, juri@linkov.net
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	[thread overview]
Message-ID: <83a782es0a.fsf@gnu.org> (raw)
In-Reply-To: <52b437d6-cbee-cea9-71c8-2a39311e602c@yandex.ru> (message from Dmitry Gutov on Sun, 8 Dec 2019 12:39:06 +0200)

> Cc: 37774@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> 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





  reply	other threads:[~2019-12-08 15:50 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-16  7:00 bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages Andrey Orst
2019-10-16  7:53 ` Eli Zaretskii
2019-10-16  8:12   ` Andrey Orst
2019-10-16 11:05     ` Eli Zaretskii
2019-10-16 14:20       ` Stefan Kangas
2019-10-16 16:25         ` Eli Zaretskii
2019-10-16  8:59   ` Michael Heerdegen
2019-10-16  9:17     ` martin rudalics
2019-10-16 11:26     ` Eli Zaretskii
2019-10-16 11:38       ` Michael Heerdegen
2019-10-16 12:59         ` Eli Zaretskii
2019-10-16 11:10   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 11:17     ` Andrey Orst
2019-10-16 11:41       ` Eli Zaretskii
2019-10-16 12:08         ` Andrey Orst
2019-10-16 13:05           ` Eli Zaretskii
2019-10-16 11:42     ` Eli Zaretskii
2019-10-16 13:18       ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 13:33         ` Andrey Orst
2019-10-16 14:21           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 17:33         ` Eli Zaretskii
2019-10-16 18:13           ` Andrey Orst
2019-10-16 18:50             ` Eli Zaretskii
2019-10-17 19:05             ` Kévin Le Gouguec
2019-10-17 20:38               ` Kévin Le Gouguec
2019-10-17 11:36           ` Michael Albinus
2019-10-17 12:38             ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-17 13:05             ` Eli Zaretskii
2019-10-17 16:18               ` Michael Albinus
2019-10-17 16:39                 ` Eli Zaretskii
2019-10-16 17:27     ` Juri Linkov
2019-10-16 18:07       ` Andrey Orst
2019-10-16 18:18         ` Juri Linkov
2019-10-16 18:54           ` Eli Zaretskii
2019-10-16 19:52             ` Juri Linkov
2019-10-16 20:06               ` Eli Zaretskii
2019-10-16 19:55             ` Eli Zaretskii
2019-10-16 20:14               ` Juri Linkov
2019-10-17  6:18                 ` Eli Zaretskii
2019-10-17  8:52                   ` Andrey Orst
2019-10-17  8:59                     ` Eli Zaretskii
2019-10-17  9:20                       ` Andrey Orst
2019-10-17 12:54                         ` Eli Zaretskii
2019-10-17 13:40                           ` Andrey Orst
2019-10-17 14:02                             ` Eli Zaretskii
2019-10-17 14:13                               ` Andrey Orst
2019-10-17 14:38                                 ` Eli Zaretskii
2019-10-17 14:02                       ` Dmitry Gutov
2019-10-17 16:20                         ` Eli Zaretskii
2019-10-17 16:46                           ` Dmitry Gutov
2019-10-17 17:20                             ` Eli Zaretskii
2019-10-17  8:25                 ` martin rudalics
2019-10-17 14:08                   ` Dmitry Gutov
2019-10-17 16:29                     ` Eli Zaretskii
2019-10-17 16:50                       ` Dmitry Gutov
2019-10-17 17:23                         ` Eli Zaretskii
2019-10-18 14:25                           ` Dmitry Gutov
2019-10-20 20:07                           ` Dmitry Gutov
2019-10-21  6:10                             ` Eli Zaretskii
2019-10-23 12:47                               ` Dmitry Gutov
2019-10-23 15:39                                 ` Eli Zaretskii
2019-10-23 16:12                                   ` Dmitry Gutov
2019-10-23 18:04                                     ` Eli Zaretskii
2019-10-23 20:28                                       ` Dmitry Gutov
2019-10-24 14:56                                         ` Eli Zaretskii
2019-10-24 17:04                                         ` Kévin Le Gouguec
2019-10-17 22:22                   ` Juri Linkov
2019-10-18  5:28                     ` Andrey Orst
2019-10-18  6:53                     ` Eli Zaretskii
2019-10-19 20:53                       ` Juri Linkov
2019-10-20  6:03                         ` Eli Zaretskii
2019-10-20 15:42                           ` Juri Linkov
2019-10-20 16:59                             ` Eli Zaretskii
2019-10-21 21:29                         ` Juri Linkov
2019-10-18  8:25                     ` martin rudalics
2019-10-18 12:41                       ` Dmitry Gutov
2019-10-18 13:04                       ` Andrey Orst
2019-10-17 13:56                 ` Dmitry Gutov
2019-10-17 16:28                   ` Eli Zaretskii
2019-10-17 13:54             ` Dmitry Gutov
2019-10-16 18:46       ` Eli Zaretskii
2019-10-16 19:46         ` Juri Linkov
2019-10-16 20:03           ` Eli Zaretskii
2019-10-16 20:23             ` Juri Linkov
2019-10-16 20:37               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-17  6:43                 ` Eli Zaretskii
2019-10-17  6:40               ` Eli Zaretskii
2019-10-16 20:29             ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-17 14:12               ` Dmitry Gutov
2019-10-16 20:23           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-10-16 11:33 ` Andrey Orst
2019-10-16 15:30   ` Eli Zaretskii
2019-10-31 16:06 ` Jonas Bernoulli
2019-10-31 16:48   ` Eli Zaretskii
2019-11-06 16:26     ` Jonas Bernoulli
2019-11-06 17:06       ` martin rudalics
2019-11-07 13:58         ` Eli Zaretskii
2019-11-07 15:41           ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-11-07 17:10             ` martin rudalics
2019-11-07 21:57               ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-11-08  9:20                 ` martin rudalics
2019-11-08 10:37                   ` Eli Zaretskii
2019-11-08 18:26                     ` martin rudalics
2019-11-08 19:14                       ` Eli Zaretskii
2019-11-08 11:39                   ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-11-08 18:27                     ` martin rudalics
2019-12-05 16:48                   ` Kévin Le Gouguec
2019-12-05 18:02                     ` Eli Zaretskii
2019-12-05 19:05                       ` Kévin Le Gouguec
2019-12-05 19:19                         ` Eli Zaretskii
2019-12-05 18:22                     ` Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2019-12-05 18:47                       ` Eli Zaretskii
2019-11-07 13:59       ` Eli Zaretskii
2019-11-11 10:52         ` Jonas Bernoulli
2019-11-11 19:03           ` Jonas Bernoulli
2019-11-14 11:33             ` Eli Zaretskii
2019-11-14 14:14               ` Dmitry Gutov
2019-11-14 14:41                 ` Eli Zaretskii
2019-11-14 22:42                   ` Dmitry Gutov
2019-11-15  8:08                     ` Eli Zaretskii
2019-11-15 23:50                       ` Dmitry Gutov
2019-11-16  8:09                         ` Eli Zaretskii
2019-11-16  8:17                           ` Dmitry Gutov
2019-11-23 23:20             ` Juri Linkov
2019-11-24 16:14               ` Eli Zaretskii
2019-11-25 23:45                 ` Juanma Barranquero
2019-11-26 17:44                   ` Eli Zaretskii
2019-11-25  0:29               ` Dmitry Gutov
2019-11-25 16:00                 ` Eli Zaretskii
2019-11-25 23:50                   ` Dmitry Gutov
2019-11-26 17:43                     ` Eli Zaretskii
2019-12-02  0:05                       ` Dmitry Gutov
2019-12-02 16:21                         ` Eli Zaretskii
2019-12-03  0:01                           ` Dmitry Gutov
2019-12-03 16:21                             ` Eli Zaretskii
2019-12-05  1:44                               ` Dmitry Gutov
2019-12-05 15:47                                 ` Eli Zaretskii
2019-12-06 15:44                                   ` Dmitry Gutov
2019-12-06 16:18                                     ` Eli Zaretskii
2019-12-06 16:58                                       ` Dmitry Gutov
2019-12-06 17:31                                         ` Eli Zaretskii
2019-12-07  1:06                                           ` Dmitry Gutov
2019-12-07  7:53                                             ` Eli Zaretskii
2019-12-07 17:06                                               ` Dmitry Gutov
2019-12-07 17:53                                                 ` Eli Zaretskii
2019-12-07 18:55                                                   ` Dmitry Gutov
2019-12-07 19:14                                                     ` Eli Zaretskii
2019-12-08  0:42                                                       ` Dmitry Gutov
2019-12-08  3:32                                                         ` Eli Zaretskii
2019-12-08 10:39                                                           ` Dmitry Gutov
2019-12-08 15:50                                                             ` Eli Zaretskii [this message]
2019-12-08 21:20                                                               ` Dmitry Gutov
2019-12-09 12:59                                                                 ` Eli Zaretskii
2019-12-09 14:07                                                                   ` Dmitry Gutov
2019-12-09 14:42                                                                     ` Eli Zaretskii
2019-12-09 15:24                                                                       ` Dmitry Gutov
2019-12-09 15:42                                                                         ` Eli Zaretskii
2019-12-10  0:20                                                                           ` Dmitry Gutov
2019-12-10 15:57                                                                             ` Eli Zaretskii
2019-12-11 23:02                                                                               ` Juri Linkov
2019-12-12  4:36                                                                                 ` Eli Zaretskii
2019-12-12 23:44                                                                                   ` Juri Linkov
2019-12-13  7:03                                                                                     ` Eli Zaretskii
2019-11-27 21:30                     ` Juri Linkov
2019-11-27 23:34                       ` Dmitry Gutov
2019-11-28 15:19                         ` Eli Zaretskii
2019-11-28 15:16                       ` Eli Zaretskii
2019-11-30 11:35                         ` Eli Zaretskii
2019-12-02  0:07                           ` Dmitry Gutov
2019-10-31 17:29   ` Dmitry Gutov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83a782es0a.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=37774@debbugs.gnu.org \
    --cc=dgutov@yandex.ru \
    --cc=juri@linkov.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).