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