Date: Mon, 29 Oct 2012 20:57:00 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: James Vasile <james@hackervisions.org>
Subject: Re: [PATCH] don't show x-foo tags in search view
Message-ID: <20121030005700.GE15377@mit.edu>
References: <87fw4x3y3e.fsf@hackervisions.org>
	<87liepw0b4.fsf@convex-new.cs.unb.ca>
	<87625tc6xd.fsf@hackervisions.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87625tc6xd.fsf@hackervisions.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch mailing list <notmuch@notmuchmail.org>
Precedence: list

I like it.

Quoth James Vasile on Oct 29 at  5:19 pm:
> This patch hides any tags in search view that match the regex specified
> in `notmuch-search-hide-tag-regex`.  That variable can be set via setq
> or the customize interface.  To hide all tags that begin with "x-" or
> "X-", set `notmuch-search-hide-tag-regex` to "^X-".
> 
> ---
>  emacs/notmuch.el |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index f9454d8..4bff538 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -775,6 +775,14 @@ non-authors is found, assume that all of the authors match."
>           (overlay-put overlay 'isearch-open-invisible #'delete-overlay)))
>        (insert padding))))
>  
> +(defcustom notmuch-search-hide-tag-regex ""
> +  "Regex specifying tags to hide in search view.

I have no idea why, but Emacs typically uses "regexp" instead of
"regex".

> +
> +Leave blank to disable hiding of tags in search view.

Saying "Leave blank" supposes that the user knows what the default
value is.  How about "An empty string disables hiding of tags in
search view."?

Even better, though, would be to use nil to indicate this, since "" is
a perfectly valid regexp and matches everything.  In that case, this
should say something like "If nil, no tags will be hidden in search
view."

> +Note: elisp regexes are case-insensitive"

Likewise, "regexps".  Also, Elisp regexps are not, in general,
case-insensitive.  If we want to control this, we should bind
case-fold-search to nil around the string-match below and say
something here like "Matching is case-insensitive."

> +  :type 'string

Better would be 'regexp.  Or, '(choice (const :tag "None" nil) regexp)
to allow nil or a regexp.

> +  :group 'notmuch-search)
> +
>  (defun notmuch-search-insert-field (field format-string result)
>    (cond
>     ((string-equal field "date")
> @@ -793,7 +801,13 @@ non-authors is found, assume that all of the authors match."
>      (notmuch-search-insert-authors format-string (plist-get result :authors)))
>  
>     ((string-equal field "tags")
> -    (let ((tags-str (mapconcat 'identity (plist-get result :tags) " ")))
> +    (let ((tags-str (mapconcat 'identity
> +                              (delq nil
> +                                    (mapcar (lambda (x) (if (and (not (equal notmuch-search-hide-tag-regex ""))
> +                                                                 (string-match notmuch-search-hide-tag-regex x))
> +                                                            nil
> +                                                            x)) (plist-get result :tags)))
> +                              " ")))

It would be simpler and more robust to use remove-if here.  What about
something like

  (let ((tags-str
         (mapconcat 'identity
                    (if notmuch-search-hide-tag-regex
                        (let ((case-fold-search t))
                          (remove-if
                           (apply-partially #'string-match
                                            notmuch-search-hide-tag-regex)
                           (plist-get result :tags)))
                      (plist-get result :tags))
                    " ")))

?

>        (insert (propertize (format format-string tags-str)
>                           'face 'notmuch-tag-face))))))
>