From 70bf0b6af6e10e1fa81ebdd946461493a62ef135 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sun, 18 Mar 2012 12:51:20 +2000 Subject: [PATCH] Re: [PATCH 4/5] cli: move show to the new --exclude= option naming scheme. --- 63/8530b6255e70bbd7c0d43599525eb8782ed716 | 283 ++++++++++++++++++++++ 1 file changed, 283 insertions(+) create mode 100644 63/8530b6255e70bbd7c0d43599525eb8782ed716 diff --git a/63/8530b6255e70bbd7c0d43599525eb8782ed716 b/63/8530b6255e70bbd7c0d43599525eb8782ed716 new file mode 100644 index 000000000..1f0f316bf --- /dev/null +++ b/63/8530b6255e70bbd7c0d43599525eb8782ed716 @@ -0,0 +1,283 @@ +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 C43AF431FB6 + for ; Sat, 17 Mar 2012 09:51:23 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 NvAj79+wpn3Q for ; + Sat, 17 Mar 2012 09:51:23 -0700 (PDT) +Received: from dmz-mailsec-scanner-5.mit.edu (DMZ-MAILSEC-SCANNER-5.MIT.EDU + [18.7.68.34]) + by olra.theworths.org (Postfix) with ESMTP id DA6D2431FAE + for ; Sat, 17 Mar 2012 09:51:22 -0700 (PDT) +X-AuditID: 12074422-b7fd66d0000008f9-d3-4f64c10a1ec3 +Received: from mailhub-auth-1.mit.edu ( [18.9.21.35]) + by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP + id 03.7B.02297.A01C46F4; Sat, 17 Mar 2012 12:51:22 -0400 (EDT) +Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) + by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id q2HGpLmA013802; + Sat, 17 Mar 2012 12:51:22 -0400 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q2HGpKaZ012098 + (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); + Sat, 17 Mar 2012 12:51:21 -0400 (EDT) +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) + (envelope-from ) + id 1S8wqe-000476-95; Sat, 17 Mar 2012 12:51:20 -0400 +Date: Sat, 17 Mar 2012 12:51:20 -0400 +From: Austin Clements +To: Mark Walters +Subject: Re: [PATCH 4/5] cli: move show to the new --exclude= option naming + scheme. +Message-ID: <20120317165119.GI2670@mit.edu> +References: <1331836925-31437-1-git-send-email-markwalters1009@gmail.com> + <1331836925-31437-5-git-send-email-markwalters1009@gmail.com> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: <1331836925-31437-5-git-send-email-markwalters1009@gmail.com> +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFupjleLIzCtJLcpLzFFi42IR4hRV1uU6mOJvsP6psMXquTwW12/OZHZg + 8tg56y67x7NVt5gDmKK4bFJSczLLUov07RK4MuZ/6GUv2GRTsfNCJ2MD4z/dLkZODgkBE4lF + 29cyQdhiEhfurWfrYuTiEBLYxyhxqOE0C4SzgVHi8s11zBDOSSaJUy2boDJLGCWmfpnHBtLP + IqAqcWzBQrBZbAIaEtv2L2cEsUUEdCRuH1rADmIzC0hLfPvdDFYjLBAq0fZuHVAvBwevgLbE + jCYxiJmdjBK33k0Cq+cVEJQ4OfMJC0SvlsSNfy+ZQOpB5iz/xwFicgp4SUz9oAJSISqgIjHl + 5Da2CYxCs5A0z0LSPAuheQEj8ypG2ZTcKt3cxMyc4tRk3eLkxLy81CJdU73czBK91JTSTYyg + oGZ3UdrB+POg0iFGAQ5GJR5ejgnJ/kKsiWXFlbmHGCU5mJREeRkPpPgL8SXlp1RmJBZnxBeV + 5qQWH2KU4GBWEuGVWg6U401JrKxKLcqHSUlzsCiJ86prvfMTEkhPLEnNTk0tSC2CycpwcChJ + 8PqADBUsSk1PrUjLzClBSDNxcIIM5wEa7gpSw1tckJhbnJkOkT/FqMvRPfXRJUYhlrz8vFQp + cV5TkCIBkKKM0jy4ObBk9IpRHOgtYd44kCoeYCKDm/QKaAkT0JKZZckgS0oSEVJSDYyzV7Ww + S+zQmG2l03jspfwURdt/Ag4h+yas/XL1C7vL/wd6ddrBS65dNudp55fkLefdqz/h69WXQW8M + BCYIZH640vVxmbWW7ZxFe0IuzLrqdIY7Iv7UyStOrS9XfTWNWyW9+LDXbe+cnwvfr4rcmH7g + nWmABHtb8pRszRtSCl1skXe51/WlP9ukxFKckWioxVxUnAgAFMAwgCEDAAA= +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: Sat, 17 Mar 2012 16:51:23 -0000 + +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/ + +> 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. + +> 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 + +(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. + +> + +> +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. + +> + 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); +> } +> -- 2.26.2