* [PATCH] emacs: make the remaining faces configurable.
@ 2016-08-14 22:17 Matt Armstrong
2016-09-12 11:16 ` David Bremner
2016-09-18 10:57 ` [PATCH] emacs: tag deleted face bugfix Mark Walters
0 siblings, 2 replies; 6+ messages in thread
From: Matt Armstrong @ 2016-08-14 22:17 UTC (permalink / raw)
To: notmuch
I believe this moves all "anonymous" face specifications in notmuch
code into a configurable defface.
---
NEWS | 8 ++++----
emacs/notmuch-tag.el | 45 ++++++++++++++++++++++++++++++++++-----------
2 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/NEWS b/NEWS
index 3a9c8d3..2ff78e9 100644
--- a/NEWS
+++ b/NEWS
@@ -6,10 +6,10 @@ Emacs
Face customization is easier
- New faces `notmuch-search-flagged-face` and
- `notmuch-search-unread-face` are used by default by
- `notmuch-search-line-faces`. Customize `notmuch-faces` to modify
- them.
+ New faces `notmuch-tag-unread`, `notmuch-tag-flagged`,
+ `notmuch-tag-deleted`, `notmuch-tag-added`,
+ `notmuch-search-flagged-face` and `notmuch-search-unread-face` are
+ now used by default. Customize `notmuch-faces` to modify them.
Ruby Bindings
-------------
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index a3f0c52..ec3c964 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -56,9 +56,23 @@
(string :tag "Custom")))
(sexp :tag "Custom")))))
+(defface notmuch-tag-unread
+ '((t :foreground "red"))
+ "Default face used for the unread tag.
+
+Used in the default value of `notmuch-tag-formats`."
+ :group 'notmuch-faces)
+
+(defface notmuch-tag-flagged
+ '((t :foreground "blue"))
+ "Face used for the flagged tag.
+
+Used in the default value of `notmuch-tag-formats`."
+ :group 'notmuch-faces)
+
(defcustom notmuch-tag-formats
- '(("unread" (propertize tag 'face '(:foreground "red")))
- ("flagged" (propertize tag 'face '(:foreground "blue"))
+ '(("unread" (propertize tag 'face 'notmuch-tag-unread))
+ ("flagged" (propertize tag 'face 'notmuch-tag-flagged)
(notmuch-tag-format-image-data tag (notmuch-tag-star-icon))))
"Custom formats for individual tags.
@@ -90,15 +104,17 @@ with images."
:group 'notmuch-faces
:type 'notmuch-tag-format-type)
+(defface notmuch-tag-deleted
+ '((((class color) (supports :strike-through)) :strike-through "red")
+ (t :inverse-video t))
+ "Face used to display deleted tags.
+
+Used in the default value of `notmuch-tag-deleted-formats`."
+ :group 'notmuch-faces)
+
(defcustom notmuch-tag-deleted-formats
- '(("unread" (notmuch-apply-face bare-tag
- (if (display-supports-face-attributes-p '(:strike-through "red"))
- '(:strike-through "red")
- '(:inverse-video t))))
- (".*" (notmuch-apply-face tag
- (if (display-supports-face-attributes-p '(:strike-through "red"))
- '(:strike-through "red")
- '(:inverse-video t)))))
+ '(("unread" (notmuch-apply-face bare-tag `notmuch-tag-deleted))
+ (".*" (notmuch-apply-face tag `notmuch-tag-deleted)))
"Custom formats for tags when deleted.
For deleted tags the formats in `notmuch-tag-formats` are applied
@@ -118,8 +134,15 @@ See `notmuch-tag-formats' for full documentation."
:group 'notmuch-faces
:type 'notmuch-tag-format-type)
+(defface notmuch-tag-added
+ '((t :underline "green"))
+ "Default face used for added tags.
+
+Used in the default value for `notmuch-tag-added-formats`."
+ :group 'notmuch-faces)
+
(defcustom notmuch-tag-added-formats
- '((".*" (notmuch-apply-face tag '(:underline "green"))))
+ '((".*" (notmuch-apply-face tag 'notmuch-tag-added)))
"Custom formats for tags when added.
For added tags the formats in `notmuch-tag-formats` are applied
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] emacs: make the remaining faces configurable.
2016-08-14 22:17 [PATCH] emacs: make the remaining faces configurable Matt Armstrong
@ 2016-09-12 11:16 ` David Bremner
2016-09-18 10:57 ` [PATCH] emacs: tag deleted face bugfix Mark Walters
1 sibling, 0 replies; 6+ messages in thread
From: David Bremner @ 2016-09-12 11:16 UTC (permalink / raw)
To: Matt Armstrong, notmuch
Matt Armstrong <marmstrong@google.com> writes:
> I believe this moves all "anonymous" face specifications in notmuch
> code into a configurable defface.
> ---
pushed,
d
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] emacs: tag deleted face bugfix
2016-08-14 22:17 [PATCH] emacs: make the remaining faces configurable Matt Armstrong
2016-09-12 11:16 ` David Bremner
@ 2016-09-18 10:57 ` Mark Walters
2016-09-18 11:20 ` Tomi Ollila
` (2 more replies)
1 sibling, 3 replies; 6+ messages in thread
From: Mark Walters @ 2016-09-18 10:57 UTC (permalink / raw)
To: notmuch, marmstrong
Commit d25d33ff cleaned up some of the tag face code. However, for the
face notmuch-tag-deleted it used the test
((class color) (supports :strike-through))
to decide whether to use red strikethrough or inverse-video (emacs in
a terminal typically doesn't support red strikethrough, but in X it does).
However, it seems that test often returns true even though red
strikethrough is not supported. This breaks the tag update code -- the
wrong thing is displayed to the user.
Thus we make the test explicitly more specific, changing the test to
((class color) (supports :strike-through "red"))
---
Tomi found this bug today, and narrowed it down to a recent notmuch
change. This seems to fix it, and the code now seems to work as
expected in terminals and in X. However I am not an expert on emacs
faces so there may be a better way.
Best wishes
Mark
emacs/notmuch-tag.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 644ce40..e59f148 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -137,7 +137,7 @@ with images."
:type 'notmuch-tag-format-type)
(defface notmuch-tag-deleted
- '((((class color) (supports :strike-through)) :strike-through "red")
+ '((((class color) (supports :strike-through "red")) :strike-through "red")
(t :inverse-video t))
"Face used to display deleted tags.
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] emacs: tag deleted face bugfix
2016-09-18 10:57 ` [PATCH] emacs: tag deleted face bugfix Mark Walters
@ 2016-09-18 11:20 ` Tomi Ollila
2016-09-20 0:42 ` Matt Armstrong
2016-09-25 10:52 ` David Bremner
2 siblings, 0 replies; 6+ messages in thread
From: Tomi Ollila @ 2016-09-18 11:20 UTC (permalink / raw)
To: Mark Walters, notmuch, marmstrong
On Sun, Sep 18 2016, Mark Walters <markwalters1009@gmail.com> wrote:
> Commit d25d33ff cleaned up some of the tag face code. However, for the
> face notmuch-tag-deleted it used the test
>
> ((class color) (supports :strike-through))
>
> to decide whether to use red strikethrough or inverse-video (emacs in
> a terminal typically doesn't support red strikethrough, but in X it does).
>
> However, it seems that test often returns true even though red
> strikethrough is not supported. This breaks the tag update code -- the
> wrong thing is displayed to the user.
>
> Thus we make the test explicitly more specific, changing the test to
>
> ((class color) (supports :strike-through "red"))
> ---
>
> Tomi found this bug today, and narrowed it down to a recent notmuch
> change. This seems to fix it, and the code now seems to work as
> expected in terminals and in X. However I am not an expert on emacs
> faces so there may be a better way.
The change works for me and IMO it looks sensible
>- '((((class color) (supports :strike-through)) :strike-through "red")
>+ '((((class color) (supports :strike-through "red")) :strike-through "red")
I tried to look documentation and grepped some emacs (lisp!) source to
verify that this fix is exactly as it should be, but could not find any.
Well, at least it looks better than what it used to be...
+1
Tomi
>
> Best wishes
>
> Mark
>
>
>
> emacs/notmuch-tag.el | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index 644ce40..e59f148 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -137,7 +137,7 @@ with images."
> :type 'notmuch-tag-format-type)
>
> (defface notmuch-tag-deleted
> - '((((class color) (supports :strike-through)) :strike-through "red")
> + '((((class color) (supports :strike-through "red")) :strike-through "red")
> (t :inverse-video t))
> "Face used to display deleted tags.
>
> --
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] emacs: tag deleted face bugfix
2016-09-18 10:57 ` [PATCH] emacs: tag deleted face bugfix Mark Walters
2016-09-18 11:20 ` Tomi Ollila
@ 2016-09-20 0:42 ` Matt Armstrong
2016-09-25 10:52 ` David Bremner
2 siblings, 0 replies; 6+ messages in thread
From: Matt Armstrong @ 2016-09-20 0:42 UTC (permalink / raw)
To: Mark Walters, notmuch
(notmuch-apply-face tag
(if (display-supports-face-attributes-p '(:strike-through "red"))
'(:strike-through "red")
'(:inverse-video t)))
Mark Walters <markwalters1009@gmail.com> writes:
> Commit d25d33ff cleaned up some of the tag face code. However, for the
> face notmuch-tag-deleted it used the test
>
> ((class color) (supports :strike-through))
>
> to decide whether to use red strikethrough or inverse-video (emacs in
> a terminal typically doesn't support red strikethrough, but in X it does).
>
> However, it seems that test often returns true even though red
> strikethrough is not supported. This breaks the tag update code -- the
> wrong thing is displayed to the user.
>
> Thus we make the test explicitly more specific, changing the test to
>
> ((class color) (supports :strike-through "red"))
> ---
>
> Tomi found this bug today, and narrowed it down to a recent notmuch
> change. This seems to fix it, and the code now seems to work as
> expected in terminals and in X. However I am not an expert on emacs
> faces so there may be a better way.
>
> Best wishes
>
> Mark
Mark, thanks, the patch looks correct to me. I apologize for the
regression.
My commit d25d33ff intended to adapt following literal face spec in the
call to notmuch-apply-face into a new face declared with defface:
(notmuch-apply-face
tag (if (display-supports-face-attributes-p
'(:strike-through "red"))
'(:strike-through "red")
'(:inverse-video t)))
Which would be what you've done in this patch:
(((class color) (supports :strike-through "red")) :strike-through "red")
I omitted the "red" portion of the supports string, which looks to be an
error.
Info docs for face attributes has:
‘:strike-through’
Whether or not characters should be strike-through, and in what
color. The value is used like that of ‘:overline’.
This suggests that supplying a color is appropriate.
> emacs/notmuch-tag.el | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
> index 644ce40..e59f148 100644
> --- a/emacs/notmuch-tag.el
> +++ b/emacs/notmuch-tag.el
> @@ -137,7 +137,7 @@ with images."
> :type 'notmuch-tag-format-type)
>
> (defface notmuch-tag-deleted
> - '((((class color) (supports :strike-through)) :strike-through "red")
> + '((((class color) (supports :strike-through "red")) :strike-through "red")
> (t :inverse-video t))
> "Face used to display deleted tags.
>
> --
> 2.1.4
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] emacs: tag deleted face bugfix
2016-09-18 10:57 ` [PATCH] emacs: tag deleted face bugfix Mark Walters
2016-09-18 11:20 ` Tomi Ollila
2016-09-20 0:42 ` Matt Armstrong
@ 2016-09-25 10:52 ` David Bremner
2 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2016-09-25 10:52 UTC (permalink / raw)
To: Mark Walters, notmuch, marmstrong
Mark Walters <markwalters1009@gmail.com> writes:
> Commit d25d33ff cleaned up some of the tag face code. However, for the
> face notmuch-tag-deleted it used the test
>
pushed,
d
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-25 10:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-14 22:17 [PATCH] emacs: make the remaining faces configurable Matt Armstrong
2016-09-12 11:16 ` David Bremner
2016-09-18 10:57 ` [PATCH] emacs: tag deleted face bugfix Mark Walters
2016-09-18 11:20 ` Tomi Ollila
2016-09-20 0:42 ` Matt Armstrong
2016-09-25 10:52 ` David Bremner
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.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).