Hi Many thanks for the review. On Sat, 22 Mar 2014, Austin Clements <amdragon@MIT.EDU> wrote: > Quoth Mark Walters on Mar 12 at 4:09 am: >> Add customize options for deleted/added tag formats. These are not >> used yet but will be later in the series. >> >> We switch to using `notmuch-apply-face' rather than `propertize' in >> the defcustom for faces so that the faces for deleted/added tags add >> to the default face attributes for the tag. >> >> We special case deleting the unread tag as that tag is a strong visual >> cue and we don't need that cue when we are just saying it used to be >> unread. Thus, we revert to the normal tag face with strikethough for >> deleted unread tags. >> --- >> emacs/notmuch-tag.el | 37 ++++++++++++++++++++++++++++++++++++- >> 1 files changed, 36 insertions(+), 1 deletions(-) >> >> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el >> index a4dea39..4698856 100644 >> --- a/emacs/notmuch-tag.el >> +++ b/emacs/notmuch-tag.el >> @@ -39,7 +39,7 @@ (define-widget 'notmuch-tag-format-type 'lazy >> (string :tag "Display as") >> (list :tag "Face" :extra-offset -4 >> (const :format "" :inline t >> - (propertize tag 'face)) >> + (notmuch-apply-face tag)) >> (list :format "%v" >> (const :format "" quote) >> custom-face-edit)) >> @@ -89,6 +89,41 @@ (defcustom notmuch-tag-formats >> :group 'notmuch-show >> :type 'notmuch-tag-format-type) >> >> +(defcustom notmuch-tag-deleted-formats >> + '(("unread" (notmuch-apply-face "unread" >> + (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))))) > > Aren't the two formatters above the same thing? Is this somehow > different from just having the .* formatter? The indentation also > seems a little odd, though that may just be the diff. These are not the same because these functions apply to tag which is the partially formatted tag, not tag itself. Thus the latter applies the above formatting to the string "unread" rather than `tag' which would be the string unread with previous formatting applied. It might be nice to change the name in the function so a formatter has access to tag and formatted-tag but that would break people's existing formats. > >> + "Custom formats for tags when deleted. >> + >> +For deleted tags the formats in `notmuch-tag-formats` are applied >> +first and then these formats are applied on top. >> + >> +By default this shows deleted tags with strike-through in red, >> +unless strike-through is not available (e.g., emacs is running in >> +a terminal) in which case it uses inverse video. To hide deleted >> +tags completely set this to >> + '((\".*\" nil)) >> + >> +See `notmuch-tag-formats' for full documentation." >> + :group 'notmuch-show >> + :type 'notmuch-tag-format-type) >> + >> +(defcustom notmuch-tag-added-formats >> + '((".*" (notmuch-apply-face tag '(:underline "green")))) >> + "Custom formats for tags when added. >> + >> +For added tags the formats in `notmuch-tag-formats` are applied >> +first and then these formats are applied on top. > > Maybe add > > To disable special formatting of added tags, set this to (). > > to parallel notmuch-tag-deleted-formats? Yes this is a good point as it is not obvious what the correct form is as it is different from the deleted case (here we want to apply no further formatting to the tag so it shows as normal, there we wanted to hide the tag) Best wishes Mark > >> + >> +See `notmuch-tag-formats' for full documentation." >> + :group 'notmuch-show >> + :type 'notmuch-tag-format-type) >> + >> (defun notmuch-tag-format-image-data (tag data) >> "Replace TAG with image DATA, if available. >>