From: Mark Walters <markwalters1009@gmail.com>
To: David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH v1] emacs: `notmuch-search-find-stable-query-region'
	should never return an empty query.
In-Reply-To: <1400515756-31779-1-git-send-email-dme@dme.org>
References: <1400515756-31779-1-git-send-email-dme@dme.org>
User-Agent: Notmuch/0.15.2+615~g78e3a93 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Mon, 19 May 2014 22:55:53 +0100
Message-ID: <87lhtxzb4m.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


On Mon, 19 May 2014, David Edmondson <dme@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!) 

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.

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