From 89c439145a31600fd8296432f0d362170becc6cc Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Fri, 9 Jan 2015 17:31:22 +0100 Subject: [PATCH] Re: [PATCH v3 10/10] cli: address: Add --filter-by option to configure address filtering --- 0b/7821cc2c5cc825f261347f589ba26d04e65726 | 301 ++++++++++++++++++++++ 1 file changed, 301 insertions(+) create mode 100644 0b/7821cc2c5cc825f261347f589ba26d04e65726 diff --git a/0b/7821cc2c5cc825f261347f589ba26d04e65726 b/0b/7821cc2c5cc825f261347f589ba26d04e65726 new file mode 100644 index 000000000..761d4c145 --- /dev/null +++ b/0b/7821cc2c5cc825f261347f589ba26d04e65726 @@ -0,0 +1,301 @@ +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 5EB54431FDB + for ; Fri, 9 Jan 2015 08:31:37 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0.138 +X-Spam-Level: +X-Spam-Status: No, score=0.138 tagged_above=-999 required=5 + tests=[DNS_FROM_AHBL_RHSBL=2.438, 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 v-FuTaOBJyc4 for ; + Fri, 9 Jan 2015 08:31:32 -0800 (PST) +Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36]) + by olra.theworths.org (Postfix) with ESMTP id A6A66431FD8 + for ; Fri, 9 Jan 2015 08:31:32 -0800 (PST) +Received: from localhost (unknown [192.168.200.7]) + by max.feld.cvut.cz (Postfix) with ESMTP id 841B73CFEF8; + Fri, 9 Jan 2015 17:31:27 +0100 (CET) +X-Virus-Scanned: IMAP STYX AMAVIS +Received: from max.feld.cvut.cz ([192.168.200.1]) + by localhost (styx.feld.cvut.cz [192.168.200.7]) (amavisd-new, + port 10044) + with ESMTP id h_s2mZ4PUkMI; Fri, 9 Jan 2015 17:31:23 +0100 (CET) +Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34]) + by max.feld.cvut.cz (Postfix) with ESMTP id 87A453CFEAD; + Fri, 9 Jan 2015 17:31:23 +0100 (CET) +Received: from wsh by steelpick.2x.cz with local (Exim 4.84) + (envelope-from ) + id 1Y9cT4-0006RW-Di; Fri, 09 Jan 2015 17:31:22 +0100 +From: Michal Sojka +To: Tomi Ollila , David Bremner , + notmuch@notmuchmail.org +Subject: Re: [PATCH v3 10/10] cli: address: Add --filter-by + option to configure address filtering +In-Reply-To: <871tn46khe.fsf@steelpick.2x.cz> +References: <1415147159-19946-1-git-send-email-sojkam1@fel.cvut.cz> + <1415147159-19946-11-git-send-email-sojkam1@fel.cvut.cz> + <87vbkrfs66.fsf@maritornes.cs.unb.ca> + + <87egr46qcs.fsf@steelpick.2x.cz> + + <871tn46khe.fsf@steelpick.2x.cz> +User-Agent: Notmuch/0.18.2+178~g6e9e8bb (http://notmuchmail.org) Emacs/24.4.1 + (x86_64-pc-linux-gnu) +Date: Fri, 09 Jan 2015 17:31:22 +0100 +Message-ID: <87y4pb6hlx.fsf@steelpick.2x.cz> +MIME-Version: 1.0 +Content-Type: text/plain +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: Fri, 09 Jan 2015 16:31:37 -0000 + +On Fri, Jan 09 2015, Michal Sojka wrote: +> On Fri, Jan 09 2015, Tomi Ollila wrote: +>> On Fri, Jan 09 2015, Michal Sojka wrote: +>> +>>> Hi, +>>> +>>> sorry for longer response time :) +>>> +>>> On Thu, Jan 01 2015, Tomi Ollila wrote: +>>>> On Wed, Dec 31 2014, David Bremner wrote: +>>>> +>>>>> Michal Sojka writes: +>>>>> +>>>>>> This option allows to configure the criterion for duplicate address +>>>>>> filtering. Without this option, all unique combinations of name and +>>>>>> address parts are printed. This option allows to filter the output +>>>>>> more, for example to only contain unique address parts. +>>>>> +>>>>> I had the feeling there was some "controversy" about the UI here, but +>>>>> following back the 3 versions of the series I didn't see it. Does that +>>>>> mean we just need to sanity check the code, or are there outstanding +>>>>> bikes to shed? +>>> +>>> I'd tend to rename this option to --unique as it was in some previous +>>> version of the patch. Another thing in my mind is the implementation of +>>> the --complete option mentioned in id:878uid9qjl.fsf@nautilus.nautilus. +>>> This would also involve some kind of address filtering. I'll look into +>>> this and send patches later. +>>> +>>>> I have intentionally been guiet on this during the review process of the +>>>> other patches to not slow down the acceptance of the others. I have not +>>>> got enough time to look the implemenentation or think this last patch +>>>> further -- from the user interface point of view I recall seeing there +>>>> both useless features (but which might be warranted by implementation +>>>> simplicity) and missing features (but which might not be there due to +>>>> difficulty in implementation). Also, I am not sure whether the --filter-by +>>>> is good option (and options descriptive...)... +>>> +>>> I'd be interested in what are these "missing features". +>> +>> Last night when I tried to catch sleep I was also thinking of this... +>> ... let's see what I remember... +>> +>> First, Currently if we have addresses: +>> +>> "Uni Que" +>> "Uni Que" +>> +>> I presume these are thought as a separate addresses -- and an option to +>> thought these as the same would be useful. +> +> Yes, this would correspond to --unique=addrfold or --unique=nameaddrfold +> from my patch. +> +>> but let's consider second set of addresses: +>> +>> "Uni Que" +>> "Uni Keko" +>> +>> Now, if there were an option to consider these 2 as the same, that would +>> hide user from one of the names -- It is clear that "Uni Que" is the right +>> one but if only "Uni Keko" (sleepyhead, that is) is shown user don't have +>> a choice to select the right one. I am not sure what the use case for +>> "uniquing" these 2 were. +> +> For example, when you are interested in the number of people involved in +> a discussion. You care only about the address and not about the names. +> Perhaps you'd like to see only the addresses in the output and not the +> names in this case, wouldn't you? + +I meant something like the patch bellow. Unique options would be +no/yes/yes-but-case-insensitive and the type of uniqueness would be +determined by the --output flags. I don't like one thing on this +approach: --output flags now determine visibility of both rows and +columns in the output. But this is already present in master, because we +have count column there. + +-Michal + +diff --git a/notmuch-search.c b/notmuch-search.c +index 14b9f01..760f59a 100644 +--- a/notmuch-search.c ++++ b/notmuch-search.c +@@ -34,6 +34,8 @@ typedef enum { + OUTPUT_SENDER = 1 << 5, + OUTPUT_RECIPIENTS = 1 << 6, + OUTPUT_COUNT = 1 << 7, ++ OUTPUT_NAME = 1 << 8, ++ OUTPUT_ADDR = 1 << 9, + } output_t; + + typedef enum { +@@ -43,6 +45,12 @@ typedef enum { + NOTMUCH_FORMAT_SEXP + } format_sel_t; + ++typedef enum { ++ UNIQUE_NO = 0, ++ UNIQUE_YES, ++ UNIQUE_CASEFOLD, ++} unique_t; ++ + typedef struct { + notmuch_database_t *notmuch; + format_sel_t format_sel; +@@ -55,6 +63,7 @@ typedef struct { + int limit; + int dupe; + GHashTable *addresses; ++ unique_t unique; + } search_context_t; + + typedef struct { +@@ -243,18 +252,45 @@ do_search_threads (search_context_t *ctx) + return 0; + } + +-/* Returns TRUE iff name and addr is duplicate. If not, stores the +- * name/addr pair in order to detect subsequent duplicates. */ ++/* Returns TRUE iff name and/or addr is considered duplicate. If not, ++ * stores the name/addr pair in order to detect subsequent ++ * duplicates. */ + static notmuch_bool_t + is_duplicate (const search_context_t *ctx, const char *name, const char *addr) + { + notmuch_bool_t duplicate; + char *key; ++ gchar *addrfold = NULL; ++ gchar *namefold = NULL; + mailbox_t *mailbox; + +- key = talloc_asprintf (ctx->format, "%s <%s>", name, addr); ++ if (ctx->unique == UNIQUE_CASEFOLD) { ++ addrfold = g_utf8_casefold (addr, -1); ++ namefold = g_utf8_casefold (name, -1); ++ } ++ ++ switch (ctx->output & (OUTPUT_NAME | OUTPUT_ADDR)) { ++ case OUTPUT_NAME | OUTPUT_ADDR: ++ key = talloc_asprintf (ctx->format, "%s <%s>", namefold ? namefold : name, ++ addrfold ? addrfold : addr); ++ break; ++ case OUTPUT_NAME: ++ key = talloc_strdup (ctx->format, namefold ? namefold : name); /* !name results in !key */ ++ break; ++ case OUTPUT_ADDR: ++ key = talloc_strdup (ctx->format, addrfold ? addrfold : addr); ++ break; ++ default: ++ INTERNAL_ERROR("invalid --output flags"); ++ } ++ ++ if (addrfold) { ++ g_free (addrfold); ++ g_free (namefold); ++ } ++ + if (! key) +- return FALSE; ++ return TRUE; + + duplicate = g_hash_table_lookup_extended (ctx->addresses, key, NULL, (gpointer)&mailbox); + +@@ -281,6 +317,7 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox) + sprinter_t *format = ctx->format; + InternetAddress *ia = internet_address_mailbox_new (name, addr); + char *name_addr; ++ notmuch_bool_t no_name_and_addr = !(ctx->output & OUTPUT_NAME || ctx->output & OUTPUT_ADDR); /* Backward compatible behavior */ + + /* name_addr has the name part quoted if necessary. Compare + * 'John Doe ' vs. '"Doe, John" ' */ +@@ -291,16 +328,27 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox) + format->integer (format, count); + format->string (format, "\t"); + } +- format->string (format, name_addr); ++ if ((ctx->output & OUTPUT_NAME && ctx->output & OUTPUT_ADDR) || no_name_and_addr) ++ format->string (format, name_addr); ++ else if (ctx->output & OUTPUT_NAME) ++ format->string (format, name); ++ else if (ctx->output & OUTPUT_ADDR) ++ format->string (format, addr); + format->separator (format); + } else { + format->begin_map (format); +- format->map_key (format, "name"); +- format->string (format, name); +- format->map_key (format, "address"); +- format->string (format, addr); +- format->map_key (format, "name-addr"); +- format->string (format, name_addr); ++ if (ctx->output & OUTPUT_NAME || no_name_and_addr) { ++ format->map_key (format, "name"); ++ format->string (format, name); ++ } ++ if (ctx->output & OUTPUT_ADDR || no_name_and_addr) { ++ format->map_key (format, "address"); ++ format->string (format, addr); ++ } ++ if ((ctx->output & OUTPUT_NAME && ctx->output & OUTPUT_ADDR) || no_name_and_addr) { ++ format->map_key (format, "name-addr"); ++ format->string (format, name_addr); ++ } + if (count > 0) { + format->map_key (format, "count"); + format->integer (format, count); +@@ -631,6 +679,7 @@ static search_context_t search_context = { + .offset = 0, + .limit = -1, /* unlimited */ + .dupe = -1, ++ .unique = UNIQUE_NO, + }; + + static const notmuch_opt_desc_t common_options[] = { +@@ -722,11 +771,18 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[]) + (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER }, + { "recipients", OUTPUT_RECIPIENTS }, + { "count", OUTPUT_COUNT }, ++ { "name", OUTPUT_NAME }, ++ { "addr", OUTPUT_ADDR }, + { 0, 0 } } }, + { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x', + (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE }, + { "false", NOTMUCH_EXCLUDE_FALSE }, + { 0, 0 } } }, ++ { NOTMUCH_OPT_KEYWORD, &ctx->unique, "unique", 'b', ++ (notmuch_keyword_t []){ { "no", UNIQUE_NO }, ++ { "yes", UNIQUE_YES }, ++ { "casefold", UNIQUE_CASEFOLD }, ++ { 0, 0 } } }, + { NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 }, + { 0, 0, 0, 0, 0 } + }; + -- 2.26.2