On Wed, 25 May 2011 15:11:16 -0400, Austin Clements <amdragon@mit.edu> wrote:
> At least in Emacs 23.3.1, it has to be (const :tag "tag:TAG" nil).  I
> didn't think the order mattered, but the tag didn't display otherwise.
>  It would also be good to give descriptive tags to the other choices.
> 
> It would be more consistent if the function form of this also returned
> a filter expression, rather than a whole expression.  This also
> simplifies the documentation.

Thanks for the suggestions. (At first, I wanted to allow the user
to be able to also include messages that don't have that tag, but this
really no longer belongs in the all tags-section).

> So, perhaps something like
>
> (defcustom notmuch-hello-tag-list-counts nil
>   "Method for generating counts displayed in the all tags list.
> 
> This variable controls the query used to generate counts for each
> tag in the \"all tags\" list.  If nil, the tag list will count
> all messages with each tag.  This can be a query string that will
> filter the messages counted for each tag.  Finally, this can be a
> function that will be called for each tag and should return a
> filter query for that tag, or nil to hide the tag."
>   :type '(choice (const :tag "Count all messages" nil)
>                  (const :tag "Count unread messages" "tag:unread")
>                  (const :tag "Custom filter" string)
>                  (const :tag "Custom filter function" function))
>   :group 'notmuch)

That is slightly less accurate though, since the query is not only used
to generate the counts, but also the searches that are performed when
one presses return on a tag in the list.

> > +(defun notmuch-hello-generate-tag-alist ()
> > +  "Return an alist from tags to queries to display in the all-tags section."
> > +  (notmuch-filter
> > +   'cdr
> > +   (mapcar '(lambda (tag)
> 
> You don't need the quote before a lambda form.  (You can also change
> 'cdr to #'cdr to further hint the compiler, though it appears to not
> matter in this case.)

Oh, yeah, normally I'm aware of that, don't know what made me quote it
at the time I wrote it.

> > +(defun notmuch-filter (pred lst)
> > +  "Return a list containing all elements from LST that satisfy PRED."
> 
> notmuch-remove-if-not would be more canonical (yeah, it's unwieldy,
> blame Common Lisp).  Also, since Elisp doesn't do tail-recursion, the
> standard way to define a remove-if-not function is
> 
> (defun notmuch-remove-if-not (predicate list)
>   "Return a copy of LIST with all items not satisfying PREDICATE removed."
>   (let (out)
>     (while list
>       (when (funcall predicate (car list))
>         (push (car list) out))
>       (setq list (cdr list)))
>     (nreverse out)))
> 
> (Why oh why Elisp hasn't just made remove-if and remove-if-not
> standard I don't know; they're redefined all over the Elisp code base.
>  Any everybody's afraid to use cl-seq's remove-if-not because it's so
> ridiculously complicated.)

Ah, thanks. I guess too much exposure to Haskell (and how smart GHC is)
makes me write things in a functional style without thinking about
performance.

I incorporated most of you recommendations in the attached patch.