unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* V3 of non-destructive excludes patches
@ 2016-11-22  2:27 David Bremner
  2016-11-22  2:27 ` [PATCH 1/4] lib: eagerly parse queries David Bremner
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: David Bremner @ 2016-11-22  2:27 UTC (permalink / raw)
  To: notmuch

Obsoletes id:20161002021328.30487-1-david@tethera.net

There is only trivial whitespace changes in the final diff, but I did
move several small changes / fixes farther back in the series.

As before, it's a bit of an open question whether breaking the ABI is
worth it to return a proper status value (see patch 4/4).

d

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

* [PATCH 1/4] lib: eagerly parse queries
  2016-11-22  2:27 V3 of non-destructive excludes patches David Bremner
@ 2016-11-22  2:27 ` David Bremner
  2016-11-22  2:27 ` [PATCH 2/4] lib/query: make query parsing lazy again, keep centralized David Bremner
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2016-11-22  2:27 UTC (permalink / raw)
  To: notmuch

Rather than waiting for a call to count/search, parse the query string when the
notmuch_query_t is created. This is a small reduction in duplicated
code, and a potential efficiency improvement if many count/search
operations are called on the same query (although the latter sounds a
bit unusual). The main goal is to prepare the way for
non-destructive (or at least less destructive) exclude tag handling.

It does introduce a not-very-nice error path where running out of memory
is not easily distinguishable from a query syntax error.
---
 lib/query.cc | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/lib/query.cc b/lib/query.cc
index 53efd4e..f2a95da 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -29,6 +29,7 @@ struct _notmuch_query {
     notmuch_sort_t sort;
     notmuch_string_list_t *exclude_terms;
     notmuch_exclude_t omit_excluded;
+    Xapian::Query xapian_query;
 };
 
 typedef struct _notmuch_mset_messages {
@@ -71,6 +72,13 @@ _debug_query (void)
     return (env && strcmp (env, "") != 0);
 }
 
+/* Explicit destructor call for placement new */
+static int
+_notmuch_query_destructor (notmuch_query_t *query) {
+    query->xapian_query.~Query();
+    return 0;
+}
+
 notmuch_query_t *
 notmuch_query_create (notmuch_database_t *notmuch,
 		      const char *query_string)
@@ -84,6 +92,10 @@ notmuch_query_create (notmuch_database_t *notmuch,
     if (unlikely (query == NULL))
 	return NULL;
 
+    new (&query->xapian_query) Xapian::Query ();
+
+    talloc_set_destructor (query, _notmuch_query_destructor);
+
     query->notmuch = notmuch;
 
     query->query_string = talloc_strdup (query, query_string);
@@ -94,6 +106,21 @@ notmuch_query_create (notmuch_database_t *notmuch,
 
     query->omit_excluded = NOTMUCH_EXCLUDE_TRUE;
 
+    try {
+	query->xapian_query =
+	    notmuch->query_parser->parse_query (query_string, NOTMUCH_QUERY_PARSER_FLAGS);
+    } catch (const Xapian::Error &error) {
+	_notmuch_database_log (notmuch,
+			       "A Xapian exception occured parsing query: %s\n",
+			       error.get_msg().c_str());
+	_notmuch_database_log_append (notmuch,
+			       "Query string was: %s\n",
+			       query->query_string);
+
+	talloc_free (query);
+	query = NULL;
+    }
+
     return query;
 }
 
@@ -217,7 +244,7 @@ _notmuch_query_search_documents (notmuch_query_t *query,
 	Xapian::Query mail_query (talloc_asprintf (query, "%s%s",
 						   _find_prefix ("type"),
 						   type));
-	Xapian::Query string_query, final_query, exclude_query;
+	Xapian::Query final_query, exclude_query;
 	Xapian::MSet mset;
 	Xapian::MSetIterator iterator;
 
@@ -226,10 +253,8 @@ _notmuch_query_search_documents (notmuch_query_t *query,
 	{
 	    final_query = mail_query;
 	} else {
-	    string_query = notmuch->query_parser->
-		parse_query (query_string, NOTMUCH_QUERY_PARSER_FLAGS);
 	    final_query = Xapian::Query (Xapian::Query::OP_AND,
-					 mail_query, string_query);
+					 mail_query, query->xapian_query);
 	}
 	messages->base.excluded_doc_ids = NULL;
 
@@ -572,7 +597,7 @@ _notmuch_query_count_documents (notmuch_query_t *query, const char *type, unsign
 	Xapian::Query mail_query (talloc_asprintf (query, "%s%s",
 						   _find_prefix ("type"),
 						   type));
-	Xapian::Query string_query, final_query, exclude_query;
+	Xapian::Query final_query, exclude_query;
 	Xapian::MSet mset;
 
 	if (strcmp (query_string, "") == 0 ||
@@ -580,10 +605,8 @@ _notmuch_query_count_documents (notmuch_query_t *query, const char *type, unsign
 	{
 	    final_query = mail_query;
 	} else {
-	    string_query = notmuch->query_parser->
-		parse_query (query_string, NOTMUCH_QUERY_PARSER_FLAGS);
 	    final_query = Xapian::Query (Xapian::Query::OP_AND,
-					 mail_query, string_query);
+					 mail_query, query->xapian_query);
 	}
 
 	exclude_query = _notmuch_exclude_tags (query, final_query);
-- 
2.10.2

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

* [PATCH 2/4] lib/query: make query parsing lazy again, keep centralized.
  2016-11-22  2:27 V3 of non-destructive excludes patches David Bremner
  2016-11-22  2:27 ` [PATCH 1/4] lib: eagerly parse queries David Bremner
@ 2016-11-22  2:27 ` David Bremner
  2016-11-22  2:27 ` [PATCH 3/4] lib: query make exclude handling non-destructive David Bremner
  2016-11-22  2:27 ` [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value David Bremner
  3 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2016-11-22  2:27 UTC (permalink / raw)
  To: notmuch

This is mainly to fix the nasty error path introduced in the last
commit, by moving the parsing into functions where the API is already
set up to return error status.  It preserves the feature of having a
pre-parsed query available for further processing.
---
 lib/query.cc | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/lib/query.cc b/lib/query.cc
index f2a95da..c356fe5 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -29,6 +29,7 @@ struct _notmuch_query {
     notmuch_sort_t sort;
     notmuch_string_list_t *exclude_terms;
     notmuch_exclude_t omit_excluded;
+    notmuch_bool_t parsed;
     Xapian::Query xapian_query;
 };
 
@@ -93,6 +94,7 @@ notmuch_query_create (notmuch_database_t *notmuch,
 	return NULL;
 
     new (&query->xapian_query) Xapian::Query ();
+    query->parsed = FALSE;
 
     talloc_set_destructor (query, _notmuch_query_destructor);
 
@@ -106,22 +108,33 @@ notmuch_query_create (notmuch_database_t *notmuch,
 
     query->omit_excluded = NOTMUCH_EXCLUDE_TRUE;
 
+    return query;
+}
+
+static notmuch_status_t
+_notmuch_query_ensure_parsed (notmuch_query_t *query)
+{
+    if (query->parsed)
+	return NOTMUCH_STATUS_SUCCESS;
+
     try {
 	query->xapian_query =
-	    notmuch->query_parser->parse_query (query_string, NOTMUCH_QUERY_PARSER_FLAGS);
+	    query->notmuch->query_parser->
+		parse_query (query->query_string, NOTMUCH_QUERY_PARSER_FLAGS);
+
+	query->parsed = TRUE;
+
     } catch (const Xapian::Error &error) {
-	_notmuch_database_log (notmuch,
+	_notmuch_database_log (query->notmuch,
 			       "A Xapian exception occured parsing query: %s\n",
 			       error.get_msg().c_str());
-	_notmuch_database_log_append (notmuch,
+	_notmuch_database_log_append (query->notmuch,
 			       "Query string was: %s\n",
 			       query->query_string);
 
-	talloc_free (query);
-	query = NULL;
+	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
-
-    return query;
+    return NOTMUCH_STATUS_SUCCESS;
 }
 
 const char *
@@ -225,6 +238,11 @@ _notmuch_query_search_documents (notmuch_query_t *query,
     notmuch_database_t *notmuch = query->notmuch;
     const char *query_string = query->query_string;
     notmuch_mset_messages_t *messages;
+    notmuch_status_t status;
+
+    status = _notmuch_query_ensure_parsed (query);
+    if (status)
+	return status;
 
     messages = talloc (query, notmuch_mset_messages_t);
     if (unlikely (messages == NULL))
@@ -591,6 +609,11 @@ _notmuch_query_count_documents (notmuch_query_t *query, const char *type, unsign
     notmuch_database_t *notmuch = query->notmuch;
     const char *query_string = query->query_string;
     Xapian::doccount count = 0;
+    notmuch_status_t status;
+
+    status = _notmuch_query_ensure_parsed (query);
+    if (status)
+	return status;
 
     try {
 	Xapian::Enquire enquire (*notmuch->xapian_db);
-- 
2.10.2

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

* [PATCH 3/4] lib: query make exclude handling non-destructive
  2016-11-22  2:27 V3 of non-destructive excludes patches David Bremner
  2016-11-22  2:27 ` [PATCH 1/4] lib: eagerly parse queries David Bremner
  2016-11-22  2:27 ` [PATCH 2/4] lib/query: make query parsing lazy again, keep centralized David Bremner
@ 2016-11-22  2:27 ` David Bremner
  2016-11-22  2:27 ` [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value David Bremner
  3 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2016-11-22  2:27 UTC (permalink / raw)
  To: notmuch

We filter added exclude at add time, rather than modifying the query by
count search. As noted in the comments, there are several ignored
conditions here. Returning proper status is split into a separate commit
because it is ABI breaking.
---
 lib/query.cc       | 47 +++++++++++++++++++++++++++++------------------
 test/T060-count.sh |  1 -
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/lib/query.cc b/lib/query.cc
index c356fe5..961cb76 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -31,6 +31,7 @@ struct _notmuch_query {
     notmuch_exclude_t omit_excluded;
     notmuch_bool_t parsed;
     Xapian::Query xapian_query;
+    std::set<std::string> terms;
 };
 
 typedef struct _notmuch_mset_messages {
@@ -77,6 +78,7 @@ _debug_query (void)
 static int
 _notmuch_query_destructor (notmuch_query_t *query) {
     query->xapian_query.~Query();
+    query->terms.~set<std::string>();
     return 0;
 }
 
@@ -94,6 +96,7 @@ notmuch_query_create (notmuch_database_t *notmuch,
 	return NULL;
 
     new (&query->xapian_query) Xapian::Query ();
+    new (&query->terms) std::set<std::string> ();
     query->parsed = FALSE;
 
     talloc_set_destructor (query, _notmuch_query_destructor);
@@ -122,6 +125,15 @@ _notmuch_query_ensure_parsed (notmuch_query_t *query)
 	    query->notmuch->query_parser->
 		parse_query (query->query_string, NOTMUCH_QUERY_PARSER_FLAGS);
 
+       /* Xapian doesn't support skip_to on terms from a query since
+	*  they are unordered, so cache a copy of all terms in
+	*  something searchable.
+	*/
+
+	for (Xapian::TermIterator t = query->xapian_query.get_terms_begin();
+	     t != query->xapian_query.get_terms_end(); ++t)
+	    query->terms.insert(*t);
+
 	query->parsed = TRUE;
 
     } catch (const Xapian::Error &error) {
@@ -165,7 +177,17 @@ notmuch_query_get_sort (const notmuch_query_t *query)
 void
 notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
 {
-    char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
+    notmuch_status_t status;
+    char *term;
+
+    status = _notmuch_query_ensure_parsed (query);
+    if (status)
+	return; /* XXX report error */
+
+    term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
+    if (query->terms.count(term) != 0)
+	return; /* XXX report ignoring exclude? */
+
     _notmuch_string_list_append (query->exclude_terms, term);
 }
 
@@ -185,28 +207,17 @@ _notmuch_messages_destructor (notmuch_mset_messages_t *messages)
 }
 
 /* Return a query that matches messages with the excluded tags
- * registered with query.  Any tags that explicitly appear in xquery
- * will not be excluded, and will be removed from the list of exclude
- * tags.  The caller of this function has to combine the returned
+ * registered with query. The caller of this function has to combine the returned
  * query appropriately.*/
 static Xapian::Query
-_notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
+_notmuch_exclude_tags (notmuch_query_t *query)
 {
     Xapian::Query exclude_query = Xapian::Query::MatchNothing;
 
     for (notmuch_string_node_t *term = query->exclude_terms->head; term;
 	 term = term->next) {
-	Xapian::TermIterator it = xquery.get_terms_begin ();
-	Xapian::TermIterator end = xquery.get_terms_end ();
-	for (; it != end; it++) {
-	    if ((*it).compare (term->string) == 0)
-		break;
-	}
-	if (it == end)
-	    exclude_query = Xapian::Query (Xapian::Query::OP_OR,
-				    exclude_query, Xapian::Query (term->string));
-	else
-	    term->string = talloc_strdup (query, "");
+	exclude_query = Xapian::Query (Xapian::Query::OP_OR,
+				       exclude_query, Xapian::Query (term->string));
     }
     return exclude_query;
 }
@@ -277,7 +288,7 @@ _notmuch_query_search_documents (notmuch_query_t *query,
 	messages->base.excluded_doc_ids = NULL;
 
 	if ((query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) && (query->exclude_terms)) {
-	    exclude_query = _notmuch_exclude_tags (query, final_query);
+	    exclude_query = _notmuch_exclude_tags (query);
 
 	    if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
 		query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
@@ -632,7 +643,7 @@ _notmuch_query_count_documents (notmuch_query_t *query, const char *type, unsign
 					 mail_query, query->xapian_query);
 	}
 
-	exclude_query = _notmuch_exclude_tags (query, final_query);
+	exclude_query = _notmuch_exclude_tags (query);
 
 	final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
 					 final_query, exclude_query);
diff --git a/test/T060-count.sh b/test/T060-count.sh
index 69ab591..b82583b 100755
--- a/test/T060-count.sh
+++ b/test/T060-count.sh
@@ -127,7 +127,6 @@ test_expect_equal_file EXPECTED OUTPUT.clean
 restore_database
 
 test_begin_subtest "count library function is non-destructive"
-test_subtest_known_broken
 cat<<EOF > EXPECTED
 1: 52 messages
 2: 52 messages
-- 
2.10.2

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

* [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value
  2016-11-22  2:27 V3 of non-destructive excludes patches David Bremner
                   ` (2 preceding siblings ...)
  2016-11-22  2:27 ` [PATCH 3/4] lib: query make exclude handling non-destructive David Bremner
@ 2016-11-22  2:27 ` David Bremner
  3 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2016-11-22  2:27 UTC (permalink / raw)
  To: notmuch

Since this is an ABI breaking change, bump the SONAME.
---
 lib/notmuch.h    | 23 +++++++++++++++++++----
 lib/query.cc     |  7 ++++---
 notmuch-count.c  |  9 +++++++--
 notmuch-search.c | 12 ++++++++++--
 notmuch-show.c   | 13 +++++++++++--
 5 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 18678c0..054d86a 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -55,8 +55,8 @@ NOTMUCH_BEGIN_DECLS
  * The library version number.  This must agree with the soname
  * version in Makefile.local.
  */
-#define LIBNOTMUCH_MAJOR_VERSION	4
-#define LIBNOTMUCH_MINOR_VERSION	4
+#define LIBNOTMUCH_MAJOR_VERSION	5
+#define LIBNOTMUCH_MINOR_VERSION	0
 #define LIBNOTMUCH_MICRO_VERSION	0
 
 
@@ -180,6 +180,11 @@ typedef enum _notmuch_status {
      */
     NOTMUCH_STATUS_PATH_ERROR,
     /**
+     * The requested operation was ignored. Depending on the function,
+     * this may not be an actual error.
+     */
+    NOTMUCH_STATUS_IGNORED,
+    /**
      * One of the arguments violates the preconditions for the
      * function, in a way not covered by a more specific argument.
      */
@@ -812,10 +817,20 @@ notmuch_query_get_sort (const notmuch_query_t *query);
 
 /**
  * Add a tag that will be excluded from the query results by default.
- * This exclusion will be overridden if this tag appears explicitly in
+ * This exclusion will be ignored if this tag appears explicitly in
  * the query.
+ *
+ * @returns
+ *
+ * NOTMUCH_STATUS_SUCCESS: excluded was added successfully.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured.
+ *      Most likely a problem lazily parsing the query string.
+ *
+ * NOTMUCH_STATUS_IGNORED: tag is explicitely present in the query, so
+ *		not excluded.
  */
-void
+notmuch_status_t
 notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag);
 
 /**
diff --git a/lib/query.cc b/lib/query.cc
index 961cb76..fba6e07 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -174,7 +174,7 @@ notmuch_query_get_sort (const notmuch_query_t *query)
     return query->sort;
 }
 
-void
+notmuch_status_t
 notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
 {
     notmuch_status_t status;
@@ -182,13 +182,14 @@ notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
 
     status = _notmuch_query_ensure_parsed (query);
     if (status)
-	return; /* XXX report error */
+	return status;
 
     term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
     if (query->terms.count(term) != 0)
-	return; /* XXX report ignoring exclude? */
+	return NOTMUCH_STATUS_IGNORED;
 
     _notmuch_string_list_append (query->exclude_terms, term);
+    return NOTMUCH_STATUS_SUCCESS;
 }
 
 /* We end up having to call the destructors explicitly because we had
diff --git a/notmuch-count.c b/notmuch-count.c
index 35a2aa7..3207c01 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -87,8 +87,13 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 	return -1;
     }
 
-    for (i = 0; i < exclude_tags_length; i++)
-	notmuch_query_add_tag_exclude (query, exclude_tags[i]);
+    for (i = 0; i < exclude_tags_length; i++) {
+	status = notmuch_query_add_tag_exclude (query, exclude_tags[i]);
+	if (status && status != NOTMUCH_STATUS_IGNORED) {
+	    print_status_query ("notmuch count", query, status);
+	    return -1;
+	}
+    }
 
     switch (output) {
     case OUTPUT_MESSAGES:
diff --git a/notmuch-search.c b/notmuch-search.c
index 8c65d5a..64a9811 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -735,11 +735,19 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar
     if (ctx->exclude != NOTMUCH_EXCLUDE_FALSE) {
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
+	notmuch_status_t status;
 
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
-	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (ctx->query, search_exclude_tags[i]);
+
+	for (i = 0; i < search_exclude_tags_length; i++) {
+	    status = notmuch_query_add_tag_exclude (ctx->query, search_exclude_tags[i]);
+	    if (status && status != NOTMUCH_STATUS_IGNORED) {
+		print_status_query ("notmuch search", ctx->query, status);
+		return EXIT_FAILURE;
+	    }
+	}
+
 	notmuch_query_set_omit_excluded (ctx->query, ctx->exclude);
     }
 
diff --git a/notmuch-show.c b/notmuch-show.c
index 22fa655..4c63959 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1157,11 +1157,19 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
 	unsigned int i;
+	notmuch_status_t status;
 
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
-	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+
+	for (i = 0; i < search_exclude_tags_length; i++) {
+	    status = notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+	    if (status && status != NOTMUCH_STATUS_IGNORED) {
+		print_status_query ("notmuch show", query, status);
+		ret = -1;
+		goto DONE;
+	    }
+	}
 
 	if (exclude == EXCLUDE_FALSE) {
 	    notmuch_query_set_omit_excluded (query, FALSE);
@@ -1171,6 +1179,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	ret = do_show (config, query, format, sprinter, &params);
     }
 
+ DONE:
     notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
-- 
2.10.2

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

* [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value
  2017-02-18 15:08 v4 of nondestructive excludes patches David Bremner
@ 2017-02-18 15:08 ` David Bremner
  2017-03-22 11:54   ` David Bremner
  0 siblings, 1 reply; 7+ messages in thread
From: David Bremner @ 2017-02-18 15:08 UTC (permalink / raw)
  To: notmuch

Since this is an ABI breaking change, bump the SONAME.
---
 lib/notmuch.h    | 23 +++++++++++++++++++----
 lib/query.cc     |  7 ++++---
 notmuch-count.c  |  9 +++++++--
 notmuch-search.c | 12 ++++++++++--
 notmuch-show.c   | 13 +++++++++++--
 5 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 16da8be9..74043819 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -55,8 +55,8 @@ NOTMUCH_BEGIN_DECLS
  * The library version number.  This must agree with the soname
  * version in Makefile.local.
  */
-#define LIBNOTMUCH_MAJOR_VERSION	4
-#define LIBNOTMUCH_MINOR_VERSION	4
+#define LIBNOTMUCH_MAJOR_VERSION	5
+#define LIBNOTMUCH_MINOR_VERSION	0
 #define LIBNOTMUCH_MICRO_VERSION	0
 
 
@@ -180,6 +180,11 @@ typedef enum _notmuch_status {
      */
     NOTMUCH_STATUS_PATH_ERROR,
     /**
+     * The requested operation was ignored. Depending on the function,
+     * this may not be an actual error.
+     */
+    NOTMUCH_STATUS_IGNORED,
+    /**
      * One of the arguments violates the preconditions for the
      * function, in a way not covered by a more specific argument.
      */
@@ -812,10 +817,20 @@ notmuch_query_get_sort (const notmuch_query_t *query);
 
 /**
  * Add a tag that will be excluded from the query results by default.
- * This exclusion will be overridden if this tag appears explicitly in
+ * This exclusion will be ignored if this tag appears explicitly in
  * the query.
+ *
+ * @returns
+ *
+ * NOTMUCH_STATUS_SUCCESS: excluded was added successfully.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured.
+ *      Most likely a problem lazily parsing the query string.
+ *
+ * NOTMUCH_STATUS_IGNORED: tag is explicitely present in the query, so
+ *		not excluded.
  */
-void
+notmuch_status_t
 notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag);
 
 /**
diff --git a/lib/query.cc b/lib/query.cc
index c0a1cdf8..c027edd3 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -174,7 +174,7 @@ notmuch_query_get_sort (const notmuch_query_t *query)
     return query->sort;
 }
 
-void
+notmuch_status_t
 notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
 {
     notmuch_status_t status;
@@ -182,13 +182,14 @@ notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
 
     status = _notmuch_query_ensure_parsed (query);
     if (status)
-	return; /* XXX report error */
+	return status;
 
     term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
     if (query->terms.count(term) != 0)
-	return; /* XXX report ignoring exclude? */
+	return NOTMUCH_STATUS_IGNORED;
 
     _notmuch_string_list_append (query->exclude_terms, term);
+    return NOTMUCH_STATUS_SUCCESS;
 }
 
 /* We end up having to call the destructors explicitly because we had
diff --git a/notmuch-count.c b/notmuch-count.c
index 35a2aa70..3207c015 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -87,8 +87,13 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 	return -1;
     }
 
-    for (i = 0; i < exclude_tags_length; i++)
-	notmuch_query_add_tag_exclude (query, exclude_tags[i]);
+    for (i = 0; i < exclude_tags_length; i++) {
+	status = notmuch_query_add_tag_exclude (query, exclude_tags[i]);
+	if (status && status != NOTMUCH_STATUS_IGNORED) {
+	    print_status_query ("notmuch count", query, status);
+	    return -1;
+	}
+    }
 
     switch (output) {
     case OUTPUT_MESSAGES:
diff --git a/notmuch-search.c b/notmuch-search.c
index 8c65d5ad..64a98110 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -735,11 +735,19 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar
     if (ctx->exclude != NOTMUCH_EXCLUDE_FALSE) {
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
+	notmuch_status_t status;
 
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
-	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (ctx->query, search_exclude_tags[i]);
+
+	for (i = 0; i < search_exclude_tags_length; i++) {
+	    status = notmuch_query_add_tag_exclude (ctx->query, search_exclude_tags[i]);
+	    if (status && status != NOTMUCH_STATUS_IGNORED) {
+		print_status_query ("notmuch search", ctx->query, status);
+		return EXIT_FAILURE;
+	    }
+	}
+
 	notmuch_query_set_omit_excluded (ctx->query, ctx->exclude);
     }
 
diff --git a/notmuch-show.c b/notmuch-show.c
index 22fa655a..4c63959f 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1157,11 +1157,19 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
 	unsigned int i;
+	notmuch_status_t status;
 
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
-	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+
+	for (i = 0; i < search_exclude_tags_length; i++) {
+	    status = notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
+	    if (status && status != NOTMUCH_STATUS_IGNORED) {
+		print_status_query ("notmuch show", query, status);
+		ret = -1;
+		goto DONE;
+	    }
+	}
 
 	if (exclude == EXCLUDE_FALSE) {
 	    notmuch_query_set_omit_excluded (query, FALSE);
@@ -1171,6 +1179,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	ret = do_show (config, query, format, sprinter, &params);
     }
 
+ DONE:
     notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
-- 
2.11.0

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

* Re: [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value
  2017-02-18 15:08 ` [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value David Bremner
@ 2017-03-22 11:54   ` David Bremner
  0 siblings, 0 replies; 7+ messages in thread
From: David Bremner @ 2017-03-22 11:54 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> Since this is an ABI breaking change, bump the SONAME.

pushed, although the SONAME bump was already there from the previous
series.

d

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

end of thread, other threads:[~2017-03-22 11:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-22  2:27 V3 of non-destructive excludes patches David Bremner
2016-11-22  2:27 ` [PATCH 1/4] lib: eagerly parse queries David Bremner
2016-11-22  2:27 ` [PATCH 2/4] lib/query: make query parsing lazy again, keep centralized David Bremner
2016-11-22  2:27 ` [PATCH 3/4] lib: query make exclude handling non-destructive David Bremner
2016-11-22  2:27 ` [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value David Bremner
  -- strict thread matches above, loose matches on Subject: below --
2017-02-18 15:08 v4 of nondestructive excludes patches David Bremner
2017-02-18 15:08 ` [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value David Bremner
2017-03-22 11:54   ` David Bremner

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