On Mon, May 19 2014, Mark Walters wrote: > On Mon, 19 May 2014, David Edmondson 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@notmuchmail.org >> http://notmuchmail.org/mailman/listinfo/notmuch