From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>
Subject: Re: [PATCH 4/5] emacs: hello: switch notmuch-hello-insert-buttons to
	plists
In-Reply-To: <20140406012425.GF15472@mit.edu>
References: <1396733065-32602-1-git-send-email-markwalters1009@gmail.com>
	<1396733065-32602-5-git-send-email-markwalters1009@gmail.com>
	<20140406012425.GF15472@mit.edu>
User-Agent: Notmuch/0.15.2+615~g78e3a93 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sun, 06 Apr 2014 06:31:29 +0100
Message-ID: <87vbunuiq6.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: notmuch@notmuchmail.org
Precedence: list


Hello

>> @@ -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.

This was actually a bug on my part: the filtered query can be nil (which
is used to mean hide this search); in earlier versions having a setq
inside the `and' achieved this but obviously not in this plist form. I
have switched to the when as suggested and added a test for
filtered-query being nil.

Best wishes

Mark