[PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t
Mark Walters
markwalters1009 at gmail.com
Sat Jun 22 03:10:32 PDT 2013
Tomi Ollila <tomi.ollila at iki.fi> writes:
> On Thu, Jun 20 2013, Mark Walters <markwalters1009 at gmail.com> wrote:
>
>> 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.
>
> So, the choice here is to choose between
>
> id:"1368454815-1854-1-git-send-email-novalazy at gmail.com"
> and
> id:"87bo8edif8.fsf at qmul.ac.uk"
I think if we go with the latter then it would make sense to also do
id:1368301809-12532-9-git-send-email-markwalters1009 at gmail.com
(or something similar)
Best wishes
Mark
>
> (might be hard to catch from this thread -- was easier to take from
> http://nmbug.tethera.net/status/ )
>
> Both apply cleanly to current master ( d0bd88f )
>
> While Peter's version surely looks simpler (and may work, didn't test atm)
> the comments Mark presents makes sense (especially the "subtlety" thing ;)
>
> IMHO Mark's version makes future development "safer" and therefore my
> vote (or million of those) goes to id:"87bo8edif8.fsf at qmul.ac.uk"
>
>> Best wishes
>>
>> Mark
>
> Tomi
>
>
>>
>>
>>
>> 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
>> _______________________________________________
>> notmuch mailing list
>> notmuch at notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list