[PATCH v2 0/8] search --exclude=all

[PATCH v2 0/8] search --exclude=all

[Mark Walters]

This is a rebased version of
id:1340198947-29370-1-git-send-email-novalazy@gmail.com I have not
tested it significantly in this form but the rebasing looks ok and all
the tests pass.

Best wishes

Mark




Mark Walters (2):
  lib: add --exclude=all option
  cli: add --exclude=all option to notmuch-search.c

Peter Wang (6):
  test: add tests for search --exclude=all
  man: clarify search --exclude documentation
  man: clarify search --exclude=flag
  man: document search --exclude=all
  lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  cli: use notmuch_exclude_t in option parser

 lib/notmuch-private.h     |    1 +
 lib/notmuch.h             |   23 ++++++++++++++++-------
 lib/query.cc              |   14 +++++++++-----
 lib/thread.cc             |   41 ++++++++++++++++++++++++++++++-----------
 man/man1/notmuch-search.1 |   29 +++++++++++++++++++++++------
 notmuch-search.c          |   24 +++++++++---------------
 test/excludes             |   22 ++++++++++++++++++++++
 7 files changed, 110 insertions(+), 44 deletions(-)

-- 
1.7.9.1

[PATCH v2 1/8] lib: add --exclude=all option

[Mark Walters]

Adds a exclude all option to the lib which means that excluded
messages are completely ignored (as if they had actually been
deleted).
---
 lib/notmuch-private.h |    1 +
 lib/notmuch.h         |   22 +++++++++++++++-------
 lib/query.cc          |   10 ++++++----
 lib/thread.cc         |   41 ++++++++++++++++++++++++++++++-----------
 4 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index f38ccb3..cc55bb9 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -236,6 +236,7 @@ _notmuch_thread_create (void *ctx,
 			unsigned int seed_doc_id,
 			notmuch_doc_id_set_t *match_set,
 			notmuch_string_list_t *excluded_terms,
+			notmuch_exclude_t omit_exclude,
 			notmuch_sort_t sort);
 
 /* message.cc */
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 3739336..27b43ff 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -500,14 +500,22 @@ typedef enum {
 const char *
 notmuch_query_get_query_string (notmuch_query_t *query);
 
+/* Exclude values for notmuch_query_set_omit_excluded */
+typedef enum {
+    NOTMUCH_EXCLUDE_FALSE,
+    NOTMUCH_EXCLUDE_TRUE,
+    NOTMUCH_EXCLUDE_ALL
+} notmuch_exclude_t;
+
 /* 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 match in at least one non-excluded
- * message.
+ * If set to TRUE or ALL, notmuch_query_search_messages will omit excluded
+ * messages from the results, and notmuch_query_search_threads will omit
+ * threads that match only in excluded messages.  If set to TRUE,
+ * notmuch_query_search_threads will include all messages in threads that
+ * match in at least one non-excluded message.  Otherwise, if set to ALL,
+ * notmuch_query_search_threads will omit excluded messages from all threads.
  *
  * The performance difference when calling
  * notmuch_query_search_messages should be relatively small (and both
@@ -516,9 +524,9 @@ notmuch_query_get_query_string (notmuch_query_t *query);
  * excluded messages as it does not need to construct the threads that
  * only match in excluded messages.
  */
-
 void
-notmuch_query_set_omit_excluded (notmuch_query_t *query, notmuch_bool_t omit_excluded);
+notmuch_query_set_omit_excluded (notmuch_query_t *query,
+				 notmuch_exclude_t omit_excluded);
 
 /* Specify the sorting desired for this query. */
 void
diff --git a/lib/query.cc b/lib/query.cc
index 7381a54..1cc768f 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;
+    notmuch_exclude_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 = TRUE;
+    query->omit_excluded = NOTMUCH_EXCLUDE_TRUE;
 
     return query;
 }
@@ -104,7 +104,8 @@ notmuch_query_get_query_string (notmuch_query_t *query)
 }
 
 void
-notmuch_query_set_omit_excluded (notmuch_query_t *query, notmuch_bool_t omit_excluded)
+notmuch_query_set_omit_excluded (notmuch_query_t *query,
+				 notmuch_exclude_t omit_excluded)
 {
     query->omit_excluded = omit_excluded;
 }
@@ -220,7 +221,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)
+	    if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)
 		final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
 					     final_query, exclude_query);
 	    else {
@@ -486,6 +487,7 @@ notmuch_threads_get (notmuch_threads_t *threads)
 				   doc_id,
 				   &threads->match_set,
 				   threads->query->exclude_terms,
+				   threads->query->omit_excluded,
 				   threads->query->sort);
 }
 
diff --git a/lib/thread.cc b/lib/thread.cc
index 50bdef1..bc07877 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -227,7 +227,8 @@ _thread_cleanup_author (notmuch_thread_t *thread,
 static void
 _thread_add_message (notmuch_thread_t *thread,
 		     notmuch_message_t *message,
-		     notmuch_string_list_t *exclude_terms)
+		     notmuch_string_list_t *exclude_terms,
+		     notmuch_exclude_t omit_exclude)
 {
     notmuch_tags_t *tags;
     const char *tag;
@@ -235,6 +236,28 @@ _thread_add_message (notmuch_thread_t *thread,
     InternetAddress *address;
     const char *from, *author;
     char *clean_author;
+    notmuch_bool_t message_excluded = FALSE;
+
+    for (tags = notmuch_message_get_tags (message);
+	 notmuch_tags_valid (tags);
+	 notmuch_tags_move_to_next (tags))
+    {
+	tag = notmuch_tags_get (tags);
+	/* Is message excluded? */
+	for (notmuch_string_node_t *term = exclude_terms->head;
+	     term != NULL;
+	     term = term->next)
+	{
+	    /* We ignore initial 'K'. */
+	    if (strcmp(tag, (term->string + 1)) == 0) {
+		message_excluded = TRUE;
+		break;
+	    }
+	}
+    }
+
+    if (message_excluded && omit_exclude == NOTMUCH_EXCLUDE_ALL)
+	return;
 
     _notmuch_message_list_add_message (thread->message_list,
 				       talloc_steal (thread, message));
@@ -275,17 +298,12 @@ _thread_add_message (notmuch_thread_t *thread,
 	 notmuch_tags_move_to_next (tags))
     {
 	tag = notmuch_tags_get (tags);
-	/* Mark excluded messages. */
-	for (notmuch_string_node_t *term = exclude_terms->head; term;
-	     term = term->next) {
-	    /* We ignore initial 'K'. */
-	    if (strcmp(tag, (term->string + 1)) == 0) {
-		notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED, TRUE);
-		break;
-	    }
-	}
 	g_hash_table_insert (thread->tags, xstrdup (tag), NULL);
     }
+
+    /* Mark excluded messages. */
+    if (message_excluded)
+	notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED, TRUE);
 }
 
 static void
@@ -409,6 +427,7 @@ _notmuch_thread_create (void *ctx,
 			unsigned int seed_doc_id,
 			notmuch_doc_id_set_t *match_set,
 			notmuch_string_list_t *exclude_terms,
+			notmuch_exclude_t omit_excluded,
 			notmuch_sort_t sort)
 {
     void *local = talloc_new (ctx);
@@ -488,7 +507,7 @@ _notmuch_thread_create (void *ctx,
 	if (doc_id == seed_doc_id)
 	    message = seed_message;
 
-	_thread_add_message (thread, message, exclude_terms);
+	_thread_add_message (thread, message, exclude_terms, omit_excluded);
 
 	if ( _notmuch_doc_id_set_contains (match_set, doc_id)) {
 	    _notmuch_doc_id_set_remove (match_set, doc_id);
-- 
1.7.9.1

[PATCH v2 2/8] cli: add --exclude=all option to notmuch-search.c

[Mark Walters]

Add a --exclude=all option to notmuch search.
---
 notmuch-search.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index e658639..4323201 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -287,6 +287,7 @@ enum {
     EXCLUDE_TRUE,
     EXCLUDE_FALSE,
     EXCLUDE_FLAG,
+    EXCLUDE_ALL
 };
 
 int
@@ -334,6 +335,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
           (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
                                   { "false", EXCLUDE_FALSE },
                                   { "flag", EXCLUDE_FLAG },
+                                  { "all", EXCLUDE_ALL },
                                   { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
 	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
@@ -400,7 +402,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	exclude = EXCLUDE_FALSE;
     }
 
-    if (exclude == EXCLUDE_TRUE || exclude == EXCLUDE_FLAG) {
+    if (exclude != EXCLUDE_FALSE) {
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
 
@@ -409,7 +411,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	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 (query, FALSE);
+	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FALSE);
+	if (exclude == EXCLUDE_ALL)
+	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
     }
 
     switch (output) {
-- 
1.7.9.1

[PATCH v2 3/8] test: add tests for search --exclude=all

[Mark Walters]

From: Peter Wang <novalazy@gmail.com>

Test the new search --exclude=all option.
---
 test/excludes |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/test/excludes b/test/excludes
index 24d653e..f1ae9ea 100755
--- a/test/excludes
+++ b/test/excludes
@@ -166,6 +166,16 @@ ${matching_message_ids[3]}
 ${matching_message_ids[4]}
 ${matching_message_ids[5]}"
 
+test_begin_subtest "Search, exclude=all (thread summary)"
+output=$(notmuch search --exclude=all 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: reply 4 (inbox test unread)
+thread:XXX   2001-01-05 [1/6] Notmuch Test Suite; No messages excluded: single match: reply 3 (inbox test unread)"
+
+test_begin_subtest "Search, exclude=all (messages)"
+output=$(notmuch search --exclude=all --output=messages tag:test | notmuch_search_sanitize)
+test_expect_equal "$output" "${matching_message_ids[4]}
+${matching_message_ids[5]}"
+
 test_begin_subtest "Search, 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/6] Notmuch Test Suite; All messages excluded: single match: reply 2 (deleted inbox test unread)
@@ -218,6 +228,18 @@ ${matching_message_ids[1]}
 ${matching_message_ids[2]}
 ${matching_message_ids[3]}"
 
+test_begin_subtest "Search, exclude=all: tag in query (thread summary)"
+output=$(notmuch search --exclude=all tag:test and tag:deleted | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/6] Notmuch Test Suite; All messages excluded: single match: reply 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [2/6] Notmuch Test Suite; All messages excluded: double match: reply 2 (deleted inbox test unread)
+thread:XXX   2001-01-05 [1/6] Notmuch Test Suite; Some messages excluded: single excluded match: reply 3 (deleted inbox test unread)"
+
+test_begin_subtest "Search, exclude=all: tag in query (messages)"
+output=$(notmuch search --exclude=all --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]}"
 
 #########################################################
 # Notmuch count tests
-- 
1.7.9.1

[PATCH v2 4/8] man: clarify search --exclude documentation

[Mark Walters]

From: Peter Wang <novalazy@gmail.com>

Highlight "excluded messages" as a term with a meaning that
may not be obvious.

Be explicit about the effects of search --exclude=true and
--exclude=false.
---
 man/man1/notmuch-search.1 |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index d3391f8..59e8f34 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -132,8 +132,19 @@ Limit the number of displayed results to N.
 .TP 4
 .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
+A message is called "excluded" if it matches at least one tag in
+search.tag_exclude that does not appear explicitly in the search terms.
+This option specifies whether to omit excluded messages in the search
+process.
+
+The default value,
+.BR true ,
+prevents excluded messages from matching the search terms.
+
+.B false
+allows excluded messages to match search terms and appear in displayed
+results. Excluded messages are still marked in the relevant outputs.
+
 .B flag
 only has an effect when
 .B --output=summary
-- 
1.7.9.1

Re: [PATCH v2 4/8] man: clarify search --exclude documentation

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> From: Peter Wang <novalazy@gmail.com>
>
> Highlight "excluded messages" as a term with a meaning that
> may not be obvious.
>

Pushed the two man page updates that don't relate to new features.

d

[PATCH v2 5/8] man: clarify search --exclude=flag

[Mark Walters]

From: Peter Wang <novalazy@gmail.com>

Improve the description of the search --exclude=flag option,
using text taken from the commit that introduced the option.
---
 man/man1/notmuch-search.1 |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 59e8f34..da2f1dd 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -147,9 +147,11 @@ results. Excluded messages are still marked in the relevant outputs.
 
 .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.
+.BR --output=summary .
+The output is almost identical to
+.BR false ,
+but the "match count" is the number of matching non-excluded messages in the
+thread, rather than the number of matching messages.
 .RE
 
 .SH EXIT STATUS
-- 
1.7.9.1

[PATCH v2 6/8] man: document search --exclude=all

[Mark Walters]

From: Peter Wang <novalazy@gmail.com>

Document the new search --exclude=all option.
---
 man/man1/notmuch-search.1 |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index da2f1dd..1c1e049 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -130,7 +130,7 @@ Limit the number of displayed results to N.
 
 .RS 4
 .TP 4
-.BR \-\-exclude=(true|false|flag)
+.BR \-\-exclude=(true|false|all|flag)
 
 A message is called "excluded" if it matches at least one tag in
 search.tag_exclude that does not appear explicitly in the search terms.
@@ -141,6 +141,10 @@ The default value,
 .BR true ,
 prevents excluded messages from matching the search terms.
 
+.B all
+additionally prevents excluded messages from appearing in displayed
+results, in effect behaving as though the excluded messages do not exist.
+
 .B false
 allows excluded messages to match search terms and appear in displayed
 results. Excluded messages are still marked in the relevant outputs.
-- 
1.7.9.1

[PATCH v2 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Mark Walters]

From: Peter Wang <novalazy@gmail.com>

Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
cover all four values of search --exclude in the cli.
---
 lib/notmuch.h    |    1 +
 lib/query.cc     |    6 ++++--
 notmuch-search.c |    2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 27b43ff..35795bb 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -504,6 +504,7 @@ notmuch_query_get_query_string (notmuch_query_t *query);
 typedef enum {
     NOTMUCH_EXCLUDE_FALSE,
     NOTMUCH_EXCLUDE_TRUE,
+    NOTMUCH_EXCLUDE_FLAG,
     NOTMUCH_EXCLUDE_ALL
 } notmuch_exclude_t;
 
diff --git a/lib/query.cc b/lib/query.cc
index 1cc768f..e61d11e 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -221,10 +221,12 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	if (query->exclude_terms) {
 	    exclude_query = _notmuch_exclude_tags (query, final_query);
 
-	    if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)
+	    if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
+		query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
+	    {
 		final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
 					     final_query, exclude_query);
-	    else {
+	    } else {
 		exclude_query = Xapian::Query (Xapian::Query::OP_AND,
 					   exclude_query, final_query);
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 4323201..a20791a 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -411,7 +411,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	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 (query, NOTMUCH_EXCLUDE_FALSE);
+	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
 	if (exclude == EXCLUDE_ALL)
 	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
     }
-- 
1.7.9.1

Re: [PATCH v2 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> -	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FALSE);
> +	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);

Shouldn't the documentation be updated to cover NOTMUCH_EXCLUDE_FLAG?  I
realize it is not _wrong_ as written, but it is a bit confusing not to
cover all possible values of the enum.

Other than that, the series LGTM.

d

Re: [PATCH v2 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Peter Wang]

On Sun, 12 May 2013 08:20:04 -0300, David Bremner <david@tethera.net> wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
> 
> > -	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FALSE);
> > +	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
> 
> Shouldn't the documentation be updated to cover NOTMUCH_EXCLUDE_FLAG?  I
> realize it is not _wrong_ as written, but it is a bit confusing not to
> cover all possible values of the enum.

Actually, please drop patch 7 and patch 8.  It was wrong to combine the
CLI-level EXCLUDE_* constants with the lib-level NOTMUCH_EXCLUDE_* constants.

Peter

[PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Mark Walters]

Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
cover all four values of search --exclude in the cli.

Previously the way to avoid any message being marked excluded was to
pass in an empty list of excluded tags: since we now have an explicit
option we might as well honour it.

The enum is in a slightly strange order as the existing FALSE/TRUE
options correspond to the new
NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options so this means we do
not need to bump the version number.

Indeed, an example of this is that the cli count and show still use
FALSE/TRUE and still work.
---

This is a new version of this single patch. In Peter's version
NOTMUCH_EXCLUDE_FLAG and NOTMUCH_EXCLUDE_FALSE were treated as synonyms:
this makes them behave in the obvious way and documents them.

I think the only subtlety is the enum order mentioned in the commit
message. Additionally it would be good to update cli count and show and,
at for the latter, it would be good to add an option exclude=all too (so
excluded messages are completely omitted). Those should both be simple
but we need to decide whether to allow all four options
(false/flag/true/all) always or not. Always allowing all 4 is nicely
consistent but sometimes redundant. Additionally we would need some
tests.

I think this patch should go in with the main series as I think it is
worth reserving the NOTMUCH_EXCLUDE_FALSE #define/value so that we don't
break code in future if we do wish to add it.

Best wishes

Mark


 
 lib/notmuch.h    |   18 ++++++++++++++++--
 lib/query.cc     |    8 +++++---
 lib/thread.cc    |   28 +++++++++++++++-------------
 notmuch-search.c |    2 +-
 4 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 27b43ff..73c85a4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -500,10 +500,15 @@ typedef enum {
 const char *
 notmuch_query_get_query_string (notmuch_query_t *query);
 
-/* Exclude values for notmuch_query_set_omit_excluded */
+/* Exclude values for notmuch_query_set_omit_excluded. The strange
+ * order is to maintain backward compatibility: the old FALSE/TRUE
+ * options correspond to the new
+ * NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options.
+ */
 typedef enum {
-    NOTMUCH_EXCLUDE_FALSE,
+    NOTMUCH_EXCLUDE_FLAG,
     NOTMUCH_EXCLUDE_TRUE,
+    NOTMUCH_EXCLUDE_FALSE,
     NOTMUCH_EXCLUDE_ALL
 } notmuch_exclude_t;
 
@@ -517,6 +522,15 @@ typedef enum {
  * match in at least one non-excluded message.  Otherwise, if set to ALL,
  * notmuch_query_search_threads will omit excluded messages from all threads.
  *
+ * If set to FALSE or FLAG then both notmuch_query_search_messages and
+ * notmuch_query_search_threads will return all matching
+ * messages/threads regardless of exclude status. If set to FLAG then
+ * the exclude flag will be set for any excluded message that is
+ * returned by notmuch_query_search_messages, and the thread counts
+ * for threads returned by notmuch_query_search_threads will be the
+ * number of non-excluded messages/matches. Otherwise, if set to
+ * FALSE, then the exclude status is completely ignored.
+ *
  * The performance difference when calling
  * notmuch_query_search_messages should be relatively small (and both
  * should be very fast).  However, in some cases,
diff --git a/lib/query.cc b/lib/query.cc
index 1cc768f..69668a4 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -218,13 +218,15 @@ notmuch_query_search_messages (notmuch_query_t *query)
 	}
 	messages->base.excluded_doc_ids = NULL;
 
-	if (query->exclude_terms) {
+	if ((query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) && (query->exclude_terms)) {
 	    exclude_query = _notmuch_exclude_tags (query, final_query);
 
-	    if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)
+	    if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
+		query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
+	    {
 		final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
 					     final_query, exclude_query);
-	    else {
+	    } else { /* NOTMUCH_EXCLUDE_FLAG */
 		exclude_query = Xapian::Query (Xapian::Query::OP_AND,
 					   exclude_query, final_query);
 
diff --git a/lib/thread.cc b/lib/thread.cc
index bc07877..4dcf705 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -238,20 +238,22 @@ _thread_add_message (notmuch_thread_t *thread,
     char *clean_author;
     notmuch_bool_t message_excluded = FALSE;
 
-    for (tags = notmuch_message_get_tags (message);
-	 notmuch_tags_valid (tags);
-	 notmuch_tags_move_to_next (tags))
-    {
-	tag = notmuch_tags_get (tags);
-	/* Is message excluded? */
-	for (notmuch_string_node_t *term = exclude_terms->head;
-	     term != NULL;
-	     term = term->next)
+    if (omit_exclude != NOTMUCH_EXCLUDE_FALSE) {
+	for (tags = notmuch_message_get_tags (message);
+	     notmuch_tags_valid (tags);
+	     notmuch_tags_move_to_next (tags))
 	{
-	    /* We ignore initial 'K'. */
-	    if (strcmp(tag, (term->string + 1)) == 0) {
-		message_excluded = TRUE;
-		break;
+	    tag = notmuch_tags_get (tags);
+	    /* Is message excluded? */
+	    for (notmuch_string_node_t *term = exclude_terms->head;
+		 term != NULL;
+		 term = term->next)
+	    {
+		/* We ignore initial 'K'. */
+		if (strcmp(tag, (term->string + 1)) == 0) {
+		    message_excluded = TRUE;
+		    break;
+		}
 	    }
 	}
     }
diff --git a/notmuch-search.c b/notmuch-search.c
index 4323201..a20791a 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -411,7 +411,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	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 (query, NOTMUCH_EXCLUDE_FALSE);
+	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
 	if (exclude == EXCLUDE_ALL)
 	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
     }
-- 
1.7.9.1

Re: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Mark Walters]

I think we should decide before 0.16 whether to go with this patch and
patch 8/8 or for Peter's suggestion at
id:1368454815-1854-1-git-send-email-novalazy@gmail.com

Now we have an enum for the NOTMUCH_EXCLUDE possibilities (rather than a
bool) I think it makes sense to implement NOTMUCH_EXCLUDE_FALSE properly
but I am happy with either choice.

Best wishes

Mark



On Mon, 13 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
> cover all four values of search --exclude in the cli.
>
> Previously the way to avoid any message being marked excluded was to
> pass in an empty list of excluded tags: since we now have an explicit
> option we might as well honour it.
>
> The enum is in a slightly strange order as the existing FALSE/TRUE
> options correspond to the new
> NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options so this means we do
> not need to bump the version number.
>
> Indeed, an example of this is that the cli count and show still use
> FALSE/TRUE and still work.
> ---
>
> This is a new version of this single patch. In Peter's version
> NOTMUCH_EXCLUDE_FLAG and NOTMUCH_EXCLUDE_FALSE were treated as synonyms:
> this makes them behave in the obvious way and documents them.
>
> I think the only subtlety is the enum order mentioned in the commit
> message. Additionally it would be good to update cli count and show and,
> at for the latter, it would be good to add an option exclude=all too (so
> excluded messages are completely omitted). Those should both be simple
> but we need to decide whether to allow all four options
> (false/flag/true/all) always or not. Always allowing all 4 is nicely
> consistent but sometimes redundant. Additionally we would need some
> tests.
>
> I think this patch should go in with the main series as I think it is
> worth reserving the NOTMUCH_EXCLUDE_FALSE #define/value so that we don't
> break code in future if we do wish to add it.
>
> Best wishes
>
> Mark
>
>
>  
>  lib/notmuch.h    |   18 ++++++++++++++++--
>  lib/query.cc     |    8 +++++---
>  lib/thread.cc    |   28 +++++++++++++++-------------
>  notmuch-search.c |    2 +-
>  4 files changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 27b43ff..73c85a4 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -500,10 +500,15 @@ typedef enum {
>  const char *
>  notmuch_query_get_query_string (notmuch_query_t *query);
>  
> -/* Exclude values for notmuch_query_set_omit_excluded */
> +/* Exclude values for notmuch_query_set_omit_excluded. The strange
> + * order is to maintain backward compatibility: the old FALSE/TRUE
> + * options correspond to the new
> + * NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options.
> + */
>  typedef enum {
> -    NOTMUCH_EXCLUDE_FALSE,
> +    NOTMUCH_EXCLUDE_FLAG,
>      NOTMUCH_EXCLUDE_TRUE,
> +    NOTMUCH_EXCLUDE_FALSE,
>      NOTMUCH_EXCLUDE_ALL
>  } notmuch_exclude_t;
>  
> @@ -517,6 +522,15 @@ typedef enum {
>   * match in at least one non-excluded message.  Otherwise, if set to ALL,
>   * notmuch_query_search_threads will omit excluded messages from all threads.
>   *
> + * If set to FALSE or FLAG then both notmuch_query_search_messages and
> + * notmuch_query_search_threads will return all matching
> + * messages/threads regardless of exclude status. If set to FLAG then
> + * the exclude flag will be set for any excluded message that is
> + * returned by notmuch_query_search_messages, and the thread counts
> + * for threads returned by notmuch_query_search_threads will be the
> + * number of non-excluded messages/matches. Otherwise, if set to
> + * FALSE, then the exclude status is completely ignored.
> + *
>   * The performance difference when calling
>   * notmuch_query_search_messages should be relatively small (and both
>   * should be very fast).  However, in some cases,
> diff --git a/lib/query.cc b/lib/query.cc
> index 1cc768f..69668a4 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -218,13 +218,15 @@ notmuch_query_search_messages (notmuch_query_t *query)
>  	}
>  	messages->base.excluded_doc_ids = NULL;
>  
> -	if (query->exclude_terms) {
> +	if ((query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) && (query->exclude_terms)) {
>  	    exclude_query = _notmuch_exclude_tags (query, final_query);
>  
> -	    if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)
> +	    if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
> +		query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
> +	    {
>  		final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
>  					     final_query, exclude_query);
> -	    else {
> +	    } else { /* NOTMUCH_EXCLUDE_FLAG */
>  		exclude_query = Xapian::Query (Xapian::Query::OP_AND,
>  					   exclude_query, final_query);
>  
> diff --git a/lib/thread.cc b/lib/thread.cc
> index bc07877..4dcf705 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -238,20 +238,22 @@ _thread_add_message (notmuch_thread_t *thread,
>      char *clean_author;
>      notmuch_bool_t message_excluded = FALSE;
>  
> -    for (tags = notmuch_message_get_tags (message);
> -	 notmuch_tags_valid (tags);
> -	 notmuch_tags_move_to_next (tags))
> -    {
> -	tag = notmuch_tags_get (tags);
> -	/* Is message excluded? */
> -	for (notmuch_string_node_t *term = exclude_terms->head;
> -	     term != NULL;
> -	     term = term->next)
> +    if (omit_exclude != NOTMUCH_EXCLUDE_FALSE) {
> +	for (tags = notmuch_message_get_tags (message);
> +	     notmuch_tags_valid (tags);
> +	     notmuch_tags_move_to_next (tags))
>  	{
> -	    /* We ignore initial 'K'. */
> -	    if (strcmp(tag, (term->string + 1)) == 0) {
> -		message_excluded = TRUE;
> -		break;
> +	    tag = notmuch_tags_get (tags);
> +	    /* Is message excluded? */
> +	    for (notmuch_string_node_t *term = exclude_terms->head;
> +		 term != NULL;
> +		 term = term->next)
> +	    {
> +		/* We ignore initial 'K'. */
> +		if (strcmp(tag, (term->string + 1)) == 0) {
> +		    message_excluded = TRUE;
> +		    break;
> +		}
>  	    }
>  	}
>      }
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 4323201..a20791a 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -411,7 +411,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  	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 (query, NOTMUCH_EXCLUDE_FALSE);
> +	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
>  	if (exclude == EXCLUDE_ALL)
>  	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
>      }
> -- 
> 1.7.9.1

Re: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Tomi Ollila]

On Thu, Jun 20 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> I think we should decide before 0.16 whether to go with this patch and
> patch 8/8 or for Peter's suggestion at
> id:1368454815-1854-1-git-send-email-novalazy@gmail.com
>
> Now we have an enum for the NOTMUCH_EXCLUDE possibilities (rather than a
> bool) I think it makes sense to implement NOTMUCH_EXCLUDE_FALSE properly
> but I am happy with either choice.

So, the choice here is to choose between

id:"1368454815-1854-1-git-send-email-novalazy@gmail.com"
and
id:"87bo8edif8.fsf@qmul.ac.uk" 

(might be hard to catch from this thread -- was easier to take from
http://nmbug.tethera.net/status/ )

Both apply cleanly to current master ( d0bd88f )

While Peter's version surely looks simpler (and may work, didn't test atm)
the comments Mark presents makes sense (especially the "subtlety" thing ;)

IMHO Mark's version makes future development "safer" and therefore my
vote (or million of those) goes to id:"87bo8edif8.fsf@qmul.ac.uk"

> Best wishes
>
> Mark

Tomi


>
>
>
> On Mon, 13 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>> Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
>> cover all four values of search --exclude in the cli.
>>
>> Previously the way to avoid any message being marked excluded was to
>> pass in an empty list of excluded tags: since we now have an explicit
>> option we might as well honour it.
>>
>> The enum is in a slightly strange order as the existing FALSE/TRUE
>> options correspond to the new
>> NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options so this means we do
>> not need to bump the version number.
>>
>> Indeed, an example of this is that the cli count and show still use
>> FALSE/TRUE and still work.
>> ---
>>
>> This is a new version of this single patch. In Peter's version
>> NOTMUCH_EXCLUDE_FLAG and NOTMUCH_EXCLUDE_FALSE were treated as synonyms:
>> this makes them behave in the obvious way and documents them.
>>
>> I think the only subtlety is the enum order mentioned in the commit
>> message. Additionally it would be good to update cli count and show and,
>> at for the latter, it would be good to add an option exclude=all too (so
>> excluded messages are completely omitted). Those should both be simple
>> but we need to decide whether to allow all four options
>> (false/flag/true/all) always or not. Always allowing all 4 is nicely
>> consistent but sometimes redundant. Additionally we would need some
>> tests.
>>
>> I think this patch should go in with the main series as I think it is
>> worth reserving the NOTMUCH_EXCLUDE_FALSE #define/value so that we don't
>> break code in future if we do wish to add it.
>>
>> Best wishes
>>
>> Mark
>>
>>
>>  
>>  lib/notmuch.h    |   18 ++++++++++++++++--
>>  lib/query.cc     |    8 +++++---
>>  lib/thread.cc    |   28 +++++++++++++++-------------
>>  notmuch-search.c |    2 +-
>>  4 files changed, 37 insertions(+), 19 deletions(-)
>>
>> diff --git a/lib/notmuch.h b/lib/notmuch.h
>> index 27b43ff..73c85a4 100644
>> --- a/lib/notmuch.h
>> +++ b/lib/notmuch.h
>> @@ -500,10 +500,15 @@ typedef enum {
>>  const char *
>>  notmuch_query_get_query_string (notmuch_query_t *query);
>>  
>> -/* Exclude values for notmuch_query_set_omit_excluded */
>> +/* Exclude values for notmuch_query_set_omit_excluded. The strange
>> + * order is to maintain backward compatibility: the old FALSE/TRUE
>> + * options correspond to the new
>> + * NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options.
>> + */
>>  typedef enum {
>> -    NOTMUCH_EXCLUDE_FALSE,
>> +    NOTMUCH_EXCLUDE_FLAG,
>>      NOTMUCH_EXCLUDE_TRUE,
>> +    NOTMUCH_EXCLUDE_FALSE,
>>      NOTMUCH_EXCLUDE_ALL
>>  } notmuch_exclude_t;
>>  
>> @@ -517,6 +522,15 @@ typedef enum {
>>   * match in at least one non-excluded message.  Otherwise, if set to ALL,
>>   * notmuch_query_search_threads will omit excluded messages from all threads.
>>   *
>> + * If set to FALSE or FLAG then both notmuch_query_search_messages and
>> + * notmuch_query_search_threads will return all matching
>> + * messages/threads regardless of exclude status. If set to FLAG then
>> + * the exclude flag will be set for any excluded message that is
>> + * returned by notmuch_query_search_messages, and the thread counts
>> + * for threads returned by notmuch_query_search_threads will be the
>> + * number of non-excluded messages/matches. Otherwise, if set to
>> + * FALSE, then the exclude status is completely ignored.
>> + *
>>   * The performance difference when calling
>>   * notmuch_query_search_messages should be relatively small (and both
>>   * should be very fast).  However, in some cases,
>> diff --git a/lib/query.cc b/lib/query.cc
>> index 1cc768f..69668a4 100644
>> --- a/lib/query.cc
>> +++ b/lib/query.cc
>> @@ -218,13 +218,15 @@ notmuch_query_search_messages (notmuch_query_t *query)
>>  	}
>>  	messages->base.excluded_doc_ids = NULL;
>>  
>> -	if (query->exclude_terms) {
>> +	if ((query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) && (query->exclude_terms)) {
>>  	    exclude_query = _notmuch_exclude_tags (query, final_query);
>>  
>> -	    if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)
>> +	    if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
>> +		query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
>> +	    {
>>  		final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
>>  					     final_query, exclude_query);
>> -	    else {
>> +	    } else { /* NOTMUCH_EXCLUDE_FLAG */
>>  		exclude_query = Xapian::Query (Xapian::Query::OP_AND,
>>  					   exclude_query, final_query);
>>  
>> diff --git a/lib/thread.cc b/lib/thread.cc
>> index bc07877..4dcf705 100644
>> --- a/lib/thread.cc
>> +++ b/lib/thread.cc
>> @@ -238,20 +238,22 @@ _thread_add_message (notmuch_thread_t *thread,
>>      char *clean_author;
>>      notmuch_bool_t message_excluded = FALSE;
>>  
>> -    for (tags = notmuch_message_get_tags (message);
>> -	 notmuch_tags_valid (tags);
>> -	 notmuch_tags_move_to_next (tags))
>> -    {
>> -	tag = notmuch_tags_get (tags);
>> -	/* Is message excluded? */
>> -	for (notmuch_string_node_t *term = exclude_terms->head;
>> -	     term != NULL;
>> -	     term = term->next)
>> +    if (omit_exclude != NOTMUCH_EXCLUDE_FALSE) {
>> +	for (tags = notmuch_message_get_tags (message);
>> +	     notmuch_tags_valid (tags);
>> +	     notmuch_tags_move_to_next (tags))
>>  	{
>> -	    /* We ignore initial 'K'. */
>> -	    if (strcmp(tag, (term->string + 1)) == 0) {
>> -		message_excluded = TRUE;
>> -		break;
>> +	    tag = notmuch_tags_get (tags);
>> +	    /* Is message excluded? */
>> +	    for (notmuch_string_node_t *term = exclude_terms->head;
>> +		 term != NULL;
>> +		 term = term->next)
>> +	    {
>> +		/* We ignore initial 'K'. */
>> +		if (strcmp(tag, (term->string + 1)) == 0) {
>> +		    message_excluded = TRUE;
>> +		    break;
>> +		}
>>  	    }
>>  	}
>>      }
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index 4323201..a20791a 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -411,7 +411,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  	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 (query, NOTMUCH_EXCLUDE_FALSE);
>> +	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
>>  	if (exclude == EXCLUDE_ALL)
>>  	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
>>      }
>> -- 
>> 1.7.9.1
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Peter Wang]

On Fri, 21 Jun 2013 13:59:57 +0300, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Thu, Jun 20 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> > I think we should decide before 0.16 whether to go with this patch and
> > patch 8/8 or for Peter's suggestion at
> > id:1368454815-1854-1-git-send-email-novalazy@gmail.com
> >
> > Now we have an enum for the NOTMUCH_EXCLUDE possibilities (rather than a
> > bool) I think it makes sense to implement NOTMUCH_EXCLUDE_FALSE properly
> > but I am happy with either choice.
> 
> So, the choice here is to choose between
> 
> id:"1368454815-1854-1-git-send-email-novalazy@gmail.com"
> and
> id:"87bo8edif8.fsf@qmul.ac.uk" 
> 
> (might be hard to catch from this thread -- was easier to take from
> http://nmbug.tethera.net/status/ )
> 
> Both apply cleanly to current master ( d0bd88f )
> 
> While Peter's version surely looks simpler (and may work, didn't test atm)
> the comments Mark presents makes sense (especially the "subtlety" thing ;)
> 
> IMHO Mark's version makes future development "safer" and therefore my
> vote (or million of those) goes to id:"87bo8edif8.fsf@qmul.ac.uk"

Sure, I don't care.

Peter

Re: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Mark Walters]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Thu, Jun 20 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> I think we should decide before 0.16 whether to go with this patch and
>> patch 8/8 or for Peter's suggestion at
>> id:1368454815-1854-1-git-send-email-novalazy@gmail.com
>>
>> Now we have an enum for the NOTMUCH_EXCLUDE possibilities (rather than a
>> bool) I think it makes sense to implement NOTMUCH_EXCLUDE_FALSE properly
>> but I am happy with either choice.
>
> So, the choice here is to choose between
>
> id:"1368454815-1854-1-git-send-email-novalazy@gmail.com"
> and
> id:"87bo8edif8.fsf@qmul.ac.uk" 

I think if we go with the latter then it would make sense to also do
id:1368301809-12532-9-git-send-email-markwalters1009@gmail.com
(or something similar)

Best wishes

Mark

>
> (might be hard to catch from this thread -- was easier to take from
> http://nmbug.tethera.net/status/ )
>
> Both apply cleanly to current master ( d0bd88f )
>
> While Peter's version surely looks simpler (and may work, didn't test atm)
> the comments Mark presents makes sense (especially the "subtlety" thing ;)
>
> IMHO Mark's version makes future development "safer" and therefore my
> vote (or million of those) goes to id:"87bo8edif8.fsf@qmul.ac.uk"
>
>> Best wishes
>>
>> Mark
>
> Tomi
>
>
>>
>>
>>
>> On Mon, 13 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>>> Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
>>> cover all four values of search --exclude in the cli.
>>>
>>> Previously the way to avoid any message being marked excluded was to
>>> pass in an empty list of excluded tags: since we now have an explicit
>>> option we might as well honour it.
>>>
>>> The enum is in a slightly strange order as the existing FALSE/TRUE
>>> options correspond to the new
>>> NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options so this means we do
>>> not need to bump the version number.
>>>
>>> Indeed, an example of this is that the cli count and show still use
>>> FALSE/TRUE and still work.
>>> ---
>>>
>>> This is a new version of this single patch. In Peter's version
>>> NOTMUCH_EXCLUDE_FLAG and NOTMUCH_EXCLUDE_FALSE were treated as synonyms:
>>> this makes them behave in the obvious way and documents them.
>>>
>>> I think the only subtlety is the enum order mentioned in the commit
>>> message. Additionally it would be good to update cli count and show and,
>>> at for the latter, it would be good to add an option exclude=all too (so
>>> excluded messages are completely omitted). Those should both be simple
>>> but we need to decide whether to allow all four options
>>> (false/flag/true/all) always or not. Always allowing all 4 is nicely
>>> consistent but sometimes redundant. Additionally we would need some
>>> tests.
>>>
>>> I think this patch should go in with the main series as I think it is
>>> worth reserving the NOTMUCH_EXCLUDE_FALSE #define/value so that we don't
>>> break code in future if we do wish to add it.
>>>
>>> Best wishes
>>>
>>> Mark
>>>
>>>
>>>  
>>>  lib/notmuch.h    |   18 ++++++++++++++++--
>>>  lib/query.cc     |    8 +++++---
>>>  lib/thread.cc    |   28 +++++++++++++++-------------
>>>  notmuch-search.c |    2 +-
>>>  4 files changed, 37 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/lib/notmuch.h b/lib/notmuch.h
>>> index 27b43ff..73c85a4 100644
>>> --- a/lib/notmuch.h
>>> +++ b/lib/notmuch.h
>>> @@ -500,10 +500,15 @@ typedef enum {
>>>  const char *
>>>  notmuch_query_get_query_string (notmuch_query_t *query);
>>>  
>>> -/* Exclude values for notmuch_query_set_omit_excluded */
>>> +/* Exclude values for notmuch_query_set_omit_excluded. The strange
>>> + * order is to maintain backward compatibility: the old FALSE/TRUE
>>> + * options correspond to the new
>>> + * NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options.
>>> + */
>>>  typedef enum {
>>> -    NOTMUCH_EXCLUDE_FALSE,
>>> +    NOTMUCH_EXCLUDE_FLAG,
>>>      NOTMUCH_EXCLUDE_TRUE,
>>> +    NOTMUCH_EXCLUDE_FALSE,
>>>      NOTMUCH_EXCLUDE_ALL
>>>  } notmuch_exclude_t;
>>>  
>>> @@ -517,6 +522,15 @@ typedef enum {
>>>   * match in at least one non-excluded message.  Otherwise, if set to ALL,
>>>   * notmuch_query_search_threads will omit excluded messages from all threads.
>>>   *
>>> + * If set to FALSE or FLAG then both notmuch_query_search_messages and
>>> + * notmuch_query_search_threads will return all matching
>>> + * messages/threads regardless of exclude status. If set to FLAG then
>>> + * the exclude flag will be set for any excluded message that is
>>> + * returned by notmuch_query_search_messages, and the thread counts
>>> + * for threads returned by notmuch_query_search_threads will be the
>>> + * number of non-excluded messages/matches. Otherwise, if set to
>>> + * FALSE, then the exclude status is completely ignored.
>>> + *
>>>   * The performance difference when calling
>>>   * notmuch_query_search_messages should be relatively small (and both
>>>   * should be very fast).  However, in some cases,
>>> diff --git a/lib/query.cc b/lib/query.cc
>>> index 1cc768f..69668a4 100644
>>> --- a/lib/query.cc
>>> +++ b/lib/query.cc
>>> @@ -218,13 +218,15 @@ notmuch_query_search_messages (notmuch_query_t *query)
>>>  	}
>>>  	messages->base.excluded_doc_ids = NULL;
>>>  
>>> -	if (query->exclude_terms) {
>>> +	if ((query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) && (query->exclude_terms)) {
>>>  	    exclude_query = _notmuch_exclude_tags (query, final_query);
>>>  
>>> -	    if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)
>>> +	    if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
>>> +		query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
>>> +	    {
>>>  		final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
>>>  					     final_query, exclude_query);
>>> -	    else {
>>> +	    } else { /* NOTMUCH_EXCLUDE_FLAG */
>>>  		exclude_query = Xapian::Query (Xapian::Query::OP_AND,
>>>  					   exclude_query, final_query);
>>>  
>>> diff --git a/lib/thread.cc b/lib/thread.cc
>>> index bc07877..4dcf705 100644
>>> --- a/lib/thread.cc
>>> +++ b/lib/thread.cc
>>> @@ -238,20 +238,22 @@ _thread_add_message (notmuch_thread_t *thread,
>>>      char *clean_author;
>>>      notmuch_bool_t message_excluded = FALSE;
>>>  
>>> -    for (tags = notmuch_message_get_tags (message);
>>> -	 notmuch_tags_valid (tags);
>>> -	 notmuch_tags_move_to_next (tags))
>>> -    {
>>> -	tag = notmuch_tags_get (tags);
>>> -	/* Is message excluded? */
>>> -	for (notmuch_string_node_t *term = exclude_terms->head;
>>> -	     term != NULL;
>>> -	     term = term->next)
>>> +    if (omit_exclude != NOTMUCH_EXCLUDE_FALSE) {
>>> +	for (tags = notmuch_message_get_tags (message);
>>> +	     notmuch_tags_valid (tags);
>>> +	     notmuch_tags_move_to_next (tags))
>>>  	{
>>> -	    /* We ignore initial 'K'. */
>>> -	    if (strcmp(tag, (term->string + 1)) == 0) {
>>> -		message_excluded = TRUE;
>>> -		break;
>>> +	    tag = notmuch_tags_get (tags);
>>> +	    /* Is message excluded? */
>>> +	    for (notmuch_string_node_t *term = exclude_terms->head;
>>> +		 term != NULL;
>>> +		 term = term->next)
>>> +	    {
>>> +		/* We ignore initial 'K'. */
>>> +		if (strcmp(tag, (term->string + 1)) == 0) {
>>> +		    message_excluded = TRUE;
>>> +		    break;
>>> +		}
>>>  	    }
>>>  	}
>>>      }
>>> diff --git a/notmuch-search.c b/notmuch-search.c
>>> index 4323201..a20791a 100644
>>> --- a/notmuch-search.c
>>> +++ b/notmuch-search.c
>>> @@ -411,7 +411,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>>  	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 (query, NOTMUCH_EXCLUDE_FALSE);
>>> +	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_FLAG);
>>>  	if (exclude == EXCLUDE_ALL)
>>>  	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
>>>      }
>>> -- 
>>> 1.7.9.1
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
> cover all four values of search --exclude in the cli.

pushed,

d

[PATCH v2 8/8] cli: use notmuch_exclude_t in option parser

[Mark Walters]

From: Peter Wang <novalazy@gmail.com>

Use notmuch_exclude_t constants directly instead of a redundant
enumeration while parsing search --exclude keyword arguments.
---
 notmuch-search.c |   28 +++++++++-------------------
 1 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index a20791a..a96f07d 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -283,13 +283,6 @@ do_search_tags (notmuch_database_t *notmuch,
     return 0;
 }
 
-enum {
-    EXCLUDE_TRUE,
-    EXCLUDE_FALSE,
-    EXCLUDE_FLAG,
-    EXCLUDE_ALL
-};
-
 int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
@@ -302,7 +295,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     output_t output = OUTPUT_SUMMARY;
     int offset = 0;
     int limit = -1; /* unlimited */
-    int exclude = EXCLUDE_TRUE;
+    notmuch_exclude_t exclude = NOTMUCH_EXCLUDE_TRUE;
     unsigned int i;
 
     enum {
@@ -332,10 +325,10 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "tags", OUTPUT_TAGS },
 				  { 0, 0 } } },
         { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
-          (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
-                                  { "false", EXCLUDE_FALSE },
-                                  { "flag", EXCLUDE_FLAG },
-                                  { "all", EXCLUDE_ALL },
+          (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
+                                  { "false", NOTMUCH_EXCLUDE_FALSE },
+                                  { "flag", NOTMUCH_EXCLUDE_FLAG },
+                                  { "all", NOTMUCH_EXCLUDE_ALL },
                                   { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
 	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
@@ -394,15 +387,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_query_set_sort (query, sort);
 
-    if (exclude == EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {
+    if (exclude == NOTMUCH_EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {
 	/* If we are not doing summary output there is nowhere to
 	 * print the excluded flag so fall back on including the
 	 * excluded messages. */
 	fprintf (stderr, "Warning: this output format cannot flag excluded messages.\n");
-	exclude = EXCLUDE_FALSE;
+	exclude = NOTMUCH_EXCLUDE_FALSE;
     }
 
-    if (exclude != EXCLUDE_FALSE) {
+    if (exclude != NOTMUCH_EXCLUDE_FALSE) {
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
 
@@ -410,10 +403,7 @@ notmuch_search_command (notmuch_config_t *config, 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 (query, NOTMUCH_EXCLUDE_FLAG);
-	if (exclude == EXCLUDE_ALL)
-	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
+	notmuch_query_set_omit_excluded (query, exclude);
     }
 
     switch (output) {
-- 
1.7.9.1

Re: [PATCH v2 8/8] cli: use notmuch_exclude_t in option parser

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> From: Peter Wang <novalazy@gmail.com>
>
> Use notmuch_exclude_t constants directly instead of a redundant
> enumeration while parsing search --exclude keyword arguments.
> ---

pushed,

d

[PATCH v2] cli: clarify correspondence of --exclude to omit_excluded in search

[Peter Wang]

Make it obvious how the --exclude command-line option affects the
omit_excluded field in notmuch_query_t objects, with an explicit and
exhaustive switch.  Do not expect the reader to know the default value
of omit_excluded.
---
This can be inserted after patch 2.

 notmuch-search.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 4323201..893df10 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -290,6 +290,24 @@ enum {
     EXCLUDE_ALL
 };
 
+static int
+exclude_option_to_omit_excluded (int exclude)
+{
+    switch (exclude) {
+    case EXCLUDE_TRUE:
+	return NOTMUCH_EXCLUDE_TRUE;
+    case EXCLUDE_FALSE:
+	return NOTMUCH_EXCLUDE_FALSE;
+    case EXCLUDE_FLAG:
+	return NOTMUCH_EXCLUDE_FALSE;
+    case EXCLUDE_ALL:
+	return NOTMUCH_EXCLUDE_ALL;
+    default:
+	INTERNAL_ERROR ("unhandled exclude option %d", exclude);
+	/*UNREACHED*/
+    }
+}
+
 int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
@@ -410,11 +428,8 @@ notmuch_search_command (notmuch_config_t *config, 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 (query, NOTMUCH_EXCLUDE_FALSE);
-	if (exclude == EXCLUDE_ALL)
-	    notmuch_query_set_omit_excluded (query, NOTMUCH_EXCLUDE_ALL);
     }
+    notmuch_query_set_omit_excluded (query, exclude_option_to_omit_excluded (exclude));
 
     switch (output) {
     default:
-- 
1.7.12.1

Re: [PATCH v2 0/8] search --exclude=all

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> This is a rebased version of
> id:1340198947-29370-1-git-send-email-novalazy@gmail.com I have not
> tested it significantly in this form but the rebasing looks ok and all
> the tests pass.

Pushed all but 7/8. I leave it to Peter and Mark to discuss which of
their patches to use.

d