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

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

[Mark Walters]

This is version 2 of the exclude= series (version 1 is [1]).

The main changes are the addition of some systematic notmuch-search
tests for all the exclude options, and I no longer add the
exclude=flag option to notmuch-show.c (the output was too similar to
exclude=false to be worth keeping).

Best wishes

Mark


id:"1331836925-31437-1-git-send-email-markwalters1009@gmail.com".





Mark Walters (6):
  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.
  test: add some exclude tests
  cli: move show to the new --exclude= option naming scheme.
  emacs: make show set --exclude=false

 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            |   50 +++++++----
 test/count                |    4 +-
 test/notmuch-test         |    1 +
 test/search               |   48 ----------
 test/search-excludes      |  214 +++++++++++++++++++++++++++++++++++++++++++++
 14 files changed, 325 insertions(+), 102 deletions(-)
 create mode 100755 test/search-excludes

-- 
1.7.9.1

[PATCH v2 1/6] 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 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

[PATCH v2 2/6] 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 35ecc53..d6cbf07 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 v2 3/6] 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 +++++++++++++++++++++++---------
 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
+	 * 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) {
-- 
1.7.9.1

[PATCH v2 4/6] test: add some exclude tests

[Mark Walters]

Systematically test the exclude options for search. Also move the
search existing exclude tests into the new test. There is some overlap
between the two sets of tests but many of the existing ones are there
because they triggered bugs in the past so I have kept them to ensure
coverage.
---
 test/notmuch-test    |    1 +
 test/search          |   48 -----------
 test/search-excludes |  214 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+), 48 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 b/test/search
index 17af6a2..a7a0b18 100755
--- a/test/search
+++ b/test/search
@@ -129,52 +129,4 @@ add_message '[subject]="utf8-message-body-subject"' '[date]="Sat, 01 Jan 2000 12
 output=$(notmuch search "bödý" | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; utf8-message-body-subject (inbox unread)"
 
-test_begin_subtest "Exclude \"deleted\" messages from search"
-notmuch config set search.exclude_tags deleted
-generate_message '[subject]="Not deleted"'
-not_deleted_id=$gen_msg_id
-generate_message '[subject]="Deleted"'
-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_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_expect_equal "$output" "id:$not_deleted_id
-id:$deleted_id"
-
-test_begin_subtest "Exclude \"deleted\" messages from message search (non-existent exclude-tag)"
-notmuch config set search.exclude_tags deleted non_existent_tag
-output=$(notmuch search --output=messages subject:deleted | notmuch_search_sanitize)
-test_expect_equal "$output" "id:$not_deleted_id"
-notmuch config set search.exclude_tags deleted
-
-test_begin_subtest "Exclude \"deleted\" messages from search, overridden"
-output=$(notmuch search subject:deleted and tag:deleted | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Deleted (deleted inbox unread)"
-
-test_begin_subtest "Exclude \"deleted\" messages from threads"
-add_message '[subject]="Not deleted reply"' '[in-reply-to]="<$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 [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_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)"
-
-test_begin_subtest "Don't exclude \"deleted\" messages from search if not configured"
-notmuch config set search.exclude_tags
-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 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)"
-
 test_done
diff --git a/test/search-excludes b/test/search-excludes
new file mode 100755
index 0000000..63acb7b
--- /dev/null
+++ b/test/search-excludes
@@ -0,0 +1,214 @@
+#!/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
+}
+
+# These are the original search exclude tests.
+
+test_begin_subtest "Exclude \"deleted\" messages from search"
+notmuch config set search.exclude_tags deleted
+generate_message '[subject]="Not deleted"'
+not_deleted_id=$gen_msg_id
+generate_message '[subject]="Deleted"'
+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)"
+
+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 --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"
+
+test_begin_subtest "Exclude \"deleted\" messages from message search (non-existent exclude-tag)"
+notmuch config set search.exclude_tags deleted non_existent_tag
+output=$(notmuch search --output=messages subject:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "id:$not_deleted_id"
+notmuch config set search.exclude_tags deleted
+
+test_begin_subtest "Exclude \"deleted\" messages from search, overridden"
+output=$(notmuch search subject:deleted and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Deleted (deleted inbox unread)"
+
+test_begin_subtest "Exclude \"deleted\" messages from threads"
+add_message '[subject]="Not deleted reply"' '[in-reply-to]="<$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 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread)"
+
+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 [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
+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 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)"
+
+
+
+
+# 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'
+
+# Temporarily remove excludes to get list of matching messages
+notmuch config set search.exclude_tags
+matching_message_ids=( `notmuch search --output=messages tag:test` )
+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" "${matching_message_ids[4]}
+${matching_message_ids[5]}"
+
+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" "${matching_message_ids[4]}
+${matching_message_ids[5]}"
+
+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" "${matching_message_ids[0]}
+${matching_message_ids[1]}
+${matching_message_ids[2]}
+${matching_message_ids[3]}
+${matching_message_ids[4]}
+${matching_message_ids[5]}"
+
+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" "${matching_message_ids[0]}
+${matching_message_ids[1]}
+${matching_message_ids[2]}
+${matching_message_ids[3]}
+${matching_message_ids[4]}
+${matching_message_ids[5]}"
+
+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" "${matching_message_ids[0]}
+${matching_message_ids[1]}
+${matching_message_ids[2]}
+${matching_message_ids[3]}"
+
+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" "${matching_message_ids[0]}
+${matching_message_ids[1]}
+${matching_message_ids[2]}
+${matching_message_ids[3]}"
+
+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" "${matching_message_ids[0]}
+${matching_message_ids[1]}
+${matching_message_ids[2]}
+${matching_message_ids[3]}"
+
+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" "${matching_message_ids[0]}
+${matching_message_ids[1]}
+${matching_message_ids[2]}
+${matching_message_ids[3]}"
+
+
+
+test_done
-- 
1.7.9.1

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

[Mark Walters]

This moves notmuch show to the --exclude=(true|false) naming
scheme. When exclude=false show returns all threads that match
including those that only match in an excluded message. The excluded
messages are flagged.

When exclude=true the behaviour depends on whether --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
the threads that match in a non-excluded message, flagging the excluded
messages in these threads. 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          |   50 +++++++++++++++++++++++++++++-----------------
 3 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index b81cce6..83cc575 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -135,9 +135,21 @@ content.
 
 .RS 4
 .TP 4
-.B \-\-no-exclude
+.BR \-\-exclude=(true|false)
+
+Specify whether to omit threads only matching search.tag_exclude from
+the search results (the default) or not. In either case the excluded
+message will be marked with the exclude flag (except when output=mbox
+when there is nowhere to put the 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 fa04fa2..97e0539 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 ff9d427..8f15bc9 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -851,6 +851,7 @@ show_messages (void *ctx,
 {
     notmuch_message_t *message;
     notmuch_bool_t match;
+    notmuch_bool_t excluded;
     int first_set = 1;
     int next_indent;
     notmuch_status_t status, res = NOTMUCH_STATUS_SUCCESS;
@@ -870,10 +871,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) {
 	    status = show_message (ctx, format, message, indent, params);
 	    if (status && !res)
 		res = status;
@@ -981,6 +983,12 @@ enum {
     NOTMUCH_FORMAT_RAW
 };
 
+/* The following is to allow future options to be added more easily */
+enum {
+    EXCLUDE_TRUE,
+    EXCLUDE_FALSE,
+};
+
 int
 notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 {
@@ -990,10 +998,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',
@@ -1002,11 +1010,14 @@ 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 },
+                                  { 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 }
     };
 
@@ -1095,29 +1106,30 @@ 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) {
-	    const char **search_exclude_tags;
-	    size_t search_exclude_tags_length;
-	    unsigned int i;
-
-	    search_exclude_tags = notmuch_config_get_search_exclude_tags
-		(config, &search_exclude_tags_length);
-	    for (i = 0; i < search_exclude_tags_length; i++)
-		notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+	/* We always apply set the exclude flag. The
+	 * exclude=true|false option controls whether or not we return
+	 * threads that only match in an excluded message */
+	const char **search_exclude_tags;
+	size_t search_exclude_tags_length;
+	unsigned int i;
+
+	search_exclude_tags = notmuch_config_get_search_exclude_tags
+	    (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_FALSE) {
+	    notmuch_query_set_omit_excluded_messages(query, FALSE);
+	    params.omit_excluded = FALSE;
 	}
+
 	ret = do_show (ctx, query, format, &params);
     }
 
-
     notmuch_query_destroy (query);
     notmuch_database_close (notmuch);
 
-- 
1.7.9.1

[PATCH v2 6/6] emacs: make show set --exclude=false

[Mark Walters]

Show has to set --exclude=false 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 6d3fe62..30b26d1 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1060,13 +1060,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
+				     (cons "--exclude=false" 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
+	    (cons "--exclude=false" basic-args)))))
 
       (jit-lock-register #'notmuch-show-buttonise-links)
 
-- 
1.7.9.1

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

[Jameson Graef Rollins]

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

On Sat, Mar 31 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> This is version 2 of the exclude= series (version 1 is [1]).
>
> The main changes are the addition of some systematic notmuch-search
> tests for all the exclude options, and I no longer add the
> exclude=flag option to notmuch-show.c (the output was too similar to
> exclude=false to be worth keeping).

Thanks so much for staying on top of this series.  I think this is a
really important new feature and I really appreciate you ferrying it all
the way through to the finish line.

These changes are critical to the functionality as well, as the exclude
stuff currently causes severe performance degradation.  So this is all
very welcome.

I've been using a previous version of this series for weeks now with no
problems at all.  The emacs UI is snappy again.

This new version applies cleanly to the current master, and all tests
pass.  And in my cursory review everything LGTM.

jamie.

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

Re: [PATCH v2 1/6] lib: change default for notmuch_query_set_omit_excluded

[Austin Clements]

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

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

[Austin Clements]

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

Re: [PATCH v2 4/6] test: add some exclude tests

[Austin Clements]

On Sat, 31 Mar 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> Systematically test the exclude options for search. Also move the
> search existing exclude tests into the new test. There is some overlap
> between the two sets of tests but many of the existing ones are there
> because they triggered bugs in the past so I have kept them to ensure
> coverage.
> ---
>  test/notmuch-test    |    1 +
>  test/search          |   48 -----------
>  test/search-excludes |  214 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 215 insertions(+), 48 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 b/test/search
> index 17af6a2..a7a0b18 100755
> --- a/test/search
> +++ b/test/search
> @@ -129,52 +129,4 @@ add_message '[subject]="utf8-message-body-subject"' '[date]="Sat, 01 Jan 2000 12
>  output=$(notmuch search "bödý" | notmuch_search_sanitize)
>  test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; utf8-message-body-subject (inbox unread)"
>  
> -test_begin_subtest "Exclude \"deleted\" messages from search"
> -notmuch config set search.exclude_tags deleted
> -generate_message '[subject]="Not deleted"'
> -not_deleted_id=$gen_msg_id
> -generate_message '[subject]="Deleted"'
> -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_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_expect_equal "$output" "id:$not_deleted_id
> -id:$deleted_id"
> -
> -test_begin_subtest "Exclude \"deleted\" messages from message search (non-existent exclude-tag)"
> -notmuch config set search.exclude_tags deleted non_existent_tag
> -output=$(notmuch search --output=messages subject:deleted | notmuch_search_sanitize)
> -test_expect_equal "$output" "id:$not_deleted_id"
> -notmuch config set search.exclude_tags deleted
> -
> -test_begin_subtest "Exclude \"deleted\" messages from search, overridden"
> -output=$(notmuch search subject:deleted and tag:deleted | notmuch_search_sanitize)
> -test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Deleted (deleted inbox unread)"
> -
> -test_begin_subtest "Exclude \"deleted\" messages from threads"
> -add_message '[subject]="Not deleted reply"' '[in-reply-to]="<$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 [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_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)"
> -
> -test_begin_subtest "Don't exclude \"deleted\" messages from search if not configured"
> -notmuch config set search.exclude_tags
> -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 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)"
> -
>  test_done
> diff --git a/test/search-excludes b/test/search-excludes
> new file mode 100755
> index 0000000..63acb7b
> --- /dev/null
> +++ b/test/search-excludes
> @@ -0,0 +1,214 @@
> +#!/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
> +}
> +
> +# These are the original search exclude tests.
> +
> +test_begin_subtest "Exclude \"deleted\" messages from search"
> +notmuch config set search.exclude_tags deleted
> +generate_message '[subject]="Not deleted"'
> +not_deleted_id=$gen_msg_id
> +generate_message '[subject]="Deleted"'
> +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)"
> +
> +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 --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"
> +
> +test_begin_subtest "Exclude \"deleted\" messages from message search (non-existent exclude-tag)"
> +notmuch config set search.exclude_tags deleted non_existent_tag
> +output=$(notmuch search --output=messages subject:deleted | notmuch_search_sanitize)
> +test_expect_equal "$output" "id:$not_deleted_id"
> +notmuch config set search.exclude_tags deleted
> +
> +test_begin_subtest "Exclude \"deleted\" messages from search, overridden"
> +output=$(notmuch search subject:deleted and tag:deleted | notmuch_search_sanitize)
> +test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Deleted (deleted inbox unread)"
> +
> +test_begin_subtest "Exclude \"deleted\" messages from threads"
> +add_message '[subject]="Not deleted reply"' '[in-reply-to]="<$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 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread)"
> +
> +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 [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
> +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 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)"
> +
> +
> +
> +
> +# 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*"

I think you meant
  notmuch tag +deleted 'subject:"All messages excluded: single match"'
since what you wrote will search for "All" in the subject and the rest
of the words anywhere in the message and the glob at the end won't do
anything.  The same goes for all of your other queries.

Alternatively, maybe generate_thread could do a query to get the thread
ID from $gen_msg_id and stuff that in a variable (e.g., $gen_thread_id)
that you could use for tagging.  That would probably be much less
painful.  It could also stuff the generated message IDs in an array that
you could use to refer to individual messages.

> +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'
> +
> +# Temporarily remove excludes to get list of matching messages
> +notmuch config set search.exclude_tags
> +matching_message_ids=( `notmuch search --output=messages tag:test` )
> +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" "${matching_message_ids[4]}
> +${matching_message_ids[5]}"

Just a thought...  Are we depending on notmuch using a stable sort to
guarantee this order, given that it's sorting by timestamps with only
second granularity?  We make this same assumption all over the test
suite, so I don't think you have to worry about it here, but this hadn't
occurred to me before.

> +
> +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" "${matching_message_ids[4]}
> +${matching_message_ids[5]}"
> +
> +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" "${matching_message_ids[0]}
> +${matching_message_ids[1]}
> +${matching_message_ids[2]}
> +${matching_message_ids[3]}
> +${matching_message_ids[4]}
> +${matching_message_ids[5]}"
> +
> +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" "${matching_message_ids[0]}
> +${matching_message_ids[1]}
> +${matching_message_ids[2]}
> +${matching_message_ids[3]}
> +${matching_message_ids[4]}
> +${matching_message_ids[5]}"
> +
> +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" "${matching_message_ids[0]}
> +${matching_message_ids[1]}
> +${matching_message_ids[2]}
> +${matching_message_ids[3]}"
> +
> +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" "${matching_message_ids[0]}
> +${matching_message_ids[1]}
> +${matching_message_ids[2]}
> +${matching_message_ids[3]}"
> +
> +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" "${matching_message_ids[0]}
> +${matching_message_ids[1]}
> +${matching_message_ids[2]}
> +${matching_message_ids[3]}"
> +
> +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" "${matching_message_ids[0]}
> +${matching_message_ids[1]}
> +${matching_message_ids[2]}
> +${matching_message_ids[3]}"
> +
> +
> +
> +test_done

Awesome.

> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Austin Clements]

On Sat, 31 Mar 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> This is version 2 of the exclude= series (version 1 is [1]).
>
> The main changes are the addition of some systematic notmuch-search
> tests for all the exclude options, and I no longer add the
> exclude=flag option to notmuch-show.c (the output was too similar to
> exclude=false to be worth keeping).
>
> Best wishes
>
> Mark
>
>
> id:"1331836925-31437-1-git-send-email-markwalters1009@gmail.com".

Sorry for the slow review.  Besides my fairly superficial comments,
everything else LGTM.

It's great to see exclude really turning into a robust feature!  I had
no idea the path would be this long when I wrote that original little
hack.  I'm thankful you're sticking with it.