[PATCH v1] emacs: `notmuch-search-find-stable-query-region' should never return an empty query.

[David Edmondson]

On Mon, May 19 2014, Mark Walters wrote:
> On Mon, 19 May 2014, David Edmondson <dme at dme.org> wrote:
>> `notmuch-search-find-stable-query-region' is expected to examine the
>> region between `beg' and `end' to generate a query that can be used to
>> include all threads in that region. If the region contains no threads,
>> it should throw an error rather than generating an empty query.
>
> Hi
>
> This seems a very definite bug (when testing I managed to archive a
> whole chunk of random messages!) 

Do you understand why? I couldn't see a path from this problem to that
failure mode.

> However, I think I would prefer not to signal an error and just do
> nothing. How about making notmuch-tag check for a nil query (and do
> nothing it's nil). Then rather than an error n.s.f.s.q.r can just return
> nil (this still needs to be special cased as otherwise we get "()" as
> the query.

Looking around for similar issues (like
`notmuch-show-get-message-properties'), it seems that we would have to
add checks in a lot of places for functions such as this returning `nil'
when they cannot find some state or context.

Throwing an error makes it clear to the user that nothing is going to
happen - it's not just silent failure.

(If it's not clear - that was all "I like that it calls `error' :-).

> Doing this would also fix a bug I found (when seeing what we did
> elsewhere based on the above) in notmuch-tree: trying to change tags at
> the end of the buffer gives an error).
>
> Best wishes
>
> Mark
>
>
>
>> ---
>>
>> Whilst logging calls to 'notmuch' from the UI, I noticed that it would generate
>>   notmuch tag -inbox -- ()
>> if I hit 'a' at the very end of a search buffer. That seems at least
>> useless and possibly bad, so flag an error in this case instead.
>>
>> Oh, the first bit is just cleanup.
>>
>>  emacs/notmuch.el | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index 8aa0104..74103a6 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -429,12 +429,15 @@ matched and unmatched messages in the current thread."
>>  
>>  If ONLY-MATCHED is non-nil, include only matched messages.  If it
>>  is nil, include both matched and unmatched messages."
>> -  (let ((query-list nil) (all (not only-matched)))
>> +  (let ((all (not only-matched))
>> +	query-list)
>>      (dolist (queries (notmuch-search-properties-in-region :query beg end))
>>        (when (first queries)
>>  	(push (first queries) query-list))
>>        (when (and all (second queries))
>>  	(push (second queries) query-list)))
>> +    (unless query-list
>> +      (error "No threads in region."))
>>      (concat "(" (mapconcat 'identity query-list ") or (") ")")))
>>  
>>  (defun notmuch-search-find-authors ()
>> -- 
>> 2.0.0.rc0
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch at notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 310 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20140520/b8d6e2d0/attachment.pgp>