From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Tomi Ollila <tomi.ollila@iki.fi>, David Bremner <david@tethera.net>,
	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>
	<m2d26yhfmk.fsf@guru.guru-group.fi>
	<87egr46qcs.fsf@steelpick.2x.cz>
	<m2h9w0ujo3.fsf@guru.guru-group.fi>
	<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
Precedence: list

On Fri, Jan 09 2015, Michal Sojka wrote:
> On Fri, Jan 09 2015, Tomi Ollila wrote:
>> On Fri, Jan 09 2015, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>>
>>> Hi,
>>>
>>> sorry for longer response time :)
>>>
>>> On Thu, Jan 01 2015, Tomi Ollila wrote:
>>>> On Wed, Dec 31 2014, David Bremner <david@tethera.net> wrote:
>>>>
>>>>> Michal Sojka <sojkam1@fel.cvut.cz> 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" <unique@example.org>
>>  "Uni Que" <Unique@Example.Org>
>>
>> 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" <unique@example.org>
>>  "Uni Keko" <unique@example.org>
>>
>> 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 <john@doe.com>' vs. '"Doe, John" <john@doe.com>' */
@@ -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 }
     };