On Tue, 20 May 2014, David Edmondson <dme at dme.org> wrote: > 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.
Ok I now understand some of why it is a bug and it is a pretty gross mix: emacs does something odd, then notmuch-tag.c then xapian. It only happens in some cases: WARNING the following will archive some random messages: have a dump or backup or something! if you do "a" at the end of the buffer then the emacs code creates the trivial query: () Then runs notmuch tag -inbox -- () in notmuch-tag.c we are clever and optimise the query (to avoid trying to remove the tag from messages that don't have it) and this becomes the query-string ( () ) and ( tag:inbox ) for reasons which aren't clear to me this matches some but not all of the messages with tag:inbox. This is despite the fact that searching for ( () ) or () by itself yields no results. You can test this bit just with notmuch search or count notmuch count "(()) and tag:inbox" Since this email is rather long I will reply to the rest separately. Best wishes Mark > >> 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