From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 3/6] cli: move search to the new --exclude= naming
	scheme.
In-Reply-To: <1333234442-28616-4-git-send-email-markwalters1009@gmail.com>
References: <1333234442-28616-1-git-send-email-markwalters1009@gmail.com>
	<1333234442-28616-4-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:18:15 -0400
Message-ID: <87398hd5ew.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:
> This commit replaces the --no-exclude option with a
> --exclude=(true|false|flag) option. The default is to omit the
> excluded messages.
>
> The flag option only makes sense if output=summary (as otherwise there
> is nowhere to print the flag). In summary output exclude=false and
> exclude=flag give almost identical output:
> they differ in that with the exclude=flag option the match count
> (i.e., the x in [x/n] in the output) is the number of matching
> non-excluded messages rather than the number of matching messages.
>
> Note this changes the default for output=summary when no --exclude=
> option is given: it used to default to flag and now defaults to true
> (i.e. omit excluded messages). This is neccesary to keep the cli
> output uncluttered and for speed reasons.
> ---
>  man/man1/notmuch-search.1 |   12 +++++++++---
>  notmuch-search.c          |   32 +++++++++++++++++++++++---------
>  2 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
> index 06d81a6..ebb61fc 100644
> --- a/man/man1/notmuch-search.1
> +++ b/man/man1/notmuch-search.1
> @@ -114,9 +114,15 @@ Limit the number of displayed results to N.
>  
>  .RS 4
>  .TP 4
> -.BR \-\-no\-exclude
> -
> -Do not exclude the messages matching search.exclude_tags in the config file.
> +.BR \-\-exclude=(true|false|flag)
> +
> +Specify whether to omit messages matching search.tag_exclude from the
> +search results (the default) or not. The extra option
> +.B flag
> +only has an effect when
> +.B --output=summary
> +In this case all matching threads are returned but the "match count"
> +is the number of matching non-excluded messages in the thread.
>  .RE
>  
>  .SH SEE ALSO
> diff --git a/notmuch-search.c b/notmuch-search.c
> index f6061e4..fe18a93 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -210,9 +210,6 @@ do_search_threads (const search_format_t *format,
>      int first_thread = 1;
>      int i;
>  
> -    if (output == OUTPUT_THREADS)
> -	notmuch_query_set_omit_excluded_messages (query, TRUE);
> -
>      if (offset < 0) {
>  	offset += notmuch_query_count_threads (query);
>  	if (offset < 0)
> @@ -303,8 +300,6 @@ do_search_messages (const search_format_t *format,
>      int first_message = 1;
>      int i;
>  
> -    notmuch_query_set_omit_excluded_messages (query, TRUE);
> -
>      if (offset < 0) {
>  	offset += notmuch_query_count_messages (query);
>  	if (offset < 0)
> @@ -376,7 +371,6 @@ do_search_tags (notmuch_database_t *notmuch,
>      const char *tag;
>      int first_tag = 1;
>  
> -    notmuch_query_set_omit_excluded_messages (query, TRUE);
>      /* should the following only special case if no excluded terms
>       * specified? */
>  
> @@ -422,6 +416,12 @@ do_search_tags (notmuch_database_t *notmuch,
>      return 0;
>  }
>  
> +enum {
> +    EXCLUDE_TRUE,
> +    EXCLUDE_FALSE,
> +    EXCLUDE_FLAG,
> +};
> +
>  int
>  notmuch_search_command (void *ctx, int argc, char *argv[])
>  {
> @@ -435,7 +435,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>      output_t output = OUTPUT_SUMMARY;
>      int offset = 0;
>      int limit = -1; /* unlimited */
> -    notmuch_bool_t no_exclude = FALSE;
> +    int exclude = EXCLUDE_TRUE;
>      unsigned int i;
>  
>      enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
> @@ -457,7 +457,11 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>  				  { "files", OUTPUT_FILES },
>  				  { "tags", OUTPUT_TAGS },
>  				  { 0, 0 } } },
> -	{ NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'd', 0 },
> +        { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
> +          (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
> +                                  { "false", EXCLUDE_FALSE },
> +                                  { "flag", EXCLUDE_FLAG },
> +                                  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
>  	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
>  	{ 0, 0, 0, 0, 0 }
> @@ -505,7 +509,15 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>  
>      notmuch_query_set_sort (query, sort);
>  
> -    if (!no_exclude) {
> +    if (exclude == EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {
> +	/* if we are not doing summary output there is no where to

s/no where/nowhere/.  Also, s/if/If/ for style consistency.

> +	 * print the excluded flag so fall back on including the
> +	 * excluded messages */
> +	fprintf (stderr, "Cannot flag excluded messages with this output: fall back on just including them\n");

I commented on the equivalent of this message in the last version of the
show patch (and now that code is gone anyway), but must have missed it
here.  How about just "Warning: this output format cannot flag excluded
messages"?  Flag already implies including them, so all you're not doing
in this case is flagging them.

> +	exclude = EXCLUDE_FALSE;
> +    }
> +
> +    if (exclude == EXCLUDE_TRUE || exclude == EXCLUDE_FLAG) {
>  	const char **search_exclude_tags;
>  	size_t search_exclude_tags_length;
>  
> @@ -513,6 +525,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>  	    (config, &search_exclude_tags_length);
>  	for (i = 0; i < search_exclude_tags_length; i++)
>  	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
> +	if (exclude == EXCLUDE_FLAG)
> +	    notmuch_query_set_omit_excluded_messages (query, FALSE);
>      }
>  
>      switch (output) {
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch