Re: [PATCH] emacs: tag deleted face bugfix

[Matt Armstrong <marmstrong@google.com>]

(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