unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/8] search --exclude=all
@ 2012-06-20 13:28 Peter Wang
  2012-06-20 13:29 ` [PATCH 1/8] lib: add --exclude=all option Peter Wang
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:28 UTC (permalink / raw)
  To: notmuch

Hi,

This is Mark's change to add search --exclude=all.
I mainly just updated a comment and added some documentation.

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 |   28 ++++++++++++++++++++++------
 notmuch-search.c          |   24 +++++++++---------------
 test/excludes             |   22 ++++++++++++++++++++++
 7 files changed, 109 insertions(+), 44 deletions(-)

-- 
1.7.4.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/8] lib: add --exclude=all option
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2013-05-11 18:07   ` David Bremner
  2012-06-20 13:29 ` [PATCH 2/8] cli: add --exclude=all option to notmuch-search.c Peter Wang
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

From: Mark Walters <markwalters1009@gmail.com>

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 bfb4111..aa799b4 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -232,6 +232,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 3633bed..1280afd 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 e9c1a2d..f752452 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 e976d64..4cb0896 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -215,7 +215,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;
@@ -223,6 +224,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));
@@ -263,17 +286,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
@@ -404,6 +422,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)
 {
     notmuch_thread_t *thread;
@@ -479,7 +498,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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/8] cli: add --exclude=all option to notmuch-search.c
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
  2012-06-20 13:29 ` [PATCH 1/8] lib: add --exclude=all option Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2012-06-20 13:29 ` [PATCH 3/8] test: add tests for search --exclude=all Peter Wang
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

From: Mark Walters <markwalters1009@gmail.com>

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 3be296d..89b5bf9 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -420,6 +420,7 @@ enum {
     EXCLUDE_TRUE,
     EXCLUDE_FALSE,
     EXCLUDE_FLAG,
+    EXCLUDE_ALL
 };
 
 int
@@ -461,6 +462,7 @@ notmuch_search_command (void *ctx, 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  },
@@ -516,7 +518,7 @@ notmuch_search_command (void *ctx, 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;
 
@@ -525,7 +527,9 @@ notmuch_search_command (void *ctx, 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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/8] test: add tests for search --exclude=all
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
  2012-06-20 13:29 ` [PATCH 1/8] lib: add --exclude=all option Peter Wang
  2012-06-20 13:29 ` [PATCH 2/8] cli: add --exclude=all option to notmuch-search.c Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2012-06-20 13:29 ` [PATCH 4/8] man: clarify search --exclude documentation Peter Wang
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/8] man: clarify search --exclude documentation
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
                   ` (2 preceding siblings ...)
  2012-06-20 13:29 ` [PATCH 3/8] test: add tests for search --exclude=all Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2012-06-20 20:08   ` Mark Walters
  2012-06-20 13:29 ` [PATCH 5/8] man: clarify search --exclude=flag Peter Wang
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

Highlight "excluded messages" as a piece of jargon 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 |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 5c72c4b..003b742 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -116,8 +116,18 @@ 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
+Messages matching search.tag_exclude are called "excluded messages".
+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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/8] man: clarify search --exclude=flag
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
                   ` (3 preceding siblings ...)
  2012-06-20 13:29 ` [PATCH 4/8] man: clarify search --exclude documentation Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2012-06-20 13:29 ` [PATCH 6/8] man: document search --exclude=all Peter Wang
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

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 003b742..e23ca30 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -130,9 +130,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 SEE ALSO
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/8] man: document search --exclude=all
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
                   ` (4 preceding siblings ...)
  2012-06-20 13:29 ` [PATCH 5/8] man: clarify search --exclude=flag Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2012-06-20 13:29 ` [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t Peter Wang
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

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 e23ca30..1d8d57a 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -114,7 +114,7 @@ Limit the number of displayed results to N.
 
 .RS 4
 .TP 4
-.BR \-\-exclude=(true|false|flag)
+.BR \-\-exclude=(true|false|all|flag)
 
 Messages matching search.tag_exclude are called "excluded messages".
 This option specifies whether to omit excluded messages in the search
@@ -124,6 +124,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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
                   ` (5 preceding siblings ...)
  2012-06-20 13:29 ` [PATCH 6/8] man: document search --exclude=all Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2012-10-19  5:15   ` Ethan Glasser-Camp
  2012-06-20 13:29 ` [PATCH 8/8] cli: use notmuch_exclude_t in option parser Peter Wang
  2012-07-22  1:18 ` [PATCH v2 4/8] man: clarify search --exclude documentation Peter Wang
  8 siblings, 1 reply; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

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 1280afd..f4381ae 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 f752452..1e0f292 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 89b5bf9..027923d 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -527,7 +527,7 @@ notmuch_search_command (void *ctx, 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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 8/8] cli: use notmuch_exclude_t in option parser
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
                   ` (6 preceding siblings ...)
  2012-06-20 13:29 ` [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t Peter Wang
@ 2012-06-20 13:29 ` Peter Wang
  2012-07-22  1:18 ` [PATCH v2 4/8] man: clarify search --exclude documentation Peter Wang
  8 siblings, 0 replies; 20+ messages in thread
From: Peter Wang @ 2012-06-20 13:29 UTC (permalink / raw)
  To: notmuch

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 027923d..23cf342 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -416,13 +416,6 @@ do_search_tags (notmuch_database_t *notmuch,
     return 0;
 }
 
-enum {
-    EXCLUDE_TRUE,
-    EXCLUDE_FALSE,
-    EXCLUDE_FLAG,
-    EXCLUDE_ALL
-};
-
 int
 notmuch_search_command (void *ctx, int argc, char *argv[])
 {
@@ -436,7 +429,7 @@ notmuch_search_command (void *ctx, 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 { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
@@ -459,10 +452,10 @@ notmuch_search_command (void *ctx, 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  },
@@ -510,15 +503,15 @@ notmuch_search_command (void *ctx, 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;
 
@@ -526,10 +519,7 @@ 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 (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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/8] man: clarify search --exclude documentation
  2012-06-20 13:29 ` [PATCH 4/8] man: clarify search --exclude documentation Peter Wang
@ 2012-06-20 20:08   ` Mark Walters
  2012-06-23  2:02     ` Peter Wang
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Walters @ 2012-06-20 20:08 UTC (permalink / raw)
  To: Peter Wang, notmuch


I have reviewed all the new parts of this series (judged as being
patches 3-8) and the changes made to my two patches and they are all
fine (with one small comment below). Patch 1/8 does need a proper review
though as it ended up more intrusive than I would have liked.

> +Messages matching search.tag_exclude are called "excluded messages".

My one comment is that this is not quite true if the corresponding tag
is in the query. Since you are defining the term it would be nice to
mention that, but I can't see a clean wording.

This should not hold up the series as it your wording is clearly an
improvement on the current wording.

Best wishes

Mark

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/8] man: clarify search --exclude documentation
  2012-06-20 20:08   ` Mark Walters
@ 2012-06-23  2:02     ` Peter Wang
  2012-06-23  7:14       ` Mark Walters
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Wang @ 2012-06-23  2:02 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

On Wed, 20 Jun 2012 21:08:05 +0100, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> I have reviewed all the new parts of this series (judged as being
> patches 3-8) and the changes made to my two patches and they are all
> fine (with one small comment below). Patch 1/8 does need a proper review
> though as it ended up more intrusive than I would have liked.
> 
> > +Messages matching search.tag_exclude are called "excluded messages".
> 
> My one comment is that this is not quite true if the corresponding tag
> is in the query. Since you are defining the term it would be nice to
> mention that, but I can't see a clean wording.

How about:

    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.

Or less densely:

    Let "excluded tags" be the set of tags listed in search.tag_exclude,
    minus any tags which appear explicitly in the search terms.
    A message is an "excluded message" if it matches one or more
    excluded tags.

Peter

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/8] man: clarify search --exclude documentation
  2012-06-23  2:02     ` Peter Wang
@ 2012-06-23  7:14       ` Mark Walters
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Walters @ 2012-06-23  7:14 UTC (permalink / raw)
  To: Peter Wang; +Cc: notmuch

Peter Wang <novalazy@gmail.com> writes:

> On Wed, 20 Jun 2012 21:08:05 +0100, Mark Walters <markwalters1009@gmail.com> wrote:
>> 
>> I have reviewed all the new parts of this series (judged as being
>> patches 3-8) and the changes made to my two patches and they are all
>> fine (with one small comment below). Patch 1/8 does need a proper review
>> though as it ended up more intrusive than I would have liked.
>> 
>> > +Messages matching search.tag_exclude are called "excluded messages".
>> 
>> My one comment is that this is not quite true if the corresponding tag
>> is in the query. Since you are defining the term it would be nice to
>> mention that, but I can't see a clean wording.
>
> How about:
>
>     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.

I think this wording is excellent (and prefer it to the less dense wording).

Best wishes

Mark

> Or less densely:
>
>     Let "excluded tags" be the set of tags listed in search.tag_exclude,
>     minus any tags which appear explicitly in the search terms.
>     A message is an "excluded message" if it matches one or more
>     excluded tags.
>
> Peter

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 4/8] man: clarify search --exclude documentation
  2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
                   ` (7 preceding siblings ...)
  2012-06-20 13:29 ` [PATCH 8/8] cli: use notmuch_exclude_t in option parser Peter Wang
@ 2012-07-22  1:18 ` Peter Wang
  8 siblings, 0 replies; 20+ messages in thread
From: Peter Wang @ 2012-07-22  1:18 UTC (permalink / raw)
  To: notmuch

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 5c72c4b..52782c7 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -116,8 +116,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.4.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  2012-06-20 13:29 ` [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t Peter Wang
@ 2012-10-19  5:15   ` Ethan Glasser-Camp
  2012-10-19 21:23     ` Ethan Glasser-Camp
  2012-10-21  2:04     ` Peter Wang
  0 siblings, 2 replies; 20+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-19  5:15 UTC (permalink / raw)
  To: Peter Wang, notmuch

Peter Wang <novalazy@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.

This series looks good to me. It's a nice clean up and a nice new
feature. Patches all apply.

However, I'm getting test failures like:

 FAIL   Search, exclude "deleted" messages from message search --exclude=false
        --- excludes.3.expected 2012-10-19 04:45:06.900518377 +0000
        +++ excludes.3.output   2012-10-19 04:45:06.900518377 +0000
        @@ -1,2 +1,2 @@
        -id:msg-001@notmuch-test-suite
         id:msg-002@notmuch-test-suite
        +id:msg-001@notmuch-test-suite

 FAIL   Search, don't exclude "deleted" messages when --exclude=flag specified
        --- excludes.7.expected 2012-10-19 04:45:07.004518378 +0000
        +++ excludes.7.output   2012-10-19 04:45:07.004518378 +0000
        @@ -1,2 +1,2 @@
        -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)
        +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)

 FAIL   Search, don't exclude "deleted" messages from search if not configured
        --- excludes.8.expected 2012-10-19 04:45:07.028518377 +0000
        +++ excludes.8.output   2012-10-19 04:45:07.028518377 +0000
        @@ -1,2 +1,2 @@
        -thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
         thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)
        +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)

In other words, threads and messages are coming up out of order. I'm not
sure of the right way to fix this. If you would like me to try sticking
"| sort" here and there in the tests I will do so. I'm not sure if the
test suite is guaranteed to scan messages in a certain order.

Mark Walters wrote in
id:"1340198947-29370-5-git-send-email-novalazy@gmail.com" that he
thought patch 1/8 seemed more intrusive than he liked. Maybe I just have
a higher standard for "intrusive" than he does ;) but I thought it was
fine.

It looks like you have better wording for patch 4/8 so I'd like to see
you resend it.

> -         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 {

"House style" is to not put braces around one-line then-clauses. This is
the only place where you did that.

I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

Ethan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  2012-10-19  5:15   ` Ethan Glasser-Camp
@ 2012-10-19 21:23     ` Ethan Glasser-Camp
  2012-10-20 10:04       ` Tomi Ollila
  2012-10-21  2:04     ` Peter Wang
  1 sibling, 1 reply; 20+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-19 21:23 UTC (permalink / raw)
  To: Ethan Glasser-Camp, Peter Wang, notmuch

Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:

> It looks like you have better wording for patch 4/8 so I'd like to see
> you resend it.
>
> I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

It turns out that patch 4 already has a v2 in the thread, but I didn't
see it due to some kind of selective blindness. It would be nice if
nmbug had grouped it as part of the same patch series. 

I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain.

Ethan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  2012-10-19 21:23     ` Ethan Glasser-Camp
@ 2012-10-20 10:04       ` Tomi Ollila
  0 siblings, 0 replies; 20+ messages in thread
From: Tomi Ollila @ 2012-10-20 10:04 UTC (permalink / raw)
  To: Ethan Glasser-Camp, notmuch

On Sat, Oct 20 2012, Ethan Glasser-Camp wrote:

> Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:
>
>> It looks like you have better wording for patch 4/8 so I'd like to see
>> you resend it.
>>
>> I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!
>
> It turns out that patch 4 already has a v2 in the thread, but I didn't
> see it due to some kind of selective blindness. It would be nice if
> nmbug had grouped it as part of the same patch series. 

I noticed the same today when I tagged your NEWS patch.

By looking contrib/nmbug/nmbug-status the reason seems obvious:
it iterates over messages and resets thread when thread id changes
(and message ordering is not based on threads).

There is 2 ways to "fix" this: 

1) Iterate over threads and then messges over these threads. I looked
the bindings and this didn't seem so easy as one could imagine..

2) Iterate over messages like now, but cache the content to an objects
in hash where thread-id is the key.
 
If no-one points me an easy way to do (1), I'll attempt (2) (in a distant
future ;)

>
> I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain.
>
> Ethan

Tomi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  2012-10-19  5:15   ` Ethan Glasser-Camp
  2012-10-19 21:23     ` Ethan Glasser-Camp
@ 2012-10-21  2:04     ` Peter Wang
  2012-10-21 12:10       ` Tomi Ollila
  2012-10-21 13:03       ` Ethan Glasser-Camp
  1 sibling, 2 replies; 20+ messages in thread
From: Peter Wang @ 2012-10-21  2:04 UTC (permalink / raw)
  To: notmuch

On Fri, 19 Oct 2012 01:15:31 -0400, Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> wrote:
> Peter Wang <novalazy@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.
> 
> This series looks good to me. It's a nice clean up and a nice new
> feature. Patches all apply.

Thanks for the review.

> However, I'm getting test failures like:
> 
>  FAIL   Search, exclude "deleted" messages from message search --exclude=false
>         --- excludes.3.expected 2012-10-19 04:45:06.900518377 +0000
>         +++ excludes.3.output   2012-10-19 04:45:06.900518377 +0000
>         @@ -1,2 +1,2 @@
>         -id:msg-001@notmuch-test-suite
>          id:msg-002@notmuch-test-suite
>         +id:msg-001@notmuch-test-suite
> 
>  FAIL   Search, don't exclude "deleted" messages when --exclude=flag specified
>         --- excludes.7.expected 2012-10-19 04:45:07.004518378 +0000
>         +++ excludes.7.output   2012-10-19 04:45:07.004518378 +0000
>         @@ -1,2 +1,2 @@
>         -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)
>         +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
> 
>  FAIL   Search, don't exclude "deleted" messages from search if not configured
>         --- excludes.8.expected 2012-10-19 04:45:07.028518377 +0000
>         +++ excludes.8.output   2012-10-19 04:45:07.028518377 +0000
>         @@ -1,2 +1,2 @@
>         -thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
>          thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)
>         +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
> 
> In other words, threads and messages are coming up out of order. I'm not
> sure of the right way to fix this. If you would like me to try sticking
> "| sort" here and there in the tests I will do so. I'm not sure if the
> test suite is guaranteed to scan messages in a certain order.

Does it help if you add a "sleep 1" before the second generate_message
call, i.e. on line 35?

> > -         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 {
> 
> "House style" is to not put braces around one-line then-clauses. This is
> the only place where you did that.

I have to disagree.  The condition is wrapped over two lines.  The then
part is wrapped over two lines.  The else part already has braces.
All suggest braces around the then part.

Peter

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  2012-10-21  2:04     ` Peter Wang
@ 2012-10-21 12:10       ` Tomi Ollila
  2012-10-21 13:03       ` Ethan Glasser-Camp
  1 sibling, 0 replies; 20+ messages in thread
From: Tomi Ollila @ 2012-10-21 12:10 UTC (permalink / raw)
  To: Peter Wang, notmuch

On Sun, Oct 21 2012, Peter Wang wrote:

> On Fri, 19 Oct 2012 01:15:31 -0400, Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> wrote:
>> Peter Wang <novalazy@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.
>> 
>> This series looks good to me. It's a nice clean up and a nice new
>> feature. Patches all apply.
>
> Thanks for the review.
>
>> However, I'm getting test failures like:
>> 
>>  FAIL   Search, exclude "deleted" messages from message search --exclude=false
>>         --- excludes.3.expected 2012-10-19 04:45:06.900518377 +0000
>>         +++ excludes.3.output   2012-10-19 04:45:06.900518377 +0000
>>         @@ -1,2 +1,2 @@
>>         -id:msg-001@notmuch-test-suite
>>          id:msg-002@notmuch-test-suite
>>         +id:msg-001@notmuch-test-suite
>> 
>>  FAIL   Search, don't exclude "deleted" messages when --exclude=flag specified
>>         --- excludes.7.expected 2012-10-19 04:45:07.004518378 +0000
>>         +++ excludes.7.output   2012-10-19 04:45:07.004518378 +0000
>>         @@ -1,2 +1,2 @@
>>         -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)
>>         +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
>> 
>>  FAIL   Search, don't exclude "deleted" messages from search if not configured
>>         --- excludes.8.expected 2012-10-19 04:45:07.028518377 +0000
>>         +++ excludes.8.output   2012-10-19 04:45:07.028518377 +0000
>>         @@ -1,2 +1,2 @@
>>         -thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
>>          thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)
>>         +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
>> 
>> In other words, threads and messages are coming up out of order. I'm not
>> sure of the right way to fix this. If you would like me to try sticking
>> "| sort" here and there in the tests I will do so. I'm not sure if the
>> test suite is guaranteed to scan messages in a certain order.
>
> Does it help if you add a "sleep 1" before the second generate_message
> call, i.e. on line 35?
>
>> > -         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 {
>> 
>> "House style" is to not put braces around one-line then-clauses. This is
>> the only place where you did that.
>
> I have to disagree.  The condition is wrapped over two lines.  The then
> part is wrapped over two lines.  The else part already has braces.
> All suggest braces around the then part.

Well, I personally would count none of these as convincing suggestions ;),
but IMHO the braces are OK here (I don't start judging which I'd like more).

> Peter

Tomi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
  2012-10-21  2:04     ` Peter Wang
  2012-10-21 12:10       ` Tomi Ollila
@ 2012-10-21 13:03       ` Ethan Glasser-Camp
  1 sibling, 0 replies; 20+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-21 13:03 UTC (permalink / raw)
  To: Peter Wang, notmuch

Peter Wang <novalazy@gmail.com> writes:

> Does it help if you add a "sleep 1" before the second generate_message
> call, i.e. on line 35?

It turns out that this test failure is sporadic (perhaps due to the fact
that I'm running on tmpfs) and exists even before this series. Doing
"sleep 1" makes it go away, but that "fix" is unrelated to this series
and ought to be fixed elsewhere.

> I have to disagree.  The condition is wrapped over two lines.  The then
> part is wrapped over two lines.  The else part already has braces.
> All suggest braces around the then part.

If Tomi is OK with it, then I guess it's fine. And it's true that there
are a couple of places where braces are used with long conditions and
then-parts.

This series has my +1.

Ethan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/8] lib: add --exclude=all option
  2012-06-20 13:29 ` [PATCH 1/8] lib: add --exclude=all option Peter Wang
@ 2013-05-11 18:07   ` David Bremner
  0 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2013-05-11 18:07 UTC (permalink / raw)
  To: Peter Wang, notmuch

Peter Wang <novalazy@gmail.com> writes:

> From: Mark Walters <markwalters1009@gmail.com>
>
> Adds a exclude all option to the lib which means that excluded
> messages are completely ignored (as if they had actually been
> deleted).

Hi Peter, Mark;
 
This patch doesn't apply anymore, which makes it hard to test the
series.  At least by eye the series looks OK to me. Unless
somebody objects, I'd be inclined to push a rebased version of this
series.

d

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-05-11 18:07 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-20 13:28 [PATCH 0/8] search --exclude=all Peter Wang
2012-06-20 13:29 ` [PATCH 1/8] lib: add --exclude=all option Peter Wang
2013-05-11 18:07   ` David Bremner
2012-06-20 13:29 ` [PATCH 2/8] cli: add --exclude=all option to notmuch-search.c Peter Wang
2012-06-20 13:29 ` [PATCH 3/8] test: add tests for search --exclude=all Peter Wang
2012-06-20 13:29 ` [PATCH 4/8] man: clarify search --exclude documentation Peter Wang
2012-06-20 20:08   ` Mark Walters
2012-06-23  2:02     ` Peter Wang
2012-06-23  7:14       ` Mark Walters
2012-06-20 13:29 ` [PATCH 5/8] man: clarify search --exclude=flag Peter Wang
2012-06-20 13:29 ` [PATCH 6/8] man: document search --exclude=all Peter Wang
2012-06-20 13:29 ` [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t Peter Wang
2012-10-19  5:15   ` Ethan Glasser-Camp
2012-10-19 21:23     ` Ethan Glasser-Camp
2012-10-20 10:04       ` Tomi Ollila
2012-10-21  2:04     ` Peter Wang
2012-10-21 12:10       ` Tomi Ollila
2012-10-21 13:03       ` Ethan Glasser-Camp
2012-06-20 13:29 ` [PATCH 8/8] cli: use notmuch_exclude_t in option parser Peter Wang
2012-07-22  1:18 ` [PATCH v2 4/8] man: clarify search --exclude documentation Peter Wang

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).