From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id E8937431FAE for ; Sat, 22 Jun 2013 03:10:53 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: 1.401 X-Spam-Level: * X-Spam-Status: No, score=1.401 tagged_above=-999 required=5 tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, FREEMAIL_REPLY=2.499, NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4VKkL8mFe7wb for ; Sat, 22 Jun 2013 03:10:43 -0700 (PDT) Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 0AE78431FB6 for ; Sat, 22 Jun 2013 03:10:43 -0700 (PDT) Received: from smtp.qmul.ac.uk ([138.37.6.40]) by mail2.qmul.ac.uk with esmtp (Exim 4.71) (envelope-from ) id 1UqKm9-00043c-Sx; Sat, 22 Jun 2013 11:10:39 +0100 Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost) by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71) (envelope-from ) id 1UqKm9-0001NP-Bv; Sat, 22 Jun 2013 11:10:33 +0100 From: Mark Walters To: Tomi Ollila , notmuch@notmuchmail.org Subject: Re: [PATCH v3 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t In-Reply-To: References: <1368301809-12532-1-git-send-email-markwalters1009@gmail.com> <1368301809-12532-8-git-send-email-markwalters1009@gmail.com> <87bo8edif8.fsf@qmul.ac.uk> <87fvwdx19x.fsf@qmul.ac.uk> User-Agent: Notmuch/0.15.2+171~ge2f30a2 (http://notmuchmail.org) Emacs/23.4.1 (i486-pc-linux-gnu) Date: Sat, 22 Jun 2013 11:10:32 +0100 Message-ID: <87ppve317r.fsf@qmul.ac.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Sender-Host-Address: 93.97.24.31 X-QM-SPAM-Info: Sender has good ham record. :) X-QM-Body-MD5: 1a987b67cd04be55696806cf47de126c (of first 20000 bytes) X-SpamAssassin-Score: 0.6 X-SpamAssassin-SpamBar: / X-SpamAssassin-Report: The QM spam filters have analysed this message to determine if it is spam. We require at least 5.0 points to mark a message as spam. This message scored 0.6 points. Summary of the scoring: * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider * (markwalters1009[at]gmail.com) * 1.0 FREEMAIL_REPLY From and body contain different freemails * -0.4 AWL AWL: From: address is in the auto white-list X-QM-Scan-Virus: ClamAV says the message is clean X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 22 Jun 2013 10:10:54 -0000 Tomi Ollila writes: > On Thu, Jun 20 2013, Mark Walters 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@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@gmail.com" > and > id:"87bo8edif8.fsf@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@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@qmul.ac.uk" > >> Best wishes >> >> Mark > > Tomi > > >> >> >> >> On Mon, 13 May 2013, Mark Walters 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@notmuchmail.org >> http://notmuchmail.org/mailman/listinfo/notmuch