[PATCH 0/5] Move --no-exclude to --exclude=(true|false|flag)

[PATCH 0/5] Move --no-exclude to --exclude=(true|false|flag)

[Mark Walters]

This series changes the --no-exclude options to count, search and show
to --exclude=(true|false|flag). It also changes the default to true
rather than flag for speed and for uncluttered command line output.

This series replaces the series
id:"1330779918-28024-1-git-send-email-markwalters1009@gmail.com". Jani
suggested the --exclude= rather than --with-excluded approach as being
both clearer and easier to extend.

It is intend to apply on top of the bugfix series
id:"1331728014-32698-1-git-send-email-markwalters1009@gmail.com".

There should be no significant logic change (i.e. the logic in the lib
is unchanged) but the defaults for the command lines are different.

The general idea is that when complete threads are requested then we
always return complete threads including any messages matching the
exclude tags (where appropriate these are flagged excluded), but we
may not return threads that only match in excluded messages.

The rationale is that it is awkward to deal with a thread with
"missing" messages.

Best wishes

Mark

Mark Walters (5):
  lib: change default for notmuch_query_set_omit_excluded
  cli: move count to the new --exclude=(true|false|flag) naming scheme.
  cli: move search to the new --exclude= naming scheme.
  cli: move show to the new --exclude= option naming scheme.
  emacs: make show set --exclude=flag

 emacs/notmuch-show.el     |    6 ++++--
 lib/notmuch.h             |   11 ++++++-----
 lib/query.cc              |   10 +++++-----
 man/man1/notmuch-count.1  |    5 +++--
 man/man1/notmuch-search.1 |   12 +++++++++---
 man/man1/notmuch-show.1   |   16 ++++++++++++++--
 notmuch-client.h          |    1 +
 notmuch-count.c           |   17 ++++++++++++-----
 notmuch-search.c          |   32 +++++++++++++++++++++++---------
 notmuch-show.c            |   39 +++++++++++++++++++++++++++++----------
 test/count                |    4 ++--
 test/search               |   13 ++++++-------
 12 files changed, 114 insertions(+), 52 deletions(-)

-- 
1.7.9.1

[PATCH 1/5] lib: change default for notmuch_query_set_omit_excluded

[Mark Walters]

---
 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. */
 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);
 
 /* Specify the sorting desired for this query. */
 void
diff --git a/lib/query.cc b/lib/query.cc
index 2b73d72..44fc0ab 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 {
@@ -86,7 +86,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;
 }
@@ -98,9 +98,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
@@ -214,7 +214,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

[PATCH 2/5] cli: move count to the new --exclude=(true|false|flag) naming scheme.

[Mark Walters]

Move the option --no-exclude to the --exclude= scheme. Since there is
no way to flag messages only true and false are implemented. Note
that, for consistency with other commands, this is implemented as a
keyword option rather than a boolean option.
---
 man/man1/notmuch-count.1 |    5 +++--
 notmuch-count.c          |   17 ++++++++++++-----
 test/count               |    4 ++--
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/man/man1/notmuch-count.1 b/man/man1/notmuch-count.1
index 805a8ae..b169f93 100644
--- a/man/man1/notmuch-count.1
+++ b/man/man1/notmuch-count.1
@@ -41,9 +41,10 @@ Output the number of matching threads.
 
 .RS 4
 .TP 4
-.BR \-\-no\-exclude
+.BR \-\-exclude=(true|false)
 
-Do not exclude the messages matching search.exclude_tags in the config file.
+Specify whether to omit messages matching search.tag_exclude from the
+count (the default) or not.
 .RE
 .RE
 .RE
diff --git a/notmuch-count.c b/notmuch-count.c
index 46b76ae..b76690c 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -26,6 +26,12 @@ enum {
     OUTPUT_MESSAGES,
 };
 
+/* The following is to allow future options to be added more easily */
+enum {
+    EXCLUDE_TRUE,
+    EXCLUDE_FALSE,
+};
+
 int
 notmuch_count_command (void *ctx, int argc, char *argv[])
 {
@@ -35,7 +41,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
     char *query_str;
     int opt_index;
     int output = OUTPUT_MESSAGES;
-    notmuch_bool_t no_exclude = FALSE;
+    int exclude = EXCLUDE_TRUE;
     unsigned int i;
 
     notmuch_opt_desc_t options[] = {
@@ -43,7 +49,10 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
 				  { 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 },
+				  { 0, 0 } } },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -78,7 +87,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    if (!no_exclude) {
+    if (exclude == EXCLUDE_TRUE) {
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
 
@@ -88,8 +97,6 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
     }
 
-    notmuch_query_set_omit_excluded_messages (query, TRUE);
-
     switch (output) {
     case OUTPUT_MESSAGES:
 	printf ("%u\n", notmuch_query_count_messages (query));
diff --git a/test/count b/test/count
index b97fc06..fd387e5 100755
--- a/test/count
+++ b/test/count
@@ -53,9 +53,9 @@ test_expect_equal \
     "1" \
     "`notmuch count subject:deleted and tag:deleted`"
 
-test_begin_subtest "count \"deleted\" messages, with --no-exclude"
+test_begin_subtest "count \"deleted\" messages, --exclude=false"
 test_expect_equal \
     "3" \
-    "`notmuch count --no-exclude subject:deleted`"
+    "`notmuch count --exclude=false subject:deleted`"
 
 test_done
-- 
1.7.9.1

[PATCH 3/5] cli: move search to the new --exclude= naming scheme.

[Mark Walters]

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 +++++++++++++++++++++++---------
 test/search               |   13 ++++++-------
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 8426aa3..49fbd0d 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
+	 * 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");
+	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) {
diff --git a/test/search b/test/search
index 17af6a2..318fdb8 100755
--- a/test/search
+++ b/test/search
@@ -138,15 +138,14 @@ notmuch new > /dev/null
 notmuch tag +deleted id:$gen_msg_id
 deleted_id=$gen_msg_id
 output=$(notmuch search subject:deleted | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
-thread:XXX   2001-01-05 [0/1] Notmuch Test Suite; Deleted (deleted inbox unread)"
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)"
 
 test_begin_subtest "Exclude \"deleted\" messages from message search"
 output=$(notmuch search --output=messages subject:deleted | notmuch_search_sanitize)
 test_expect_equal "$output" "id:$not_deleted_id"
 
-test_begin_subtest "Exclude \"deleted\" messages from message search (no-exclude)"
-output=$(notmuch search --no-exclude --output=messages subject:deleted | notmuch_search_sanitize)
+test_begin_subtest "Exclude \"deleted\" messages from message search --exclude=false"
+output=$(notmuch search --exclude=false --output=messages subject:deleted | notmuch_search_sanitize)
 test_expect_equal "$output" "id:$not_deleted_id
 id:$deleted_id"
 
@@ -166,10 +165,10 @@ output=$(notmuch search subject:deleted | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
 thread:XXX   2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread)"
 
-test_begin_subtest "Don't exclude \"deleted\" messages when --no-exclude specified"
-output=$(notmuch search --no-exclude subject:deleted | notmuch_search_sanitize)
+test_begin_subtest "Don't exclude \"deleted\" messages when --exclude=flag specified"
+output=$(notmuch search --exclude=flag subject:deleted | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
-thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)"
+thread:XXX   2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread)"
 
 test_begin_subtest "Don't exclude \"deleted\" messages from search if not configured"
 notmuch config set search.exclude_tags
-- 
1.7.9.1

[PATCH 4/5] cli: move show to the new --exclude= option naming scheme.

[Mark Walters]

This moves show to the --exclude=(true|false|flag) naming
scheme. When `exclude' is false or flag show returns all threads
that match including those that only match in an excluded message, the
difference being whether excluded messages are flagged excluded.

When exclude=true the behaviour depends on whether --entire-threads
is set. If it is not set then show only returns the messages which
match and are not excluded. If it is set then show returns all
messages in these threads flagging the excluded messages. The
rationale is that it is awkward to use a thread with some missing
messages.
---
 man/man1/notmuch-show.1 |   16 ++++++++++++++--
 notmuch-client.h        |    1 +
 notmuch-show.c          |   39 +++++++++++++++++++++++++++++----------
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index d75d971..801b7f1 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -130,9 +130,21 @@ content.
 
 .RS 4
 .TP 4
-.B \-\-no-exclude
+.BR \-\-exclude=(true|false|flag)
+
+Specify whether to omit threads only matching search.tag_exclude from
+the search results (the default) or not. The extra option
+.B flag
+includes these messages but marks them with the excluded flag.
+
+If --entire-thread is specified then complete threads are returned
+regardless (with the excluded flag being set when appropriate) but
+threads that only match in an excluded message are not returned when
+.B --exclude=true.
+
+The default is
+.B --exclude=true.
 
-Do not exclude the messages matching search.exclude_tags in the config file.
 .RE
 
 A common use of
diff --git a/notmuch-client.h b/notmuch-client.h
index f4a62cc..e36148b 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -99,6 +99,7 @@ typedef struct notmuch_show_format {
 
 typedef struct notmuch_show_params {
     notmuch_bool_t entire_thread;
+    notmuch_bool_t omit_excluded;
     notmuch_bool_t raw;
     int part;
 #ifdef GMIME_ATLEAST_26
diff --git a/notmuch-show.c b/notmuch-show.c
index 05d51b2..20d6635 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -812,6 +812,7 @@ show_messages (void *ctx,
 {
     notmuch_message_t *message;
     notmuch_bool_t match;
+    notmuch_bool_t excluded;
     int first_set = 1;
     int next_indent;
 
@@ -830,10 +831,11 @@ show_messages (void *ctx,
 	message = notmuch_messages_get (messages);
 
 	match = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH);
+	excluded = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
 
 	next_indent = indent;
 
-	if (match || params->entire_thread) {
+	if ((match && (!excluded || !params->omit_excluded)) || params->entire_thread) {
 	    show_message (ctx, format, message, indent, params);
 	    next_indent = indent + 1;
 
@@ -974,6 +976,12 @@ enum {
     NOTMUCH_FORMAT_RAW
 };
 
+enum {
+    EXCLUDE_TRUE,
+    EXCLUDE_FALSE,
+    EXCLUDE_FLAG,
+};
+
 int
 notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 {
@@ -983,10 +991,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     char *query_string;
     int opt_index, ret;
     const notmuch_show_format_t *format = &format_text;
-    notmuch_show_params_t params = { .part = -1 };
+    notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
     int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
     notmuch_bool_t verify = FALSE;
-    notmuch_bool_t no_exclude = FALSE;
+    int exclude = EXCLUDE_TRUE;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
@@ -995,11 +1003,15 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 				  { "mbox", NOTMUCH_FORMAT_MBOX },
 				  { "raw", NOTMUCH_FORMAT_RAW },
 				  { 0, 0 } } },
+        { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
+          (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
+                                  { "false", EXCLUDE_FALSE },
+                                  { "flag", EXCLUDE_FLAG },
+                                  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.entire_thread, "entire-thread", 't', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.decrypt, "decrypt", 'd', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 },
-	{ NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'n', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -1088,16 +1100,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
-    /* if format=mbox then we can not output excluded messages as
-     * there is no way to make the exclude flag available */
-    if (format_sel == NOTMUCH_FORMAT_MBOX)
-	notmuch_query_set_omit_excluded_messages (query, TRUE);
-
     /* If a single message is requested we do not use search_excludes. */
     if (params.part >= 0)
 	ret = do_show_single (ctx, query, format, &params);
     else {
-	if (!no_exclude) {
+	if (format == &format_mbox && exclude == EXCLUDE_FLAG) {
+	    /* there is no where to mark flagged messages so fall back on
+	     * including the excluded messages */
+	    fprintf (stderr, "Cannot flag excluded messages with format=mbox: fall back on just including them\n");
+	    exclude = EXCLUDE_FALSE;
+	}
+
+	if (exclude != EXCLUDE_FALSE) {
 	    const char **search_exclude_tags;
 	    size_t search_exclude_tags_length;
 	    unsigned int i;
@@ -1106,7 +1120,12 @@ notmuch_show_command (void *ctx, unused (int argc), unused (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);
+		params.omit_excluded = FALSE;
+	    }
 	}
+
 	ret = do_show (ctx, query, format, &params);
     }
 
-- 
1.7.9.1

[PATCH 5/5] emacs: make show set --exclude=flag

[Mark Walters]

Show has to set --exclude=flag to deal with cases where it is asked
to show a single excluded message. It uses JSON so it can easily pass
the exclude information to the user.
---
 emacs/notmuch-show.el |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 4a60631..8ab2485 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1099,13 +1099,15 @@ function is used."
 		       (append (list "\'") basic-args
 			       (list "and (" notmuch-show-query-context ")\'"))
 		     (append (list "\'") basic-args (list "\'")))))
-	(notmuch-show-insert-forest (notmuch-query-get-threads args))
+	(notmuch-show-insert-forest (notmuch-query-get-threads
+				     (append (list "--exclude=flag") args)))
 	;; If the query context reduced the results to nothing, run
 	;; the basic query.
 	(when (and (eq (buffer-size) 0)
 		   notmuch-show-query-context)
 	  (notmuch-show-insert-forest
-	   (notmuch-query-get-threads basic-args))))
+	   (notmuch-query-get-threads
+	    (append (list "--exclude=flag") basic-args)))))
 
       (jit-lock-register #'notmuch-show-buttonise-links)
 
-- 
1.7.9.1

Re: [PATCH 0/5] Move --no-exclude to --exclude=(true|false|flag)

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

On Thu, 15 Mar 2012 18:42:00 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> This series changes the --no-exclude options to count, search and show
> to --exclude=(true|false|flag). It also changes the default to true
> rather than flag for speed and for uncluttered command line output.
> 
> This series replaces the series
> id:"1330779918-28024-1-git-send-email-markwalters1009@gmail.com". Jani
> suggested the --exclude= rather than --with-excluded approach as being
> both clearer and easier to extend.
> 
> It is intend to apply on top of the bugfix series
> id:"1331728014-32698-1-git-send-email-markwalters1009@gmail.com".

And indeed it does.  I've tested and reviewed and it looks and works
great.

> There should be no significant logic change (i.e. the logic in the lib
> is unchanged) but the defaults for the command lines are different.

I fully support this change.  The new logic makes sense to me and is a
good compromise between usability and flexibility.

> The general idea is that when complete threads are requested then we
> always return complete threads including any messages matching the
> exclude tags (where appropriate these are flagged excluded), but we
> may not return threads that only match in excluded messages.
> 
> The rationale is that it is awkward to deal with a thread with
> "missing" messages.

Agreed.  I should note that for all uses of excludes that I can think of
one would not expect to be excluding messages from threads that one
would otherwise be keeping.  So I certainly have no problem not
excluding 

Thanks again for working on this, Mark.  Your efforts are much
appreciated.  This is nice new functionality.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 3/5] cli: move search to the new --exclude= naming scheme.

[Austin Clements]

Quoth Mark Walters on Mar 15 at  6:42 pm:
> 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.
> ---

Code looks good, but I think there need to be a few more tests.  In
particular, I think we need all 10 valid combinations of

* --output=summary or --output=messages (those two are probably
  enough)

* The three types of exclude (or two for --output=messages)

* Whether or not an excluded term is in the query

Finally, the corpus should have three threads of interest: a thread
with no excluded messages, a thread with both excluded and
non-excluded messages, and a thread consisting entirely of excluded
messages.

We already have many of these, but it would be good to systematically
cover the full gamut of possibilities.

Re: [PATCH 4/5] cli: move show to the new --exclude= option naming scheme.

[Austin Clements]

Quoth Mark Walters on Mar 15 at  6:42 pm:
> This moves show to the --exclude=(true|false|flag) naming
> scheme. When `exclude' is false or flag show returns all threads
> that match including those that only match in an excluded message, the
> difference being whether excluded messages are flagged excluded.
> 
> When exclude=true the behaviour depends on whether --entire-threads

s/--entire-threads/--entire-thread/

> is set. If it is not set then show only returns the messages which
> match and are not excluded. If it is set then show returns all
> messages in these threads flagging the excluded messages. The

Parse error.

> rationale is that it is awkward to use a thread with some missing
> messages.
> ---
>  man/man1/notmuch-show.1 |   16 ++++++++++++++--
>  notmuch-client.h        |    1 +
>  notmuch-show.c          |   39 +++++++++++++++++++++++++++++----------
>  3 files changed, 44 insertions(+), 12 deletions(-)
> 
> diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
> index d75d971..801b7f1 100644
> --- a/man/man1/notmuch-show.1
> +++ b/man/man1/notmuch-show.1
> @@ -130,9 +130,21 @@ content.
>  
>  .RS 4
>  .TP 4
> -.B \-\-no-exclude
> +.BR \-\-exclude=(true|false|flag)
> +
> +Specify whether to omit threads only matching search.tag_exclude from
> +the search results (the default) or not. The extra option
> +.B flag
> +includes these messages but marks them with the excluded flag.
> +
> +If --entire-thread is specified then complete threads are returned
> +regardless (with the excluded flag being set when appropriate) but
> +threads that only match in an excluded message are not returned when
> +.B --exclude=true.

I found this a bit confusing.  There are two orthogonal things going
on here: what happens to excluded messages and what happens to
fully-excluded threads.  Is the following table accurate?

         --entire-thread=false
      excl. messages   excl. threads
true       omit            omit
false    include         include
flag   include,flag      include

         --entire-thread=true
      excl. messages   excl. threads
true   include,flag        omit
false    include         include
flag   include,flag      include

(My reasoning: --exclude=false is equivalent to not having any
excludes configured, --exclude=true omits excluded messages from the
seed set and filters them in show_messages, --exclude=flag does not
exclude messages from the seed set nor filter them in show_messages.)

If this is right, then what's the point of having both false and flag
for show?  I'm pretty sure their performance will be
indistinguishable.

> +
> +The default is
> +.B --exclude=true.
>  
> -Do not exclude the messages matching search.exclude_tags in the config file.
>  .RE
>  
>  A common use of
> diff --git a/notmuch-client.h b/notmuch-client.h
> index f4a62cc..e36148b 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -99,6 +99,7 @@ typedef struct notmuch_show_format {
>  
>  typedef struct notmuch_show_params {
>      notmuch_bool_t entire_thread;
> +    notmuch_bool_t omit_excluded;
>      notmuch_bool_t raw;
>      int part;
>  #ifdef GMIME_ATLEAST_26
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 05d51b2..20d6635 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -812,6 +812,7 @@ show_messages (void *ctx,
>  {
>      notmuch_message_t *message;
>      notmuch_bool_t match;
> +    notmuch_bool_t excluded;
>      int first_set = 1;
>      int next_indent;
>  
> @@ -830,10 +831,11 @@ show_messages (void *ctx,
>  	message = notmuch_messages_get (messages);
>  
>  	match = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH);
> +	excluded = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
>  
>  	next_indent = indent;
>  
> -	if (match || params->entire_thread) {
> +	if ((match && (!excluded || !params->omit_excluded)) || params->entire_thread) {
>  	    show_message (ctx, format, message, indent, params);
>  	    next_indent = indent + 1;
>  
> @@ -974,6 +976,12 @@ enum {
>      NOTMUCH_FORMAT_RAW
>  };
>  
> +enum {
> +    EXCLUDE_TRUE,
> +    EXCLUDE_FALSE,
> +    EXCLUDE_FLAG,
> +};
> +
>  int
>  notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  {
> @@ -983,10 +991,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>      char *query_string;
>      int opt_index, ret;
>      const notmuch_show_format_t *format = &format_text;
> -    notmuch_show_params_t params = { .part = -1 };
> +    notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
>      int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
>      notmuch_bool_t verify = FALSE;
> -    notmuch_bool_t no_exclude = FALSE;
> +    int exclude = EXCLUDE_TRUE;
>  
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
> @@ -995,11 +1003,15 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  				  { "mbox", NOTMUCH_FORMAT_MBOX },
>  				  { "raw", NOTMUCH_FORMAT_RAW },
>  				  { 0, 0 } } },
> +        { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
> +          (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
> +                                  { "false", EXCLUDE_FALSE },
> +                                  { "flag", EXCLUDE_FLAG },
> +                                  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &params.entire_thread, "entire-thread", 't', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &params.decrypt, "decrypt", 'd', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 },
> -	{ NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'n', 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -1088,16 +1100,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  	return 1;
>      }
>  
> -    /* if format=mbox then we can not output excluded messages as
> -     * there is no way to make the exclude flag available */
> -    if (format_sel == NOTMUCH_FORMAT_MBOX)
> -	notmuch_query_set_omit_excluded_messages (query, TRUE);
> -
>      /* If a single message is requested we do not use search_excludes. */
>      if (params.part >= 0)
>  	ret = do_show_single (ctx, query, format, &params);
>      else {
> -	if (!no_exclude) {
> +	if (format == &format_mbox && exclude == EXCLUDE_FLAG) {
> +	    /* there is no where to mark flagged messages so fall back on

s/no where/nowhere/  Also, s/there/There/ for style consistency.

> +	     * including the excluded messages */
> +	    fprintf (stderr, "Cannot flag excluded messages with format=mbox: fall back on just including them\n");

This is a bit verbose.  How about just "Warning: mbox cannot flag
excluded messages"?  Flag already means that the messages should be
included, so this message states exactly what mbox isn't doing that
you might expect it to.

> +	    exclude = EXCLUDE_FALSE;
> +	}
> +
> +	if (exclude != EXCLUDE_FALSE) {
>  	    const char **search_exclude_tags;
>  	    size_t search_exclude_tags_length;
>  	    unsigned int i;
> @@ -1106,7 +1120,12 @@ notmuch_show_command (void *ctx, unused (int argc), unused (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);
> +		params.omit_excluded = FALSE;
> +	    }
>  	}
> +
>  	ret = do_show (ctx, query, format, &params);
>      }
>  

Re: [PATCH 5/5] emacs: make show set --exclude=flag

[Austin Clements]

Quoth Mark Walters on Mar 15 at  6:42 pm:
> Show has to set --exclude=flag to deal with cases where it is asked
> to show a single excluded message. It uses JSON so it can easily pass
> the exclude information to the user.
> ---
>  emacs/notmuch-show.el |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 4a60631..8ab2485 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1099,13 +1099,15 @@ function is used."
>  		       (append (list "\'") basic-args
>  			       (list "and (" notmuch-show-query-context ")\'"))
>  		     (append (list "\'") basic-args (list "\'")))))
> -	(notmuch-show-insert-forest (notmuch-query-get-threads args))
> +	(notmuch-show-insert-forest (notmuch-query-get-threads
> +				     (append (list "--exclude=flag") args)))

(cons "--exclude=flag" args)

>  	;; If the query context reduced the results to nothing, run
>  	;; the basic query.
>  	(when (and (eq (buffer-size) 0)
>  		   notmuch-show-query-context)
>  	  (notmuch-show-insert-forest
> -	   (notmuch-query-get-threads basic-args))))
> +	   (notmuch-query-get-threads
> +	    (append (list "--exclude=flag") basic-args)))))

Same here.

>  
>        (jit-lock-register #'notmuch-show-buttonise-links)
>  

Re: [PATCH 3/5] cli: move search to the new --exclude= naming scheme.

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

On Sat, 17 Mar 2012 11:50:23 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> We already have many of these, but it would be good to systematically
> cover the full gamut of possibilities.

agreed.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

[PATCH] test: add some exclude tests

[Mark Walters]

Here are some tests for search exclude working in a systematic fashion
as suggested by Austin.

In principle I think something like the generate_thread function could
go in to test-lib.sh, but it would need to be written by someone much
familiar with bash quoting than I am.

At the moment I have left the current exclude tests in the "search"
test: should they be moved here?

Best wishes 

Mark

---
 test/notmuch-test    |    1 +
 test/search-excludes |  160 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 161 insertions(+), 0 deletions(-)
 create mode 100755 test/search-excludes

diff --git a/test/notmuch-test b/test/notmuch-test
index f03b594..4dcd8c6 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -27,6 +27,7 @@ TESTS="
   search-position-overlap-bug
   search-insufficient-from-quoting
   search-limiting
+  search-excludes
   tagging
   json
   multipart
diff --git a/test/search-excludes b/test/search-excludes
new file mode 100755
index 0000000..81c284e
--- /dev/null
+++ b/test/search-excludes
@@ -0,0 +1,160 @@
+#!/usr/bin/env bash
+test_description='"notmuch search" with excludes in several variations'
+. ./test-lib.sh
+
+# Generates a thread of 'length' messages. The subject of the nth
+# message in the thread is 'subject: message n'
+generate_thread ()
+{
+    local subject="$1"
+    local length="$2"
+    generate_message '[subject]="'"${subject}: message 1"'"'
+    parent_id=$gen_msg_id
+    for i in `seq 2 $length`
+    do
+	generate_message '[subject]="'"${subject}: message $i"'"' \
+	                 "[in-reply-to]=\<$parent_id\>"
+	parent_id=$gen_msg_id
+    done
+    notmuch new > /dev/null
+}
+
+# We construct some threads for the tests. We use the tag "test" to
+# indicate which messages we will search for.
+
+# A thread of deleted messages; test matches one of them.
+generate_thread "All messages excluded: single match" 5
+notmuch tag +deleted subject:"All messages excluded: single match*"
+notmuch tag +test 'subject:All messages excluded: single match: message 2'
+
+# A thread of deleted messages; test matches two of them.
+generate_thread "All messages excluded: double match" 5
+notmuch tag +deleted subject:"All messages excluded: double match*"
+notmuch tag +test 'subject:All messages excluded: double match: message 2'
+notmuch tag +test 'subject:All messages excluded: double match: message 4'
+
+# A thread some messages deleted; test only matches a deleted message.
+generate_thread "Some messages excluded: single excluded match" 5
+notmuch tag +deleted +test 'subject:Some messages excluded: single excluded match: message 3'
+
+# A thread some messages deleted; test only matches a non-deleted message.
+generate_thread "Some messages excluded: single non-excluded match" 5
+notmuch tag +deleted 'subject:Some messages excluded: single non-excluded match: message 2'
+notmuch tag +test 'subject:Some messages excluded: single non-excluded match: message 4'
+
+# A thread no messages deleted; test matches a message.
+generate_thread "No messages excluded: single match" 5
+notmuch tag +test 'subject:No messages excluded: single match: message 3'
+
+# Set up excludes
+notmuch config set search.exclude_tags deleted
+
+test_begin_subtest "Default exclusion (thread summary)"
+output=$(notmuch search tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single non-excluded match: message 4 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; No messages excluded: single match: message 3 (inbox test unread)"
+
+test_begin_subtest "Default exclusion (messages)"
+output=$(notmuch search --output=messages tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-019@notmuch-test-suite
+id:msg-023@notmuch-test-suite"
+
+test_begin_subtest "exclude=true (thread summary)"
+output=$(notmuch search --exclude=true tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single non-excluded match: message 4 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; No messages excluded: single match: message 3 (inbox test unread)"
+
+test_begin_subtest "exclude=true (messages)"
+output=$(notmuch search --exclude=true --output=messages tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-019@notmuch-test-suite
+id:msg-023@notmuch-test-suite"
+
+test_begin_subtest "exclude=false (thread summary)"
+output=$(notmuch search --exclude=false tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; All messages excluded: single match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [2/5] Notmuch Test Suite; All messages excluded: double match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single excluded match: message 3 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single non-excluded match: message 4 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; No messages excluded: single match: message 3 (inbox test unread)"
+
+test_begin_subtest "exclude=false (messages)"
+output=$(notmuch search --exclude=false --output=messages tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-002@notmuch-test-suite
+id:msg-007@notmuch-test-suite
+id:msg-009@notmuch-test-suite
+id:msg-013@notmuch-test-suite
+id:msg-019@notmuch-test-suite
+id:msg-023@notmuch-test-suite"
+
+test_begin_subtest "exclude=flag (thread summary)"
+output=$(notmuch search --exclude=flag tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [0/5] Notmuch Test Suite; All messages excluded: single match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [0/5] Notmuch Test Suite; All messages excluded: double match: message 4 (deleted inbox test unread)
+thread:XXX   2001-01-05 [0/5] Notmuch Test Suite; Some messages excluded: single excluded match: message 3 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single non-excluded match: message 4 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; No messages excluded: single match: message 3 (inbox test unread)"
+
+test_begin_subtest "exclude=flag (messages)"
+output=$(notmuch search --exclude=flag --output=messages tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-002@notmuch-test-suite
+id:msg-007@notmuch-test-suite
+id:msg-009@notmuch-test-suite
+id:msg-013@notmuch-test-suite
+id:msg-019@notmuch-test-suite
+id:msg-023@notmuch-test-suite"
+
+test_begin_subtest "Default exclusion: tag in query (thread summary)"
+output=$(notmuch search tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; All messages excluded: single match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [2/5] Notmuch Test Suite; All messages excluded: double match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single excluded match: message 3 (deleted inbox test unread)"
+
+test_begin_subtest "Default exclusion: tag in query (messages)"
+output=$(notmuch search --output=messages tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-002@notmuch-test-suite
+id:msg-007@notmuch-test-suite
+id:msg-009@notmuch-test-suite
+id:msg-013@notmuch-test-suite"
+
+test_begin_subtest "exclude=true: tag in query (thread summary)"
+output=$(notmuch search --exclude=true tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; All messages excluded: single match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [2/5] Notmuch Test Suite; All messages excluded: double match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single excluded match: message 3 (deleted inbox test unread)"
+
+test_begin_subtest "exclude=true: tag in query (messages)"
+output=$(notmuch search --exclude=true --output=messages tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-002@notmuch-test-suite
+id:msg-007@notmuch-test-suite
+id:msg-009@notmuch-test-suite
+id:msg-013@notmuch-test-suite"
+
+test_begin_subtest "exclude=false: tag in query (thread summary)"
+output=$(notmuch search --exclude=false tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; All messages excluded: single match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [2/5] Notmuch Test Suite; All messages excluded: double match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single excluded match: message 3 (deleted inbox test unread)"
+
+test_begin_subtest "exclude=false: tag in query (messages)"
+output=$(notmuch search --exclude=false --output=messages tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-002@notmuch-test-suite
+id:msg-007@notmuch-test-suite
+id:msg-009@notmuch-test-suite
+id:msg-013@notmuch-test-suite"
+
+test_begin_subtest "exclude=flag: tag in query (thread summary)"
+output=$(notmuch search --exclude=flag tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; All messages excluded: single match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [2/5] Notmuch Test Suite; All messages excluded: double match: message 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/5] Notmuch Test Suite; Some messages excluded: single excluded match: message 3 (deleted inbox test unread)"
+
+test_begin_subtest "exclude=flag: tag in query (messages)"
+output=$(notmuch search --exclude=flag --output=messages tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "id:msg-002@notmuch-test-suite
+id:msg-007@notmuch-test-suite
+id:msg-009@notmuch-test-suite
+id:msg-013@notmuch-test-suite"
+
+
+
+test_done
-- 
1.7.9.1

Re: [PATCH 4/5] cli: move show to the new --exclude= option naming scheme.

[Mark Walters]

On Sat, 17 Mar 2012 12:51:20 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Mark Walters on Mar 15 at  6:42 pm:
> > This moves show to the --exclude=(true|false|flag) naming
> > scheme. When `exclude' is false or flag show returns all threads
> > that match including those that only match in an excluded message, the
> > difference being whether excluded messages are flagged excluded.
> > 
> > When exclude=true the behaviour depends on whether --entire-threads
> 
> s/--entire-threads/--entire-thread/

Will fix.

> > is set. If it is not set then show only returns the messages which
> > match and are not excluded. If it is set then show returns all
> > messages in these threads flagging the excluded messages. The
> 
> Parse error.

Will try and reword.

> > rationale is that it is awkward to use a thread with some missing
> > messages.
> > ---
> >  man/man1/notmuch-show.1 |   16 ++++++++++++++--
> >  notmuch-client.h        |    1 +
> >  notmuch-show.c          |   39 +++++++++++++++++++++++++++++----------
> >  3 files changed, 44 insertions(+), 12 deletions(-)
> > 
> > diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
> > index d75d971..801b7f1 100644
> > --- a/man/man1/notmuch-show.1
> > +++ b/man/man1/notmuch-show.1
> > @@ -130,9 +130,21 @@ content.
> >  
> >  .RS 4
> >  .TP 4
> > -.B \-\-no-exclude
> > +.BR \-\-exclude=(true|false|flag)
> > +
> > +Specify whether to omit threads only matching search.tag_exclude from
> > +the search results (the default) or not. The extra option
> > +.B flag
> > +includes these messages but marks them with the excluded flag.
> > +
> > +If --entire-thread is specified then complete threads are returned
> > +regardless (with the excluded flag being set when appropriate) but
> > +threads that only match in an excluded message are not returned when
> > +.B --exclude=true.
> 
> I found this a bit confusing.  There are two orthogonal things going
> on here: what happens to excluded messages and what happens to
> fully-excluded threads.  Is the following table accurate?
> 
>          --entire-thread=false
>       excl. messages   excl. threads
> true       omit            omit
> false    include         include
> flag   include,flag      include
> 
>          --entire-thread=true
>       excl. messages   excl. threads
> true   include,flag        omit
> false    include         include
> flag   include,flag      include

This table is accurate.

> (My reasoning: --exclude=false is equivalent to not having any
> excludes configured, --exclude=true omits excluded messages from the
> seed set and filters them in show_messages, --exclude=flag does not
> exclude messages from the seed set nor filter them in show_messages.)
> 
> If this is right, then what's the point of having both false and flag
> for show?  I'm pretty sure their performance will be
> indistinguishable.

I included the three options partly for completeness, partly to be the
same as notmuch-search and partly because a "flag" for mbox feels
wrong. I am happy to change it though. Do you have any good naming
suggestions?

> > +
> > +The default is
> > +.B --exclude=true.
> >  
> > -Do not exclude the messages matching search.exclude_tags in the config file.
> >  .RE
> >  
> >  A common use of
> > diff --git a/notmuch-client.h b/notmuch-client.h
> > index f4a62cc..e36148b 100644
> > --- a/notmuch-client.h
> > +++ b/notmuch-client.h
> > @@ -99,6 +99,7 @@ typedef struct notmuch_show_format {
> >  
> >  typedef struct notmuch_show_params {
> >      notmuch_bool_t entire_thread;
> > +    notmuch_bool_t omit_excluded;
> >      notmuch_bool_t raw;
> >      int part;
> >  #ifdef GMIME_ATLEAST_26
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index 05d51b2..20d6635 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -812,6 +812,7 @@ show_messages (void *ctx,
> >  {
> >      notmuch_message_t *message;
> >      notmuch_bool_t match;
> > +    notmuch_bool_t excluded;
> >      int first_set = 1;
> >      int next_indent;
> >  
> > @@ -830,10 +831,11 @@ show_messages (void *ctx,
> >  	message = notmuch_messages_get (messages);
> >  
> >  	match = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH);
> > +	excluded = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
> >  
> >  	next_indent = indent;
> >  
> > -	if (match || params->entire_thread) {
> > +	if ((match && (!excluded || !params->omit_excluded)) || params->entire_thread) {
> >  	    show_message (ctx, format, message, indent, params);
> >  	    next_indent = indent + 1;
> >  
> > @@ -974,6 +976,12 @@ enum {
> >      NOTMUCH_FORMAT_RAW
> >  };
> >  
> > +enum {
> > +    EXCLUDE_TRUE,
> > +    EXCLUDE_FALSE,
> > +    EXCLUDE_FLAG,
> > +};
> > +
> >  int
> >  notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> >  {
> > @@ -983,10 +991,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> >      char *query_string;
> >      int opt_index, ret;
> >      const notmuch_show_format_t *format = &format_text;
> > -    notmuch_show_params_t params = { .part = -1 };
> > +    notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
> >      int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
> >      notmuch_bool_t verify = FALSE;
> > -    notmuch_bool_t no_exclude = FALSE;
> > +    int exclude = EXCLUDE_TRUE;
> >  
> >      notmuch_opt_desc_t options[] = {
> >  	{ NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
> > @@ -995,11 +1003,15 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> >  				  { "mbox", NOTMUCH_FORMAT_MBOX },
> >  				  { "raw", NOTMUCH_FORMAT_RAW },
> >  				  { 0, 0 } } },
> > +        { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
> > +          (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
> > +                                  { "false", EXCLUDE_FALSE },
> > +                                  { "flag", EXCLUDE_FLAG },
> > +                                  { 0, 0 } } },
> >  	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
> >  	{ NOTMUCH_OPT_BOOLEAN, &params.entire_thread, "entire-thread", 't', 0 },
> >  	{ NOTMUCH_OPT_BOOLEAN, &params.decrypt, "decrypt", 'd', 0 },
> >  	{ NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 },
> > -	{ NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'n', 0 },
> >  	{ 0, 0, 0, 0, 0 }
> >      };
> >  
> > @@ -1088,16 +1100,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> >  	return 1;
> >      }
> >  
> > -    /* if format=mbox then we can not output excluded messages as
> > -     * there is no way to make the exclude flag available */
> > -    if (format_sel == NOTMUCH_FORMAT_MBOX)
> > -	notmuch_query_set_omit_excluded_messages (query, TRUE);
> > -
> >      /* If a single message is requested we do not use search_excludes. */
> >      if (params.part >= 0)
> >  	ret = do_show_single (ctx, query, format, &params);
> >      else {
> > -	if (!no_exclude) {
> > +	if (format == &format_mbox && exclude == EXCLUDE_FLAG) {
> > +	    /* there is no where to mark flagged messages so fall back on
> 
> s/no where/nowhere/  Also, s/there/There/ for style consistency.
> 
> > +	     * including the excluded messages */
> > +	    fprintf (stderr, "Cannot flag excluded messages with format=mbox: fall back on just including them\n");
> 
> This is a bit verbose.  How about just "Warning: mbox cannot flag
> excluded messages"?  Flag already means that the messages should be
> included, so this message states exactly what mbox isn't doing that
> you might expect it to.

I like this and will change to it.

> > +	    exclude = EXCLUDE_FALSE;
> > +	}
> > +
> > +	if (exclude != EXCLUDE_FALSE) {
> >  	    const char **search_exclude_tags;
> >  	    size_t search_exclude_tags_length;
> >  	    unsigned int i;
> > @@ -1106,7 +1120,12 @@ notmuch_show_command (void *ctx, unused (int argc), unused (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);
> > +		params.omit_excluded = FALSE;
> > +	    }
> >  	}
> > +
> >  	ret = do_show (ctx, query, format, &params);
> >      }
> >  

Thanks

Mark

Re: [PATCH] test: add some exclude tests

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]

On Sun, 18 Mar 2012 17:23:01 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> Here are some tests for search exclude working in a systematic fashion
> as suggested by Austin.

Awesome.  More tests = good.

> In principle I think something like the generate_thread function could
> go in to test-lib.sh, but it would need to be written by someone much
> familiar with bash quoting than I am.

Yeah, that's probably a good idea.  It seems generally useful.

> At the moment I have left the current exclude tests in the "search"
> test: should they be moved here?

I would probably put all of the exclude tests (even count/show exclude
tests?)  into one test script.  Might make debugging a bit easier down
the line.  But there are also plenty of tests that fall under multiple
categories, so it's kind of hard to just pick one.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]