From: Mark Walters <markwalters1009@gmail.com>
To: notmuch@notmuchmail.org
Subject: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
In-Reply-To: <1368301809-12532-8-git-send-email-markwalters1009@gmail.com>
References: <1368301809-12532-1-git-send-email-markwalters1009@gmail.com>
	<1368301809-12532-8-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.14+255~gff3cc55 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Mon, 13 May 2013 16:10:51 +0100
Message-ID: <87bo8edif8.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


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