Date: Sat, 5 Apr 2014 21:24:25 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Subject: Re: [PATCH 4/5] emacs: hello: switch notmuch-hello-insert-buttons to
	plists
Message-ID: <20140406012425.GF15472@mit.edu>
References: <1396733065-32602-1-git-send-email-markwalters1009@gmail.com>
	<1396733065-32602-5-git-send-email-markwalters1009@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1396733065-32602-5-git-send-email-markwalters1009@gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Mark Walters on Apr 05 at 10:24 pm:
> Switching notmuch-hello-insert-buttons to plists means we can easily
> pass extra options through to the buttons.
> ---
>  emacs/notmuch-hello.el |   33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index c729af2..aa40e6f 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -425,7 +425,9 @@ (defun notmuch-hello-query-counts (query-list &rest options)
>  these is a plist but other options are available for backwards
>  compatibility: see notmuch-saved-search-get for details.
>  
> -The result is the list of elements of the form (NAME QUERY COUNT).
> +The result is a list of plists each of which includes the
> +pairs :name NAME, :query QUERY and :count COUNT, together with
> +any pairs in the original saved-search.

"Pair" can be synonymous with "cons", which is not what you mean here.
"Properties"?

>  
>  The values :show-empty-searches, :filter and :filter-count from
>  options will be handled as specified for
> @@ -455,23 +457,26 @@ (defun notmuch-hello-query-counts (query-list &rest options)
>       #'identity
>       (mapcar
>        (lambda (elem)
> -	(let ((name (notmuch-saved-search-get elem :name))
> -	      (search-query (notmuch-saved-search-get elem :query))
> -	      (message-count (prog1 (read (current-buffer))
> +	(let* ((elem-plist (notmuch-hello-saved-search-to-plist elem))
> +	       (search-query (plist-get elem-plist :query))
> +	       (filtered-query (notmuch-hello-filtered-query
> +				search-query (plist-get options :filter)))
> +	       (message-count (prog1 (read (current-buffer))
>  				(forward-line 1))))
>  	  (and (or (plist-get options :show-empty-searches) (> message-count 0))
> -	       (list name (notmuch-hello-filtered-query
> -			   search-query (plist-get options :filter))
> -		     message-count))))
> +	       (setq elem-plist (plist-put elem-plist :query filtered-query))

This technically works, but `setq' is a strange thing to see in an
`and'.  But the problem isn't the `setq'; it's that crazy `and'.  I'd
replace the `and' with `when', keep the `setq' and `plist-put' in the
body, and squint a lot less at this code.

> +	       (plist-put elem-plist :count message-count))))
>        query-list))))
>  
>  (defun notmuch-hello-insert-buttons (searches)
>    "Insert buttons for SEARCHES.
>  
> -SEARCHES must be a list containing lists of the form (NAME QUERY COUNT), where
> -QUERY is the query to start when the button for the corresponding entry is
> -activated. COUNT should be the number of messages matching the query.
> -Such a list can be computed with `notmuch-hello-query-counts'."
> +SEARCHES must be a list of plists each of which should contain at
> +least pairs for :name NAME :query QUERY and :count COUNT, where

Same comment about "pairs".

> +QUERY is the query to start when the button for the corresponding
> +entry is activated, and COUNT should be the number of messages
> +matching the query.  Such a plist can be computed with
> +`notmuch-hello-query-counts'."
>    (let* ((widest (notmuch-hello-longest-label searches))
>  	 (tags-and-width (notmuch-hello-tags-per-line widest))
>  	 (tags-per-line (car tags-and-width))
> @@ -489,9 +494,9 @@ (defun notmuch-hello-insert-buttons (searches)
>  	    (when elem
>  	      (if (> column-indent 0)
>  		  (widget-insert (make-string column-indent ? )))
> -	      (let* ((name (first elem))
> -		     (query (second elem))
> -		     (msg-count (third elem)))
> +	      (let* ((name (plist-get elem :name))
> +		     (query (plist-get elem :query))
> +		     (msg-count (plist-get elem :count)))
>  		(widget-insert (format "%8s "
>  				       (notmuch-hello-nice-number msg-count)))
>  		(widget-create 'push-button