From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 1/6] lib: change default for
	notmuch_query_set_omit_excluded
In-Reply-To: <1333234442-28616-2-git-send-email-markwalters1009@gmail.com>
References: <1333234442-28616-1-git-send-email-markwalters1009@gmail.com>
	<1333234442-28616-2-git-send-email-markwalters1009@gmail.com>User-Agent:
	Notmuch/0.11.1+358~g3b96caf (http://notmuchmail.org)
	Emacs/23.3.1 (i486-pc-linux-gnu)
Date: Thu, 05 Apr 2012 21:09:40 -0400
Message-ID: <8762ddd5t7.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Sat, 31 Mar 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> ---
>  lib/notmuch.h |   11 ++++++-----
>  lib/query.cc  |   10 +++++-----
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index babd208..029a2c3 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -449,12 +449,13 @@ typedef enum {
>  const char *
>  notmuch_query_get_query_string (notmuch_query_t *query);
>  
> -/* Specify whether to results should omit the excluded results rather
> - * than just marking them excluded. This is useful for passing a
> - * notmuch_messages_t not containing the excluded messages to other
> - * functions. */
> +/* Specify whether to omit the excluded results or just flag
> + * them. Note when calling notmuch_query_search_threads, the returned
> + * thread will contain all messages regardless of this setting but,
> + * unless this is unset, only threads matching in a non-excluded
> + * message will be returned. */

The docs seem a bit awkward (I suppose the interface is slightly
awkward, but the comment makes it sound much more so) and I don't see a
mention of the default behavior anywhere.  How about something like

  Specify whether to omit excluded results or simply flag them.  By
  default, this is set to TRUE.

  If this is TRUE, notmuch_query_search_messages will omit excluded
  messages from the results.  notmuch_query_search_threads will omit
  threads that match only in excluded messages, but will include all
  messages in threads that are only partially excluded.

It might be nice to mention the performance implications of the various
options, too.

>  void
> -notmuch_query_set_omit_excluded_messages (notmuch_query_t *query, notmuch_bool_t omit);
> +notmuch_query_set_omit_excluded_messages (notmuch_query_t *query, notmuch_bool_t omit_excluded);

Should this be notmuch_query_set_omit_excluded?

>  
>  /* Specify the sorting desired for this query. */
>  void
> diff --git a/lib/query.cc b/lib/query.cc
> index 68ac1e4..f7c7099 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -28,7 +28,7 @@ struct _notmuch_query {
>      const char *query_string;
>      notmuch_sort_t sort;
>      notmuch_string_list_t *exclude_terms;
> -    notmuch_bool_t omit_excluded_messages;
> +    notmuch_bool_t omit_excluded;
>  };
>  
>  typedef struct _notmuch_mset_messages {
> @@ -92,7 +92,7 @@ notmuch_query_create (notmuch_database_t *notmuch,
>  
>      query->exclude_terms = _notmuch_string_list_create (query);
>  
> -    query->omit_excluded_messages = FALSE;
> +    query->omit_excluded = TRUE;
>  
>      return query;
>  }
> @@ -104,9 +104,9 @@ notmuch_query_get_query_string (notmuch_query_t *query)
>  }
>  
>  void
> -notmuch_query_set_omit_excluded_messages (notmuch_query_t *query, notmuch_bool_t omit)
> +notmuch_query_set_omit_excluded_messages (notmuch_query_t *query, notmuch_bool_t omit_excluded)
>  {
> -    query->omit_excluded_messages = omit;
> +    query->omit_excluded = omit_excluded;
>  }
>  
>  void
> @@ -220,7 +220,7 @@ notmuch_query_search_messages (notmuch_query_t *query)
>  	if (query->exclude_terms) {
>  	    exclude_query = _notmuch_exclude_tags (query, final_query);
>  
> -	    if (query->omit_excluded_messages)
> +	    if (query->omit_excluded)
>  		final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
>  					     final_query, exclude_query);
>  	    else {
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch