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