From 971187d89bd1adee56614faff0d4b6567714906f Mon Sep 17 00:00:00 2001 From: Mark Walters Date: Sun, 18 Mar 2012 17:30:31 +0000 Subject: [PATCH] Re: [PATCH 4/5] cli: move show to the new --exclude= option naming scheme. --- 86/82ddcea06ec2dfc3bfa0c3a5d2334fac75d923 | 303 ++++++++++++++++++++++ 1 file changed, 303 insertions(+) create mode 100644 86/82ddcea06ec2dfc3bfa0c3a5d2334fac75d923 diff --git a/86/82ddcea06ec2dfc3bfa0c3a5d2334fac75d923 b/86/82ddcea06ec2dfc3bfa0c3a5d2334fac75d923 new file mode 100644 index 000000000..1cc2c0a67 --- /dev/null +++ b/86/82ddcea06ec2dfc3bfa0c3a5d2334fac75d923 @@ -0,0 +1,303 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 61B50431FB6 + for ; Sun, 18 Mar 2012 10:30:30 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -1.098 +X-Spam-Level: +X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5 + tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, + 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 QZdCY3eVJojM for ; + Sun, 18 Mar 2012 10:30:29 -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 36B53431FAF + for ; Sun, 18 Mar 2012 10:30:29 -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 1S9Jvy-00057l-Nn; Sun, 18 Mar 2012 17:30:25 +0000 +Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223] + helo=localhost) + by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69) + (envelope-from ) + id 1S9Jvy-0006EJ-7R; Sun, 18 Mar 2012 17:30:22 +0000 +From: Mark Walters +To: Austin Clements +Subject: Re: [PATCH 4/5] cli: move show to the new --exclude= option naming + scheme. +In-Reply-To: <20120317165119.GI2670@mit.edu> +References: <1331836925-31437-1-git-send-email-markwalters1009@gmail.com> + <1331836925-31437-5-git-send-email-markwalters1009@gmail.com> + <20120317165119.GI2670@mit.edu> +User-Agent: Notmuch/0.11.1+302~g99b9184 (http://notmuchmail.org) Emacs/23.3.1 + (x86_64-pc-linux-gnu) +Warning: No xauth data; using fake authentication data for X11 forwarding. +Date: Sun, 18 Mar 2012 17:30:31 +0000 +Message-ID: <87aa3dhl54.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Sender-Host-Address: 94.192.233.223 +X-QM-SPAM-Info: Sender has good ham record. :) +X-QM-Body-MD5: 13f51f946ee8ad6adefe67e26ef17091 (of first 20000 bytes) +X-SpamAssassin-Score: -1.8 +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 -1.8 points. + Summary of the scoring: + * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, + * medium trust + * [138.37.6.40 listed in list.dnswl.org] + * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail + provider * (markwalters1009[at]gmail.com) + * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay + * domain + * 0.5 AWL AWL: From: address is in the auto white-list +X-QM-Scan-Virus: ClamAV says the message is clean +Cc: notmuch@notmuchmail.org +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: Sun, 18 Mar 2012 17:30:30 -0000 + +On Sat, 17 Mar 2012 12:51:20 -0400, Austin Clements wrote: +> Quoth Mark Walters on Mar 15 at 6:42 pm: +> > This moves show to the --exclude=(true|false|flag) naming +> > scheme. When `exclude' is false or flag show returns all threads +> > that match including those that only match in an excluded message, the +> > difference being whether excluded messages are flagged excluded. +> > +> > When exclude=true the behaviour depends on whether --entire-threads +> +> s/--entire-threads/--entire-thread/ + +Will fix. + +> > is set. If it is not set then show only returns the messages which +> > match and are not excluded. If it is set then show returns all +> > messages in these threads flagging the excluded messages. The +> +> Parse error. + +Will try and reword. + +> > rationale is that it is awkward to use a thread with some missing +> > messages. +> > --- +> > man/man1/notmuch-show.1 | 16 ++++++++++++++-- +> > notmuch-client.h | 1 + +> > notmuch-show.c | 39 +++++++++++++++++++++++++++++---------- +> > 3 files changed, 44 insertions(+), 12 deletions(-) +> > +> > diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1 +> > index d75d971..801b7f1 100644 +> > --- a/man/man1/notmuch-show.1 +> > +++ b/man/man1/notmuch-show.1 +> > @@ -130,9 +130,21 @@ content. +> > +> > .RS 4 +> > .TP 4 +> > -.B \-\-no-exclude +> > +.BR \-\-exclude=(true|false|flag) +> > + +> > +Specify whether to omit threads only matching search.tag_exclude from +> > +the search results (the default) or not. The extra option +> > +.B flag +> > +includes these messages but marks them with the excluded flag. +> > + +> > +If --entire-thread is specified then complete threads are returned +> > +regardless (with the excluded flag being set when appropriate) but +> > +threads that only match in an excluded message are not returned when +> > +.B --exclude=true. +> +> I found this a bit confusing. There are two orthogonal things going +> on here: what happens to excluded messages and what happens to +> fully-excluded threads. Is the following table accurate? +> +> --entire-thread=false +> excl. messages excl. threads +> true omit omit +> false include include +> flag include,flag include +> +> --entire-thread=true +> excl. messages excl. threads +> true include,flag omit +> false include include +> flag include,flag include + +This table is accurate. + +> (My reasoning: --exclude=false is equivalent to not having any +> excludes configured, --exclude=true omits excluded messages from the +> seed set and filters them in show_messages, --exclude=flag does not +> exclude messages from the seed set nor filter them in show_messages.) +> +> If this is right, then what's the point of having both false and flag +> for show? I'm pretty sure their performance will be +> indistinguishable. + +I included the three options partly for completeness, partly to be the +same as notmuch-search and partly because a "flag" for mbox feels +wrong. I am happy to change it though. Do you have any good naming +suggestions? + +> > + +> > +The default is +> > +.B --exclude=true. +> > +> > -Do not exclude the messages matching search.exclude_tags in the config file. +> > .RE +> > +> > A common use of +> > diff --git a/notmuch-client.h b/notmuch-client.h +> > index f4a62cc..e36148b 100644 +> > --- a/notmuch-client.h +> > +++ b/notmuch-client.h +> > @@ -99,6 +99,7 @@ typedef struct notmuch_show_format { +> > +> > typedef struct notmuch_show_params { +> > notmuch_bool_t entire_thread; +> > + notmuch_bool_t omit_excluded; +> > notmuch_bool_t raw; +> > int part; +> > #ifdef GMIME_ATLEAST_26 +> > diff --git a/notmuch-show.c b/notmuch-show.c +> > index 05d51b2..20d6635 100644 +> > --- a/notmuch-show.c +> > +++ b/notmuch-show.c +> > @@ -812,6 +812,7 @@ show_messages (void *ctx, +> > { +> > notmuch_message_t *message; +> > notmuch_bool_t match; +> > + notmuch_bool_t excluded; +> > int first_set = 1; +> > int next_indent; +> > +> > @@ -830,10 +831,11 @@ show_messages (void *ctx, +> > message = notmuch_messages_get (messages); +> > +> > match = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH); +> > + excluded = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED); +> > +> > next_indent = indent; +> > +> > - if (match || params->entire_thread) { +> > + if ((match && (!excluded || !params->omit_excluded)) || params->entire_thread) { +> > show_message (ctx, format, message, indent, params); +> > next_indent = indent + 1; +> > +> > @@ -974,6 +976,12 @@ enum { +> > NOTMUCH_FORMAT_RAW +> > }; +> > +> > +enum { +> > + EXCLUDE_TRUE, +> > + EXCLUDE_FALSE, +> > + EXCLUDE_FLAG, +> > +}; +> > + +> > int +> > notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> > { +> > @@ -983,10 +991,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> > char *query_string; +> > int opt_index, ret; +> > const notmuch_show_format_t *format = &format_text; +> > - notmuch_show_params_t params = { .part = -1 }; +> > + notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE }; +> > int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +> > notmuch_bool_t verify = FALSE; +> > - notmuch_bool_t no_exclude = FALSE; +> > + int exclude = EXCLUDE_TRUE; +> > +> > notmuch_opt_desc_t options[] = { +> > { NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f', +> > @@ -995,11 +1003,15 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> > { "mbox", NOTMUCH_FORMAT_MBOX }, +> > { "raw", NOTMUCH_FORMAT_RAW }, +> > { 0, 0 } } }, +> > + { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x', +> > + (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE }, +> > + { "false", EXCLUDE_FALSE }, +> > + { "flag", EXCLUDE_FLAG }, +> > + { 0, 0 } } }, +> > { NOTMUCH_OPT_INT, ¶ms.part, "part", 'p', 0 }, +> > { NOTMUCH_OPT_BOOLEAN, ¶ms.entire_thread, "entire-thread", 't', 0 }, +> > { NOTMUCH_OPT_BOOLEAN, ¶ms.decrypt, "decrypt", 'd', 0 }, +> > { NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 }, +> > - { NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'n', 0 }, +> > { 0, 0, 0, 0, 0 } +> > }; +> > +> > @@ -1088,16 +1100,18 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> > return 1; +> > } +> > +> > - /* if format=mbox then we can not output excluded messages as +> > - * there is no way to make the exclude flag available */ +> > - if (format_sel == NOTMUCH_FORMAT_MBOX) +> > - notmuch_query_set_omit_excluded_messages (query, TRUE); +> > - +> > /* If a single message is requested we do not use search_excludes. */ +> > if (params.part >= 0) +> > ret = do_show_single (ctx, query, format, ¶ms); +> > else { +> > - if (!no_exclude) { +> > + if (format == &format_mbox && exclude == EXCLUDE_FLAG) { +> > + /* there is no where to mark flagged messages so fall back on +> +> s/no where/nowhere/ Also, s/there/There/ for style consistency. +> +> > + * including the excluded messages */ +> > + fprintf (stderr, "Cannot flag excluded messages with format=mbox: fall back on just including them\n"); +> +> This is a bit verbose. How about just "Warning: mbox cannot flag +> excluded messages"? Flag already means that the messages should be +> included, so this message states exactly what mbox isn't doing that +> you might expect it to. + +I like this and will change to it. + +> > + exclude = EXCLUDE_FALSE; +> > + } +> > + +> > + if (exclude != EXCLUDE_FALSE) { +> > const char **search_exclude_tags; +> > size_t search_exclude_tags_length; +> > unsigned int i; +> > @@ -1106,7 +1120,12 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) +> > (config, &search_exclude_tags_length); +> > 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_messages(query, FALSE); +> > + params.omit_excluded = FALSE; +> > + } +> > } +> > + +> > ret = do_show (ctx, query, format, ¶ms); +> > } +> > + +Thanks + +Mark -- 2.26.2