On Tue, 11 Feb 2014, Mark Walters <markwalters1009@gmail.com> wrote: > Thanks for the review. > > On Mon, 10 Feb 2014, Austin Clements <amdragon@MIT.EDU> wrote: >> On Sat, 18 Jan 2014, Mark Walters <markwalters1009@gmail.com> wrote: >>> Allow an empty string in notmuch-tag-formats which matches all tags >>> except those matched explicitly matched. This allows the user to tell >> >> Typo. > > Will fix. > >>> notmuch to hide all tags except those specified. >>> >>> This will be useful once formatting for deleted/added tags is added >>> later in the series: a user might want to hide all deleted tags for >>> example. >>> --- >>> emacs/notmuch-tag.el | 20 +++++++++++--------- >>> 1 files changed, 11 insertions(+), 9 deletions(-) >>> >>> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el >>> index 2153068..92c1249 100644 >>> --- a/emacs/notmuch-tag.el >>> +++ b/emacs/notmuch-tag.el >>> @@ -65,14 +65,15 @@ >>> This gives a list that maps from tag names to lists of formatting >>> expressions. The car of each element gives a tag name and the >>> cdr gives a list of Elisp expressions that modify the tag. If >>> -the list is empty, the tag will simply be hidden. Otherwise, >>> -each expression will be evaluated in order: for the first >>> -expression, the variable `tag' will be bound to the tag name; for >>> -each later expression, the variable `tag' will be bound to the >>> -result of the previous expression. In this way, each expression >>> -can build on the formatting performed by the previous expression. >>> -The result of the last expression will displayed in place of the >>> -tag. >>> +the car is an empty string it matches all tags that do not have >>> +an explicit match. If the list is empty, the tag will simply be >> >> Hmm. I'm not sure I like overloading of the meanings of strings. Could >> we instead use a symbol to represent this case? For example, `t' would >> parallel the fall-through case of `cond' and `case', or `_' would >> parallel `pcase' [1]. Or even a separate variable like >> notmuch-tag-default-format? > > I would prefer not to have a separate variable as I want the default > case for added/deleted tags too (see next patch), so it would need to be > 3 separate variables. But maybe that makes things clearer and is worth > doing? Ah, I see. In that case I agree this shouldn't be a separate variable (that idea was just a workaround anyway). > One other possibility that would solve the customize problem would be to > allow regexps for the matches. The regexp would have to match the > complete tag and only the first match would be used. This has advantages > (the user can highlight all notmuch::.* tags or can hide all X- > tags). The downside is that finding/testing for a regexp match is about > 20 times slower than using assoc, primarily because assoc is written in > C and the regexp match bit in lisp. I really like the idea of using regexps for this. I wouldn't worry about the performance. We can almost certainly eliminate that problem with a sprinkle of caching (which would probably be even faster than assoc), and that can be added separately. >> The former would require some tweaking of the customize widget, but that >> should probably happen anyway to support this special case. >> Unfortunately, we may need a custom alist widget variant to do that. (I >> tried and failed to tweak it in a way that both worked and looked >> decent.) Hence my suggestion of a separate variable, which would only >> require pulling out the :value-type into its own define-widget. > > If we go this route I may need some help getting this to work: pulling > out value-type didn't work on my first attempt. > >> I'm also slightly bothered that this would introduce a second way to >> control the default formatting of tags in addition to notmuch-tag-face, >> but only slightly. > > Yes that slightly bothered me but I didn't see a solution. > >> [1] It's unfortunate that pcase wasn't introduced until Emacs 24. I've >> been tempted to backport it for notmuch multiple times now. Then we >> could just treat notmuch-tag-formats as a list of pcase conditions. > > Would that still involve a lisp loop so would be comparable to the > regexp bit above? I haven't looked at pcase enough to work out its > details. pcase compiles into a decision tree, which could in principle reduce this to a fast lookup when possible, though I don't know if the implementation is actually that smart. At any rate, I like the idea of using regexps for this better anyway. > Best wishes > > Mark > > >> >>> +hidden. Otherwise, each expression will be evaluated in order: >>> +for the first expression, the variable `tag' will be bound to the >>> +tag name; for each later expression, the variable `tag' will be >>> +bound to the result of the previous expression. In this way, >>> +each expression can build on the formatting performed by the >>> +previous expression. The result of the last expression will >>> +displayed in place of the tag. >>> >>> For example, to replace a tag with another string, simply use >>> that string as a formatting expression. To change the foreground >>> @@ -140,7 +141,8 @@ This can be used with `notmuch-tag-format-image-data'." >>> >>> (defun notmuch-tag-format-tag (tag) >>> "Format TAG by looking into `notmuch-tag-formats'." >>> - (let ((formats (assoc tag notmuch-tag-formats))) >>> + (let ((formats (or (assoc tag notmuch-tag-formats) >>> + (assoc "" notmuch-tag-formats)))) >>> (cond >>> ((null formats) ;; - Tag not in `notmuch-tag-formats', >>> tag) ;; the format is the tag itself. >>> -- >>> 1.7.9.1 >>> >>> _______________________________________________ >>> notmuch mailing list >>> notmuch@notmuchmail.org >>> http://notmuchmail.org/mailman/listinfo/notmuch