V2 non-destructive excludes
This mainly rebases against master, and fixes the two minor problems already pointed out. I would like feedback on breaking the API for this. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/5] test: add known broken test for nondestructiveness of count
Thanks to Lucas (id:147263183913.27784.12274024193186585889@mbp) for the bug report and the test case. I decided to use the python version because the python bindings could use more exercise. --- test/T060-count.sh | 28 1 file changed, 28 insertions(+) diff --git a/test/T060-count.sh b/test/T060-count.sh index d6933a7..69ab591 100755 --- a/test/T060-count.sh +++ b/test/T060-count.sh @@ -126,4 +126,32 @@ sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean test_expect_equal_file EXPECTED OUTPUT.clean restore_database +test_begin_subtest "count library function is non-destructive" +test_subtest_known_broken +cat< EXPECTED +1: 52 messages +2: 52 messages +Exclude 'spam' +3: 52 messages +4: 52 messages +EOF +test_python
[PATCH 2/5] lib/query: make query parsing lazy again, keep centralized.
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 | 36
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/lib/query.cc b/lib/query.cc
index 098ed8f..ba5465d 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,23 +108,31 @@ 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 {
- new (&query->xapian_query) Xapian::Query ();
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 *
@@ -226,6 +236,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))
@@ -592,6 +607,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.9.3
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/5] lib: make notmuch_query_add_tag_exclude return a status value
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 251c29b..3b4a3aa 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 afbabb6..d8cbc7b 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -173,7 +173,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;
@@ -181,13 +181,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_e
[PATCH 4/5] lib: query make exclude handling non-destructive
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 | 48 ++--
test/T060-count.sh | 1 -
2 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/lib/query.cc b/lib/query.cc
index ba5465d..afbabb6 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 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();
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 ();
query->parsed = FALSE;
talloc_set_destructor (query, _notmuch_query_destructor);
@@ -121,6 +124,16 @@ _notmuch_query_ensure_parsed (notmuch_query_t *query)
query->xapian_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) {
_notmuch_database_log (query->notmuch,
@@ -163,7 +176,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);
}
@@ -183,28 +206,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;
}
@@ -275,7 +287,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)
@@ -630,7 +642,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
[PATCH 1/5] lib: eagerly parse queries
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 | 40
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/lib/query.cc b/lib/query.cc
index 53efd4e..098ed8f 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,22 @@ notmuch_query_create (notmuch_database_t *notmuch,
query->omit_excluded = NOTMUCH_EXCLUDE_TRUE;
+try {
+ new (&query->xapian_query) Xapian::Query ();
+ 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 +245,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 +254,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 +598,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 +606,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.9.3
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] lib: bump minor version to mark added symbols
This should not change the SONAME, and therefore won't change the dynamic linking behaviour, but it may help some users debug missing symbols in case their libnotmuch is too old. --- this should probably go into 0.23 lib/notmuch.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/notmuch.h b/lib/notmuch.h index e03a05d..251c29b 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -56,7 +56,7 @@ NOTMUCH_BEGIN_DECLS * version in Makefile.local. */ #define LIBNOTMUCH_MAJOR_VERSION 4 -#define LIBNOTMUCH_MINOR_VERSION 3 +#define LIBNOTMUCH_MINOR_VERSION 4 #define LIBNOTMUCH_MICRO_VERSION 0 -- 2.9.3 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] emacs: postpone/resume support
Mark Walters writes: > This provides preliminary support for postponing and resuming in the > emacs frontend. On postponing it uses notmuch insert to put the > message in the notmuch database; resume gets the raw file from notmuch > and using the emacs function mime-to-mml reconstructs the message > (including attachments). I was thinking about this a bit more, in the context of your other patch to scan for #secure, and it occured to me one option would be would be to add a header to the draft being written with the secure (and other relevant info). As a proof of concept I added a constant header to all drafts [1]. An outline of the approach would be - scan for #secure tags, error if they are not in the right place - if found at the top of the document, extract the contents, remove the mml tag and put in a header like e.g. X-Notmuch-Emacs-Secure: method=pgpmime mode=sign - on reading back, check for, remove that header, add the corresponding #secure tag. It might be possible to re-use functions e.g. mml-secure-message and mml-unsecure-message from mml-sec.el Compared to the other approach we discussed (using message-properties to store the data), this has the advantage of not requiring new features in the CLI. Also, it would probably be a potential source of user error to sync drafts between machines via some imap/rsync mechanism, without syncing message metadata. Our usual distaste for writing headers is that we don't want to modify delivered mail; this seems a non-issue here to me since there is no delivered mail. Others might disagree, of course. [1] diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el index 95e5650..412da94 100644 --- a/emacs/notmuch-maildir-fcc.el +++ b/emacs/notmuch-maildir-fcc.el @@ -163,6 +163,7 @@ by notmuch-mua-mail" "Setup message for saving. Should be called on a temporary copy. This is taken from the function message-do-fcc." + (message-add-header "X-Notmuch-Emacs-Draft: true") (message-encode-message-body) (save-restriction (message-narrow-to-headers) signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Last call for 0.23 NEWS
David Bremner writes: > We are now frozen for 0.23. I have tagged 0.23_rc0 in git, and uploaded > it to debian experimental. At this point I expect only NEWS changes and > serious bug fixes before release. I have tagged 0.23_rc1 in git (and uploaded to Debian experimental). If I don't hear of any serious problems before Monday, I'll release that, along with any last minute NEWS patches as 0.23. d signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
