(notmuch-apply-face tag
   (if (display-supports-face-attributes-p            '(:strike-through "red"))
       '(:strike-through "red")
             '(:inverse-video t)))

Mark Walters <markwalters1...@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

My commit d25d33ff intended to adapt following literal face spec in the
call to notmuch-apply-face into a new face declared with defface:

 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

Info docs for face attributes has:

     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
notmuch mailing list

Reply via email to