unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/5] notmuch search --output=addresses
@ 2014-09-22  9:37 Michal Sojka
  2014-09-22  9:37 ` [PATCH 1/5] cli: Refactor option passing in the search command Michal Sojka
                   ` (4 more replies)
  0 siblings, 5 replies; 31+ messages in thread
From: Michal Sojka @ 2014-09-22  9:37 UTC (permalink / raw)
  To: notmuch

Hello all,

this patch series adds support for --output=addresses and similar
arguments in notmuch search. It is a reworked and extended version of
id:1410021689-15901-1-git-send-email-jani@nikula.org.

Compared to Jani's patches, I do not use keyword-flag command line
parser for the --output option, because this would not be useful for
most but one combination of keywords. Instead, this one combination is
explicitely mentioned as another keyword.

The flag command-line parser is introduced later, with (IMHO) better
syntax: --flags=one,two,... This feature is used to control address
deduplication via the --unique option.

Other extensions are added documentation, tests and shell completions.

The whole test suite passes with these patches.

The functionality presented here will be useful for improving Emacs
address completion (id:1411150602-21892-1-git-send-email-sojkam1@fel.cvut.cz).

Another nice use case is sending patched with git:

  git send-email --cc-cmd='nmsenders id:xxxx' ...

where nmsenders script contains:

  notmuch search --output=sender $(notmuch search --output=threads $1)



Michal Sojka (5):
  cli: Refactor option passing in the search command
  cli: Extend the search command for --output=addresses and similar
  cli: Add support for parsing command line "flag" options
  cli: Add configurable address deduplication for --output=addresses
  cli: Add tests for 'search --output=addresses' and similar

 command-line-arguments.c           |  40 +++++
 command-line-arguments.h           |   1 +
 completion/notmuch-completion.bash |   8 +-
 completion/notmuch-completion.zsh  |   4 +-
 doc/man1/notmuch-search.rst        |  55 ++++++-
 notmuch-search.c                   | 311 +++++++++++++++++++++++++++++--------
 test/Makefile.local                |   2 +-
 test/T090-search-output.sh         |  59 +++++++
 test/T095-search-unique.sh         |  63 ++++++++
 test/T410-argument-parsing.sh      |   3 +-
 test/arg-test.c                    |   8 +
 11 files changed, 482 insertions(+), 72 deletions(-)
 create mode 100755 test/T095-search-unique.sh

-- 
2.1.0

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/5] cli: Refactor option passing in the search command
  2014-09-22  9:37 [PATCH 0/5] notmuch search --output=addresses Michal Sojka
@ 2014-09-22  9:37 ` Michal Sojka
  2014-09-25 19:13   ` Tomi Ollila
  2014-09-22  9:37 ` [PATCH 2/5] cli: Extend the search command for --output=addresses and similar Michal Sojka
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 31+ messages in thread
From: Michal Sojka @ 2014-09-22  9:37 UTC (permalink / raw)
  To: notmuch

Many functions that implement the search command need to access command
line options. Instead of passing each option in a separate variable, put
them in a structure and pass only this structure.

This will become handy in the following patches.
---
 notmuch-search.c | 122 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 62 insertions(+), 60 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index bc9be45..5ac2a26 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -30,6 +30,16 @@ typedef enum {
     OUTPUT_TAGS
 } output_t;
 
+typedef struct {
+    sprinter_t *format;
+    notmuch_query_t *query;
+    notmuch_sort_t sort;
+    output_t output;
+    int offset;
+    int limit;
+    int dupe;
+} search_options_t;
+
 /* Return two stable query strings that identify exactly the matched
  * and unmatched messages currently in thread.  If there are no
  * matched or unmatched messages, the returned buffers will be
@@ -70,46 +80,42 @@ get_thread_query (notmuch_thread_t *thread,
 }
 
 static int
-do_search_threads (sprinter_t *format,
-		   notmuch_query_t *query,
-		   notmuch_sort_t sort,
-		   output_t output,
-		   int offset,
-		   int limit)
+do_search_threads (search_options_t *o)
 {
     notmuch_thread_t *thread;
     notmuch_threads_t *threads;
     notmuch_tags_t *tags;
+    sprinter_t *format = o->format;
     time_t date;
     int i;
 
-    if (offset < 0) {
-	offset += notmuch_query_count_threads (query);
-	if (offset < 0)
-	    offset = 0;
+    if (o->offset < 0) {
+	o->offset += notmuch_query_count_threads (o->query);
+	if (o->offset < 0)
+	    o->offset = 0;
     }
 
-    threads = notmuch_query_search_threads (query);
+    threads = notmuch_query_search_threads (o->query);
     if (threads == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
+	 notmuch_threads_valid (threads) && (o->limit < 0 || i < o->offset + o->limit);
 	 notmuch_threads_move_to_next (threads), i++)
     {
 	thread = notmuch_threads_get (threads);
 
-	if (i < offset) {
+	if (i < o->offset) {
 	    notmuch_thread_destroy (thread);
 	    continue;
 	}
 
-	if (output == OUTPUT_THREADS) {
+	if (o->output == OUTPUT_THREADS) {
 	    format->set_prefix (format, "thread");
 	    format->string (format,
-			    notmuch_thread_get_thread_id (thread));
+			       notmuch_thread_get_thread_id (thread));
 	    format->separator (format);
 	} else { /* output == OUTPUT_SUMMARY */
 	    void *ctx_quote = talloc_new (thread);
@@ -123,7 +129,7 @@ do_search_threads (sprinter_t *format,
 
 	    format->begin_map (format);
 
-	    if (sort == NOTMUCH_SORT_OLDEST_FIRST)
+	    if (o->sort == NOTMUCH_SORT_OLDEST_FIRST)
 		date = notmuch_thread_get_oldest_date (thread);
 	    else
 		date = notmuch_thread_get_newest_date (thread);
@@ -215,40 +221,36 @@ do_search_threads (sprinter_t *format,
 }
 
 static int
-do_search_messages (sprinter_t *format,
-		    notmuch_query_t *query,
-		    output_t output,
-		    int offset,
-		    int limit,
-		    int dupe)
+do_search_messages (search_options_t *o)
 {
     notmuch_message_t *message;
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
+    sprinter_t *format = o->format;
     int i;
 
-    if (offset < 0) {
-	offset += notmuch_query_count_messages (query);
-	if (offset < 0)
-	    offset = 0;
+    if (o->offset < 0) {
+	o->offset += notmuch_query_count_messages (o->query);
+	if (o->offset < 0)
+	    o->offset = 0;
     }
 
-    messages = notmuch_query_search_messages (query);
+    messages = notmuch_query_search_messages (o->query);
     if (messages == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit);
+	 notmuch_messages_valid (messages) && (o->limit < 0 || i < o->offset + o->limit);
 	 notmuch_messages_move_to_next (messages), i++)
     {
-	if (i < offset)
+	if (i < o->offset)
 	    continue;
 
 	message = notmuch_messages_get (messages);
 
-	if (output == OUTPUT_FILES) {
+	if (o->output == OUTPUT_FILES) {
 	    int j;
 	    filenames = notmuch_message_get_filenames (message);
 
@@ -256,7 +258,7 @@ do_search_messages (sprinter_t *format,
 		 notmuch_filenames_valid (filenames);
 		 notmuch_filenames_move_to_next (filenames), j++)
 	    {
-		if (dupe < 0 || dupe == j) {
+		if (o->dupe < 0 || o->dupe == j) {
 		    format->string (format, notmuch_filenames_get (filenames));
 		    format->separator (format);
 		}
@@ -333,16 +335,16 @@ int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
     notmuch_database_t *notmuch;
-    notmuch_query_t *query;
+    search_options_t o = {
+	.sort = NOTMUCH_SORT_NEWEST_FIRST,
+	.output = OUTPUT_SUMMARY,
+	.offset = 0,
+	.limit = -1, /* unlimited */
+	.dupe = -1,
+    };
     char *query_str;
-    notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;
-    sprinter_t *format = NULL;
     int opt_index, ret;
-    output_t output = OUTPUT_SUMMARY;
-    int offset = 0;
-    int limit = -1; /* unlimited */
     notmuch_exclude_t exclude = NOTMUCH_EXCLUDE_TRUE;
-    int dupe = -1;
     unsigned int i;
 
     enum {
@@ -353,7 +355,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     } format_sel = NOTMUCH_FORMAT_TEXT;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',
+	{ NOTMUCH_OPT_KEYWORD, &o.sort, "sort", 's',
 	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
 				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
 				  { 0, 0 } } },
@@ -364,7 +366,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "text0", NOTMUCH_FORMAT_TEXT0 },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
+	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
@@ -377,9 +379,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
                                   { "flag", NOTMUCH_EXCLUDE_FLAG },
                                   { "all", NOTMUCH_EXCLUDE_ALL },
                                   { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
-	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
-	{ NOTMUCH_OPT_INT, &dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_INT, &o.offset, "offset", 'O', 0 },
+	{ NOTMUCH_OPT_INT, &o.limit, "limit", 'L', 0  },
+	{ NOTMUCH_OPT_INT, &o.dupe, "duplicate", 'D', 0  },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -389,20 +391,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	format = sprinter_text_create (config, stdout);
+	o.format = sprinter_text_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_TEXT0:
-	if (output == OUTPUT_SUMMARY) {
+	if (o.output == OUTPUT_SUMMARY) {
 	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
 	    return EXIT_FAILURE;
 	}
-	format = sprinter_text0_create (config, stdout);
+	o.format = sprinter_text0_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	format = sprinter_json_create (config, stdout);
+	o.format = sprinter_json_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_SEXP:
-	format = sprinter_sexp_create (config, stdout);
+	o.format = sprinter_sexp_create (config, stdout);
 	break;
     default:
 	/* this should never happen */
@@ -425,15 +427,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    query = notmuch_query_create (notmuch, query_str);
-    if (query == NULL) {
+    o.query = notmuch_query_create (notmuch, query_str);
+    if (o.query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
     }
 
-    notmuch_query_set_sort (query, sort);
+    notmuch_query_set_sort (o.query, o.sort);
 
-    if (exclude == NOTMUCH_EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {
+    if (exclude == NOTMUCH_EXCLUDE_FLAG && o.output != OUTPUT_SUMMARY) {
 	/* If we are not doing summary output there is nowhere to
 	 * print the excluded flag so fall back on including the
 	 * excluded messages. */
@@ -448,29 +450,29 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
 	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
-	notmuch_query_set_omit_excluded (query, exclude);
+	    notmuch_query_add_tag_exclude (o.query, search_exclude_tags[i]);
+	notmuch_query_set_omit_excluded (o.query, exclude);
     }
 
-    switch (output) {
+    switch (o.output) {
     default:
     case OUTPUT_SUMMARY:
     case OUTPUT_THREADS:
-	ret = do_search_threads (format, query, sort, output, offset, limit);
+	ret = do_search_threads (&o);
 	break;
     case OUTPUT_MESSAGES:
     case OUTPUT_FILES:
-	ret = do_search_messages (format, query, output, offset, limit, dupe);
+	ret = do_search_messages (&o);
 	break;
     case OUTPUT_TAGS:
-	ret = do_search_tags (notmuch, format, query);
+	ret = do_search_tags (notmuch, o.format, o.query);
 	break;
     }
 
-    notmuch_query_destroy (query);
+    notmuch_query_destroy (o.query);
     notmuch_database_destroy (notmuch);
 
-    talloc_free (format);
+    talloc_free (o.format);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 2/5] cli: Extend the search command for --output=addresses and similar
  2014-09-22  9:37 [PATCH 0/5] notmuch search --output=addresses Michal Sojka
  2014-09-22  9:37 ` [PATCH 1/5] cli: Refactor option passing in the search command Michal Sojka
@ 2014-09-22  9:37 ` Michal Sojka
  2014-09-22  9:37 ` [PATCH 3/5] cli: Add support for parsing command line "flag" options Michal Sojka
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-09-22  9:37 UTC (permalink / raw)
  To: notmuch

The new outputs allow printing senders, recipients or both of matching
messages.

This code based on a patch from Jani Nikula.
---
 completion/notmuch-completion.bash |   2 +-
 completion/notmuch-completion.zsh  |   3 +-
 doc/man1/notmuch-search.rst        |  22 +++++++-
 notmuch-search.c                   | 100 ++++++++++++++++++++++++++++++++++---
 4 files changed, 118 insertions(+), 9 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 0571dc9..c37ddf5 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -294,7 +294,7 @@ _notmuch_search()
 	    return
 	    ;;
 	--output)
-	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients addresses" -- "${cur}" ) )
 	    return
 	    ;;
 	--sort)
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index 67a9aba..bff8fd5 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -52,7 +52,8 @@ _notmuch_search()
   _arguments -s : \
     '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
     '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
-    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
+    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
+    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index 90160f2..6094906 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -35,7 +35,7 @@ Supported options for **search** include
         intended for programs that invoke **notmuch(1)** internally. If
         omitted, the latest supported version will be used.
 
-    ``--output=(summary|threads|messages|files|tags)``
+    ``--output=(summary|threads|messages|files|tags|sender|recipients|addresses)``
 
         **summary**
             Output a summary of each thread with any message matching
@@ -78,6 +78,26 @@ Supported options for **search** include
             by null characters (--format=text0), as a JSON array
             (--format=json), or as an S-Expression list (--format=sexp).
 
+	**sender**
+            Output all addresses from the *From* header that appear on
+            any message matching the search terms, either one per line
+            (--format=text), separated by null characters
+            (--format=text0), as a JSON array (--format=json), or as
+            an S-Expression list (--format=sexp).
+
+	    Note: Searching for **sender** should much be faster than
+	    searching for **recipients** or **addresses**, because
+	    sender addresses are cached directly in the database
+	    whereas other addresses need to be fetched from the
+	    message file by parsing it.
+
+	**recipients**
+            Like **sender** but for addresses from *To*, *Cc* and
+	    *Bcc* headers.
+
+	**addresses**
+	    Like **sender** and **recipients** together.
+
     ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
         This option can be used to present results in either
         chronological order (**oldest-first**) or reverse chronological
diff --git a/notmuch-search.c b/notmuch-search.c
index 5ac2a26..0614f10 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -23,11 +23,14 @@
 #include "string-util.h"
 
 typedef enum {
-    OUTPUT_SUMMARY,
-    OUTPUT_THREADS,
-    OUTPUT_MESSAGES,
-    OUTPUT_FILES,
-    OUTPUT_TAGS
+    OUTPUT_SUMMARY	= 1 << 0,
+    OUTPUT_THREADS	= 1 << 1,
+    OUTPUT_MESSAGES	= 1 << 2,
+    OUTPUT_FILES	= 1 << 3,
+    OUTPUT_TAGS		= 1 << 4,
+    OUTPUT_SENDER	= 1 << 5,
+    OUTPUT_RECIPIENTS	= 1 << 6,
+    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
 } output_t;
 
 typedef struct {
@@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
     return 0;
 }
 
+static void
+print_address_list (const search_options_t *o, InternetAddressList *list)
+{
+    InternetAddress *address;
+    int i;
+
+    for (i = 0; i < internet_address_list_length (list); i++) {
+	address = internet_address_list_get_address (list, i);
+	if (INTERNET_ADDRESS_IS_GROUP (address)) {
+	    InternetAddressGroup *group;
+	    InternetAddressList *group_list;
+
+	    group = INTERNET_ADDRESS_GROUP (address);
+	    group_list = internet_address_group_get_members (group);
+	    if (group_list == NULL)
+		continue;
+
+	    print_address_list (o, group_list);
+	} else {
+	    InternetAddressMailbox *mailbox;
+	    const char *name;
+	    const char *addr;
+	    char *full_address;
+
+	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
+
+	    name = internet_address_get_name (address);
+	    addr = internet_address_mailbox_get_addr (mailbox);
+
+	    if (name && *name)
+		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
+	    else
+		full_address = talloc_strdup (o->format, addr);
+
+	    if (!full_address) {
+		fprintf (stderr, "Error: out of memory\n");
+		break;
+	    }
+	    o->format->string (o->format, full_address);
+	    o->format->separator (o->format);
+
+	    talloc_free (full_address);
+	}
+    }
+}
+
+static void
+print_address_string (const search_options_t *o, const char *recipients)
+{
+    InternetAddressList *list;
+
+    if (recipients == NULL)
+	return;
+
+    list = internet_address_list_parse_string (recipients);
+    if (list == NULL)
+	return;
+
+    print_address_list (o, list);
+}
+
 static int
 do_search_messages (search_options_t *o)
 {
@@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
 	    
 	    notmuch_filenames_destroy( filenames );
 
-	} else { /* output == OUTPUT_MESSAGES */
+	} else if (o->output == OUTPUT_MESSAGES) {
 	    format->set_prefix (format, "id");
 	    format->string (format,
 			    notmuch_message_get_message_id (message));
 	    format->separator (format);
+	} else {
+	    if (o->output & OUTPUT_SENDER) {
+		const char *addrs;
+
+		addrs = notmuch_message_get_header (message, "from");
+		print_address_string (o, addrs);
+	    }
+
+	    if (o->output & OUTPUT_RECIPIENTS) {
+		const char *hdrs[] = { "to", "cc", "bcc" };
+		const char *addrs;
+		size_t j;
+
+		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
+		    addrs = notmuch_message_get_header (message, hdrs[j]);
+		    print_address_string (o, addrs);
+		}
+	    }
 	}
 
 	notmuch_message_destroy (message);
@@ -370,6 +452,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
+				  { "sender", OUTPUT_SENDER },
+				  { "recipients", OUTPUT_RECIPIENTS },
+				  { "addresses", OUTPUT_ADDRESSES },
 				  { "files", OUTPUT_FILES },
 				  { "tags", OUTPUT_TAGS },
 				  { 0, 0 } } },
@@ -461,6 +546,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	ret = do_search_threads (&o);
 	break;
     case OUTPUT_MESSAGES:
+    case OUTPUT_SENDER:
+    case OUTPUT_RECIPIENTS:
+    case OUTPUT_ADDRESSES:
     case OUTPUT_FILES:
 	ret = do_search_messages (&o);
 	break;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 3/5] cli: Add support for parsing command line "flag" options
  2014-09-22  9:37 [PATCH 0/5] notmuch search --output=addresses Michal Sojka
  2014-09-22  9:37 ` [PATCH 1/5] cli: Refactor option passing in the search command Michal Sojka
  2014-09-22  9:37 ` [PATCH 2/5] cli: Extend the search command for --output=addresses and similar Michal Sojka
@ 2014-09-22  9:37 ` Michal Sojka
  2014-09-22  9:37 ` [PATCH 4/5] cli: Add configurable address deduplication for --output=addresses Michal Sojka
  2014-09-22  9:37 ` [PATCH 5/5] cli: Add tests for 'search --output=addresses' and similar Michal Sojka
  4 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-09-22  9:37 UTC (permalink / raw)
  To: notmuch

This allows having multiple flags as a value of a command line
option (e.g. --foo=bar,baz). The values of the given flags are OR'd
together.

This was inspired by a similar patch from Jani Nikula.
---
 command-line-arguments.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 command-line-arguments.h      |  1 +
 test/Makefile.local           |  2 +-
 test/T410-argument-parsing.sh |  3 ++-
 test/arg-test.c               |  8 ++++++++
 5 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 844d6c3..50723e4 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -3,6 +3,7 @@
 #include <stdio.h>
 #include "error_util.h"
 #include "command-line-arguments.h"
+#include "string-util.h"
 
 /*
   Search the array of keywords for a given argument, assigning the
@@ -36,6 +37,43 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
     return FALSE;
 }
 
+/*
+  An arguments composed of comma-separated flags is parsed and the
+  output variable is assigned the ORed flag values. Return FALSE in
+  case of error.
+*/
+static notmuch_bool_t
+_process_flags_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
+
+    const notmuch_keyword_t *keyword;
+    const char *flag = arg_str;
+    size_t flen = 0;
+    notmuch_bool_t match;
+
+    if (next == '\0' || *arg_str == '\0') {
+	/* No flag given */
+	fprintf (stderr, "Option \"%s\" needs an flags argument.\n", arg_desc->name);
+	return FALSE;
+    }
+
+    while ((flag = strtok_len_c (flag + flen, ",", &flen))) {
+	for (keyword = arg_desc->keywords, match = FALSE; keyword->name; keyword++) {
+	    if (strncmp (flag, keyword->name, flen) == 0 &&
+		flen == strlen (keyword->name)) {
+		*((int *)arg_desc->output_var) |= keyword->value;
+		match = TRUE;
+		break;
+	    }
+	}
+	if (! match) {
+	    fprintf (stderr, "Unknown flag argument \"%.*s\" for option \"%s\".\n", (int)flen, flag, arg_desc->name);
+	    return FALSE;
+	}
+    }
+    return TRUE;
+}
+
+
 static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
 
@@ -153,6 +191,8 @@ parse_option (const char *arg,
 	switch (try->opt_type) {
 	case NOTMUCH_OPT_KEYWORD:
 	    return _process_keyword_arg (try, next, value);
+	case NOTMUCH_OPT_FLAGS:
+	    return _process_flags_arg (try, next, value);
 	case NOTMUCH_OPT_BOOLEAN:
 	    return _process_boolean_arg (try, next, value);
 	case NOTMUCH_OPT_INT:
diff --git a/command-line-arguments.h b/command-line-arguments.h
index de1734a..192ce06 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -8,6 +8,7 @@ enum notmuch_opt_type {
     NOTMUCH_OPT_BOOLEAN,	/* --verbose              */
     NOTMUCH_OPT_INT,		/* --frob=8               */
     NOTMUCH_OPT_KEYWORD,	/* --format=raw|json|text */
+    NOTMUCH_OPT_FLAGS,  	/* --flags=name,addr,casefold */
     NOTMUCH_OPT_STRING,		/* --file=/tmp/gnarf.txt  */
     NOTMUCH_OPT_POSITION	/* notmuch dump pos_arg   */
 };
diff --git a/test/Makefile.local b/test/Makefile.local
index a2d58fc..efa3410 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -13,7 +13,7 @@ smtp_dummy_srcs =		\
 smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 
 $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a
-	$(call quiet,CC) $^ -o $@
+	$(call quiet,CC) $^ -o $@ -ltalloc
 
 $(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
 	$(call quiet,CC) $^ $(TALLOC_LDFLAGS) -o $@
diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 94e9087..9c4cc84 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -3,9 +3,10 @@ test_description="argument parsing"
 . ./test-lib.sh
 
 test_begin_subtest "sanity check"
-$TEST_DIRECTORY/arg-test  pos1  --keyword=one --string=foo pos2 --int=7 > OUTPUT
+$TEST_DIRECTORY/arg-test  pos1  --keyword=one --flags=one,two --string=foo pos2 --int=7 > OUTPUT
 cat <<EOF > EXPECTED
 keyword 1
+flags 3
 int 7
 string foo
 positional arg 1 pos1
diff --git a/test/arg-test.c b/test/arg-test.c
index 6c49eac..0d75fe7 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -7,6 +7,7 @@ int main(int argc, char **argv){
     int opt_index=1;
 
     int kw_val=0;
+    int fl_val=0;
     int int_val=0;
     char *pos_arg1=NULL;
     char *pos_arg2=NULL;
@@ -17,6 +18,10 @@ int main(int argc, char **argv){
 	  (notmuch_keyword_t []){ { "one", 1 },
 				  { "two", 2 },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_FLAGS, &fl_val, "flags", 'f',
+	  (notmuch_keyword_t []){ { "one", 1 << 0},
+				  { "two", 1 << 1 },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &int_val, "int", 'i', 0},
 	{ NOTMUCH_OPT_STRING, &string_val, "string", 's', 0},
 	{ NOTMUCH_OPT_POSITION, &pos_arg1, 0,0, 0},
@@ -31,6 +36,9 @@ int main(int argc, char **argv){
     if (kw_val)
 	printf("keyword %d\n", kw_val);
 
+    if (fl_val)
+	printf("flags %d\n", fl_val);
+
     if (int_val)
 	printf("int %d\n", int_val);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 4/5] cli: Add configurable address deduplication for --output=addresses
  2014-09-22  9:37 [PATCH 0/5] notmuch search --output=addresses Michal Sojka
                   ` (2 preceding siblings ...)
  2014-09-22  9:37 ` [PATCH 3/5] cli: Add support for parsing command line "flag" options Michal Sojka
@ 2014-09-22  9:37 ` Michal Sojka
  2014-09-22  9:37 ` [PATCH 5/5] cli: Add tests for 'search --output=addresses' and similar Michal Sojka
  4 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-09-22  9:37 UTC (permalink / raw)
  To: notmuch

The code here is an extended version of a path from Jani Nikula.
---
 completion/notmuch-completion.bash |   6 ++-
 completion/notmuch-completion.zsh  |   3 +-
 doc/man1/notmuch-search.rst        |  33 ++++++++++++
 notmuch-search.c                   | 101 ++++++++++++++++++++++++++++++++++---
 4 files changed, 135 insertions(+), 8 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index c37ddf5..8bc7874 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -305,12 +305,16 @@ _notmuch_search()
 	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
 	    return
 	    ;;
+	--unique)
+	    COMPREPLY=( $( compgen -W "none addr addrfold name" -- "${cur}" ) )
+	    return
+	    ;;
     esac
 
     ! $split &&
     case "${cur}" in
 	-*)
-	    local options="--format= --output= --sort= --offset= --limit= --exclude= --duplicate="
+	    local options="--format= --output= --sort= --offset= --limit= --exclude= --duplicate= --unique="
 	    compopt -o nospace
 	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
 	    ;;
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index bff8fd5..cf4968c 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -53,7 +53,8 @@ _notmuch_search()
     '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
     '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
     '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
-    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))'
+    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))' \
+    '--unique=[address deduplication]:unique:((none\:"no deduplication" addr\:"deduplicate by address" addrfold\:"deduplicate by case-insensitive address" name\:"deduplicate by name"))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index 6094906..a92779a 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -85,6 +85,9 @@ Supported options for **search** include
             (--format=text0), as a JSON array (--format=json), or as
             an S-Expression list (--format=sexp).
 
+            Handling of duplicate addresses and/or names can be
+            controlled with the --unique option.
+
 	    Note: Searching for **sender** should much be faster than
 	    searching for **recipients** or **addresses**, because
 	    sender addresses are cached directly in the database
@@ -151,6 +154,36 @@ Supported options for **search** include
         prefix. The prefix matches messages based on filenames. This
         option filters filenames of the matching messages.
 
+    ``--unique=``\ (**none**\ \|\ **addr**\ \|\ **addrfold**\ \|\ **name**)[,\ ...]
+
+        Can be used with ``--output=addresses``, ``--output=sender``
+        or ``--output=recipients`` to control the address
+        deduplication algorithm.
+
+	**none** means that no deduplication is performed. The same
+	address can appear multiple times in the output.
+
+	**addr** means that case-sensitive deduplication is performed
+	on the address part. For example, given the addresses "John
+	Doe <john@example.com>" and "Dr. John Doe <john@example.com>",
+	only one will be printed.
+
+	**addrfold** means that case-insensitive deduplication is
+	performed on the address part. For example, given the
+	addresses "John Doe <john@example.com>" and "John Doe
+	<JOHN@EXAMPLE.COM>", only one will be printed. This is the
+	default.
+
+	**name** means that case-sensitive deduplication is performed
+	on the name part. For example, given the addresses "John Doe
+	<john@example.com>" and "John Doe <john@doe.name>", only one
+	will be printed.
+
+	It is possible to combine the above flags (except **none**) by
+	separating them with comma. For example,
+	``--unique=name,addr`` will print unique case-sensitive
+	combinations of name and address.
+
 EXIT STATUS
 ===========
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 0614f10..00d6771 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -33,6 +33,15 @@ typedef enum {
     OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
 } output_t;
 
+typedef enum {
+    UNIQUE_NONE 	  = 1 << 0,
+    UNIQUE_ADDR 	  = 1 << 1,
+    UNIQUE_NAME 	  = 1 << 2,
+    UNIQUE_ADDR_CASEFOLD  = 1 << 3,
+
+    UNIQUE_BOTH = UNIQUE_NAME | UNIQUE_ADDR,
+} unique_t;
+
 typedef struct {
     sprinter_t *format;
     notmuch_query_t *query;
@@ -41,6 +50,7 @@ typedef struct {
     int offset;
     int limit;
     int dupe;
+    unique_t unique;
 } search_options_t;
 
 /* Return two stable query strings that identify exactly the matched
@@ -223,8 +233,51 @@ do_search_threads (search_options_t *o)
     return 0;
 }
 
+/* Returns TRUE iff name and/or addr is considered unique. */
+static notmuch_bool_t
+check_unique (const search_options_t *o, GHashTable *addrs, const char *name, const char *addr)
+{
+    notmuch_bool_t unique;
+    char *key;
+
+    if (o->unique == UNIQUE_NONE)
+	return TRUE;
+
+    if (o->unique & UNIQUE_ADDR_CASEFOLD) {
+	gchar *folded = g_utf8_casefold (addr, -1);
+	addr = talloc_strdup (o->format, folded);
+	g_free (folded);
+    }
+    switch (o->unique & UNIQUE_BOTH) {
+    case UNIQUE_NAME:
+	key = talloc_strdup (o->format, name); /* !name results in !key */
+	break;
+    case UNIQUE_ADDR:
+	key = talloc_strdup (o->format, addr);
+	break;
+    case UNIQUE_BOTH:
+	key = talloc_asprintf (o->format, "%s <%s>", name, addr);
+	break;
+    default:
+	INTERNAL_ERROR("invalid --unique flags");
+    }
+
+    if (! key)
+	return FALSE;
+
+    unique = !g_hash_table_lookup_extended (addrs, key, NULL, NULL);
+
+    if (unique)
+	g_hash_table_insert (addrs, key, NULL);
+    else
+	talloc_free (key);
+
+    return unique;
+}
+
 static void
-print_address_list (const search_options_t *o, InternetAddressList *list)
+print_address_list (const search_options_t *o, GHashTable *addrs,
+		    InternetAddressList *list)
 {
     InternetAddress *address;
     int i;
@@ -240,7 +293,7 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 	    if (group_list == NULL)
 		continue;
 
-	    print_address_list (o, group_list);
+	    print_address_list (o, addrs, group_list);
 	} else {
 	    InternetAddressMailbox *mailbox;
 	    const char *name;
@@ -252,6 +305,9 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 	    name = internet_address_get_name (address);
 	    addr = internet_address_mailbox_get_addr (mailbox);
 
+	    if (!check_unique (o, addrs, name, addr))
+		continue;
+
 	    if (name && *name)
 		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
 	    else
@@ -270,7 +326,7 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 }
 
 static void
-print_address_string (const search_options_t *o, const char *recipients)
+print_address_string (const search_options_t *o, GHashTable *addrs, const char *recipients)
 {
     InternetAddressList *list;
 
@@ -281,7 +337,13 @@ print_address_string (const search_options_t *o, const char *recipients)
     if (list == NULL)
 	return;
 
-    print_address_list (o, list);
+    print_address_list (o, addrs, list);
+}
+
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+    talloc_free (ptr);
 }
 
 static int
@@ -291,8 +353,13 @@ do_search_messages (search_options_t *o)
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
     sprinter_t *format = o->format;
+    GHashTable *addresses = NULL;
     int i;
 
+    if (o->output & OUTPUT_ADDRESSES)
+	addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
+					   _my_talloc_free_for_g_hash, NULL);
+
     if (o->offset < 0) {
 	o->offset += notmuch_query_count_messages (o->query);
 	if (o->offset < 0)
@@ -340,7 +407,7 @@ do_search_messages (search_options_t *o)
 		const char *addrs;
 
 		addrs = notmuch_message_get_header (message, "from");
-		print_address_string (o, addrs);
+		print_address_string (o, addresses, addrs);
 	    }
 
 	    if (o->output & OUTPUT_RECIPIENTS) {
@@ -350,7 +417,7 @@ do_search_messages (search_options_t *o)
 
 		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
 		    addrs = notmuch_message_get_header (message, hdrs[j]);
-		    print_address_string (o, addrs);
+		    print_address_string (o, addresses, addrs);
 		}
 	    }
 	}
@@ -358,6 +425,9 @@ do_search_messages (search_options_t *o)
 	notmuch_message_destroy (message);
     }
 
+    if (addresses)
+	g_hash_table_unref (addresses);
+
     notmuch_messages_destroy (messages);
 
     format->end (format);
@@ -423,6 +493,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	.offset = 0,
 	.limit = -1, /* unlimited */
 	.dupe = -1,
+	.unique = 0,
     };
     char *query_str;
     int opt_index, ret;
@@ -467,6 +538,12 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	{ NOTMUCH_OPT_INT, &o.offset, "offset", 'O', 0 },
 	{ NOTMUCH_OPT_INT, &o.limit, "limit", 'L', 0  },
 	{ NOTMUCH_OPT_INT, &o.dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_FLAGS, &o.unique, "unique", 'u',
+	  (notmuch_keyword_t []){ { "none", UNIQUE_NONE },
+				  { "name", UNIQUE_NAME },
+				  { "addr", UNIQUE_ADDR },
+				  { "addrfold", UNIQUE_ADDR | UNIQUE_ADDR_CASEFOLD },
+				  { 0, 0 } } },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -474,6 +551,18 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
+    if (o.unique && (o.output & ~OUTPUT_ADDRESSES)) {
+	fprintf (stderr, "Error: --unique can only be used with address output.\n");
+	return EXIT_FAILURE;
+    }
+    if ((o.unique & UNIQUE_NONE) &&
+	(o.unique & ~UNIQUE_NONE)) {
+	fprintf (stderr, "Error: --unique=none cannot be combined with other flags.\n");
+	return EXIT_FAILURE;
+    }
+    if (! o.unique)
+	o.unique = UNIQUE_ADDR | UNIQUE_ADDR_CASEFOLD;
+
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
 	o.format = sprinter_text_create (config, stdout);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/5] cli: Add tests for 'search --output=addresses' and similar
  2014-09-22  9:37 [PATCH 0/5] notmuch search --output=addresses Michal Sojka
                   ` (3 preceding siblings ...)
  2014-09-22  9:37 ` [PATCH 4/5] cli: Add configurable address deduplication for --output=addresses Michal Sojka
@ 2014-09-22  9:37 ` Michal Sojka
  4 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-09-22  9:37 UTC (permalink / raw)
  To: notmuch

---
 test/T090-search-output.sh | 59 +++++++++++++++++++++++++++++++++++++++++++
 test/T095-search-unique.sh | 63 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+)
 create mode 100755 test/T095-search-unique.sh

diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
index 947d572..ebc8c37 100755
--- a/test/T090-search-output.sh
+++ b/test/T090-search-output.sh
@@ -387,6 +387,65 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "--output=sender"
+notmuch search --output=sender '*' | sort >OUTPUT
+cat <<EOF >EXPECTED
+Adrian Perez de Castro <aperez@igalia.com>
+Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+Aron Griffis <agriffis@n01se.net>
+Carl Worth <cworth@cworth.org>
+Chris Wilson <chris@chris-wilson.co.uk>
+François Boulogne <boulogne.f@gmail.com>
+Ingmar Vanhassel <ingmar@exherbo.org>
+Israel Herraiz <isra@herraiz.org>
+Jan Janak <jan@ryngle.com>
+Jjgod Jiang <gzjjgod@gmail.com>
+Keith Packard <keithp@keithp.com>
+Lars Kellogg-Stedman <lars@seas.harvard.edu>
+Mikhail Gusarov <dottedmag@dottedmag.net>
+Olivier Berger <olivier.berger@it-sudparis.eu>
+Rolland Santimano <rollandsantimano@yahoo.com>
+Stewart Smith <stewart@flamingspork.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients"
+notmuch search --output=recipients '*' | sort >OUTPUT
+cat <<EOF >EXPECTED
+Allan McRae <allan@archlinux.org>
+Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
+Keith Packard <keithp@keithp.com>
+Mikhail Gusarov <dottedmag@dottedmag.net>
+notmuch@notmuchmail.org
+olivier.berger@it-sudparis.eu
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=addresses"
+notmuch search --output=addresses '*' | sort >OUTPUT
+cat <<EOF >EXPECTED
+Adrian Perez de Castro <aperez@igalia.com>
+Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+Allan McRae <allan@archlinux.org>
+Aron Griffis <agriffis@n01se.net>
+Carl Worth <cworth@cworth.org>
+Chris Wilson <chris@chris-wilson.co.uk>
+Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
+François Boulogne <boulogne.f@gmail.com>
+Ingmar Vanhassel <ingmar@exherbo.org>
+Israel Herraiz <isra@herraiz.org>
+Jan Janak <jan@ryngle.com>
+Jjgod Jiang <gzjjgod@gmail.com>
+Keith Packard <keithp@keithp.com>
+Lars Kellogg-Stedman <lars@seas.harvard.edu>
+Mikhail Gusarov <dottedmag@dottedmag.net>
+Olivier Berger <olivier.berger@it-sudparis.eu>
+Rolland Santimano <rollandsantimano@yahoo.com>
+Stewart Smith <stewart@flamingspork.com>
+notmuch@notmuchmail.org
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
 add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
 	headers'"
diff --git a/test/T095-search-unique.sh b/test/T095-search-unique.sh
new file mode 100755
index 0000000..8fd8fc0
--- /dev/null
+++ b/test/T095-search-unique.sh
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+test_description='address deduplication in "notmuch search --output=addresses"'
+. ./test-lib.sh
+
+add_message '[to]="Real Name <foo@example.com>, Real Name <bar@example.com>"'
+add_message '[to]="Nickname <foo@example.com>"' '[cc]="Real Name <Bar@Example.COM>"'
+add_message '[to]="Nickname <foo@example.com>"' '[bcc]="Real Name <Bar@Example.COM>"'
+
+test_begin_subtest "--output=recipients"
+notmuch search --output=recipients "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=none"
+notmuch search --output=recipients --unique=none "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Nickname <foo@example.com>
+Real Name <Bar@Example.COM>
+Nickname <foo@example.com>
+Real Name <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=addr"
+notmuch search --output=recipients --unique=addr "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Real Name <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=addrfold"
+notmuch search --output=recipients --unique=addrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=name"
+notmuch search --output=recipients --unique=name "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Nickname <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=name,addrfold"
+notmuch search --output=recipients --unique=name,addrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Nickname <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/5] cli: Refactor option passing in the search command
  2014-09-22  9:37 ` [PATCH 1/5] cli: Refactor option passing in the search command Michal Sojka
@ 2014-09-25 19:13   ` Tomi Ollila
  2014-09-25 20:48     ` Michal Sojka
  0 siblings, 1 reply; 31+ messages in thread
From: Tomi Ollila @ 2014-09-25 19:13 UTC (permalink / raw)
  To: Michal Sojka, notmuch

On Mon, Sep 22 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

> Many functions that implement the search command need to access command
> line options. Instead of passing each option in a separate variable, put
> them in a structure and pass only this structure.

This patch looks good to me. 

Although the test and the implementation in the next patches look OK, I'd
prefer the FLAG implementation Jani suggested earlier. IMO now that I
compare these two it looks cleaner and simpler...

I.e. I'd prefer notmuch search --output=sender --output=recipients ...
(same output regardless the order these options given).

I'd postpone the unique handling to a bit later phase; there are quite a
few options how to do that (*)


Tomi

(*) IMO the default unique (when requested) would be exact case-sensitive
match of full name & address parts (phrase, address & comment); then
(a subset of possible) options could be: 
   +) case-insensitive (first match taken (or last match?) -- option?)
   +) unique email addresses (take phrase/comment from first/last?)
      --  or use first that has something additional to plain address
      --  or use last  that has something additional to plain address


> This will become handy in the following patches.
> ---
>  notmuch-search.c | 122 ++++++++++++++++++++++++++++---------------------------
>  1 file changed, 62 insertions(+), 60 deletions(-)
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index bc9be45..5ac2a26 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/5] cli: Refactor option passing in the search command
  2014-09-25 19:13   ` Tomi Ollila
@ 2014-09-25 20:48     ` Michal Sojka
  2014-09-29 16:04       ` Tomi Ollila
  2014-09-29 17:28       ` Jani Nikula
  0 siblings, 2 replies; 31+ messages in thread
From: Michal Sojka @ 2014-09-25 20:48 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

On Thu, Sep 25 2014, Tomi Ollila wrote:
> On Mon, Sep 22 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>
>> Many functions that implement the search command need to access command
>> line options. Instead of passing each option in a separate variable, put
>> them in a structure and pass only this structure.
>
> This patch looks good to me.

Thanks for the review.

> Although the test and the implementation in the next patches look OK, I'd
> prefer the FLAG implementation Jani suggested earlier. IMO now that I
> compare these two it looks cleaner and simpler...

The question is which kind of simplicity you have in mind. I think that
my version is simpler to type (less keystrokes). But if others have
different opinion, I don't mind.

> I.e. I'd prefer notmuch search --output=sender --output=recipients ...
> (same output regardless the order these options given).

This should be the case with both implementations.

> I'd postpone the unique handling to a bit later phase; there are quite a
> few options how to do that (*)
>
>
> Tomi
>
> (*) IMO the default unique (when requested) would be exact case-sensitive
> match of full name & address 

Why do you think that case-sensitive address matching should be the
default? In theory local-part can be case sensitive, but I've never seen
that in reality. So this default would only be useful if you want to
research how people type your email address :)

> parts (phrase, address & comment); 

What do you mean by phrase and comment? Address syntax is defined by
http://tools.ietf.org/html/rfc5322#section-3.4.1.

> then (a subset of possible) options could be:
>    +) case-insensitive (first match taken (or last match?) -- option?)
>    +) unique email addresses (take phrase/comment from first/last?)
>       --  or use first that has something additional to plain address
>       --  or use last  that has something additional to plain address

Yes, there is a lot of possible options. I don't think that notmuch has
to support all of them. If people need something special like "use last
that has something additional to plain address", they can always do
--unique=none and do their own post-processing.

-Michal

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/5] cli: Refactor option passing in the search command
  2014-09-25 20:48     ` Michal Sojka
@ 2014-09-29 16:04       ` Tomi Ollila
  2014-09-29 17:28       ` Jani Nikula
  1 sibling, 0 replies; 31+ messages in thread
From: Tomi Ollila @ 2014-09-29 16:04 UTC (permalink / raw)
  To: Michal Sojka, notmuch

On Thu, Sep 25 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

> On Thu, Sep 25 2014, Tomi Ollila wrote:
>> On Mon, Sep 22 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>>
>>> Many functions that implement the search command need to access command
>>> line options. Instead of passing each option in a separate variable, put
>>> them in a structure and pass only this structure.
>>
>> This patch looks good to me.
>
> Thanks for the review.
>
>> Although the test and the implementation in the next patches look OK, I'd
>> prefer the FLAG implementation Jani suggested earlier. IMO now that I
>> compare these two it looks cleaner and simpler...
>
> The question is which kind of simplicity you have in mind. I think that
> my version is simpler to type (less keystrokes). But if others have
> different opinion, I don't mind.

Less keystrokes for sure -- but these interfaces are usually accessed
programmatically... :D

>>
>> Tomi
>>
>> (*) IMO the default unique (when requested) would be exact case-sensitive
>> match of full name & address 
>
> Why do you think that case-sensitive address matching should be the
> default? In theory local-part can be case sensitive, but I've never seen
> that in reality. So this default would only be useful if you want to
> research how people type your email address :)

Well, in short, I think the lowest level of uniqueness should be simple
string match, and this should at least be available if not default --
to the extent gmime provides (maybe that is this way in your patch...),

...and therefore I'd like to have this address output solved first, then
we can experiment with the outputs provided and have better-educated
comments on this issue...

>> parts (phrase, address & comment); 
>
> What do you mean by phrase and comment? Address syntax is defined by
> http://tools.ietf.org/html/rfc5322#section-3.4.1.

in 

"Foo Bar" (company/city) foo.bar@example.org

and

"Foo Bar" foo.bar@example.org (company/city) 

Phrase would be "Foo Bar"
Address foo.bar@example.org
and comment (company/city)

As a side note, nottoomuch-addresses does some heuristics there, and think
the 2 options above are equal (as "Phrase" (comment) address) -- which
might the same InternetAddressMailbox provides :O

Also, it seems that nottoomuch-addresses lowercases 'address' for
comparison and storage ... I am not entirely sure whether I should provide
options to disable these heuristics -- if someone asks for the feature
then I probably will do :D

>> then (a subset of possible) options could be:
>>    +) case-insensitive (first match taken (or last match?) -- option?)
>>    +) unique email addresses (take phrase/comment from first/last?)
>>       --  or use first that has something additional to plain address
>>       --  or use last  that has something additional to plain address
>
> Yes, there is a lot of possible options. I don't think that notmuch has
> to support all of them. If people need something special like "use last
> that has something additional to plain address", they can always do
> --unique=none and do their own post-processing.

Ok, but something (we can further bikeshed with) needs to be selected :D

>
> -Michal

Tomi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/5] cli: Refactor option passing in the search command
  2014-09-25 20:48     ` Michal Sojka
  2014-09-29 16:04       ` Tomi Ollila
@ 2014-09-29 17:28       ` Jani Nikula
  2014-10-05 20:51         ` [PATCH v2 0/4] notmuch search --output=addresses Michal Sojka
  1 sibling, 1 reply; 31+ messages in thread
From: Jani Nikula @ 2014-09-29 17:28 UTC (permalink / raw)
  To: Michal Sojka, Tomi Ollila, notmuch

On Thu, 25 Sep 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> On Thu, Sep 25 2014, Tomi Ollila wrote:
>> Although the test and the implementation in the next patches look OK, I'd
>> prefer the FLAG implementation Jani suggested earlier. IMO now that I
>> compare these two it looks cleaner and simpler...
>
> The question is which kind of simplicity you have in mind. I think that
> my version is simpler to type (less keystrokes). But if others have
> different opinion, I don't mind.

I'm biased, but I do like the implementation simplicity of my
approach. Adding the bash completion support is also trivial.

BR,
Jani.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 0/4] notmuch search --output=addresses
  2014-09-29 17:28       ` Jani Nikula
@ 2014-10-05 20:51         ` Michal Sojka
  2014-10-05 20:51           ` [PATCH v2 1/4] cli: Refactor option passing in the search command Michal Sojka
                             ` (3 more replies)
  0 siblings, 4 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-05 20:51 UTC (permalink / raw)
  To: notmuch

Hi,

this is a second version of my adaptation of Jani's patch series
adding --output=addresses and similar arguments to notmuch search.

Based on the feedback from others, this version uses Jani's original
"keyword flags" implementation with --flag=a --flab=b syntax. Also the
tests for --output and --unique flags are not mixed together, but are
included in the patches that introduce the new features.

I left the default value of the --unique option the same as before,
because I'm convinced that this is what makes most sense. But of
course, we can discuss about that.

-Michal


Jani Nikula (1):
  cli: Add support for parsing multiple keyword arguments

Michal Sojka (3):
  cli: Refactor option passing in the search command
  cli: Extend the search command for --output=addresses and similar
  cli: Add configurable address deduplication for --output=addresses

 command-line-arguments.c           |   6 +-
 command-line-arguments.h           |   1 +
 completion/notmuch-completion.bash |   8 +-
 completion/notmuch-completion.zsh  |   4 +-
 doc/man1/notmuch-search.rst        |  54 ++++++-
 notmuch-search.c                   | 311 +++++++++++++++++++++++++++++--------
 test/T090-search-output.sh         |  64 ++++++++
 test/T095-search-unique.sh         |  63 ++++++++
 test/T410-argument-parsing.sh      |   3 +-
 test/arg-test.c                    |   9 ++
 10 files changed, 451 insertions(+), 72 deletions(-)
 create mode 100755 test/T095-search-unique.sh

-- 
2.1.1

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 1/4] cli: Refactor option passing in the search command
  2014-10-05 20:51         ` [PATCH v2 0/4] notmuch search --output=addresses Michal Sojka
@ 2014-10-05 20:51           ` Michal Sojka
  2014-10-05 20:51           ` [PATCH v2 2/4] cli: Extend the search command for --output=addresses and similar Michal Sojka
                             ` (2 subsequent siblings)
  3 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-05 20:51 UTC (permalink / raw)
  To: notmuch

Many functions that implement the search command need to access command
line options. Instead of passing each option in a separate variable, put
them in a structure and pass only this structure.

This will become handy in the following patches.
---
 notmuch-search.c | 122 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 62 insertions(+), 60 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index bc9be45..5ac2a26 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -30,6 +30,16 @@ typedef enum {
     OUTPUT_TAGS
 } output_t;
 
+typedef struct {
+    sprinter_t *format;
+    notmuch_query_t *query;
+    notmuch_sort_t sort;
+    output_t output;
+    int offset;
+    int limit;
+    int dupe;
+} search_options_t;
+
 /* Return two stable query strings that identify exactly the matched
  * and unmatched messages currently in thread.  If there are no
  * matched or unmatched messages, the returned buffers will be
@@ -70,46 +80,42 @@ get_thread_query (notmuch_thread_t *thread,
 }
 
 static int
-do_search_threads (sprinter_t *format,
-		   notmuch_query_t *query,
-		   notmuch_sort_t sort,
-		   output_t output,
-		   int offset,
-		   int limit)
+do_search_threads (search_options_t *o)
 {
     notmuch_thread_t *thread;
     notmuch_threads_t *threads;
     notmuch_tags_t *tags;
+    sprinter_t *format = o->format;
     time_t date;
     int i;
 
-    if (offset < 0) {
-	offset += notmuch_query_count_threads (query);
-	if (offset < 0)
-	    offset = 0;
+    if (o->offset < 0) {
+	o->offset += notmuch_query_count_threads (o->query);
+	if (o->offset < 0)
+	    o->offset = 0;
     }
 
-    threads = notmuch_query_search_threads (query);
+    threads = notmuch_query_search_threads (o->query);
     if (threads == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
+	 notmuch_threads_valid (threads) && (o->limit < 0 || i < o->offset + o->limit);
 	 notmuch_threads_move_to_next (threads), i++)
     {
 	thread = notmuch_threads_get (threads);
 
-	if (i < offset) {
+	if (i < o->offset) {
 	    notmuch_thread_destroy (thread);
 	    continue;
 	}
 
-	if (output == OUTPUT_THREADS) {
+	if (o->output == OUTPUT_THREADS) {
 	    format->set_prefix (format, "thread");
 	    format->string (format,
-			    notmuch_thread_get_thread_id (thread));
+			       notmuch_thread_get_thread_id (thread));
 	    format->separator (format);
 	} else { /* output == OUTPUT_SUMMARY */
 	    void *ctx_quote = talloc_new (thread);
@@ -123,7 +129,7 @@ do_search_threads (sprinter_t *format,
 
 	    format->begin_map (format);
 
-	    if (sort == NOTMUCH_SORT_OLDEST_FIRST)
+	    if (o->sort == NOTMUCH_SORT_OLDEST_FIRST)
 		date = notmuch_thread_get_oldest_date (thread);
 	    else
 		date = notmuch_thread_get_newest_date (thread);
@@ -215,40 +221,36 @@ do_search_threads (sprinter_t *format,
 }
 
 static int
-do_search_messages (sprinter_t *format,
-		    notmuch_query_t *query,
-		    output_t output,
-		    int offset,
-		    int limit,
-		    int dupe)
+do_search_messages (search_options_t *o)
 {
     notmuch_message_t *message;
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
+    sprinter_t *format = o->format;
     int i;
 
-    if (offset < 0) {
-	offset += notmuch_query_count_messages (query);
-	if (offset < 0)
-	    offset = 0;
+    if (o->offset < 0) {
+	o->offset += notmuch_query_count_messages (o->query);
+	if (o->offset < 0)
+	    o->offset = 0;
     }
 
-    messages = notmuch_query_search_messages (query);
+    messages = notmuch_query_search_messages (o->query);
     if (messages == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit);
+	 notmuch_messages_valid (messages) && (o->limit < 0 || i < o->offset + o->limit);
 	 notmuch_messages_move_to_next (messages), i++)
     {
-	if (i < offset)
+	if (i < o->offset)
 	    continue;
 
 	message = notmuch_messages_get (messages);
 
-	if (output == OUTPUT_FILES) {
+	if (o->output == OUTPUT_FILES) {
 	    int j;
 	    filenames = notmuch_message_get_filenames (message);
 
@@ -256,7 +258,7 @@ do_search_messages (sprinter_t *format,
 		 notmuch_filenames_valid (filenames);
 		 notmuch_filenames_move_to_next (filenames), j++)
 	    {
-		if (dupe < 0 || dupe == j) {
+		if (o->dupe < 0 || o->dupe == j) {
 		    format->string (format, notmuch_filenames_get (filenames));
 		    format->separator (format);
 		}
@@ -333,16 +335,16 @@ int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
     notmuch_database_t *notmuch;
-    notmuch_query_t *query;
+    search_options_t o = {
+	.sort = NOTMUCH_SORT_NEWEST_FIRST,
+	.output = OUTPUT_SUMMARY,
+	.offset = 0,
+	.limit = -1, /* unlimited */
+	.dupe = -1,
+    };
     char *query_str;
-    notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;
-    sprinter_t *format = NULL;
     int opt_index, ret;
-    output_t output = OUTPUT_SUMMARY;
-    int offset = 0;
-    int limit = -1; /* unlimited */
     notmuch_exclude_t exclude = NOTMUCH_EXCLUDE_TRUE;
-    int dupe = -1;
     unsigned int i;
 
     enum {
@@ -353,7 +355,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     } format_sel = NOTMUCH_FORMAT_TEXT;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',
+	{ NOTMUCH_OPT_KEYWORD, &o.sort, "sort", 's',
 	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
 				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
 				  { 0, 0 } } },
@@ -364,7 +366,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "text0", NOTMUCH_FORMAT_TEXT0 },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
+	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
@@ -377,9 +379,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
                                   { "flag", NOTMUCH_EXCLUDE_FLAG },
                                   { "all", NOTMUCH_EXCLUDE_ALL },
                                   { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
-	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
-	{ NOTMUCH_OPT_INT, &dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_INT, &o.offset, "offset", 'O', 0 },
+	{ NOTMUCH_OPT_INT, &o.limit, "limit", 'L', 0  },
+	{ NOTMUCH_OPT_INT, &o.dupe, "duplicate", 'D', 0  },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -389,20 +391,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	format = sprinter_text_create (config, stdout);
+	o.format = sprinter_text_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_TEXT0:
-	if (output == OUTPUT_SUMMARY) {
+	if (o.output == OUTPUT_SUMMARY) {
 	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
 	    return EXIT_FAILURE;
 	}
-	format = sprinter_text0_create (config, stdout);
+	o.format = sprinter_text0_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	format = sprinter_json_create (config, stdout);
+	o.format = sprinter_json_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_SEXP:
-	format = sprinter_sexp_create (config, stdout);
+	o.format = sprinter_sexp_create (config, stdout);
 	break;
     default:
 	/* this should never happen */
@@ -425,15 +427,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    query = notmuch_query_create (notmuch, query_str);
-    if (query == NULL) {
+    o.query = notmuch_query_create (notmuch, query_str);
+    if (o.query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
     }
 
-    notmuch_query_set_sort (query, sort);
+    notmuch_query_set_sort (o.query, o.sort);
 
-    if (exclude == NOTMUCH_EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {
+    if (exclude == NOTMUCH_EXCLUDE_FLAG && o.output != OUTPUT_SUMMARY) {
 	/* If we are not doing summary output there is nowhere to
 	 * print the excluded flag so fall back on including the
 	 * excluded messages. */
@@ -448,29 +450,29 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
 	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
-	notmuch_query_set_omit_excluded (query, exclude);
+	    notmuch_query_add_tag_exclude (o.query, search_exclude_tags[i]);
+	notmuch_query_set_omit_excluded (o.query, exclude);
     }
 
-    switch (output) {
+    switch (o.output) {
     default:
     case OUTPUT_SUMMARY:
     case OUTPUT_THREADS:
-	ret = do_search_threads (format, query, sort, output, offset, limit);
+	ret = do_search_threads (&o);
 	break;
     case OUTPUT_MESSAGES:
     case OUTPUT_FILES:
-	ret = do_search_messages (format, query, output, offset, limit, dupe);
+	ret = do_search_messages (&o);
 	break;
     case OUTPUT_TAGS:
-	ret = do_search_tags (notmuch, format, query);
+	ret = do_search_tags (notmuch, o.format, o.query);
 	break;
     }
 
-    notmuch_query_destroy (query);
+    notmuch_query_destroy (o.query);
     notmuch_database_destroy (notmuch);
 
-    talloc_free (format);
+    talloc_free (o.format);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 2/4] cli: Extend the search command for --output=addresses and similar
  2014-10-05 20:51         ` [PATCH v2 0/4] notmuch search --output=addresses Michal Sojka
  2014-10-05 20:51           ` [PATCH v2 1/4] cli: Refactor option passing in the search command Michal Sojka
@ 2014-10-05 20:51           ` Michal Sojka
  2014-10-06 18:56             ` Tomi Ollila
  2014-10-05 20:51           ` [PATCH v2 3/4] cli: Add support for parsing multiple keyword arguments Michal Sojka
  2014-10-05 20:51           ` [PATCH v2 4/4] cli: Add configurable address deduplication for --output=addresses Michal Sojka
  3 siblings, 1 reply; 31+ messages in thread
From: Michal Sojka @ 2014-10-05 20:51 UTC (permalink / raw)
  To: notmuch

The new outputs allow printing senders, recipients or both of matching
messages.

This code based on a patch from Jani Nikula.
---
 completion/notmuch-completion.bash |   2 +-
 completion/notmuch-completion.zsh  |   3 +-
 doc/man1/notmuch-search.rst        |  22 +++++++-
 notmuch-search.c                   | 100 ++++++++++++++++++++++++++++++++++---
 test/T090-search-output.sh         |  64 ++++++++++++++++++++++++
 5 files changed, 182 insertions(+), 9 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 0571dc9..c37ddf5 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -294,7 +294,7 @@ _notmuch_search()
 	    return
 	    ;;
 	--output)
-	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients addresses" -- "${cur}" ) )
 	    return
 	    ;;
 	--sort)
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index 67a9aba..bff8fd5 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -52,7 +52,8 @@ _notmuch_search()
   _arguments -s : \
     '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
     '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
-    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
+    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
+    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index 90160f2..3447820 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -35,7 +35,7 @@ Supported options for **search** include
         intended for programs that invoke **notmuch(1)** internally. If
         omitted, the latest supported version will be used.
 
-    ``--output=(summary|threads|messages|files|tags)``
+    ``--output=(summary|threads|messages|files|tags|sender|recipients|addresses)``
 
         **summary**
             Output a summary of each thread with any message matching
@@ -78,6 +78,26 @@ Supported options for **search** include
             by null characters (--format=text0), as a JSON array
             (--format=json), or as an S-Expression list (--format=sexp).
 
+	**sender**
+            Output all addresses from the *From* header that appear on
+            any message matching the search terms, either one per line
+            (--format=text), separated by null characters
+            (--format=text0), as a JSON array (--format=json), or as
+            an S-Expression list (--format=sexp).
+
+	    Note: Searching for **sender** should be much faster than
+	    searching for **recipients** or **addresses**, because
+	    sender addresses are cached directly in the database
+	    whereas other addresses need to be fetched from message
+	    files.
+
+	**recipients**
+            Like **sender** but for addresses from *To*, *Cc* and
+	    *Bcc* headers.
+
+	**addresses**
+	    Like **sender** and **recipients** together.
+
     ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
         This option can be used to present results in either
         chronological order (**oldest-first**) or reverse chronological
diff --git a/notmuch-search.c b/notmuch-search.c
index 5ac2a26..0614f10 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -23,11 +23,14 @@
 #include "string-util.h"
 
 typedef enum {
-    OUTPUT_SUMMARY,
-    OUTPUT_THREADS,
-    OUTPUT_MESSAGES,
-    OUTPUT_FILES,
-    OUTPUT_TAGS
+    OUTPUT_SUMMARY	= 1 << 0,
+    OUTPUT_THREADS	= 1 << 1,
+    OUTPUT_MESSAGES	= 1 << 2,
+    OUTPUT_FILES	= 1 << 3,
+    OUTPUT_TAGS		= 1 << 4,
+    OUTPUT_SENDER	= 1 << 5,
+    OUTPUT_RECIPIENTS	= 1 << 6,
+    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
 } output_t;
 
 typedef struct {
@@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
     return 0;
 }
 
+static void
+print_address_list (const search_options_t *o, InternetAddressList *list)
+{
+    InternetAddress *address;
+    int i;
+
+    for (i = 0; i < internet_address_list_length (list); i++) {
+	address = internet_address_list_get_address (list, i);
+	if (INTERNET_ADDRESS_IS_GROUP (address)) {
+	    InternetAddressGroup *group;
+	    InternetAddressList *group_list;
+
+	    group = INTERNET_ADDRESS_GROUP (address);
+	    group_list = internet_address_group_get_members (group);
+	    if (group_list == NULL)
+		continue;
+
+	    print_address_list (o, group_list);
+	} else {
+	    InternetAddressMailbox *mailbox;
+	    const char *name;
+	    const char *addr;
+	    char *full_address;
+
+	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
+
+	    name = internet_address_get_name (address);
+	    addr = internet_address_mailbox_get_addr (mailbox);
+
+	    if (name && *name)
+		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
+	    else
+		full_address = talloc_strdup (o->format, addr);
+
+	    if (!full_address) {
+		fprintf (stderr, "Error: out of memory\n");
+		break;
+	    }
+	    o->format->string (o->format, full_address);
+	    o->format->separator (o->format);
+
+	    talloc_free (full_address);
+	}
+    }
+}
+
+static void
+print_address_string (const search_options_t *o, const char *recipients)
+{
+    InternetAddressList *list;
+
+    if (recipients == NULL)
+	return;
+
+    list = internet_address_list_parse_string (recipients);
+    if (list == NULL)
+	return;
+
+    print_address_list (o, list);
+}
+
 static int
 do_search_messages (search_options_t *o)
 {
@@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
 	    
 	    notmuch_filenames_destroy( filenames );
 
-	} else { /* output == OUTPUT_MESSAGES */
+	} else if (o->output == OUTPUT_MESSAGES) {
 	    format->set_prefix (format, "id");
 	    format->string (format,
 			    notmuch_message_get_message_id (message));
 	    format->separator (format);
+	} else {
+	    if (o->output & OUTPUT_SENDER) {
+		const char *addrs;
+
+		addrs = notmuch_message_get_header (message, "from");
+		print_address_string (o, addrs);
+	    }
+
+	    if (o->output & OUTPUT_RECIPIENTS) {
+		const char *hdrs[] = { "to", "cc", "bcc" };
+		const char *addrs;
+		size_t j;
+
+		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
+		    addrs = notmuch_message_get_header (message, hdrs[j]);
+		    print_address_string (o, addrs);
+		}
+	    }
 	}
 
 	notmuch_message_destroy (message);
@@ -370,6 +452,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
+				  { "sender", OUTPUT_SENDER },
+				  { "recipients", OUTPUT_RECIPIENTS },
+				  { "addresses", OUTPUT_ADDRESSES },
 				  { "files", OUTPUT_FILES },
 				  { "tags", OUTPUT_TAGS },
 				  { 0, 0 } } },
@@ -461,6 +546,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	ret = do_search_threads (&o);
 	break;
     case OUTPUT_MESSAGES:
+    case OUTPUT_SENDER:
+    case OUTPUT_RECIPIENTS:
+    case OUTPUT_ADDRESSES:
     case OUTPUT_FILES:
 	ret = do_search_messages (&o);
 	break;
diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
index 947d572..5458de1 100755
--- a/test/T090-search-output.sh
+++ b/test/T090-search-output.sh
@@ -387,6 +387,70 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "--output=sender"
+notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
+cat <<EOF >EXPECTED
+      1 Adrian Perez de Castro <aperez@igalia.com>
+      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
+      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+      1 Aron Griffis <agriffis@n01se.net>
+     12 Carl Worth <cworth@cworth.org>
+      1 Chris Wilson <chris@chris-wilson.co.uk>
+      1 François Boulogne <boulogne.f@gmail.com>
+      1 Ingmar Vanhassel <ingmar@exherbo.org>
+      1 Israel Herraiz <isra@herraiz.org>
+      4 Jan Janak <jan@ryngle.com>
+      2 Jjgod Jiang <gzjjgod@gmail.com>
+      7 Keith Packard <keithp@keithp.com>
+      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
+      5 Mikhail Gusarov <dottedmag@dottedmag.net>
+      1 Olivier Berger <olivier.berger@it-sudparis.eu>
+      1 Rolland Santimano <rollandsantimano@yahoo.com>
+      3 Stewart Smith <stewart@flamingspork.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients"
+notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
+cat <<EOF >EXPECTED
+      1 Allan McRae <allan@archlinux.org>
+      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
+      1 Keith Packard <keithp@keithp.com>
+      1 Mikhail Gusarov <dottedmag@dottedmag.net>
+      2 notmuch <notmuch@notmuchmail.org>
+     48 notmuch@notmuchmail.org
+      1 olivier.berger@it-sudparis.eu
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=addresses"
+notmuch search --output=addresses '*' | sort | uniq --count >OUTPUT
+cat <<EOF >EXPECTED
+      1 Adrian Perez de Castro <aperez@igalia.com>
+      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
+      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+      1 Allan McRae <allan@archlinux.org>
+      1 Aron Griffis <agriffis@n01se.net>
+     12 Carl Worth <cworth@cworth.org>
+      1 Chris Wilson <chris@chris-wilson.co.uk>
+      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
+      1 François Boulogne <boulogne.f@gmail.com>
+      1 Ingmar Vanhassel <ingmar@exherbo.org>
+      1 Israel Herraiz <isra@herraiz.org>
+      4 Jan Janak <jan@ryngle.com>
+      2 Jjgod Jiang <gzjjgod@gmail.com>
+      8 Keith Packard <keithp@keithp.com>
+      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
+      6 Mikhail Gusarov <dottedmag@dottedmag.net>
+      1 Olivier Berger <olivier.berger@it-sudparis.eu>
+      1 Rolland Santimano <rollandsantimano@yahoo.com>
+      3 Stewart Smith <stewart@flamingspork.com>
+      2 notmuch <notmuch@notmuchmail.org>
+     48 notmuch@notmuchmail.org
+      1 olivier.berger@it-sudparis.eu
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
 add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
 	headers'"
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 3/4] cli: Add support for parsing multiple keyword arguments
  2014-10-05 20:51         ` [PATCH v2 0/4] notmuch search --output=addresses Michal Sojka
  2014-10-05 20:51           ` [PATCH v2 1/4] cli: Refactor option passing in the search command Michal Sojka
  2014-10-05 20:51           ` [PATCH v2 2/4] cli: Extend the search command for --output=addresses and similar Michal Sojka
@ 2014-10-05 20:51           ` Michal Sojka
  2014-10-05 20:51           ` [PATCH v2 4/4] cli: Add configurable address deduplication for --output=addresses Michal Sojka
  3 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-05 20:51 UTC (permalink / raw)
  To: notmuch

From: Jani Nikula <jani@nikula.org>

This allows having multiple --foo=bar --foo=baz options on the command
line, with the corresponding values OR'd together.

[Test added by Michal Sojka]
---
 command-line-arguments.c      | 6 +++++-
 command-line-arguments.h      | 1 +
 test/T410-argument-parsing.sh | 3 ++-
 test/arg-test.c               | 9 +++++++++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 844d6c3..c6f7269 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -23,7 +23,10 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
     while (keywords->name) {
 	if (strcmp (arg_str, keywords->name) == 0) {
 	    if (arg_desc->output_var) {
-		*((int *)arg_desc->output_var) = keywords->value;
+		if (arg_desc->opt_type == NOTMUCH_OPT_KEYWORD_FLAGS)
+		    *((int *)arg_desc->output_var) |= keywords->value;
+		else
+		    *((int *)arg_desc->output_var) = keywords->value;
 	    }
 	    return TRUE;
 	}
@@ -152,6 +155,7 @@ parse_option (const char *arg,
 
 	switch (try->opt_type) {
 	case NOTMUCH_OPT_KEYWORD:
+	case NOTMUCH_OPT_KEYWORD_FLAGS:
 	    return _process_keyword_arg (try, next, value);
 	case NOTMUCH_OPT_BOOLEAN:
 	    return _process_boolean_arg (try, next, value);
diff --git a/command-line-arguments.h b/command-line-arguments.h
index de1734a..085a492 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -8,6 +8,7 @@ enum notmuch_opt_type {
     NOTMUCH_OPT_BOOLEAN,	/* --verbose              */
     NOTMUCH_OPT_INT,		/* --frob=8               */
     NOTMUCH_OPT_KEYWORD,	/* --format=raw|json|text */
+    NOTMUCH_OPT_KEYWORD_FLAGS,  /* the above with values OR'd together */
     NOTMUCH_OPT_STRING,		/* --file=/tmp/gnarf.txt  */
     NOTMUCH_OPT_POSITION	/* notmuch dump pos_arg   */
 };
diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 94e9087..2e5d7ae 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -3,9 +3,10 @@ test_description="argument parsing"
 . ./test-lib.sh
 
 test_begin_subtest "sanity check"
-$TEST_DIRECTORY/arg-test  pos1  --keyword=one --string=foo pos2 --int=7 > OUTPUT
+$TEST_DIRECTORY/arg-test  pos1  --keyword=one --string=foo pos2 --int=7 --flag=one --flag=three > OUTPUT
 cat <<EOF > EXPECTED
 keyword 1
+flags 5
 int 7
 string foo
 positional arg 1 pos1
diff --git a/test/arg-test.c b/test/arg-test.c
index 6c49eac..736686d 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -7,6 +7,7 @@ int main(int argc, char **argv){
     int opt_index=1;
 
     int kw_val=0;
+    int fl_val=0;
     int int_val=0;
     char *pos_arg1=NULL;
     char *pos_arg2=NULL;
@@ -17,6 +18,11 @@ int main(int argc, char **argv){
 	  (notmuch_keyword_t []){ { "one", 1 },
 				  { "two", 2 },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &fl_val, "flag", 'f',
+	  (notmuch_keyword_t []){ { "one",   1 << 0},
+				  { "two",   1 << 1 },
+				  { "three", 1 << 2 },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &int_val, "int", 'i', 0},
 	{ NOTMUCH_OPT_STRING, &string_val, "string", 's', 0},
 	{ NOTMUCH_OPT_POSITION, &pos_arg1, 0,0, 0},
@@ -31,6 +37,9 @@ int main(int argc, char **argv){
     if (kw_val)
 	printf("keyword %d\n", kw_val);
 
+    if (fl_val)
+	printf("flags %d\n", fl_val);
+
     if (int_val)
 	printf("int %d\n", int_val);
 
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v2 4/4] cli: Add configurable address deduplication for --output=addresses
  2014-10-05 20:51         ` [PATCH v2 0/4] notmuch search --output=addresses Michal Sojka
                             ` (2 preceding siblings ...)
  2014-10-05 20:51           ` [PATCH v2 3/4] cli: Add support for parsing multiple keyword arguments Michal Sojka
@ 2014-10-05 20:51           ` Michal Sojka
  3 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-05 20:51 UTC (permalink / raw)
  To: notmuch

The code here is an extended version of a patch from Jani Nikula.
---
 completion/notmuch-completion.bash |   6 ++-
 completion/notmuch-completion.zsh  |   3 +-
 doc/man1/notmuch-search.rst        |  32 ++++++++++++
 notmuch-search.c                   | 101 ++++++++++++++++++++++++++++++++++---
 test/T090-search-output.sh         |   6 +--
 test/T095-search-unique.sh         |  63 +++++++++++++++++++++++
 6 files changed, 200 insertions(+), 11 deletions(-)
 create mode 100755 test/T095-search-unique.sh

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index c37ddf5..8bc7874 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -305,12 +305,16 @@ _notmuch_search()
 	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
 	    return
 	    ;;
+	--unique)
+	    COMPREPLY=( $( compgen -W "none addr addrfold name" -- "${cur}" ) )
+	    return
+	    ;;
     esac
 
     ! $split &&
     case "${cur}" in
 	-*)
-	    local options="--format= --output= --sort= --offset= --limit= --exclude= --duplicate="
+	    local options="--format= --output= --sort= --offset= --limit= --exclude= --duplicate= --unique="
 	    compopt -o nospace
 	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
 	    ;;
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index bff8fd5..cf4968c 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -53,7 +53,8 @@ _notmuch_search()
     '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
     '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
     '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
-    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))'
+    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))' \
+    '--unique=[address deduplication]:unique:((none\:"no deduplication" addr\:"deduplicate by address" addrfold\:"deduplicate by case-insensitive address" name\:"deduplicate by name"))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index 3447820..9a9d9c3 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -85,6 +85,9 @@ Supported options for **search** include
             (--format=text0), as a JSON array (--format=json), or as
             an S-Expression list (--format=sexp).
 
+            Handling of duplicate addresses and/or names can be
+            controlled with the --unique option.
+
 	    Note: Searching for **sender** should be much faster than
 	    searching for **recipients** or **addresses**, because
 	    sender addresses are cached directly in the database
@@ -151,6 +154,35 @@ Supported options for **search** include
         prefix. The prefix matches messages based on filenames. This
         option filters filenames of the matching messages.
 
+    ``--unique=``\ (**none**\ \|\ **addr**\ \|\ **addrfold**\ \|\ **name**)
+
+        Can be used with ``--output=addresses``, ``--output=sender``
+        or ``--output=recipients`` to control the address
+        deduplication algorithm.
+
+	**none** means that no deduplication is performed. The same
+	address can appear multiple times in the output.
+
+	**addr** means that case-sensitive deduplication is performed
+	on the address part. For example, given the addresses "John
+	Doe <john@example.com>" and "Dr. John Doe <john@example.com>",
+	only one will be printed.
+
+	**addrfold** is the same as **addr** but with case folding
+	applied. For example, given the addresses "John Doe
+	<john@example.com>" and "John Doe <JOHN@EXAMPLE.COM>", only
+	one will be printed. This is the default.
+
+	**name** means that case-sensitive deduplication is performed
+	on the name part. For example, given the addresses "John Doe
+	<john@example.com>" and "John Doe <john@doe.name>", only one
+	will be printed.
+
+	This option can be given multiple times to output unique
+	combinations of names and addresses. For example,
+	``--unique=name --unique=addr`` will print unique
+	case-sensitive combinations of name and address.
+
 EXIT STATUS
 ===========
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 0614f10..94d400e 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -33,6 +33,15 @@ typedef enum {
     OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
 } output_t;
 
+typedef enum {
+    UNIQUE_NONE 	  = 1 << 0,
+    UNIQUE_ADDR 	  = 1 << 1,
+    UNIQUE_NAME 	  = 1 << 2,
+    UNIQUE_ADDR_CASEFOLD  = 1 << 3,
+
+    UNIQUE_BOTH = UNIQUE_NAME | UNIQUE_ADDR,
+} unique_t;
+
 typedef struct {
     sprinter_t *format;
     notmuch_query_t *query;
@@ -41,6 +50,7 @@ typedef struct {
     int offset;
     int limit;
     int dupe;
+    unique_t unique;
 } search_options_t;
 
 /* Return two stable query strings that identify exactly the matched
@@ -223,8 +233,51 @@ do_search_threads (search_options_t *o)
     return 0;
 }
 
+/* Returns TRUE iff name and/or addr is considered unique. */
+static notmuch_bool_t
+check_unique (const search_options_t *o, GHashTable *addrs, const char *name, const char *addr)
+{
+    notmuch_bool_t unique;
+    char *key;
+
+    if (o->unique == UNIQUE_NONE)
+	return TRUE;
+
+    if (o->unique & UNIQUE_ADDR_CASEFOLD) {
+	gchar *folded = g_utf8_casefold (addr, -1);
+	addr = talloc_strdup (o->format, folded);
+	g_free (folded);
+    }
+    switch (o->unique & UNIQUE_BOTH) {
+    case UNIQUE_NAME:
+	key = talloc_strdup (o->format, name); /* !name results in !key */
+	break;
+    case UNIQUE_ADDR:
+	key = talloc_strdup (o->format, addr);
+	break;
+    case UNIQUE_BOTH:
+	key = talloc_asprintf (o->format, "%s <%s>", name, addr);
+	break;
+    default:
+	INTERNAL_ERROR("invalid --unique flags");
+    }
+
+    if (! key)
+	return FALSE;
+
+    unique = !g_hash_table_lookup_extended (addrs, key, NULL, NULL);
+
+    if (unique)
+	g_hash_table_insert (addrs, key, NULL);
+    else
+	talloc_free (key);
+
+    return unique;
+}
+
 static void
-print_address_list (const search_options_t *o, InternetAddressList *list)
+print_address_list (const search_options_t *o, GHashTable *addrs,
+		    InternetAddressList *list)
 {
     InternetAddress *address;
     int i;
@@ -240,7 +293,7 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 	    if (group_list == NULL)
 		continue;
 
-	    print_address_list (o, group_list);
+	    print_address_list (o, addrs, group_list);
 	} else {
 	    InternetAddressMailbox *mailbox;
 	    const char *name;
@@ -252,6 +305,9 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 	    name = internet_address_get_name (address);
 	    addr = internet_address_mailbox_get_addr (mailbox);
 
+	    if (!check_unique (o, addrs, name, addr))
+		continue;
+
 	    if (name && *name)
 		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
 	    else
@@ -270,7 +326,7 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 }
 
 static void
-print_address_string (const search_options_t *o, const char *recipients)
+print_address_string (const search_options_t *o, GHashTable *addrs, const char *recipients)
 {
     InternetAddressList *list;
 
@@ -281,7 +337,13 @@ print_address_string (const search_options_t *o, const char *recipients)
     if (list == NULL)
 	return;
 
-    print_address_list (o, list);
+    print_address_list (o, addrs, list);
+}
+
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+    talloc_free (ptr);
 }
 
 static int
@@ -291,8 +353,13 @@ do_search_messages (search_options_t *o)
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
     sprinter_t *format = o->format;
+    GHashTable *addresses = NULL;
     int i;
 
+    if (o->output & OUTPUT_ADDRESSES)
+	addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
+					   _my_talloc_free_for_g_hash, NULL);
+
     if (o->offset < 0) {
 	o->offset += notmuch_query_count_messages (o->query);
 	if (o->offset < 0)
@@ -340,7 +407,7 @@ do_search_messages (search_options_t *o)
 		const char *addrs;
 
 		addrs = notmuch_message_get_header (message, "from");
-		print_address_string (o, addrs);
+		print_address_string (o, addresses, addrs);
 	    }
 
 	    if (o->output & OUTPUT_RECIPIENTS) {
@@ -350,7 +417,7 @@ do_search_messages (search_options_t *o)
 
 		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
 		    addrs = notmuch_message_get_header (message, hdrs[j]);
-		    print_address_string (o, addrs);
+		    print_address_string (o, addresses, addrs);
 		}
 	    }
 	}
@@ -358,6 +425,9 @@ do_search_messages (search_options_t *o)
 	notmuch_message_destroy (message);
     }
 
+    if (addresses)
+	g_hash_table_unref (addresses);
+
     notmuch_messages_destroy (messages);
 
     format->end (format);
@@ -423,6 +493,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	.offset = 0,
 	.limit = -1, /* unlimited */
 	.dupe = -1,
+	.unique = 0,
     };
     char *query_str;
     int opt_index, ret;
@@ -467,6 +538,12 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	{ NOTMUCH_OPT_INT, &o.offset, "offset", 'O', 0 },
 	{ NOTMUCH_OPT_INT, &o.limit, "limit", 'L', 0  },
 	{ NOTMUCH_OPT_INT, &o.dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &o.unique, "unique", 'u',
+	  (notmuch_keyword_t []){ { "none", UNIQUE_NONE },
+				  { "name", UNIQUE_NAME },
+				  { "addr", UNIQUE_ADDR },
+				  { "addrfold", UNIQUE_ADDR | UNIQUE_ADDR_CASEFOLD },
+				  { 0, 0 } } },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -474,6 +551,18 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
+    if (o.unique && (o.output & ~OUTPUT_ADDRESSES)) {
+	fprintf (stderr, "Error: --unique can only be used with address output.\n");
+	return EXIT_FAILURE;
+    }
+    if ((o.unique & UNIQUE_NONE) &&
+	(o.unique & ~UNIQUE_NONE)) {
+	fprintf (stderr, "Error: --unique=none cannot be combined with other flags.\n");
+	return EXIT_FAILURE;
+    }
+    if (! o.unique)
+	o.unique = UNIQUE_ADDR | UNIQUE_ADDR_CASEFOLD;
+
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
 	o.format = sprinter_text_create (config, stdout);
diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
index 5458de1..e11cb52 100755
--- a/test/T090-search-output.sh
+++ b/test/T090-search-output.sh
@@ -388,7 +388,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--output=sender"
-notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
+notmuch search --output=sender --unique=none '*' | sort | uniq --count >OUTPUT
 cat <<EOF >EXPECTED
       1 Adrian Perez de Castro <aperez@igalia.com>
       2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
@@ -411,7 +411,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--output=recipients"
-notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
+notmuch search --output=recipients --unique=none '*' | sort | uniq --count >OUTPUT
 cat <<EOF >EXPECTED
       1 Allan McRae <allan@archlinux.org>
       1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
@@ -424,7 +424,7 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "--output=addresses"
-notmuch search --output=addresses '*' | sort | uniq --count >OUTPUT
+notmuch search --output=addresses --unique=none '*' | sort | uniq --count >OUTPUT
 cat <<EOF >EXPECTED
       1 Adrian Perez de Castro <aperez@igalia.com>
       2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
diff --git a/test/T095-search-unique.sh b/test/T095-search-unique.sh
new file mode 100755
index 0000000..1d8afac
--- /dev/null
+++ b/test/T095-search-unique.sh
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+test_description='address deduplication in "notmuch search --output=addresses"'
+. ./test-lib.sh
+
+add_message '[to]="Real Name <foo@example.com>, Real Name <bar@example.com>"'
+add_message '[to]="Nickname <foo@example.com>"' '[cc]="Real Name <Bar@Example.COM>"'
+add_message '[to]="Nickname <foo@example.com>"' '[bcc]="Real Name <Bar@Example.COM>"'
+
+test_begin_subtest "--output=recipients"
+notmuch search --output=recipients "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=none"
+notmuch search --output=recipients --unique=none "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Nickname <foo@example.com>
+Real Name <Bar@Example.COM>
+Nickname <foo@example.com>
+Real Name <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=addr"
+notmuch search --output=recipients --unique=addr "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Real Name <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=addrfold"
+notmuch search --output=recipients --unique=addrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=name"
+notmuch search --output=recipients --unique=name "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Nickname <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --unique=name --unique=addrfold"
+notmuch search --output=recipients --unique=name --unique=addrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Nickname <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 2/4] cli: Extend the search command for --output=addresses and similar
  2014-10-05 20:51           ` [PATCH v2 2/4] cli: Extend the search command for --output=addresses and similar Michal Sojka
@ 2014-10-06 18:56             ` Tomi Ollila
  2014-10-09 10:55               ` Michal Sojka
  0 siblings, 1 reply; 31+ messages in thread
From: Tomi Ollila @ 2014-10-06 18:56 UTC (permalink / raw)
  To: Michal Sojka, notmuch

On Sun, Oct 05 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

> The new outputs allow printing senders, recipients or both of matching
> messages.
>
> This code based on a patch from Jani Nikula.

OK, IMO...

1/4 OK

Before 2/4 add support for 'flag' arguments, drop the --output=addresses
option which is now done as --output=sender --output=recipients


In deduplication comment did not describe the deduplication at all...
so I looked a bit into the code now... the Default you described was
that with "John Doe" <john.doe@example.com> and "John Doe" <JOHN.DOE@EXAMPLE.COM> 
only one was printed (but not which one). Secondly, what happens
with "Doe, John" <john.doe@example.com> and "John Doe" <JOHN.DOE@EXAMPLE.COM>...
ah, it is same as *addr* with case-insensitive address.

Sorry, but IMO these options are a bit strange.

Not to go to choose which one to choose (first, last, most common) instead
of the suggested options these should be the ones:

1) "John Doe" <john.doe@example.com> and "John Doe" <JOHN.DOE@EXAMPLE.COM>: 
only one printed, but if either were "Dr. John Doe", both of these are printed
(this as default).

2) same as above, but only make case-insensitive address match -- i.e. in
the 2 above cases in option 1, print only one.

(and same name but different address to perhaps never been an option...)

I might like to have option that does case-sensitive address match, In
those cases I don't know the recipient's culture and the email he sent
to me used format <Foo.Bar@example.org> (and not knowing which one is the
first and which last name (or whatever names these are) -- just to reply
in same case format in respect...


Tomi


> ---
>  completion/notmuch-completion.bash |   2 +-
>  completion/notmuch-completion.zsh  |   3 +-
>  doc/man1/notmuch-search.rst        |  22 +++++++-
>  notmuch-search.c                   | 100 ++++++++++++++++++++++++++++++++++---
>  test/T090-search-output.sh         |  64 ++++++++++++++++++++++++
>  5 files changed, 182 insertions(+), 9 deletions(-)
>
> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
> index 0571dc9..c37ddf5 100644
> --- a/completion/notmuch-completion.bash
> +++ b/completion/notmuch-completion.bash
> @@ -294,7 +294,7 @@ _notmuch_search()
>  	    return
>  	    ;;
>  	--output)
> -	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients addresses" -- "${cur}" ) )
>  	    return
>  	    ;;
>  	--sort)
> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
> index 67a9aba..bff8fd5 100644
> --- a/completion/notmuch-completion.zsh
> +++ b/completion/notmuch-completion.zsh
> @@ -52,7 +52,8 @@ _notmuch_search()
>    _arguments -s : \
>      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
>      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
> -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
> +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))'
>  }
>  
>  _notmuch()
> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
> index 90160f2..3447820 100644
> --- a/doc/man1/notmuch-search.rst
> +++ b/doc/man1/notmuch-search.rst
> @@ -35,7 +35,7 @@ Supported options for **search** include
>          intended for programs that invoke **notmuch(1)** internally. If
>          omitted, the latest supported version will be used.
>  
> -    ``--output=(summary|threads|messages|files|tags)``
> +    ``--output=(summary|threads|messages|files|tags|sender|recipients|addresses)``
>  
>          **summary**
>              Output a summary of each thread with any message matching
> @@ -78,6 +78,26 @@ Supported options for **search** include
>              by null characters (--format=text0), as a JSON array
>              (--format=json), or as an S-Expression list (--format=sexp).
>  
> +	**sender**
> +            Output all addresses from the *From* header that appear on
> +            any message matching the search terms, either one per line
> +            (--format=text), separated by null characters
> +            (--format=text0), as a JSON array (--format=json), or as
> +            an S-Expression list (--format=sexp).
> +
> +	    Note: Searching for **sender** should be much faster than
> +	    searching for **recipients** or **addresses**, because
> +	    sender addresses are cached directly in the database
> +	    whereas other addresses need to be fetched from message
> +	    files.
> +
> +	**recipients**
> +            Like **sender** but for addresses from *To*, *Cc* and
> +	    *Bcc* headers.
> +
> +	**addresses**
> +	    Like **sender** and **recipients** together.
> +
>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>          This option can be used to present results in either
>          chronological order (**oldest-first**) or reverse chronological
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 5ac2a26..0614f10 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -23,11 +23,14 @@
>  #include "string-util.h"
>  
>  typedef enum {
> -    OUTPUT_SUMMARY,
> -    OUTPUT_THREADS,
> -    OUTPUT_MESSAGES,
> -    OUTPUT_FILES,
> -    OUTPUT_TAGS
> +    OUTPUT_SUMMARY	= 1 << 0,
> +    OUTPUT_THREADS	= 1 << 1,
> +    OUTPUT_MESSAGES	= 1 << 2,
> +    OUTPUT_FILES	= 1 << 3,
> +    OUTPUT_TAGS		= 1 << 4,
> +    OUTPUT_SENDER	= 1 << 5,
> +    OUTPUT_RECIPIENTS	= 1 << 6,
> +    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
>  } output_t;
>  
>  typedef struct {
> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
>      return 0;
>  }
>  
> +static void
> +print_address_list (const search_options_t *o, InternetAddressList *list)
> +{
> +    InternetAddress *address;
> +    int i;
> +
> +    for (i = 0; i < internet_address_list_length (list); i++) {
> +	address = internet_address_list_get_address (list, i);
> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
> +	    InternetAddressGroup *group;
> +	    InternetAddressList *group_list;
> +
> +	    group = INTERNET_ADDRESS_GROUP (address);
> +	    group_list = internet_address_group_get_members (group);
> +	    if (group_list == NULL)
> +		continue;
> +
> +	    print_address_list (o, group_list);
> +	} else {
> +	    InternetAddressMailbox *mailbox;
> +	    const char *name;
> +	    const char *addr;
> +	    char *full_address;
> +
> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +
> +	    name = internet_address_get_name (address);
> +	    addr = internet_address_mailbox_get_addr (mailbox);
> +
> +	    if (name && *name)
> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
> +	    else
> +		full_address = talloc_strdup (o->format, addr);
> +
> +	    if (!full_address) {
> +		fprintf (stderr, "Error: out of memory\n");
> +		break;
> +	    }
> +	    o->format->string (o->format, full_address);
> +	    o->format->separator (o->format);
> +
> +	    talloc_free (full_address);
> +	}
> +    }
> +}
> +
> +static void
> +print_address_string (const search_options_t *o, const char *recipients)
> +{
> +    InternetAddressList *list;
> +
> +    if (recipients == NULL)
> +	return;
> +
> +    list = internet_address_list_parse_string (recipients);
> +    if (list == NULL)
> +	return;
> +
> +    print_address_list (o, list);
> +}
> +
>  static int
>  do_search_messages (search_options_t *o)
>  {
> @@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
>  	    
>  	    notmuch_filenames_destroy( filenames );
>  
> -	} else { /* output == OUTPUT_MESSAGES */
> +	} else if (o->output == OUTPUT_MESSAGES) {
>  	    format->set_prefix (format, "id");
>  	    format->string (format,
>  			    notmuch_message_get_message_id (message));
>  	    format->separator (format);
> +	} else {
> +	    if (o->output & OUTPUT_SENDER) {
> +		const char *addrs;
> +
> +		addrs = notmuch_message_get_header (message, "from");
> +		print_address_string (o, addrs);
> +	    }
> +
> +	    if (o->output & OUTPUT_RECIPIENTS) {
> +		const char *hdrs[] = { "to", "cc", "bcc" };
> +		const char *addrs;
> +		size_t j;
> +
> +		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
> +		    addrs = notmuch_message_get_header (message, hdrs[j]);
> +		    print_address_string (o, addrs);
> +		}
> +	    }
>  	}
>  
>  	notmuch_message_destroy (message);
> @@ -370,6 +452,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>  				  { "threads", OUTPUT_THREADS },
>  				  { "messages", OUTPUT_MESSAGES },
> +				  { "sender", OUTPUT_SENDER },
> +				  { "recipients", OUTPUT_RECIPIENTS },
> +				  { "addresses", OUTPUT_ADDRESSES },
>  				  { "files", OUTPUT_FILES },
>  				  { "tags", OUTPUT_TAGS },
>  				  { 0, 0 } } },
> @@ -461,6 +546,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  	ret = do_search_threads (&o);
>  	break;
>      case OUTPUT_MESSAGES:
> +    case OUTPUT_SENDER:
> +    case OUTPUT_RECIPIENTS:
> +    case OUTPUT_ADDRESSES:
>      case OUTPUT_FILES:
>  	ret = do_search_messages (&o);
>  	break;
> diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
> index 947d572..5458de1 100755
> --- a/test/T090-search-output.sh
> +++ b/test/T090-search-output.sh
> @@ -387,6 +387,70 @@ cat <<EOF >EXPECTED
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "--output=sender"
> +notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Adrian Perez de Castro <aperez@igalia.com>
> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +      1 Aron Griffis <agriffis@n01se.net>
> +     12 Carl Worth <cworth@cworth.org>
> +      1 Chris Wilson <chris@chris-wilson.co.uk>
> +      1 François Boulogne <boulogne.f@gmail.com>
> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
> +      1 Israel Herraiz <isra@herraiz.org>
> +      4 Jan Janak <jan@ryngle.com>
> +      2 Jjgod Jiang <gzjjgod@gmail.com>
> +      7 Keith Packard <keithp@keithp.com>
> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
> +      5 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
> +      3 Stewart Smith <stewart@flamingspork.com>
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=recipients"
> +notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Allan McRae <allan@archlinux.org>
> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
> +      1 Keith Packard <keithp@keithp.com>
> +      1 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      2 notmuch <notmuch@notmuchmail.org>
> +     48 notmuch@notmuchmail.org
> +      1 olivier.berger@it-sudparis.eu
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=addresses"
> +notmuch search --output=addresses '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Adrian Perez de Castro <aperez@igalia.com>
> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +      1 Allan McRae <allan@archlinux.org>
> +      1 Aron Griffis <agriffis@n01se.net>
> +     12 Carl Worth <cworth@cworth.org>
> +      1 Chris Wilson <chris@chris-wilson.co.uk>
> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
> +      1 François Boulogne <boulogne.f@gmail.com>
> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
> +      1 Israel Herraiz <isra@herraiz.org>
> +      4 Jan Janak <jan@ryngle.com>
> +      2 Jjgod Jiang <gzjjgod@gmail.com>
> +      8 Keith Packard <keithp@keithp.com>
> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
> +      6 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
> +      3 Stewart Smith <stewart@flamingspork.com>
> +      2 notmuch <notmuch@notmuchmail.org>
> +     48 notmuch@notmuchmail.org
> +      1 olivier.berger@it-sudparis.eu
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
>  test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
>  add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
>  	headers'"
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 2/4] cli: Extend the search command for --output=addresses and similar
  2014-10-06 18:56             ` Tomi Ollila
@ 2014-10-09 10:55               ` Michal Sojka
  2014-10-12 21:41                 ` [PATCH v3 0/4] notmuch search --output=sender/recipients Michal Sojka
  0 siblings, 1 reply; 31+ messages in thread
From: Michal Sojka @ 2014-10-09 10:55 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

On Mon, Oct 06 2014, Tomi Ollila wrote:
> On Sun, Oct 05 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>
>> The new outputs allow printing senders, recipients or both of matching
>> messages.
>>
>> This code based on a patch from Jani Nikula.
>
> OK, IMO...
>
> 1/4 OK
>
> Before 2/4 add support for 'flag' arguments, drop the --output=addresses
> option which is now done as --output=sender --output=recipients

OK

> In deduplication comment did not describe the deduplication at all...
> so I looked a bit into the code now... the Default you described was
> that with "John Doe" <john.doe@example.com> and "John Doe" <JOHN.DOE@EXAMPLE.COM>
> only one was printed (but not which one).

I intentionally didn't want to define which one, but I agree that it
might be useful in same cases. It would depend on --sort option and on
the order of addresses in email headers.

> Secondly, what happens with "Doe, John" <john.doe@example.com> and
> "John Doe" <JOHN.DOE@EXAMPLE.COM>... ah, it is same as *addr* with
> case-insensitive address.
>
> Sorry, but IMO these options are a bit strange.

My impression is that I did bad job describing the deduplication
algorithm, which is why you don't understand it. Maybe, we can also
change the name of the option to --filter-by, or something like this.

When thinking about how to best document such an option, it seems that
the user must be aware that this is implemented as flags that are ORed.
Which means that the default should be what was in the previous patch
--unique=none.

What about the following?

    ``--filter-flag=``\ (**addr**\ \|\ **name**\ \|\ **addrfold**)

        Can be used with ``--output=addresses``, ``--output=sender`` or
        ``--output=recipients`` to filter out duplicate addresses. The
        filtering algorithm receives a sequence of email addresses and
        outputs the same sequence without the addresses that are
        considered a duplicate of a previously output address. What is
        considered a duplicate depends on the flags given:

	**addr** means that the address part is compared.
	Case-sensitivity can be controlled by **addrfold** flag (see
	below). For example, the addresses "John Doe <john@example.com>"
	and "Dr. John Doe <john@example.com>" will be considered
	duplicate.

	**name** means that the name part is compared in case-sensitive
	manner. For example, the addresses "John Doe <john@example.com>"
	and "John Doe <john@doe.name>" will be considered duplicate.

	**addrfold** when used with the **addr** flag, the address
	comparison is performed in case-insensitive manner. For example,
	the addresses "John Doe <john@example.com>" and "Dr. John Doe
	<JOHN@EXAMPLE.COM>" will be considered duplicate.

	To specify multiple flags, this option can be given multiple
	times. For example, ``--filter-flag=name --filter-flag=addr``
	will print unique case-sensitive combinations of both name and
	address parts.

With this, the previously default behavior would now has to be spelled
as "--filter-flag=addr --filter-flag=addrfold".

I'm not sure it is wise present such a low-level interface (flags) to
command-line users, but it is hopefully more understandable now. What do
you think?

> Not to go to choose which one to choose (first, last, most common) instead
> of the suggested options these should be the ones:
>
> 1) "John Doe" <john.doe@example.com> and "John Doe" <JOHN.DOE@EXAMPLE.COM>:
> only one printed, but if either were "Dr. John Doe", both of these are printed
> (this as default).

According to the above, which could be achieved as --filter-flag=name
--filter-flag=addr --filter-flag=addrfold.

> 2) same as above, but only make case-insensitive

case-insensitive is already in 1), you probably mean case-sensitive.

> address match -- i.e. in the 2 above cases in option 1, print only
> one.

This would be --filter-flag=name --filter-flag=addr.

> (and same name but different address to perhaps never been an option...)
>
> I might like to have option that does case-sensitive address match, 

This would be just --filter-flag=addr.

As a side note, it is interesting, that you mentioned your options as an
enumeration even though they are actually combinations of several on/off
flags. I think that it is more natural for human brains to think in
terms of simple lists than in terms of combinations of flags. That's why
I originally implemented --output=addresses as just another keyword,
rather than requiring the user to specify both sender and receivers.

Thanks for the review.
-Michal

> In those cases I don't know the recipient's culture and the email he
> sent to me used format <Foo.Bar@example.org> (and not knowing which
> one is the first and which last name (or whatever names these are) --
> just to reply in same case format in respect...


>
>
> Tomi
>
>
>> ---
>>  completion/notmuch-completion.bash |   2 +-
>>  completion/notmuch-completion.zsh  |   3 +-
>>  doc/man1/notmuch-search.rst        |  22 +++++++-
>>  notmuch-search.c                   | 100 ++++++++++++++++++++++++++++++++++---
>>  test/T090-search-output.sh         |  64 ++++++++++++++++++++++++
>>  5 files changed, 182 insertions(+), 9 deletions(-)
>>
>> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
>> index 0571dc9..c37ddf5 100644
>> --- a/completion/notmuch-completion.bash
>> +++ b/completion/notmuch-completion.bash
>> @@ -294,7 +294,7 @@ _notmuch_search()
>>  	    return
>>  	    ;;
>>  	--output)
>> -	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
>> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients addresses" -- "${cur}" ) )
>>  	    return
>>  	    ;;
>>  	--sort)
>> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
>> index 67a9aba..bff8fd5 100644
>> --- a/completion/notmuch-completion.zsh
>> +++ b/completion/notmuch-completion.zsh
>> @@ -52,7 +52,8 @@ _notmuch_search()
>>    _arguments -s : \
>>      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
>>      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
>> -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
>> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
>> +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients addresses))'
>>  }
>>
>>  _notmuch()
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 90160f2..3447820 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -35,7 +35,7 @@ Supported options for **search** include
>>          intended for programs that invoke **notmuch(1)** internally. If
>>          omitted, the latest supported version will be used.
>>
>> -    ``--output=(summary|threads|messages|files|tags)``
>> +    ``--output=(summary|threads|messages|files|tags|sender|recipients|addresses)``
>>
>>          **summary**
>>              Output a summary of each thread with any message matching
>> @@ -78,6 +78,26 @@ Supported options for **search** include
>>              by null characters (--format=text0), as a JSON array
>>              (--format=json), or as an S-Expression list (--format=sexp).
>>
>> +	**sender**
>> +            Output all addresses from the *From* header that appear on
>> +            any message matching the search terms, either one per line
>> +            (--format=text), separated by null characters
>> +            (--format=text0), as a JSON array (--format=json), or as
>> +            an S-Expression list (--format=sexp).
>> +
>> +	    Note: Searching for **sender** should be much faster than
>> +	    searching for **recipients** or **addresses**, because
>> +	    sender addresses are cached directly in the database
>> +	    whereas other addresses need to be fetched from message
>> +	    files.
>> +
>> +	**recipients**
>> +            Like **sender** but for addresses from *To*, *Cc* and
>> +	    *Bcc* headers.
>> +
>> +	**addresses**
>> +	    Like **sender** and **recipients** together.
>> +
>>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>>          This option can be used to present results in either
>>          chronological order (**oldest-first**) or reverse chronological
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index 5ac2a26..0614f10 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -23,11 +23,14 @@
>>  #include "string-util.h"
>>
>>  typedef enum {
>> -    OUTPUT_SUMMARY,
>> -    OUTPUT_THREADS,
>> -    OUTPUT_MESSAGES,
>> -    OUTPUT_FILES,
>> -    OUTPUT_TAGS
>> +    OUTPUT_SUMMARY	= 1 << 0,
>> +    OUTPUT_THREADS	= 1 << 1,
>> +    OUTPUT_MESSAGES	= 1 << 2,
>> +    OUTPUT_FILES	= 1 << 3,
>> +    OUTPUT_TAGS		= 1 << 4,
>> +    OUTPUT_SENDER	= 1 << 5,
>> +    OUTPUT_RECIPIENTS	= 1 << 6,
>> +    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
>>  } output_t;
>>
>>  typedef struct {
>> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
>>      return 0;
>>  }
>>
>> +static void
>> +print_address_list (const search_options_t *o, InternetAddressList *list)
>> +{
>> +    InternetAddress *address;
>> +    int i;
>> +
>> +    for (i = 0; i < internet_address_list_length (list); i++) {
>> +	address = internet_address_list_get_address (list, i);
>> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
>> +	    InternetAddressGroup *group;
>> +	    InternetAddressList *group_list;
>> +
>> +	    group = INTERNET_ADDRESS_GROUP (address);
>> +	    group_list = internet_address_group_get_members (group);
>> +	    if (group_list == NULL)
>> +		continue;
>> +
>> +	    print_address_list (o, group_list);
>> +	} else {
>> +	    InternetAddressMailbox *mailbox;
>> +	    const char *name;
>> +	    const char *addr;
>> +	    char *full_address;
>> +
>> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
>> +
>> +	    name = internet_address_get_name (address);
>> +	    addr = internet_address_mailbox_get_addr (mailbox);
>> +
>> +	    if (name && *name)
>> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
>> +	    else
>> +		full_address = talloc_strdup (o->format, addr);
>> +
>> +	    if (!full_address) {
>> +		fprintf (stderr, "Error: out of memory\n");
>> +		break;
>> +	    }
>> +	    o->format->string (o->format, full_address);
>> +	    o->format->separator (o->format);
>> +
>> +	    talloc_free (full_address);
>> +	}
>> +    }
>> +}
>> +
>> +static void
>> +print_address_string (const search_options_t *o, const char *recipients)
>> +{
>> +    InternetAddressList *list;
>> +
>> +    if (recipients == NULL)
>> +	return;
>> +
>> +    list = internet_address_list_parse_string (recipients);
>> +    if (list == NULL)
>> +	return;
>> +
>> +    print_address_list (o, list);
>> +}
>> +
>>  static int
>>  do_search_messages (search_options_t *o)
>>  {
>> @@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
>>
>>  	    notmuch_filenames_destroy( filenames );
>>
>> -	} else { /* output == OUTPUT_MESSAGES */
>> +	} else if (o->output == OUTPUT_MESSAGES) {
>>  	    format->set_prefix (format, "id");
>>  	    format->string (format,
>>  			    notmuch_message_get_message_id (message));
>>  	    format->separator (format);
>> +	} else {
>> +	    if (o->output & OUTPUT_SENDER) {
>> +		const char *addrs;
>> +
>> +		addrs = notmuch_message_get_header (message, "from");
>> +		print_address_string (o, addrs);
>> +	    }
>> +
>> +	    if (o->output & OUTPUT_RECIPIENTS) {
>> +		const char *hdrs[] = { "to", "cc", "bcc" };
>> +		const char *addrs;
>> +		size_t j;
>> +
>> +		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
>> +		    addrs = notmuch_message_get_header (message, hdrs[j]);
>> +		    print_address_string (o, addrs);
>> +		}
>> +	    }
>>  	}
>>
>>  	notmuch_message_destroy (message);
>> @@ -370,6 +452,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>>  				  { "threads", OUTPUT_THREADS },
>>  				  { "messages", OUTPUT_MESSAGES },
>> +				  { "sender", OUTPUT_SENDER },
>> +				  { "recipients", OUTPUT_RECIPIENTS },
>> +				  { "addresses", OUTPUT_ADDRESSES },
>>  				  { "files", OUTPUT_FILES },
>>  				  { "tags", OUTPUT_TAGS },
>>  				  { 0, 0 } } },
>> @@ -461,6 +546,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  	ret = do_search_threads (&o);
>>  	break;
>>      case OUTPUT_MESSAGES:
>> +    case OUTPUT_SENDER:
>> +    case OUTPUT_RECIPIENTS:
>> +    case OUTPUT_ADDRESSES:
>>      case OUTPUT_FILES:
>>  	ret = do_search_messages (&o);
>>  	break;
>> diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
>> index 947d572..5458de1 100755
>> --- a/test/T090-search-output.sh
>> +++ b/test/T090-search-output.sh
>> @@ -387,6 +387,70 @@ cat <<EOF >EXPECTED
>>  EOF
>>  test_expect_equal_file OUTPUT EXPECTED
>>
>> +test_begin_subtest "--output=sender"
>> +notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
>> +cat <<EOF >EXPECTED
>> +      1 Adrian Perez de Castro <aperez@igalia.com>
>> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
>> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
>> +      1 Aron Griffis <agriffis@n01se.net>
>> +     12 Carl Worth <cworth@cworth.org>
>> +      1 Chris Wilson <chris@chris-wilson.co.uk>
>> +      1 François Boulogne <boulogne.f@gmail.com>
>> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
>> +      1 Israel Herraiz <isra@herraiz.org>
>> +      4 Jan Janak <jan@ryngle.com>
>> +      2 Jjgod Jiang <gzjjgod@gmail.com>
>> +      7 Keith Packard <keithp@keithp.com>
>> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
>> +      5 Mikhail Gusarov <dottedmag@dottedmag.net>
>> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
>> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
>> +      3 Stewart Smith <stewart@flamingspork.com>
>> +EOF
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>> +test_begin_subtest "--output=recipients"
>> +notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
>> +cat <<EOF >EXPECTED
>> +      1 Allan McRae <allan@archlinux.org>
>> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
>> +      1 Keith Packard <keithp@keithp.com>
>> +      1 Mikhail Gusarov <dottedmag@dottedmag.net>
>> +      2 notmuch <notmuch@notmuchmail.org>
>> +     48 notmuch@notmuchmail.org
>> +      1 olivier.berger@it-sudparis.eu
>> +EOF
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>> +test_begin_subtest "--output=addresses"
>> +notmuch search --output=addresses '*' | sort | uniq --count >OUTPUT
>> +cat <<EOF >EXPECTED
>> +      1 Adrian Perez de Castro <aperez@igalia.com>
>> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
>> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
>> +      1 Allan McRae <allan@archlinux.org>
>> +      1 Aron Griffis <agriffis@n01se.net>
>> +     12 Carl Worth <cworth@cworth.org>
>> +      1 Chris Wilson <chris@chris-wilson.co.uk>
>> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
>> +      1 François Boulogne <boulogne.f@gmail.com>
>> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
>> +      1 Israel Herraiz <isra@herraiz.org>
>> +      4 Jan Janak <jan@ryngle.com>
>> +      2 Jjgod Jiang <gzjjgod@gmail.com>
>> +      8 Keith Packard <keithp@keithp.com>
>> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
>> +      6 Mikhail Gusarov <dottedmag@dottedmag.net>
>> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
>> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
>> +      3 Stewart Smith <stewart@flamingspork.com>
>> +      2 notmuch <notmuch@notmuchmail.org>
>> +     48 notmuch@notmuchmail.org
>> +      1 olivier.berger@it-sudparis.eu
>> +EOF
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>>  test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
>>  add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
>>  	headers'"
>> --
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 0/4] notmuch search --output=sender/recipients
  2014-10-09 10:55               ` Michal Sojka
@ 2014-10-12 21:41                 ` Michal Sojka
  2014-10-12 21:41                   ` [PATCH v3 1/4] cli: Refactor option passing in the search command Michal Sojka
                                     ` (3 more replies)
  0 siblings, 4 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-12 21:41 UTC (permalink / raw)
  To: notmuch

Hi,

this is a third version of my adaptation of Jani's patch series adding
--output=sender/recipients and related arguments to notmuch search.

The 1st patch is the same as in v2 (Marked as OK in
id:m24mvht4c4.fsf@guru.guru-group.fi).

The 2nd patch is not changed as well, but in v2 it was patch 3/4.

The 3rd patch is rewritten to use the "keyword flags" introduced in
patch 2 (requested by Tomi). The code is basically the same as in
id:1410021689-15901-1-git-send-email-jani@nikula.org, but tests are
added and shell completion is updated.

Finally, last patch adds --filter-by option that allows one to filter
out duplicate addresses. This option was called --unique in v2 and the
the semantic is slightly different now. This resulted in simpler code.
The documentation was also reworked and is hopefully more
understandable now.

-Michal


Jani Nikula (1):
  cli: Add support for parsing multiple keyword arguments

Michal Sojka (3):
  cli: Refactor option passing in the search command
  cli: Extend the search command for --output={sender,recipients}
  cli: Add an option to filter our duplicate addresses

 command-line-arguments.c           |   6 +-
 command-line-arguments.h           |   1 +
 completion/notmuch-completion.bash |   8 +-
 completion/notmuch-completion.zsh  |   4 +-
 doc/man1/notmuch-search.rst        |  54 ++++++-
 notmuch-search.c                   | 309 +++++++++++++++++++++++++++++--------
 test/T090-search-output.sh         |  64 ++++++++
 test/T095-search-filter-by.sh      |  55 +++++++
 test/T410-argument-parsing.sh      |   3 +-
 test/arg-test.c                    |   9 ++
 10 files changed, 440 insertions(+), 73 deletions(-)
 create mode 100755 test/T095-search-filter-by.sh

-- 
2.1.1

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 1/4] cli: Refactor option passing in the search command
  2014-10-12 21:41                 ` [PATCH v3 0/4] notmuch search --output=sender/recipients Michal Sojka
@ 2014-10-12 21:41                   ` Michal Sojka
  2014-10-22 10:44                     ` Mark Walters
  2014-10-12 21:41                   ` [PATCH v3 2/4] cli: Add support for parsing multiple keyword arguments Michal Sojka
                                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 31+ messages in thread
From: Michal Sojka @ 2014-10-12 21:41 UTC (permalink / raw)
  To: notmuch

Many functions that implement the search command need to access command
line options. Instead of passing each option in a separate variable, put
them in a structure and pass only this structure.

This will become handy in the following patches.
---
 notmuch-search.c | 122 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 62 insertions(+), 60 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index bc9be45..5ac2a26 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -30,6 +30,16 @@ typedef enum {
     OUTPUT_TAGS
 } output_t;
 
+typedef struct {
+    sprinter_t *format;
+    notmuch_query_t *query;
+    notmuch_sort_t sort;
+    output_t output;
+    int offset;
+    int limit;
+    int dupe;
+} search_options_t;
+
 /* Return two stable query strings that identify exactly the matched
  * and unmatched messages currently in thread.  If there are no
  * matched or unmatched messages, the returned buffers will be
@@ -70,46 +80,42 @@ get_thread_query (notmuch_thread_t *thread,
 }
 
 static int
-do_search_threads (sprinter_t *format,
-		   notmuch_query_t *query,
-		   notmuch_sort_t sort,
-		   output_t output,
-		   int offset,
-		   int limit)
+do_search_threads (search_options_t *o)
 {
     notmuch_thread_t *thread;
     notmuch_threads_t *threads;
     notmuch_tags_t *tags;
+    sprinter_t *format = o->format;
     time_t date;
     int i;
 
-    if (offset < 0) {
-	offset += notmuch_query_count_threads (query);
-	if (offset < 0)
-	    offset = 0;
+    if (o->offset < 0) {
+	o->offset += notmuch_query_count_threads (o->query);
+	if (o->offset < 0)
+	    o->offset = 0;
     }
 
-    threads = notmuch_query_search_threads (query);
+    threads = notmuch_query_search_threads (o->query);
     if (threads == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
+	 notmuch_threads_valid (threads) && (o->limit < 0 || i < o->offset + o->limit);
 	 notmuch_threads_move_to_next (threads), i++)
     {
 	thread = notmuch_threads_get (threads);
 
-	if (i < offset) {
+	if (i < o->offset) {
 	    notmuch_thread_destroy (thread);
 	    continue;
 	}
 
-	if (output == OUTPUT_THREADS) {
+	if (o->output == OUTPUT_THREADS) {
 	    format->set_prefix (format, "thread");
 	    format->string (format,
-			    notmuch_thread_get_thread_id (thread));
+			       notmuch_thread_get_thread_id (thread));
 	    format->separator (format);
 	} else { /* output == OUTPUT_SUMMARY */
 	    void *ctx_quote = talloc_new (thread);
@@ -123,7 +129,7 @@ do_search_threads (sprinter_t *format,
 
 	    format->begin_map (format);
 
-	    if (sort == NOTMUCH_SORT_OLDEST_FIRST)
+	    if (o->sort == NOTMUCH_SORT_OLDEST_FIRST)
 		date = notmuch_thread_get_oldest_date (thread);
 	    else
 		date = notmuch_thread_get_newest_date (thread);
@@ -215,40 +221,36 @@ do_search_threads (sprinter_t *format,
 }
 
 static int
-do_search_messages (sprinter_t *format,
-		    notmuch_query_t *query,
-		    output_t output,
-		    int offset,
-		    int limit,
-		    int dupe)
+do_search_messages (search_options_t *o)
 {
     notmuch_message_t *message;
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
+    sprinter_t *format = o->format;
     int i;
 
-    if (offset < 0) {
-	offset += notmuch_query_count_messages (query);
-	if (offset < 0)
-	    offset = 0;
+    if (o->offset < 0) {
+	o->offset += notmuch_query_count_messages (o->query);
+	if (o->offset < 0)
+	    o->offset = 0;
     }
 
-    messages = notmuch_query_search_messages (query);
+    messages = notmuch_query_search_messages (o->query);
     if (messages == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit);
+	 notmuch_messages_valid (messages) && (o->limit < 0 || i < o->offset + o->limit);
 	 notmuch_messages_move_to_next (messages), i++)
     {
-	if (i < offset)
+	if (i < o->offset)
 	    continue;
 
 	message = notmuch_messages_get (messages);
 
-	if (output == OUTPUT_FILES) {
+	if (o->output == OUTPUT_FILES) {
 	    int j;
 	    filenames = notmuch_message_get_filenames (message);
 
@@ -256,7 +258,7 @@ do_search_messages (sprinter_t *format,
 		 notmuch_filenames_valid (filenames);
 		 notmuch_filenames_move_to_next (filenames), j++)
 	    {
-		if (dupe < 0 || dupe == j) {
+		if (o->dupe < 0 || o->dupe == j) {
 		    format->string (format, notmuch_filenames_get (filenames));
 		    format->separator (format);
 		}
@@ -333,16 +335,16 @@ int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
     notmuch_database_t *notmuch;
-    notmuch_query_t *query;
+    search_options_t o = {
+	.sort = NOTMUCH_SORT_NEWEST_FIRST,
+	.output = OUTPUT_SUMMARY,
+	.offset = 0,
+	.limit = -1, /* unlimited */
+	.dupe = -1,
+    };
     char *query_str;
-    notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;
-    sprinter_t *format = NULL;
     int opt_index, ret;
-    output_t output = OUTPUT_SUMMARY;
-    int offset = 0;
-    int limit = -1; /* unlimited */
     notmuch_exclude_t exclude = NOTMUCH_EXCLUDE_TRUE;
-    int dupe = -1;
     unsigned int i;
 
     enum {
@@ -353,7 +355,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     } format_sel = NOTMUCH_FORMAT_TEXT;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',
+	{ NOTMUCH_OPT_KEYWORD, &o.sort, "sort", 's',
 	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
 				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
 				  { 0, 0 } } },
@@ -364,7 +366,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "text0", NOTMUCH_FORMAT_TEXT0 },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
+	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
@@ -377,9 +379,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
                                   { "flag", NOTMUCH_EXCLUDE_FLAG },
                                   { "all", NOTMUCH_EXCLUDE_ALL },
                                   { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
-	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
-	{ NOTMUCH_OPT_INT, &dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_INT, &o.offset, "offset", 'O', 0 },
+	{ NOTMUCH_OPT_INT, &o.limit, "limit", 'L', 0  },
+	{ NOTMUCH_OPT_INT, &o.dupe, "duplicate", 'D', 0  },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -389,20 +391,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	format = sprinter_text_create (config, stdout);
+	o.format = sprinter_text_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_TEXT0:
-	if (output == OUTPUT_SUMMARY) {
+	if (o.output == OUTPUT_SUMMARY) {
 	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
 	    return EXIT_FAILURE;
 	}
-	format = sprinter_text0_create (config, stdout);
+	o.format = sprinter_text0_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	format = sprinter_json_create (config, stdout);
+	o.format = sprinter_json_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_SEXP:
-	format = sprinter_sexp_create (config, stdout);
+	o.format = sprinter_sexp_create (config, stdout);
 	break;
     default:
 	/* this should never happen */
@@ -425,15 +427,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    query = notmuch_query_create (notmuch, query_str);
-    if (query == NULL) {
+    o.query = notmuch_query_create (notmuch, query_str);
+    if (o.query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
     }
 
-    notmuch_query_set_sort (query, sort);
+    notmuch_query_set_sort (o.query, o.sort);
 
-    if (exclude == NOTMUCH_EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {
+    if (exclude == NOTMUCH_EXCLUDE_FLAG && o.output != OUTPUT_SUMMARY) {
 	/* If we are not doing summary output there is nowhere to
 	 * print the excluded flag so fall back on including the
 	 * excluded messages. */
@@ -448,29 +450,29 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	search_exclude_tags = notmuch_config_get_search_exclude_tags
 	    (config, &search_exclude_tags_length);
 	for (i = 0; i < search_exclude_tags_length; i++)
-	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
-	notmuch_query_set_omit_excluded (query, exclude);
+	    notmuch_query_add_tag_exclude (o.query, search_exclude_tags[i]);
+	notmuch_query_set_omit_excluded (o.query, exclude);
     }
 
-    switch (output) {
+    switch (o.output) {
     default:
     case OUTPUT_SUMMARY:
     case OUTPUT_THREADS:
-	ret = do_search_threads (format, query, sort, output, offset, limit);
+	ret = do_search_threads (&o);
 	break;
     case OUTPUT_MESSAGES:
     case OUTPUT_FILES:
-	ret = do_search_messages (format, query, output, offset, limit, dupe);
+	ret = do_search_messages (&o);
 	break;
     case OUTPUT_TAGS:
-	ret = do_search_tags (notmuch, format, query);
+	ret = do_search_tags (notmuch, o.format, o.query);
 	break;
     }
 
-    notmuch_query_destroy (query);
+    notmuch_query_destroy (o.query);
     notmuch_database_destroy (notmuch);
 
-    talloc_free (format);
+    talloc_free (o.format);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 2/4] cli: Add support for parsing multiple keyword arguments
  2014-10-12 21:41                 ` [PATCH v3 0/4] notmuch search --output=sender/recipients Michal Sojka
  2014-10-12 21:41                   ` [PATCH v3 1/4] cli: Refactor option passing in the search command Michal Sojka
@ 2014-10-12 21:41                   ` Michal Sojka
  2014-10-12 21:41                   ` [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients} Michal Sojka
  2014-10-12 21:41                   ` [PATCH v3 4/4] cli: Add an option to filter our duplicate addresses Michal Sojka
  3 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-12 21:41 UTC (permalink / raw)
  To: notmuch

From: Jani Nikula <jani@nikula.org>

This allows having multiple --foo=bar --foo=baz options on the command
line, with the corresponding values OR'd together.

[Test added by Michal Sojka]
---
 command-line-arguments.c      | 6 +++++-
 command-line-arguments.h      | 1 +
 test/T410-argument-parsing.sh | 3 ++-
 test/arg-test.c               | 9 +++++++++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index 844d6c3..c6f7269 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -23,7 +23,10 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
     while (keywords->name) {
 	if (strcmp (arg_str, keywords->name) == 0) {
 	    if (arg_desc->output_var) {
-		*((int *)arg_desc->output_var) = keywords->value;
+		if (arg_desc->opt_type == NOTMUCH_OPT_KEYWORD_FLAGS)
+		    *((int *)arg_desc->output_var) |= keywords->value;
+		else
+		    *((int *)arg_desc->output_var) = keywords->value;
 	    }
 	    return TRUE;
 	}
@@ -152,6 +155,7 @@ parse_option (const char *arg,
 
 	switch (try->opt_type) {
 	case NOTMUCH_OPT_KEYWORD:
+	case NOTMUCH_OPT_KEYWORD_FLAGS:
 	    return _process_keyword_arg (try, next, value);
 	case NOTMUCH_OPT_BOOLEAN:
 	    return _process_boolean_arg (try, next, value);
diff --git a/command-line-arguments.h b/command-line-arguments.h
index de1734a..085a492 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -8,6 +8,7 @@ enum notmuch_opt_type {
     NOTMUCH_OPT_BOOLEAN,	/* --verbose              */
     NOTMUCH_OPT_INT,		/* --frob=8               */
     NOTMUCH_OPT_KEYWORD,	/* --format=raw|json|text */
+    NOTMUCH_OPT_KEYWORD_FLAGS,  /* the above with values OR'd together */
     NOTMUCH_OPT_STRING,		/* --file=/tmp/gnarf.txt  */
     NOTMUCH_OPT_POSITION	/* notmuch dump pos_arg   */
 };
diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh
index 94e9087..2e5d7ae 100755
--- a/test/T410-argument-parsing.sh
+++ b/test/T410-argument-parsing.sh
@@ -3,9 +3,10 @@ test_description="argument parsing"
 . ./test-lib.sh
 
 test_begin_subtest "sanity check"
-$TEST_DIRECTORY/arg-test  pos1  --keyword=one --string=foo pos2 --int=7 > OUTPUT
+$TEST_DIRECTORY/arg-test  pos1  --keyword=one --string=foo pos2 --int=7 --flag=one --flag=three > OUTPUT
 cat <<EOF > EXPECTED
 keyword 1
+flags 5
 int 7
 string foo
 positional arg 1 pos1
diff --git a/test/arg-test.c b/test/arg-test.c
index 6c49eac..736686d 100644
--- a/test/arg-test.c
+++ b/test/arg-test.c
@@ -7,6 +7,7 @@ int main(int argc, char **argv){
     int opt_index=1;
 
     int kw_val=0;
+    int fl_val=0;
     int int_val=0;
     char *pos_arg1=NULL;
     char *pos_arg2=NULL;
@@ -17,6 +18,11 @@ int main(int argc, char **argv){
 	  (notmuch_keyword_t []){ { "one", 1 },
 				  { "two", 2 },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &fl_val, "flag", 'f',
+	  (notmuch_keyword_t []){ { "one",   1 << 0},
+				  { "two",   1 << 1 },
+				  { "three", 1 << 2 },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &int_val, "int", 'i', 0},
 	{ NOTMUCH_OPT_STRING, &string_val, "string", 's', 0},
 	{ NOTMUCH_OPT_POSITION, &pos_arg1, 0,0, 0},
@@ -31,6 +37,9 @@ int main(int argc, char **argv){
     if (kw_val)
 	printf("keyword %d\n", kw_val);
 
+    if (fl_val)
+	printf("flags %d\n", fl_val);
+
     if (int_val)
 	printf("int %d\n", int_val);
 
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-12 21:41                 ` [PATCH v3 0/4] notmuch search --output=sender/recipients Michal Sojka
  2014-10-12 21:41                   ` [PATCH v3 1/4] cli: Refactor option passing in the search command Michal Sojka
  2014-10-12 21:41                   ` [PATCH v3 2/4] cli: Add support for parsing multiple keyword arguments Michal Sojka
@ 2014-10-12 21:41                   ` Michal Sojka
  2014-10-13 19:00                     ` Tomi Ollila
                                       ` (2 more replies)
  2014-10-12 21:41                   ` [PATCH v3 4/4] cli: Add an option to filter our duplicate addresses Michal Sojka
  3 siblings, 3 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-12 21:41 UTC (permalink / raw)
  To: notmuch

The new outputs allow printing senders, recipients or both of matching
messages. The --output option is converted from "keyword" argument to
"flags" argument, which means that the user can use --output=sender and
--output=recipients simultaneously, to print both. Other combinations
produce an error.

This code based on a patch from Jani Nikula.
---
 completion/notmuch-completion.bash |   2 +-
 completion/notmuch-completion.zsh  |   3 +-
 doc/man1/notmuch-search.rst        |  22 +++++++-
 notmuch-search.c                   | 110 ++++++++++++++++++++++++++++++++++---
 test/T090-search-output.sh         |  64 +++++++++++++++++++++
 5 files changed, 189 insertions(+), 12 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 0571dc9..cfbd389 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -294,7 +294,7 @@ _notmuch_search()
 	    return
 	    ;;
 	--output)
-	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients" -- "${cur}" ) )
 	    return
 	    ;;
 	--sort)
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index 67a9aba..3e52a00 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -52,7 +52,8 @@ _notmuch_search()
   _arguments -s : \
     '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
     '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
-    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
+    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
+    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index 90160f2..c9d38b1 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -35,7 +35,7 @@ Supported options for **search** include
         intended for programs that invoke **notmuch(1)** internally. If
         omitted, the latest supported version will be used.
 
-    ``--output=(summary|threads|messages|files|tags)``
+    ``--output=(summary|threads|messages|files|tags|sender|recipients)``
 
         **summary**
             Output a summary of each thread with any message matching
@@ -78,6 +78,26 @@ Supported options for **search** include
             by null characters (--format=text0), as a JSON array
             (--format=json), or as an S-Expression list (--format=sexp).
 
+	**sender**
+            Output all addresses from the *From* header that appear on
+            any message matching the search terms, either one per line
+            (--format=text), separated by null characters
+            (--format=text0), as a JSON array (--format=json), or as
+            an S-Expression list (--format=sexp).
+
+	    Note: Searching for **sender** should be much faster than
+	    searching for **recipients**, because sender addresses are
+	    cached directly in the database whereas other addresses
+	    need to be fetched from message files.
+
+	**recipients**
+            Like **sender** but for addresses from *To*, *Cc* and
+	    *Bcc* headers.
+
+	This option can be given multiple times to combine different
+	outputs. Curently, this is only supported for **sender** and
+	**recipients** outputs.
+
     ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
         This option can be used to present results in either
         chronological order (**oldest-first**) or reverse chronological
diff --git a/notmuch-search.c b/notmuch-search.c
index 5ac2a26..74588f8 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -23,11 +23,14 @@
 #include "string-util.h"
 
 typedef enum {
-    OUTPUT_SUMMARY,
-    OUTPUT_THREADS,
-    OUTPUT_MESSAGES,
-    OUTPUT_FILES,
-    OUTPUT_TAGS
+    OUTPUT_SUMMARY	= 1 << 0,
+    OUTPUT_THREADS	= 1 << 1,
+    OUTPUT_MESSAGES	= 1 << 2,
+    OUTPUT_FILES	= 1 << 3,
+    OUTPUT_TAGS		= 1 << 4,
+    OUTPUT_SENDER	= 1 << 5,
+    OUTPUT_RECIPIENTS	= 1 << 6,
+    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
 } output_t;
 
 typedef struct {
@@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
     return 0;
 }
 
+static void
+print_address_list (const search_options_t *o, InternetAddressList *list)
+{
+    InternetAddress *address;
+    int i;
+
+    for (i = 0; i < internet_address_list_length (list); i++) {
+	address = internet_address_list_get_address (list, i);
+	if (INTERNET_ADDRESS_IS_GROUP (address)) {
+	    InternetAddressGroup *group;
+	    InternetAddressList *group_list;
+
+	    group = INTERNET_ADDRESS_GROUP (address);
+	    group_list = internet_address_group_get_members (group);
+	    if (group_list == NULL)
+		continue;
+
+	    print_address_list (o, group_list);
+	} else {
+	    InternetAddressMailbox *mailbox;
+	    const char *name;
+	    const char *addr;
+	    char *full_address;
+
+	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
+
+	    name = internet_address_get_name (address);
+	    addr = internet_address_mailbox_get_addr (mailbox);
+
+	    if (name && *name)
+		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
+	    else
+		full_address = talloc_strdup (o->format, addr);
+
+	    if (!full_address) {
+		fprintf (stderr, "Error: out of memory\n");
+		break;
+	    }
+	    o->format->string (o->format, full_address);
+	    o->format->separator (o->format);
+
+	    talloc_free (full_address);
+	}
+    }
+}
+
+static void
+print_address_string (const search_options_t *o, const char *recipients)
+{
+    InternetAddressList *list;
+
+    if (recipients == NULL)
+	return;
+
+    list = internet_address_list_parse_string (recipients);
+    if (list == NULL)
+	return;
+
+    print_address_list (o, list);
+}
+
 static int
 do_search_messages (search_options_t *o)
 {
@@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
 	    
 	    notmuch_filenames_destroy( filenames );
 
-	} else { /* output == OUTPUT_MESSAGES */
+	} else if (o->output == OUTPUT_MESSAGES) {
 	    format->set_prefix (format, "id");
 	    format->string (format,
 			    notmuch_message_get_message_id (message));
 	    format->separator (format);
+	} else {
+	    if (o->output & OUTPUT_SENDER) {
+		const char *addrs;
+
+		addrs = notmuch_message_get_header (message, "from");
+		print_address_string (o, addrs);
+	    }
+
+	    if (o->output & OUTPUT_RECIPIENTS) {
+		const char *hdrs[] = { "to", "cc", "bcc" };
+		const char *addrs;
+		size_t j;
+
+		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
+		    addrs = notmuch_message_get_header (message, hdrs[j]);
+		    print_address_string (o, addrs);
+		}
+	    }
 	}
 
 	notmuch_message_destroy (message);
@@ -337,7 +419,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_database_t *notmuch;
     search_options_t o = {
 	.sort = NOTMUCH_SORT_NEWEST_FIRST,
-	.output = OUTPUT_SUMMARY,
+	.output = 0,
 	.offset = 0,
 	.limit = -1, /* unlimited */
 	.dupe = -1,
@@ -366,10 +448,12 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "text0", NOTMUCH_FORMAT_TEXT0 },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &o.output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
+				  { "sender", OUTPUT_SENDER },
+				  { "recipients", OUTPUT_RECIPIENTS },
 				  { "files", OUTPUT_FILES },
 				  { "tags", OUTPUT_TAGS },
 				  { 0, 0 } } },
@@ -389,6 +473,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
+    if (! o.output)
+	o.output = OUTPUT_SUMMARY;
+
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
 	o.format = sprinter_text_create (config, stdout);
@@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     switch (o.output) {
-    default:
     case OUTPUT_SUMMARY:
     case OUTPUT_THREADS:
 	ret = do_search_threads (&o);
 	break;
     case OUTPUT_MESSAGES:
+    case OUTPUT_SENDER:
+    case OUTPUT_RECIPIENTS:
+    case OUTPUT_ADDRESSES:
     case OUTPUT_FILES:
 	ret = do_search_messages (&o);
 	break;
     case OUTPUT_TAGS:
 	ret = do_search_tags (notmuch, o.format, o.query);
 	break;
+    default:
+	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
+	ret = 1;
     }
 
     notmuch_query_destroy (o.query);
diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
index 947d572..e696c01 100755
--- a/test/T090-search-output.sh
+++ b/test/T090-search-output.sh
@@ -387,6 +387,70 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "--output=sender"
+notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
+cat <<EOF >EXPECTED
+      1 Adrian Perez de Castro <aperez@igalia.com>
+      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
+      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+      1 Aron Griffis <agriffis@n01se.net>
+     12 Carl Worth <cworth@cworth.org>
+      1 Chris Wilson <chris@chris-wilson.co.uk>
+      1 François Boulogne <boulogne.f@gmail.com>
+      1 Ingmar Vanhassel <ingmar@exherbo.org>
+      1 Israel Herraiz <isra@herraiz.org>
+      4 Jan Janak <jan@ryngle.com>
+      2 Jjgod Jiang <gzjjgod@gmail.com>
+      7 Keith Packard <keithp@keithp.com>
+      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
+      5 Mikhail Gusarov <dottedmag@dottedmag.net>
+      1 Olivier Berger <olivier.berger@it-sudparis.eu>
+      1 Rolland Santimano <rollandsantimano@yahoo.com>
+      3 Stewart Smith <stewart@flamingspork.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients"
+notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
+cat <<EOF >EXPECTED
+      1 Allan McRae <allan@archlinux.org>
+      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
+      1 Keith Packard <keithp@keithp.com>
+      1 Mikhail Gusarov <dottedmag@dottedmag.net>
+      2 notmuch <notmuch@notmuchmail.org>
+     48 notmuch@notmuchmail.org
+      1 olivier.berger@it-sudparis.eu
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=sender --output=recipients"
+notmuch search --output=sender --output=recipients '*' | sort | uniq --count >OUTPUT
+cat <<EOF >EXPECTED
+      1 Adrian Perez de Castro <aperez@igalia.com>
+      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
+      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+      1 Allan McRae <allan@archlinux.org>
+      1 Aron Griffis <agriffis@n01se.net>
+     12 Carl Worth <cworth@cworth.org>
+      1 Chris Wilson <chris@chris-wilson.co.uk>
+      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
+      1 François Boulogne <boulogne.f@gmail.com>
+      1 Ingmar Vanhassel <ingmar@exherbo.org>
+      1 Israel Herraiz <isra@herraiz.org>
+      4 Jan Janak <jan@ryngle.com>
+      2 Jjgod Jiang <gzjjgod@gmail.com>
+      8 Keith Packard <keithp@keithp.com>
+      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
+      6 Mikhail Gusarov <dottedmag@dottedmag.net>
+      1 Olivier Berger <olivier.berger@it-sudparis.eu>
+      1 Rolland Santimano <rollandsantimano@yahoo.com>
+      3 Stewart Smith <stewart@flamingspork.com>
+      2 notmuch <notmuch@notmuchmail.org>
+     48 notmuch@notmuchmail.org
+      1 olivier.berger@it-sudparis.eu
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
 add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
 	headers'"
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 4/4] cli: Add an option to filter our duplicate addresses
  2014-10-12 21:41                 ` [PATCH v3 0/4] notmuch search --output=sender/recipients Michal Sojka
                                     ` (2 preceding siblings ...)
  2014-10-12 21:41                   ` [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients} Michal Sojka
@ 2014-10-12 21:41                   ` Michal Sojka
  3 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-12 21:41 UTC (permalink / raw)
  To: notmuch

This adds a --filter-by option to "notmuch search". It can be used to
filter out duplicate addresses in --output=sender/receivers.

The code here is an extended version of a patch from Jani Nikula.
---
 completion/notmuch-completion.bash |  6 ++-
 completion/notmuch-completion.zsh  |  3 +-
 doc/man1/notmuch-search.rst        | 32 +++++++++++++
 notmuch-search.c                   | 93 +++++++++++++++++++++++++++++++++++---
 test/T095-search-filter-by.sh      | 55 ++++++++++++++++++++++
 5 files changed, 181 insertions(+), 8 deletions(-)
 create mode 100755 test/T095-search-filter-by.sh

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index cfbd389..41dd85b 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -305,12 +305,16 @@ _notmuch_search()
 	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
 	    return
 	    ;;
+	--filter-by)
+	    COMPREPLY=( $( compgen -W "addr addrfold name" -- "${cur}" ) )
+	    return
+	    ;;
     esac
 
     ! $split &&
     case "${cur}" in
 	-*)
-	    local options="--format= --output= --sort= --offset= --limit= --exclude= --duplicate="
+	    local options="--format= --output= --sort= --offset= --limit= --exclude= --duplicate= --filter-by="
 	    compopt -o nospace
 	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
 	    ;;
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index 3e52a00..17b345f 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -53,7 +53,8 @@ _notmuch_search()
     '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
     '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
     '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
-    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
+    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))' \
+    '--filter-by=[filter out duplicate addresses]:filter-by:((addr\:"address part" addrfold\:"case-insensitive address part" name\:"name part"))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index c9d38b1..0fed76e 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -85,6 +85,9 @@ Supported options for **search** include
             (--format=text0), as a JSON array (--format=json), or as
             an S-Expression list (--format=sexp).
 
+            Handling of duplicate addresses and/or names can be
+            controlled with the --filter-by option.
+
 	    Note: Searching for **sender** should be much faster than
 	    searching for **recipients**, because sender addresses are
 	    cached directly in the database whereas other addresses
@@ -151,6 +154,35 @@ Supported options for **search** include
         prefix. The prefix matches messages based on filenames. This
         option filters filenames of the matching messages.
 
+    ``--filter-by=``\ (**addr**\ \|\ **addrfold**\ \|\ **name**)
+
+	Can be used with ``--output=sender`` or
+	``--output=recipients`` to filter out duplicate addresses. The
+	filtering algorithm receives a sequence of email addresses and
+	outputs the same sequence without the addresses that are
+	considered a duplicate of a previously output address. What is
+	considered a duplicate depends on how two addresses are
+	compared and this can be controlled by the follwing flags:
+
+	**addr** means that the address part is compared in
+	case-sensitive manner. For example, the addresses "John Doe
+	<john@example.com>" and "Dr. John Doe <john@example.com>" will
+	be considered duplicate.
+
+	**addrfold** is similar to **addr**, but in addition to it
+	case folding is performed before comparison. For example, the
+	addresses "John Doe <john@example.com>" and "Dr. John Doe
+	<JOHN@EXAMPLE.COM>" will be considered duplicate.
+
+	**name** means that the name part is compared in case-sensitive
+	manner. For example, the addresses "John Doe <me@example.com>"
+	and "John Doe <john@doe.name>" will be considered duplicate.
+
+	This option can be given multiple times to combine the effects
+	of the flags. For example,
+	``--filter-by=name --filter-by=addr`` will print unique
+	case-sensitive combinations of both name and address parts.
+
 EXIT STATUS
 ===========
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 74588f8..df678ad 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -33,6 +33,12 @@ typedef enum {
     OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
 } output_t;
 
+typedef enum {
+    FILTER_FLAG_ADDR  = 1 << 0,
+    FILTER_FLAG_NAME  = 1 << 1,
+    FILTER_FLAG_AFOLD = 1 << 2,
+} filter_flag_t;
+
 typedef struct {
     sprinter_t *format;
     notmuch_query_t *query;
@@ -41,6 +47,7 @@ typedef struct {
     int offset;
     int limit;
     int dupe;
+    filter_flag_t filter_flags;
 } search_options_t;
 
 /* Return two stable query strings that identify exactly the matched
@@ -223,8 +230,54 @@ do_search_threads (search_options_t *o)
     return 0;
 }
 
+/* Returns TRUE iff name and/or addr is considered duplicite. */
+static notmuch_bool_t
+check_duplicite (const search_options_t *o, GHashTable *addrs, const char *name, const char *addr)
+{
+    notmuch_bool_t duplicite;
+    char *key;
+
+    if (o->filter_flags == 0)
+	return FALSE;
+
+    if (o->filter_flags & FILTER_FLAG_AFOLD) {
+	gchar *folded = g_utf8_casefold (addr, -1);
+	addr = talloc_strdup (o->format, folded);
+	g_free (folded);
+    }
+    switch (o->filter_flags & (FILTER_FLAG_ADDR | FILTER_FLAG_NAME)) {
+    case FILTER_FLAG_NAME:
+	key = talloc_strdup (o->format, name); /* !name results in !key */
+	break;
+    case FILTER_FLAG_ADDR:
+	key = talloc_strdup (o->format, addr);
+	break;
+    case FILTER_FLAG_NAME | FILTER_FLAG_ADDR:
+	key = talloc_asprintf (o->format, "%s <%s>", name, addr);
+	break;
+    default:
+	INTERNAL_ERROR("invalid --filter_by flags");
+    }
+
+    if (o->filter_flags & FILTER_FLAG_AFOLD)
+	talloc_free ((char*)addr);
+
+    if (! key)
+	return FALSE;
+
+    duplicite = g_hash_table_lookup_extended (addrs, key, NULL, NULL);
+
+    if (!duplicite)
+	g_hash_table_insert (addrs, key, NULL);
+    else
+	talloc_free (key);
+
+    return duplicite;
+}
+
 static void
-print_address_list (const search_options_t *o, InternetAddressList *list)
+print_address_list (const search_options_t *o, GHashTable *addrs,
+		    InternetAddressList *list)
 {
     InternetAddress *address;
     int i;
@@ -240,7 +293,7 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 	    if (group_list == NULL)
 		continue;
 
-	    print_address_list (o, group_list);
+	    print_address_list (o, addrs, group_list);
 	} else {
 	    InternetAddressMailbox *mailbox;
 	    const char *name;
@@ -252,6 +305,9 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 	    name = internet_address_get_name (address);
 	    addr = internet_address_mailbox_get_addr (mailbox);
 
+	    if (check_duplicite (o, addrs, name, addr))
+		continue;
+
 	    if (name && *name)
 		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
 	    else
@@ -270,7 +326,7 @@ print_address_list (const search_options_t *o, InternetAddressList *list)
 }
 
 static void
-print_address_string (const search_options_t *o, const char *recipients)
+print_address_string (const search_options_t *o, GHashTable *addrs, const char *recipients)
 {
     InternetAddressList *list;
 
@@ -281,7 +337,13 @@ print_address_string (const search_options_t *o, const char *recipients)
     if (list == NULL)
 	return;
 
-    print_address_list (o, list);
+    print_address_list (o, addrs, list);
+}
+
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+    talloc_free (ptr);
 }
 
 static int
@@ -291,8 +353,13 @@ do_search_messages (search_options_t *o)
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
     sprinter_t *format = o->format;
+    GHashTable *addresses = NULL;
     int i;
 
+    if (o->output & OUTPUT_ADDRESSES)
+	addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
+					   _my_talloc_free_for_g_hash, NULL);
+
     if (o->offset < 0) {
 	o->offset += notmuch_query_count_messages (o->query);
 	if (o->offset < 0)
@@ -340,7 +407,7 @@ do_search_messages (search_options_t *o)
 		const char *addrs;
 
 		addrs = notmuch_message_get_header (message, "from");
-		print_address_string (o, addrs);
+		print_address_string (o, addresses, addrs);
 	    }
 
 	    if (o->output & OUTPUT_RECIPIENTS) {
@@ -350,7 +417,7 @@ do_search_messages (search_options_t *o)
 
 		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
 		    addrs = notmuch_message_get_header (message, hdrs[j]);
-		    print_address_string (o, addrs);
+		    print_address_string (o, addresses, addrs);
 		}
 	    }
 	}
@@ -358,6 +425,9 @@ do_search_messages (search_options_t *o)
 	notmuch_message_destroy (message);
     }
 
+    if (addresses)
+	g_hash_table_unref (addresses);
+
     notmuch_messages_destroy (messages);
 
     format->end (format);
@@ -423,6 +493,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	.offset = 0,
 	.limit = -1, /* unlimited */
 	.dupe = -1,
+	.filter_flags = 0,
     };
     char *query_str;
     int opt_index, ret;
@@ -466,6 +537,11 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	{ NOTMUCH_OPT_INT, &o.offset, "offset", 'O', 0 },
 	{ NOTMUCH_OPT_INT, &o.limit, "limit", 'L', 0  },
 	{ NOTMUCH_OPT_INT, &o.dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &o.filter_flags, "filter-by", 'b',
+	  (notmuch_keyword_t []){ { "name", FILTER_FLAG_NAME },
+				  { "addr", FILTER_FLAG_ADDR },
+				  { "addrfold", FILTER_FLAG_ADDR | FILTER_FLAG_AFOLD },
+				  { 0, 0 } } },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -476,6 +552,11 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     if (! o.output)
 	o.output = OUTPUT_SUMMARY;
 
+    if (o.filter_flags && (o.output & ~OUTPUT_ADDRESSES)) {
+	fprintf (stderr, "Error: --filter_flag can only be used with address output.\n");
+	return EXIT_FAILURE;
+    }
+
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
 	o.format = sprinter_text_create (config, stdout);
diff --git a/test/T095-search-filter-by.sh b/test/T095-search-filter-by.sh
new file mode 100755
index 0000000..20d6a34
--- /dev/null
+++ b/test/T095-search-filter-by.sh
@@ -0,0 +1,55 @@
+#!/usr/bin/env bash
+test_description='address deduplication in "notmuch search --output=addresses"'
+. ./test-lib.sh
+
+add_message '[to]="Real Name <foo@example.com>, Real Name <bar@example.com>"'
+add_message '[to]="Nickname <foo@example.com>"' '[cc]="Real Name <Bar@Example.COM>"'
+add_message '[to]="Nickname <foo@example.com>"' '[bcc]="Real Name <Bar@Example.COM>"'
+
+test_begin_subtest "--output=recipients"
+notmuch search --output=recipients "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Nickname <foo@example.com>
+Real Name <Bar@Example.COM>
+Nickname <foo@example.com>
+Real Name <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=addr"
+notmuch search --output=recipients --filter-by=addr "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Real Name <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=addrfold"
+notmuch search --output=recipients --filter-by=addrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=name"
+notmuch search --output=recipients --filter-by=name "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Nickname <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=name --filter-by=addrfold"
+notmuch search --output=recipients --filter-by=name --filter-by=addrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+Real Name <foo@example.com>
+Real Name <bar@example.com>
+Nickname <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-12 21:41                   ` [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients} Michal Sojka
@ 2014-10-13 19:00                     ` Tomi Ollila
  2014-10-13 21:38                       ` Michal Sojka
  2014-10-22 10:51                     ` Mark Walters
  2014-10-23  9:41                     ` Mark Walters
  2 siblings, 1 reply; 31+ messages in thread
From: Tomi Ollila @ 2014-10-13 19:00 UTC (permalink / raw)
  To: Michal Sojka, notmuch

On Mon, Oct 13 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

> The new outputs allow printing senders, recipients or both of matching
> messages. The --output option is converted from "keyword" argument to
> "flags" argument, which means that the user can use --output=sender and
> --output=recipients simultaneously, to print both. Other combinations
> produce an error.
>
> This code based on a patch from Jani Nikula.
> ---
>  completion/notmuch-completion.bash |   2 +-
>  completion/notmuch-completion.zsh  |   3 +-
>  doc/man1/notmuch-search.rst        |  22 +++++++-
>  notmuch-search.c                   | 110 ++++++++++++++++++++++++++++++++++---
>  test/T090-search-output.sh         |  64 +++++++++++++++++++++
>  5 files changed, 189 insertions(+), 12 deletions(-)
>
> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
> index 0571dc9..cfbd389 100644
> --- a/completion/notmuch-completion.bash
> +++ b/completion/notmuch-completion.bash
> @@ -294,7 +294,7 @@ _notmuch_search()
>  	    return
>  	    ;;
>  	--output)
> -	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients" -- "${cur}" ) )
>  	    return
>  	    ;;
>  	--sort)
> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
> index 67a9aba..3e52a00 100644
> --- a/completion/notmuch-completion.zsh
> +++ b/completion/notmuch-completion.zsh
> @@ -52,7 +52,8 @@ _notmuch_search()
>    _arguments -s : \
>      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
>      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
> -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
> +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
>  }
>  
>  _notmuch()
> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
> index 90160f2..c9d38b1 100644
> --- a/doc/man1/notmuch-search.rst
> +++ b/doc/man1/notmuch-search.rst
> @@ -35,7 +35,7 @@ Supported options for **search** include
>          intended for programs that invoke **notmuch(1)** internally. If
>          omitted, the latest supported version will be used.
>  
> -    ``--output=(summary|threads|messages|files|tags)``
> +    ``--output=(summary|threads|messages|files|tags|sender|recipients)``
>  
>          **summary**
>              Output a summary of each thread with any message matching
> @@ -78,6 +78,26 @@ Supported options for **search** include
>              by null characters (--format=text0), as a JSON array
>              (--format=json), or as an S-Expression list (--format=sexp).
>  
> +	**sender**
> +            Output all addresses from the *From* header that appear on
> +            any message matching the search terms, either one per line
> +            (--format=text), separated by null characters
> +            (--format=text0), as a JSON array (--format=json), or as
> +            an S-Expression list (--format=sexp).
> +
> +	    Note: Searching for **sender** should be much faster than
> +	    searching for **recipients**, because sender addresses are
> +	    cached directly in the database whereas other addresses
> +	    need to be fetched from message files.
> +
> +	**recipients**
> +            Like **sender** but for addresses from *To*, *Cc* and
> +	    *Bcc* headers.
> +
> +	This option can be given multiple times to combine different
> +	outputs. Curently, this is only supported for **sender** and
> +	**recipients** outputs.
> +
>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>          This option can be used to present results in either
>          chronological order (**oldest-first**) or reverse chronological
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 5ac2a26..74588f8 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -23,11 +23,14 @@
>  #include "string-util.h"
>  
>  typedef enum {
> -    OUTPUT_SUMMARY,
> -    OUTPUT_THREADS,
> -    OUTPUT_MESSAGES,
> -    OUTPUT_FILES,
> -    OUTPUT_TAGS
> +    OUTPUT_SUMMARY	= 1 << 0,
> +    OUTPUT_THREADS	= 1 << 1,
> +    OUTPUT_MESSAGES	= 1 << 2,
> +    OUTPUT_FILES	= 1 << 3,
> +    OUTPUT_TAGS		= 1 << 4,
> +    OUTPUT_SENDER	= 1 << 5,
> +    OUTPUT_RECIPIENTS	= 1 << 6,
> +    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,

leftover, like mentioned below (this comment added just before sending)

>  } output_t;
>  
>  typedef struct {
> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
>      return 0;
>  }
>  
> +static void
> +print_address_list (const search_options_t *o, InternetAddressList *list)
> +{
> +    InternetAddress *address;
> +    int i;
> +
> +    for (i = 0; i < internet_address_list_length (list); i++) {
> +	address = internet_address_list_get_address (list, i);
> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
> +	    InternetAddressGroup *group;
> +	    InternetAddressList *group_list;
> +
> +	    group = INTERNET_ADDRESS_GROUP (address);
> +	    group_list = internet_address_group_get_members (group);
> +	    if (group_list == NULL)
> +		continue;
> +
> +	    print_address_list (o, group_list);
> +	} else {
> +	    InternetAddressMailbox *mailbox;
> +	    const char *name;
> +	    const char *addr;
> +	    char *full_address;
> +
> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +
> +	    name = internet_address_get_name (address);
> +	    addr = internet_address_mailbox_get_addr (mailbox);
> +
> +	    if (name && *name)
> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
> +	    else
> +		full_address = talloc_strdup (o->format, addr);
> +
> +	    if (!full_address) {

Apart from minor style issue like space after ! and the leftover ADDRESSES
parts (w/o that I would not have commented about !<SPC>) the first 3
patches look pretty good to me. I have not tested those yet.

But we keep to have some disagreement w/ unique/duplicate/filter-by
handling ;/

I (currently) rest the case of first/last/most common handling to just
how the --sort=(newest-first|oldest-first) affects the order...

Let's consider the following list of output if no /deduplication/ is done:

  John Doe <john@example.com>
  Dr. John Doe <john@example.com>
  John Doe <JOHN@EXAMPLE.COM>
  John Doe <john@doe.name>
  Dr. John Doe <john@doe.name>
  John Doe <JOHN@doe.name.example.com>
  John Doe <john@doe.name>
  Dr. John Doe <john@example.com>
  Dr. John Doe <john@example.com>
  Dr. John Doe <john@doe.name>
  John Doe <john@example.com>
  John Doe <john@doe.name.example.com>

To stir the pool a little more, this could be the output when
--duplicate=all (the default) is given.

With --duplicate=none the output could be (first match by unique
case-insensitive address):

  John Doe <john@example.com>
  John Doe <john@doe.name>
  John Doe <john@doe.name.example.com>

(many people may have the same name, but email address is unique per person
-- therefore I think 'none' limiting that just to John Doe <john@example.com>
would be too little)

and with --duplicate=address

  John Doe <john@example.com>
  Dr. John Doe <john@example.com>
  John Doe <john@doe.name>
  Dr. John Doe <john@doe.name>
  John Doe <JOHN@doe.name.example.com>

(from this output user can choose how the recipient is to be called
(like "pseudonyms" mentioned in id:20141010113202.GE28601@TP_L520.localdomain )
when sending email)

and --duplicate=address-casesensitive

  John Doe <john@example.com>
  Dr. John Doe <john@example.com>
  John Doe <JOHN@EXAMPLE.COM>
  John Doe <john@doe.name>
  Dr. John Doe <john@doe.name>
  John Doe <JOHN@doe.name.example.com>
  John Doe <john@doe.name.example.com>

This reuse of --duplicate was thought out after Jani's irc mention of it.
This scheme would leave no room tho the filter-by=name suggestion -- for
completeness that would make this look:

  John Doe <john@example.com>
  Dr. John Doe <john@example.com>

This doesn't look too bad in this particular case but not having ability to
see all potential addresses (perhaps the only working address is now
hidden) isn't not much for general use. Well, maybe for some specific use
--duplicate=no-unique-addresses could be useful :O

Ok, this took an hour to get written to (w/ some interruptions). Healthy
criticism appreciated :D

Tomi

// stuff deleted before some 'ADDRESSES' leftover...

> @@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      }
>  
>      switch (o.output) {
> -    default:
>      case OUTPUT_SUMMARY:
>      case OUTPUT_THREADS:
>  	ret = do_search_threads (&o);
>  	break;
>      case OUTPUT_MESSAGES:
> +    case OUTPUT_SENDER:
> +    case OUTPUT_RECIPIENTS:
> +    case OUTPUT_ADDRESSES:
>      case OUTPUT_FILES:
>  	ret = do_search_messages (&o);
>  	break;
>      case OUTPUT_TAGS:
>  	ret = do_search_tags (notmuch, o.format, o.query);
>  	break;
> +    default:
> +	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
> +	ret = 1;
>      }
>  
>      notmuch_query_destroy (o.query);

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-13 19:00                     ` Tomi Ollila
@ 2014-10-13 21:38                       ` Michal Sojka
  2014-10-19 10:02                         ` Tomi Ollila
  0 siblings, 1 reply; 31+ messages in thread
From: Michal Sojka @ 2014-10-13 21:38 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

On Mon, Oct 13 2014, Tomi Ollila wrote:
> On Mon, Oct 13 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>
>> The new outputs allow printing senders, recipients or both of matching
>> messages. The --output option is converted from "keyword" argument to
>> "flags" argument, which means that the user can use --output=sender and
>> --output=recipients simultaneously, to print both. Other combinations
>> produce an error.
>>
>> This code based on a patch from Jani Nikula.
>> ---
>>  completion/notmuch-completion.bash |   2 +-
>>  completion/notmuch-completion.zsh  |   3 +-
>>  doc/man1/notmuch-search.rst        |  22 +++++++-
>>  notmuch-search.c                   | 110 ++++++++++++++++++++++++++++++++++---
>>  test/T090-search-output.sh         |  64 +++++++++++++++++++++
>>  5 files changed, 189 insertions(+), 12 deletions(-)
>>
>> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
>> index 0571dc9..cfbd389 100644
>> --- a/completion/notmuch-completion.bash
>> +++ b/completion/notmuch-completion.bash
>> @@ -294,7 +294,7 @@ _notmuch_search()
>>  	    return
>>  	    ;;
>>  	--output)
>> -	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
>> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients" -- "${cur}" ) )
>>  	    return
>>  	    ;;
>>  	--sort)
>> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
>> index 67a9aba..3e52a00 100644
>> --- a/completion/notmuch-completion.zsh
>> +++ b/completion/notmuch-completion.zsh
>> @@ -52,7 +52,8 @@ _notmuch_search()
>>    _arguments -s : \
>>      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
>>      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
>> -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
>> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
>> +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
>>  }
>>
>>  _notmuch()
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 90160f2..c9d38b1 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -35,7 +35,7 @@ Supported options for **search** include
>>          intended for programs that invoke **notmuch(1)** internally. If
>>          omitted, the latest supported version will be used.
>>
>> -    ``--output=(summary|threads|messages|files|tags)``
>> +    ``--output=(summary|threads|messages|files|tags|sender|recipients)``
>>
>>          **summary**
>>              Output a summary of each thread with any message matching
>> @@ -78,6 +78,26 @@ Supported options for **search** include
>>              by null characters (--format=text0), as a JSON array
>>              (--format=json), or as an S-Expression list (--format=sexp).
>>
>> +	**sender**
>> +            Output all addresses from the *From* header that appear on
>> +            any message matching the search terms, either one per line
>> +            (--format=text), separated by null characters
>> +            (--format=text0), as a JSON array (--format=json), or as
>> +            an S-Expression list (--format=sexp).
>> +
>> +	    Note: Searching for **sender** should be much faster than
>> +	    searching for **recipients**, because sender addresses are
>> +	    cached directly in the database whereas other addresses
>> +	    need to be fetched from message files.
>> +
>> +	**recipients**
>> +            Like **sender** but for addresses from *To*, *Cc* and
>> +	    *Bcc* headers.
>> +
>> +	This option can be given multiple times to combine different
>> +	outputs. Curently, this is only supported for **sender** and
>> +	**recipients** outputs.
>> +
>>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>>          This option can be used to present results in either
>>          chronological order (**oldest-first**) or reverse chronological
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index 5ac2a26..74588f8 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -23,11 +23,14 @@
>>  #include "string-util.h"
>>
>>  typedef enum {
>> -    OUTPUT_SUMMARY,
>> -    OUTPUT_THREADS,
>> -    OUTPUT_MESSAGES,
>> -    OUTPUT_FILES,
>> -    OUTPUT_TAGS
>> +    OUTPUT_SUMMARY	= 1 << 0,
>> +    OUTPUT_THREADS	= 1 << 1,
>> +    OUTPUT_MESSAGES	= 1 << 2,
>> +    OUTPUT_FILES	= 1 << 3,
>> +    OUTPUT_TAGS		= 1 << 4,
>> +    OUTPUT_SENDER	= 1 << 5,
>> +    OUTPUT_RECIPIENTS	= 1 << 6,
>> +    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
>
> leftover, like mentioned below (this comment added just before
> sending)

This is needed to suppress a warning about unknown enum value in the
switch statement.

>>  } output_t;
>>
>>  typedef struct {
>> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
>>      return 0;
>>  }
>>
>> +static void
>> +print_address_list (const search_options_t *o, InternetAddressList *list)
>> +{
>> +    InternetAddress *address;
>> +    int i;
>> +
>> +    for (i = 0; i < internet_address_list_length (list); i++) {
>> +	address = internet_address_list_get_address (list, i);
>> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
>> +	    InternetAddressGroup *group;
>> +	    InternetAddressList *group_list;
>> +
>> +	    group = INTERNET_ADDRESS_GROUP (address);
>> +	    group_list = internet_address_group_get_members (group);
>> +	    if (group_list == NULL)
>> +		continue;
>> +
>> +	    print_address_list (o, group_list);
>> +	} else {
>> +	    InternetAddressMailbox *mailbox;
>> +	    const char *name;
>> +	    const char *addr;
>> +	    char *full_address;
>> +
>> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
>> +
>> +	    name = internet_address_get_name (address);
>> +	    addr = internet_address_mailbox_get_addr (mailbox);
>> +
>> +	    if (name && *name)
>> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
>> +	    else
>> +		full_address = talloc_strdup (o->format, addr);
>> +
>> +	    if (!full_address) {
>
> Apart from minor style issue like space after ! and the leftover ADDRESSES
> parts (w/o that I would not have commented about !<SPC>) the first 3
> patches look pretty good to me. I have not tested those yet.

If you want, I can send another version.

> But we keep to have some disagreement w/ unique/duplicate/filter-by
> handling ;/
>
> I (currently) rest the case of first/last/most common handling to just
> how the --sort=(newest-first|oldest-first) affects the order...
>
> Let's consider the following list of output if no /deduplication/ is done:
>
>   John Doe <john@example.com>
>   Dr. John Doe <john@example.com>
>   John Doe <JOHN@EXAMPLE.COM>
>   John Doe <john@doe.name>
>   Dr. John Doe <john@doe.name>
>   John Doe <JOHN@doe.name.example.com>
>   John Doe <john@doe.name>
>   Dr. John Doe <john@example.com>
>   Dr. John Doe <john@example.com>
>   Dr. John Doe <john@doe.name>
>   John Doe <john@example.com>
>   John Doe <john@doe.name.example.com>
>
> To stir the pool a little more, this could be the output when
> --duplicate=all (the default) is given.

This seems counter-intuitive. A command without --duplicate gives you a
certain number of addresses and when you add --duplicate, you get less
addresses?

Moreover, --duplicate is already used with --output=files and has
different meaning.

> With --duplicate=none the output could be (first match by unique
> case-insensitive address):
>
>   John Doe <john@example.com>
>   John Doe <john@doe.name>
>   John Doe <john@doe.name.example.com>

This seems confusing to me. If I see "duplicate none", I would simply
expect that duplicate lines are removed from the --duplicate=all list
above. Not that just duplicate addresses would be removed.

> (many people may have the same name, but email address is unique per person
> -- therefore I think 'none' limiting that just to John Doe <john@example.com>
> would be too little)

Here I agree that filtering by address part is perhaps most useful.

> and with --duplicate=address
>
>   John Doe <john@example.com>
>   Dr. John Doe <john@example.com>
>   John Doe <john@doe.name>
>   Dr. John Doe <john@doe.name>
>   John Doe <JOHN@doe.name.example.com>

I don't get this. You say --duplicate=address but the output contains
also duplicate names.

> (from this output user can choose how the recipient is to be called
> (like "pseudonyms" mentioned in id:20141010113202.GE28601@TP_L520.localdomain )
> when sending email)
>
> and --duplicate=address-casesensitive
>
>   John Doe <john@example.com>
>   Dr. John Doe <john@example.com>
>   John Doe <JOHN@EXAMPLE.COM>
>   John Doe <john@doe.name>
>   Dr. John Doe <john@doe.name>
>   John Doe <JOHN@doe.name.example.com>
>   John Doe <john@doe.name.example.com>
>
> This reuse of --duplicate was thought out after Jani's irc mention of it.
> This scheme would leave no room tho the filter-by=name suggestion -- for
> completeness that would make this look:
>
>   John Doe <john@example.com>
>   Dr. John Doe <john@example.com>
>
> This doesn't look too bad in this particular case but not having ability to
> see all potential addresses (perhaps the only working address is now
> hidden) isn't not much for general use. Well, maybe for some specific use
> --duplicate=no-unique-addresses could be useful :O
>
> Ok, this took an hour to get written to (w/ some interruptions). Healthy
> criticism appreciated :D

I don't know. You seem to think about this in the opposite way than how
it is implemented. The implementation really filters things out whereas
you specify what not to filter.

My feeling is that if you would implement your proposal, the code would
be more complex than in my patch, because the mapping between command
line options and the actual algorithm would require some extra code. And
in a previous comment, you preferred simplicity.

Hopefully, you consider the above as "healthy" criticism.

Thanks for quick review.
-Michal

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-13 21:38                       ` Michal Sojka
@ 2014-10-19 10:02                         ` Tomi Ollila
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Ollila @ 2014-10-19 10:02 UTC (permalink / raw)
  To: Michal Sojka, notmuch


Hi Michal, been busy lately so I haven't got into this...

... I'll look into it in a bit more detail a bit later, 


On Tue, Oct 14 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

>
> I don't know. You seem to think about this in the opposite way than how
> it is implemented. The implementation really filters things out whereas
> you specify what not to filter.
>
> My feeling is that if you would implement your proposal, the code would
> be more complex than in my patch, because the mapping between command
> line options and the actual algorithm would require some extra code. And
> in a previous comment, you preferred simplicity.

Yes, maybe the choice of reusing --duplicate was bad, and perhaps took
thought from the main point -- what kind output options should be possible...

> Hopefully, you consider the above as "healthy" criticism.

Yes, I do -- I hope we can get something decent out in this in near future,
with duplicate filtering that can have useful options to be used directly
by MUAs and no duplicate filtering option which I can use in 
nottoomuch-addresses in 2016 ;D



> Thanks for quick review.
> -Michal


Tomi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 1/4] cli: Refactor option passing in the search command
  2014-10-12 21:41                   ` [PATCH v3 1/4] cli: Refactor option passing in the search command Michal Sojka
@ 2014-10-22 10:44                     ` Mark Walters
  0 siblings, 0 replies; 31+ messages in thread
From: Mark Walters @ 2014-10-22 10:44 UTC (permalink / raw)
  To: Michal Sojka, notmuch

On Sun, 12 Oct 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> Many functions that implement the search command need to access command
> line options. Instead of passing each option in a separate variable, put
> them in a structure and pass only this structure.
>
> This will become handy in the following patches.

Hi

This basically looks good to me. I have some style queries/preferences
mentioned below but am happy to be overruled by you or others.

> ---
>  notmuch-search.c | 122 ++++++++++++++++++++++++++++---------------------------
>  1 file changed, 62 insertions(+), 60 deletions(-)
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index bc9be45..5ac2a26 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -30,6 +30,16 @@ typedef enum {
>      OUTPUT_TAGS
>  } output_t;
>  
> +typedef struct {
> +    sprinter_t *format;
> +    notmuch_query_t *query;
> +    notmuch_sort_t sort;
> +    output_t output;
> +    int offset;
> +    int limit;
> +    int dupe;
> +} search_options_t;
> +

I don't think of format as being a "search option", whereas I do think
of all the others as search options. So I would prefer to omit format
from search_options_t and pass it explicitly.

>  /* Return two stable query strings that identify exactly the matched
>   * and unmatched messages currently in thread.  If there are no
>   * matched or unmatched messages, the returned buffers will be
> @@ -70,46 +80,42 @@ get_thread_query (notmuch_thread_t *thread,
>  }
>  
>  static int
> -do_search_threads (sprinter_t *format,
> -		   notmuch_query_t *query,
> -		   notmuch_sort_t sort,
> -		   output_t output,
> -		   int offset,
> -		   int limit)
> +do_search_threads (search_options_t *o)

Personally, I prefer longer variable names (and names which aren't 'o'):
even just opt would be a definite improvement in my opinion.

>  {
>      notmuch_thread_t *thread;
>      notmuch_threads_t *threads;
>      notmuch_tags_t *tags;
> +    sprinter_t *format = o->format;
>      time_t date;
>      int i;
>  
> -    if (offset < 0) {
> -	offset += notmuch_query_count_threads (query);
> -	if (offset < 0)
> -	    offset = 0;
> +    if (o->offset < 0) {
> +	o->offset += notmuch_query_count_threads (o->query);
> +	if (o->offset < 0)
> +	    o->offset = 0;
>      }
>  
> -    threads = notmuch_query_search_threads (query);
> +    threads = notmuch_query_search_threads (o->query);
>      if (threads == NULL)
>  	return 1;
>  
>      format->begin_list (format);
>  
>      for (i = 0;
> -	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
> +	 notmuch_threads_valid (threads) && (o->limit < 0 || i < o->offset + o->limit);
>  	 notmuch_threads_move_to_next (threads), i++)
>      {
>  	thread = notmuch_threads_get (threads);
>  
> -	if (i < offset) {
> +	if (i < o->offset) {
>  	    notmuch_thread_destroy (thread);
>  	    continue;
>  	}
>  
> -	if (output == OUTPUT_THREADS) {
> +	if (o->output == OUTPUT_THREADS) {
>  	    format->set_prefix (format, "thread");
>  	    format->string (format,
> -			    notmuch_thread_get_thread_id (thread));
> +			       notmuch_thread_get_thread_id (thread));
>  	    format->separator (format);
>  	} else { /* output == OUTPUT_SUMMARY */
>  	    void *ctx_quote = talloc_new (thread);
> @@ -123,7 +129,7 @@ do_search_threads (sprinter_t *format,
>  
>  	    format->begin_map (format);
>  
> -	    if (sort == NOTMUCH_SORT_OLDEST_FIRST)
> +	    if (o->sort == NOTMUCH_SORT_OLDEST_FIRST)
>  		date = notmuch_thread_get_oldest_date (thread);
>  	    else
>  		date = notmuch_thread_get_newest_date (thread);
> @@ -215,40 +221,36 @@ do_search_threads (sprinter_t *format,
>  }
>  
>  static int
> -do_search_messages (sprinter_t *format,
> -		    notmuch_query_t *query,
> -		    output_t output,
> -		    int offset,
> -		    int limit,
> -		    int dupe)
> +do_search_messages (search_options_t *o)
>  {
>      notmuch_message_t *message;
>      notmuch_messages_t *messages;
>      notmuch_filenames_t *filenames;
> +    sprinter_t *format = o->format;
>      int i;
>  
> -    if (offset < 0) {
> -	offset += notmuch_query_count_messages (query);
> -	if (offset < 0)
> -	    offset = 0;
> +    if (o->offset < 0) {
> +	o->offset += notmuch_query_count_messages (o->query);
> +	if (o->offset < 0)
> +	    o->offset = 0;
>      }
>  
> -    messages = notmuch_query_search_messages (query);
> +    messages = notmuch_query_search_messages (o->query);
>      if (messages == NULL)
>  	return 1;
>  
>      format->begin_list (format);
>  
>      for (i = 0;
> -	 notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit);
> +	 notmuch_messages_valid (messages) && (o->limit < 0 || i < o->offset + o->limit);
>  	 notmuch_messages_move_to_next (messages), i++)
>      {
> -	if (i < offset)
> +	if (i < o->offset)
>  	    continue;
>  
>  	message = notmuch_messages_get (messages);
>  
> -	if (output == OUTPUT_FILES) {
> +	if (o->output == OUTPUT_FILES) {
>  	    int j;
>  	    filenames = notmuch_message_get_filenames (message);
>  
> @@ -256,7 +258,7 @@ do_search_messages (sprinter_t *format,
>  		 notmuch_filenames_valid (filenames);
>  		 notmuch_filenames_move_to_next (filenames), j++)
>  	    {
> -		if (dupe < 0 || dupe == j) {
> +		if (o->dupe < 0 || o->dupe == j) {
>  		    format->string (format, notmuch_filenames_get (filenames));
>  		    format->separator (format);
>  		}
> @@ -333,16 +335,16 @@ int
>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  {
>      notmuch_database_t *notmuch;
> -    notmuch_query_t *query;
> +    search_options_t o = {
> +	.sort = NOTMUCH_SORT_NEWEST_FIRST,
> +	.output = OUTPUT_SUMMARY,
> +	.offset = 0,
> +	.limit = -1, /* unlimited */
> +	.dupe = -1,
> +    };
>      char *query_str;
> -    notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;
> -    sprinter_t *format = NULL;
>      int opt_index, ret;
> -    output_t output = OUTPUT_SUMMARY;
> -    int offset = 0;
> -    int limit = -1; /* unlimited */
>      notmuch_exclude_t exclude = NOTMUCH_EXCLUDE_TRUE;
> -    int dupe = -1;
>      unsigned int i;
>  
>      enum {
> @@ -353,7 +355,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      } format_sel = NOTMUCH_FORMAT_TEXT;
>  
>      notmuch_opt_desc_t options[] = {
> -	{ NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',
> +	{ NOTMUCH_OPT_KEYWORD, &o.sort, "sort", 's',
>  	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
>  				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
>  				  { 0, 0 } } },
> @@ -364,7 +366,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
> -	{ NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
> +	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>  				  { "threads", OUTPUT_THREADS },
>  				  { "messages", OUTPUT_MESSAGES },
> @@ -377,9 +379,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>                                    { "flag", NOTMUCH_EXCLUDE_FLAG },
>                                    { "all", NOTMUCH_EXCLUDE_ALL },
>                                    { 0, 0 } } },
> -	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
> -	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
> -	{ NOTMUCH_OPT_INT, &dupe, "duplicate", 'D', 0  },
> +	{ NOTMUCH_OPT_INT, &o.offset, "offset", 'O', 0 },
> +	{ NOTMUCH_OPT_INT, &o.limit, "limit", 'L', 0  },
> +	{ NOTMUCH_OPT_INT, &o.dupe, "duplicate", 'D', 0  },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -389,20 +391,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  
>      switch (format_sel) {
>      case NOTMUCH_FORMAT_TEXT:
> -	format = sprinter_text_create (config, stdout);
> +	o.format = sprinter_text_create (config, stdout);
>  	break;
>      case NOTMUCH_FORMAT_TEXT0:
> -	if (output == OUTPUT_SUMMARY) {
> +	if (o.output == OUTPUT_SUMMARY) {
>  	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
>  	    return EXIT_FAILURE;
>  	}
> -	format = sprinter_text0_create (config, stdout);
> +	o.format = sprinter_text0_create (config, stdout);
>  	break;
>      case NOTMUCH_FORMAT_JSON:
> -	format = sprinter_json_create (config, stdout);
> +	o.format = sprinter_json_create (config, stdout);
>  	break;
>      case NOTMUCH_FORMAT_SEXP:
> -	format = sprinter_sexp_create (config, stdout);
> +	o.format = sprinter_sexp_create (config, stdout);
>  	break;
>      default:
>  	/* this should never happen */
> @@ -425,15 +427,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  	return EXIT_FAILURE;
>      }
>  
> -    query = notmuch_query_create (notmuch, query_str);
> -    if (query == NULL) {
> +    o.query = notmuch_query_create (notmuch, query_str);
> +    if (o.query == NULL) {
>  	fprintf (stderr, "Out of memory\n");
>  	return EXIT_FAILURE;
>      }
>  
> -    notmuch_query_set_sort (query, sort);
> +    notmuch_query_set_sort (o.query, o.sort);
>  
> -    if (exclude == NOTMUCH_EXCLUDE_FLAG && output != OUTPUT_SUMMARY) {
> +    if (exclude == NOTMUCH_EXCLUDE_FLAG && o.output != OUTPUT_SUMMARY) {
>  	/* If we are not doing summary output there is nowhere to
>  	 * print the excluded flag so fall back on including the
>  	 * excluded messages. */
> @@ -448,29 +450,29 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  	search_exclude_tags = notmuch_config_get_search_exclude_tags
>  	    (config, &search_exclude_tags_length);
>  	for (i = 0; i < search_exclude_tags_length; i++)
> -	    notmuch_query_add_tag_exclude (query, search_exclude_tags[i]);
> -	notmuch_query_set_omit_excluded (query, exclude);
> +	    notmuch_query_add_tag_exclude (o.query, search_exclude_tags[i]);
> +	notmuch_query_set_omit_excluded (o.query, exclude);
>      }
>  
> -    switch (output) {
> +    switch (o.output) {
>      default:
>      case OUTPUT_SUMMARY:
>      case OUTPUT_THREADS:
> -	ret = do_search_threads (format, query, sort, output, offset, limit);
> +	ret = do_search_threads (&o);
>  	break;
>      case OUTPUT_MESSAGES:
>      case OUTPUT_FILES:
> -	ret = do_search_messages (format, query, output, offset, limit, dupe);
> +	ret = do_search_messages (&o);
>  	break;
>      case OUTPUT_TAGS:
> -	ret = do_search_tags (notmuch, format, query);
> +	ret = do_search_tags (notmuch, o.format, o.query);

It feels slightly strange to me to have do_search_tags taking a
different argument style than the other two do_search_* functions so I
would slightly prefer making it do_search_tags (&o).

Obviously all of these are trivialities, so as I said above, I am happy
to be overruled.

Best wishes

Mark



>  	break;
>      }
>  
> -    notmuch_query_destroy (query);
> +    notmuch_query_destroy (o.query);
>      notmuch_database_destroy (notmuch);
>  
> -    talloc_free (format);
> +    talloc_free (o.format);
>  
>      return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>  }
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-12 21:41                   ` [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients} Michal Sojka
  2014-10-13 19:00                     ` Tomi Ollila
@ 2014-10-22 10:51                     ` Mark Walters
  2014-10-24 10:57                       ` Michal Sojka
  2014-10-23  9:41                     ` Mark Walters
  2 siblings, 1 reply; 31+ messages in thread
From: Mark Walters @ 2014-10-22 10:51 UTC (permalink / raw)
  To: Michal Sojka, notmuch

On Sun, 12 Oct 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> The new outputs allow printing senders, recipients or both of matching
> messages. The --output option is converted from "keyword" argument to
> "flags" argument, which means that the user can use --output=sender and
> --output=recipients simultaneously, to print both. Other combinations
> produce an error.
>
> This code based on a patch from Jani Nikula.
> ---
>  completion/notmuch-completion.bash |   2 +-
>  completion/notmuch-completion.zsh  |   3 +-
>  doc/man1/notmuch-search.rst        |  22 +++++++-
>  notmuch-search.c                   | 110 ++++++++++++++++++++++++++++++++++---
>  test/T090-search-output.sh         |  64 +++++++++++++++++++++
>  5 files changed, 189 insertions(+), 12 deletions(-)
>
> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
> index 0571dc9..cfbd389 100644
> --- a/completion/notmuch-completion.bash
> +++ b/completion/notmuch-completion.bash
> @@ -294,7 +294,7 @@ _notmuch_search()
>  	    return
>  	    ;;
>  	--output)
> -	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients" -- "${cur}" ) )
>  	    return
>  	    ;;
>  	--sort)
> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
> index 67a9aba..3e52a00 100644
> --- a/completion/notmuch-completion.zsh
> +++ b/completion/notmuch-completion.zsh
> @@ -52,7 +52,8 @@ _notmuch_search()
>    _arguments -s : \
>      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
>      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
> -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
> +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
>  }
>  
>  _notmuch()
> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
> index 90160f2..c9d38b1 100644
> --- a/doc/man1/notmuch-search.rst
> +++ b/doc/man1/notmuch-search.rst
> @@ -35,7 +35,7 @@ Supported options for **search** include
>          intended for programs that invoke **notmuch(1)** internally. If
>          omitted, the latest supported version will be used.
>  
> -    ``--output=(summary|threads|messages|files|tags)``
> +    ``--output=(summary|threads|messages|files|tags|sender|recipients)``
>  
>          **summary**
>              Output a summary of each thread with any message matching
> @@ -78,6 +78,26 @@ Supported options for **search** include
>              by null characters (--format=text0), as a JSON array
>              (--format=json), or as an S-Expression list (--format=sexp).
>  
> +	**sender**
> +            Output all addresses from the *From* header that appear on
> +            any message matching the search terms, either one per line
> +            (--format=text), separated by null characters
> +            (--format=text0), as a JSON array (--format=json), or as
> +            an S-Expression list (--format=sexp).
> +
> +	    Note: Searching for **sender** should be much faster than
> +	    searching for **recipients**, because sender addresses are
> +	    cached directly in the database whereas other addresses
> +	    need to be fetched from message files.
> +
> +	**recipients**
> +            Like **sender** but for addresses from *To*, *Cc* and
> +	    *Bcc* headers.
> +
> +	This option can be given multiple times to combine different
> +	outputs. Curently, this is only supported for **sender** and
> +	**recipients** outputs.
> +
>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>          This option can be used to present results in either
>          chronological order (**oldest-first**) or reverse chronological
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 5ac2a26..74588f8 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -23,11 +23,14 @@
>  #include "string-util.h"
>  
>  typedef enum {
> -    OUTPUT_SUMMARY,
> -    OUTPUT_THREADS,
> -    OUTPUT_MESSAGES,
> -    OUTPUT_FILES,
> -    OUTPUT_TAGS
> +    OUTPUT_SUMMARY	= 1 << 0,
> +    OUTPUT_THREADS	= 1 << 1,
> +    OUTPUT_MESSAGES	= 1 << 2,
> +    OUTPUT_FILES	= 1 << 3,
> +    OUTPUT_TAGS		= 1 << 4,
> +    OUTPUT_SENDER	= 1 << 5,
> +    OUTPUT_RECIPIENTS	= 1 << 6,
> +    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,

I think I would drop the OUTPUT_ADDRESSES enum as the parser no longer
uses it (and replace the one use by OUTPUT_SENDER | OUTPUT_RECIPIENTS below).

>  } output_t;
>  
>  typedef struct {
> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
>      return 0;
>  }
>  
> +static void
> +print_address_list (const search_options_t *o, InternetAddressList *list)
> +{
> +    InternetAddress *address;
> +    int i;
> +
> +    for (i = 0; i < internet_address_list_length (list); i++) {
> +	address = internet_address_list_get_address (list, i);
> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
> +	    InternetAddressGroup *group;
> +	    InternetAddressList *group_list;
> +
> +	    group = INTERNET_ADDRESS_GROUP (address);
> +	    group_list = internet_address_group_get_members (group);
> +	    if (group_list == NULL)
> +		continue;
> +
> +	    print_address_list (o, group_list);
> +	} else {
> +	    InternetAddressMailbox *mailbox;
> +	    const char *name;
> +	    const char *addr;
> +	    char *full_address;
> +
> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +
> +	    name = internet_address_get_name (address);
> +	    addr = internet_address_mailbox_get_addr (mailbox);
> +
> +	    if (name && *name)
> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
> +	    else
> +		full_address = talloc_strdup (o->format, addr);
> +
> +	    if (!full_address) {
> +		fprintf (stderr, "Error: out of memory\n");
> +		break;
> +	    }
> +	    o->format->string (o->format, full_address);
> +	    o->format->separator (o->format);
> +
> +	    talloc_free (full_address);
> +	}
> +    }
> +}
> +
> +static void
> +print_address_string (const search_options_t *o, const char *recipients)
> +{
> +    InternetAddressList *list;
> +
> +    if (recipients == NULL)
> +	return;
> +
> +    list = internet_address_list_parse_string (recipients);
> +    if (list == NULL)
> +	return;
> +
> +    print_address_list (o, list);
> +}
> +
>  static int
>  do_search_messages (search_options_t *o)
>  {
> @@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
>  	    
>  	    notmuch_filenames_destroy( filenames );
>  
> -	} else { /* output == OUTPUT_MESSAGES */
> +	} else if (o->output == OUTPUT_MESSAGES) {
>  	    format->set_prefix (format, "id");
>  	    format->string (format,
>  			    notmuch_message_get_message_id (message));
>  	    format->separator (format);
> +	} else {
> +	    if (o->output & OUTPUT_SENDER) {
> +		const char *addrs;
> +
> +		addrs = notmuch_message_get_header (message, "from");
> +		print_address_string (o, addrs);
> +	    }
> +
> +	    if (o->output & OUTPUT_RECIPIENTS) {
> +		const char *hdrs[] = { "to", "cc", "bcc" };
> +		const char *addrs;
> +		size_t j;
> +
> +		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
> +		    addrs = notmuch_message_get_header (message, hdrs[j]);
> +		    print_address_string (o, addrs);
> +		}
> +	    }
>  	}
>  
>  	notmuch_message_destroy (message);
> @@ -337,7 +419,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      notmuch_database_t *notmuch;
>      search_options_t o = {
>  	.sort = NOTMUCH_SORT_NEWEST_FIRST,
> -	.output = OUTPUT_SUMMARY,
> +	.output = 0,
>  	.offset = 0,
>  	.limit = -1, /* unlimited */
>  	.dupe = -1,

It is slightly unfortunate that all the other defaults are defined here
but the default output is after the option parsing but I can't see a
nice way round that. I only mention it in case you (or anyone else) can
see a nice way round this.


> @@ -366,10 +448,12 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
> -	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &o.output, "output", 'o',
>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>  				  { "threads", OUTPUT_THREADS },
>  				  { "messages", OUTPUT_MESSAGES },
> +				  { "sender", OUTPUT_SENDER },
> +				  { "recipients", OUTPUT_RECIPIENTS },
>  				  { "files", OUTPUT_FILES },
>  				  { "tags", OUTPUT_TAGS },
>  				  { 0, 0 } } },
> @@ -389,6 +473,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      if (opt_index < 0)
>  	return EXIT_FAILURE;
>  
> +    if (! o.output)
> +	o.output = OUTPUT_SUMMARY;
> +
>      switch (format_sel) {
>      case NOTMUCH_FORMAT_TEXT:
>  	o.format = sprinter_text_create (config, stdout);
> @@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      }
>  
>      switch (o.output) {
> -    default:
>      case OUTPUT_SUMMARY:
>      case OUTPUT_THREADS:
>  	ret = do_search_threads (&o);
>  	break;
>      case OUTPUT_MESSAGES:
> +    case OUTPUT_SENDER:
> +    case OUTPUT_RECIPIENTS:
> +    case OUTPUT_ADDRESSES:

This is the one use of OUTPUT_ADDRESSES that I would replace with
OUTPUT_SENDER | OUTPUT_RECIPIENTS

Otherwise this looks good to me. (I have not tested it yet.)

Best wishes

Mark

>      case OUTPUT_FILES:
>  	ret = do_search_messages (&o);
>  	break;
>      case OUTPUT_TAGS:
>  	ret = do_search_tags (notmuch, o.format, o.query);
>  	break;
> +    default:
> +	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
> +	ret = 1;
>      }
>  
>      notmuch_query_destroy (o.query);
> diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
> index 947d572..e696c01 100755
> --- a/test/T090-search-output.sh
> +++ b/test/T090-search-output.sh
> @@ -387,6 +387,70 @@ cat <<EOF >EXPECTED
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "--output=sender"
> +notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Adrian Perez de Castro <aperez@igalia.com>
> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +      1 Aron Griffis <agriffis@n01se.net>
> +     12 Carl Worth <cworth@cworth.org>
> +      1 Chris Wilson <chris@chris-wilson.co.uk>
> +      1 François Boulogne <boulogne.f@gmail.com>
> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
> +      1 Israel Herraiz <isra@herraiz.org>
> +      4 Jan Janak <jan@ryngle.com>
> +      2 Jjgod Jiang <gzjjgod@gmail.com>
> +      7 Keith Packard <keithp@keithp.com>
> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
> +      5 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
> +      3 Stewart Smith <stewart@flamingspork.com>
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=recipients"
> +notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Allan McRae <allan@archlinux.org>
> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
> +      1 Keith Packard <keithp@keithp.com>
> +      1 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      2 notmuch <notmuch@notmuchmail.org>
> +     48 notmuch@notmuchmail.org
> +      1 olivier.berger@it-sudparis.eu
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=sender --output=recipients"
> +notmuch search --output=sender --output=recipients '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Adrian Perez de Castro <aperez@igalia.com>
> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +      1 Allan McRae <allan@archlinux.org>
> +      1 Aron Griffis <agriffis@n01se.net>
> +     12 Carl Worth <cworth@cworth.org>
> +      1 Chris Wilson <chris@chris-wilson.co.uk>
> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
> +      1 François Boulogne <boulogne.f@gmail.com>
> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
> +      1 Israel Herraiz <isra@herraiz.org>
> +      4 Jan Janak <jan@ryngle.com>
> +      2 Jjgod Jiang <gzjjgod@gmail.com>
> +      8 Keith Packard <keithp@keithp.com>
> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
> +      6 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
> +      3 Stewart Smith <stewart@flamingspork.com>
> +      2 notmuch <notmuch@notmuchmail.org>
> +     48 notmuch@notmuchmail.org
> +      1 olivier.berger@it-sudparis.eu
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
>  test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
>  add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
>  	headers'"
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-12 21:41                   ` [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients} Michal Sojka
  2014-10-13 19:00                     ` Tomi Ollila
  2014-10-22 10:51                     ` Mark Walters
@ 2014-10-23  9:41                     ` Mark Walters
  2014-10-24  9:38                       ` Tomi Ollila
  2014-10-24 10:23                       ` David Edmondson
  2 siblings, 2 replies; 31+ messages in thread
From: Mark Walters @ 2014-10-23  9:41 UTC (permalink / raw)
  To: Michal Sojka, notmuch

On Sun, 12 Oct 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> The new outputs allow printing senders, recipients or both of matching
> messages. The --output option is converted from "keyword" argument to
> "flags" argument, which means that the user can use --output=sender and
> --output=recipients simultaneously, to print both. Other combinations
> produce an error.
>
> This code based on a patch from Jani Nikula.
> ---
>  completion/notmuch-completion.bash |   2 +-
>  completion/notmuch-completion.zsh  |   3 +-
>  doc/man1/notmuch-search.rst        |  22 +++++++-
>  notmuch-search.c                   | 110 ++++++++++++++++++++++++++++++++++---
>  test/T090-search-output.sh         |  64 +++++++++++++++++++++
>  5 files changed, 189 insertions(+), 12 deletions(-)
>
> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
> index 0571dc9..cfbd389 100644
> --- a/completion/notmuch-completion.bash
> +++ b/completion/notmuch-completion.bash
> @@ -294,7 +294,7 @@ _notmuch_search()
>  	    return
>  	    ;;
>  	--output)
> -	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients" -- "${cur}" ) )
>  	    return
>  	    ;;
>  	--sort)
> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
> index 67a9aba..3e52a00 100644
> --- a/completion/notmuch-completion.zsh
> +++ b/completion/notmuch-completion.zsh
> @@ -52,7 +52,8 @@ _notmuch_search()
>    _arguments -s : \
>      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
>      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
> -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
> +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
>  }
>  
>  _notmuch()
> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
> index 90160f2..c9d38b1 100644
> --- a/doc/man1/notmuch-search.rst
> +++ b/doc/man1/notmuch-search.rst
> @@ -35,7 +35,7 @@ Supported options for **search** include
>          intended for programs that invoke **notmuch(1)** internally. If
>          omitted, the latest supported version will be used.
>  
> -    ``--output=(summary|threads|messages|files|tags)``
> +    ``--output=(summary|threads|messages|files|tags|sender|recipients)``
>  
>          **summary**
>              Output a summary of each thread with any message matching
> @@ -78,6 +78,26 @@ Supported options for **search** include
>              by null characters (--format=text0), as a JSON array
>              (--format=json), or as an S-Expression list (--format=sexp).
>  
> +	**sender**
> +            Output all addresses from the *From* header that appear on
> +            any message matching the search terms, either one per line
> +            (--format=text), separated by null characters
> +            (--format=text0), as a JSON array (--format=json), or as
> +            an S-Expression list (--format=sexp).
> +
> +	    Note: Searching for **sender** should be much faster than
> +	    searching for **recipients**, because sender addresses are
> +	    cached directly in the database whereas other addresses
> +	    need to be fetched from message files.
> +
> +	**recipients**
> +            Like **sender** but for addresses from *To*, *Cc* and
> +	    *Bcc* headers.
> +
> +	This option can be given multiple times to combine different
> +	outputs. Curently, this is only supported for **sender** and
> +	**recipients** outputs.
> +
>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>          This option can be used to present results in either
>          chronological order (**oldest-first**) or reverse chronological
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 5ac2a26..74588f8 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -23,11 +23,14 @@
>  #include "string-util.h"
>  
>  typedef enum {
> -    OUTPUT_SUMMARY,
> -    OUTPUT_THREADS,
> -    OUTPUT_MESSAGES,
> -    OUTPUT_FILES,
> -    OUTPUT_TAGS
> +    OUTPUT_SUMMARY	= 1 << 0,
> +    OUTPUT_THREADS	= 1 << 1,
> +    OUTPUT_MESSAGES	= 1 << 2,
> +    OUTPUT_FILES	= 1 << 3,
> +    OUTPUT_TAGS		= 1 << 4,
> +    OUTPUT_SENDER	= 1 << 5,
> +    OUTPUT_RECIPIENTS	= 1 << 6,
> +    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
>  } output_t;
>  
>  typedef struct {
> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
>      return 0;
>  }
>  
> +static void
> +print_address_list (const search_options_t *o, InternetAddressList *list)
> +{
> +    InternetAddress *address;
> +    int i;
> +
> +    for (i = 0; i < internet_address_list_length (list); i++) {
> +	address = internet_address_list_get_address (list, i);
> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
> +	    InternetAddressGroup *group;
> +	    InternetAddressList *group_list;
> +
> +	    group = INTERNET_ADDRESS_GROUP (address);
> +	    group_list = internet_address_group_get_members (group);
> +	    if (group_list == NULL)
> +		continue;
> +
> +	    print_address_list (o, group_list);
> +	} else {
> +	    InternetAddressMailbox *mailbox;
> +	    const char *name;
> +	    const char *addr;
> +	    char *full_address;
> +
> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +
> +	    name = internet_address_get_name (address);
> +	    addr = internet_address_mailbox_get_addr (mailbox);
> +
> +	    if (name && *name)
> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
> +	    else
> +		full_address = talloc_strdup (o->format, addr);
> +
> +	    if (!full_address) {
> +		fprintf (stderr, "Error: out of memory\n");
> +		break;
> +	    }
> +	    o->format->string (o->format, full_address);
> +	    o->format->separator (o->format);
> +
> +	    talloc_free (full_address);

Thinking about this some more how about printing the name and address as
a structured pair/map (at least for all cases except text/text0 output):
something like (in JSON)
[name: "John Doe" address: "john.doe@example.com"]

It seems wrong to me to go to the effort of separating them in the C and
then combining them in the output.

This could also help with the questions about uniqueness. If the client
can get the data ready parsed into name/address then it can deal with
much of the uniqueness itself.

My preference would be for the default to print one line for each
distinct full_address, and then any filter-by options to refine from
there.

One other advantage of structuring the output is that it is extensible:
for example, at some later stage, we could include a "count" in the map
allowing the client can pick the most popular variant.

Best wishes

Mark




> +	}
> +    }
> +}
> +
> +static void
> +print_address_string (const search_options_t *o, const char *recipients)
> +{
> +    InternetAddressList *list;
> +
> +    if (recipients == NULL)
> +	return;
> +
> +    list = internet_address_list_parse_string (recipients);
> +    if (list == NULL)
> +	return;
> +
> +    print_address_list (o, list);
> +}
> +
>  static int
>  do_search_messages (search_options_t *o)
>  {
> @@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
>  	    
>  	    notmuch_filenames_destroy( filenames );
>  
> -	} else { /* output == OUTPUT_MESSAGES */
> +	} else if (o->output == OUTPUT_MESSAGES) {
>  	    format->set_prefix (format, "id");
>  	    format->string (format,
>  			    notmuch_message_get_message_id (message));
>  	    format->separator (format);
> +	} else {
> +	    if (o->output & OUTPUT_SENDER) {
> +		const char *addrs;
> +
> +		addrs = notmuch_message_get_header (message, "from");
> +		print_address_string (o, addrs);
> +	    }
> +
> +	    if (o->output & OUTPUT_RECIPIENTS) {
> +		const char *hdrs[] = { "to", "cc", "bcc" };
> +		const char *addrs;
> +		size_t j;
> +
> +		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
> +		    addrs = notmuch_message_get_header (message, hdrs[j]);
> +		    print_address_string (o, addrs);
> +		}
> +	    }
>  	}
>  
>  	notmuch_message_destroy (message);
> @@ -337,7 +419,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      notmuch_database_t *notmuch;
>      search_options_t o = {
>  	.sort = NOTMUCH_SORT_NEWEST_FIRST,
> -	.output = OUTPUT_SUMMARY,
> +	.output = 0,
>  	.offset = 0,
>  	.limit = -1, /* unlimited */
>  	.dupe = -1,
> @@ -366,10 +448,12 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
> -	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &o.output, "output", 'o',
>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>  				  { "threads", OUTPUT_THREADS },
>  				  { "messages", OUTPUT_MESSAGES },
> +				  { "sender", OUTPUT_SENDER },
> +				  { "recipients", OUTPUT_RECIPIENTS },
>  				  { "files", OUTPUT_FILES },
>  				  { "tags", OUTPUT_TAGS },
>  				  { 0, 0 } } },
> @@ -389,6 +473,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      if (opt_index < 0)
>  	return EXIT_FAILURE;
>  
> +    if (! o.output)
> +	o.output = OUTPUT_SUMMARY;
> +
>      switch (format_sel) {
>      case NOTMUCH_FORMAT_TEXT:
>  	o.format = sprinter_text_create (config, stdout);
> @@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      }
>  
>      switch (o.output) {
> -    default:
>      case OUTPUT_SUMMARY:
>      case OUTPUT_THREADS:
>  	ret = do_search_threads (&o);
>  	break;
>      case OUTPUT_MESSAGES:
> +    case OUTPUT_SENDER:
> +    case OUTPUT_RECIPIENTS:
> +    case OUTPUT_ADDRESSES:
>      case OUTPUT_FILES:
>  	ret = do_search_messages (&o);
>  	break;
>      case OUTPUT_TAGS:
>  	ret = do_search_tags (notmuch, o.format, o.query);
>  	break;
> +    default:
> +	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
> +	ret = 1;
>      }
>  
>      notmuch_query_destroy (o.query);
> diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
> index 947d572..e696c01 100755
> --- a/test/T090-search-output.sh
> +++ b/test/T090-search-output.sh
> @@ -387,6 +387,70 @@ cat <<EOF >EXPECTED
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "--output=sender"
> +notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Adrian Perez de Castro <aperez@igalia.com>
> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +      1 Aron Griffis <agriffis@n01se.net>
> +     12 Carl Worth <cworth@cworth.org>
> +      1 Chris Wilson <chris@chris-wilson.co.uk>
> +      1 François Boulogne <boulogne.f@gmail.com>
> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
> +      1 Israel Herraiz <isra@herraiz.org>
> +      4 Jan Janak <jan@ryngle.com>
> +      2 Jjgod Jiang <gzjjgod@gmail.com>
> +      7 Keith Packard <keithp@keithp.com>
> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
> +      5 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
> +      3 Stewart Smith <stewart@flamingspork.com>
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=recipients"
> +notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Allan McRae <allan@archlinux.org>
> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
> +      1 Keith Packard <keithp@keithp.com>
> +      1 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      2 notmuch <notmuch@notmuchmail.org>
> +     48 notmuch@notmuchmail.org
> +      1 olivier.berger@it-sudparis.eu
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=sender --output=recipients"
> +notmuch search --output=sender --output=recipients '*' | sort | uniq --count >OUTPUT
> +cat <<EOF >EXPECTED
> +      1 Adrian Perez de Castro <aperez@igalia.com>
> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +      1 Allan McRae <allan@archlinux.org>
> +      1 Aron Griffis <agriffis@n01se.net>
> +     12 Carl Worth <cworth@cworth.org>
> +      1 Chris Wilson <chris@chris-wilson.co.uk>
> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
> +      1 François Boulogne <boulogne.f@gmail.com>
> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
> +      1 Israel Herraiz <isra@herraiz.org>
> +      4 Jan Janak <jan@ryngle.com>
> +      2 Jjgod Jiang <gzjjgod@gmail.com>
> +      8 Keith Packard <keithp@keithp.com>
> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
> +      6 Mikhail Gusarov <dottedmag@dottedmag.net>
> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
> +      3 Stewart Smith <stewart@flamingspork.com>
> +      2 notmuch <notmuch@notmuchmail.org>
> +     48 notmuch@notmuchmail.org
> +      1 olivier.berger@it-sudparis.eu
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
>  test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
>  add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
>  	headers'"
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-23  9:41                     ` Mark Walters
@ 2014-10-24  9:38                       ` Tomi Ollila
  2014-10-24 10:23                       ` David Edmondson
  1 sibling, 0 replies; 31+ messages in thread
From: Tomi Ollila @ 2014-10-24  9:38 UTC (permalink / raw)
  To: Mark Walters, Michal Sojka, notmuch

On Thu, Oct 23 2014, Mark Walters <markwalters1009@gmail.com> wrote:

> On Sun, 12 Oct 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> The new outputs allow printing senders, recipients or both of matching
>> messages. The --output option is converted from "keyword" argument to
>> "flags" argument, which means that the user can use --output=sender and
>> --output=recipients simultaneously, to print both. Other combinations
>> produce an error.
>>
>> ...
>>
>> +static void
>> +print_address_list (const search_options_t *o, InternetAddressList *list)
>> +{
>> +    InternetAddress *address;
>> +    int i;
>> +
>> +    for (i = 0; i < internet_address_list_length (list); i++) {
>> +	address = internet_address_list_get_address (list, i);
>> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
>> +	    InternetAddressGroup *group;
>> +	    InternetAddressList *group_list;
>> +
>> +	    group = INTERNET_ADDRESS_GROUP (address);
>> +	    group_list = internet_address_group_get_members (group);
>> +	    if (group_list == NULL)
>> +		continue;
>> +
>> +	    print_address_list (o, group_list);
>> +	} else {
>> +	    InternetAddressMailbox *mailbox;
>> +	    const char *name;
>> +	    const char *addr;
>> +	    char *full_address;
>> +
>> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
>> +
>> +	    name = internet_address_get_name (address);
>> +	    addr = internet_address_mailbox_get_addr (mailbox);
>> +
>> +	    if (name && *name)
>> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
>> +	    else
>> +		full_address = talloc_strdup (o->format, addr);
>> +
>> +	    if (!full_address) {
>> +		fprintf (stderr, "Error: out of memory\n");
>> +		break;
>> +	    }
>> +	    o->format->string (o->format, full_address);
>> +	    o->format->separator (o->format);
>> +
>> +	    talloc_free (full_address);
>
> Thinking about this some more how about printing the name and address as
> a structured pair/map (at least for all cases except text/text0 output):
> something like (in JSON)
> [name: "John Doe" address: "john.doe@example.com"]
>
> It seems wrong to me to go to the effort of separating them in the C and
> then combining them in the output.
>
> This could also help with the questions about uniqueness. If the client
> can get the data ready parsed into name/address then it can deal with
> much of the uniqueness itself.

In that case client can also filter based on some substring, reducing the
memory requirements...

>
> My preference would be for the default to print one line for each
> distinct full_address, and then any filter-by options to refine from
> there.

Hmm, now I cannot decide whether this or just print out all addresses of
messages, or do this distinct full_address output -- it looks like all
other --output options prints unique lines, but there is potential of 
quite a lot of memory usage there...

... probably the memory usage is not problem there, OOM-killer eventually
does it's job if necessary (!) (but machine may be slow (and trashing) for
a while (just thinking out loud))

(!) but could we have general filter option for search to drop data before
it is even considered for caching! -- maybe later ?


> One other advantage of structuring the output is that it is extensible:
> for example, at some later stage, we could include a "count" in the map
> allowing the client can pick the most popular variant.

, and in this case notmuch cannot print any output until the full address
list is gathered... :D

>
> Best wishes
>
> Mark

Tomi

>
>
>
>
>> +	}
>> +    }
>> +}
>> +
>> +static void
>> +print_address_string (const search_options_t *o, const char *recipients)
>> +{
>> +    InternetAddressList *list;
>> +
>> +    if (recipients == NULL)
>> +	return;
>> +
>> +    list = internet_address_list_parse_string (recipients);
>> +    if (list == NULL)
>> +	return;
>> +
>> +    print_address_list (o, list);
>> +}
>> +
>>  static int
>>  do_search_messages (search_options_t *o)
>>  {
>> @@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
>>  	    
>>  	    notmuch_filenames_destroy( filenames );
>>  
>> -	} else { /* output == OUTPUT_MESSAGES */
>> +	} else if (o->output == OUTPUT_MESSAGES) {
>>  	    format->set_prefix (format, "id");
>>  	    format->string (format,
>>  			    notmuch_message_get_message_id (message));
>>  	    format->separator (format);
>> +	} else {
>> +	    if (o->output & OUTPUT_SENDER) {
>> +		const char *addrs;
>> +
>> +		addrs = notmuch_message_get_header (message, "from");
>> +		print_address_string (o, addrs);
>> +	    }
>> +
>> +	    if (o->output & OUTPUT_RECIPIENTS) {
>> +		const char *hdrs[] = { "to", "cc", "bcc" };
>> +		const char *addrs;
>> +		size_t j;
>> +
>> +		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
>> +		    addrs = notmuch_message_get_header (message, hdrs[j]);
>> +		    print_address_string (o, addrs);
>> +		}
>> +	    }
>>  	}
>>  
>>  	notmuch_message_destroy (message);
>> @@ -337,7 +419,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      notmuch_database_t *notmuch;
>>      search_options_t o = {
>>  	.sort = NOTMUCH_SORT_NEWEST_FIRST,
>> -	.output = OUTPUT_SUMMARY,
>> +	.output = 0,
>>  	.offset = 0,
>>  	.limit = -1, /* unlimited */
>>  	.dupe = -1,
>> @@ -366,10 +448,12 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>>  				  { 0, 0 } } },
>>  	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>> -	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
>> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &o.output, "output", 'o',
>>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>>  				  { "threads", OUTPUT_THREADS },
>>  				  { "messages", OUTPUT_MESSAGES },
>> +				  { "sender", OUTPUT_SENDER },
>> +				  { "recipients", OUTPUT_RECIPIENTS },
>>  				  { "files", OUTPUT_FILES },
>>  				  { "tags", OUTPUT_TAGS },
>>  				  { 0, 0 } } },
>> @@ -389,6 +473,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      if (opt_index < 0)
>>  	return EXIT_FAILURE;
>>  
>> +    if (! o.output)
>> +	o.output = OUTPUT_SUMMARY;
>> +
>>      switch (format_sel) {
>>      case NOTMUCH_FORMAT_TEXT:
>>  	o.format = sprinter_text_create (config, stdout);
>> @@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      }
>>  
>>      switch (o.output) {
>> -    default:
>>      case OUTPUT_SUMMARY:
>>      case OUTPUT_THREADS:
>>  	ret = do_search_threads (&o);
>>  	break;
>>      case OUTPUT_MESSAGES:
>> +    case OUTPUT_SENDER:
>> +    case OUTPUT_RECIPIENTS:
>> +    case OUTPUT_ADDRESSES:
>>      case OUTPUT_FILES:
>>  	ret = do_search_messages (&o);
>>  	break;
>>      case OUTPUT_TAGS:
>>  	ret = do_search_tags (notmuch, o.format, o.query);
>>  	break;
>> +    default:
>> +	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
>> +	ret = 1;
>>      }
>>  
>>      notmuch_query_destroy (o.query);
>> diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
>> index 947d572..e696c01 100755
>> --- a/test/T090-search-output.sh
>> +++ b/test/T090-search-output.sh
>> @@ -387,6 +387,70 @@ cat <<EOF >EXPECTED
>>  EOF
>>  test_expect_equal_file OUTPUT EXPECTED
>>  
>> +test_begin_subtest "--output=sender"
>> +notmuch search --output=sender '*' | sort | uniq --count >OUTPUT
>> +cat <<EOF >EXPECTED
>> +      1 Adrian Perez de Castro <aperez@igalia.com>
>> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
>> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
>> +      1 Aron Griffis <agriffis@n01se.net>
>> +     12 Carl Worth <cworth@cworth.org>
>> +      1 Chris Wilson <chris@chris-wilson.co.uk>
>> +      1 François Boulogne <boulogne.f@gmail.com>
>> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
>> +      1 Israel Herraiz <isra@herraiz.org>
>> +      4 Jan Janak <jan@ryngle.com>
>> +      2 Jjgod Jiang <gzjjgod@gmail.com>
>> +      7 Keith Packard <keithp@keithp.com>
>> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
>> +      5 Mikhail Gusarov <dottedmag@dottedmag.net>
>> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
>> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
>> +      3 Stewart Smith <stewart@flamingspork.com>
>> +EOF
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>> +test_begin_subtest "--output=recipients"
>> +notmuch search --output=recipients '*' | sort | uniq --count >OUTPUT
>> +cat <<EOF >EXPECTED
>> +      1 Allan McRae <allan@archlinux.org>
>> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
>> +      1 Keith Packard <keithp@keithp.com>
>> +      1 Mikhail Gusarov <dottedmag@dottedmag.net>
>> +      2 notmuch <notmuch@notmuchmail.org>
>> +     48 notmuch@notmuchmail.org
>> +      1 olivier.berger@it-sudparis.eu
>> +EOF
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>> +test_begin_subtest "--output=sender --output=recipients"
>> +notmuch search --output=sender --output=recipients '*' | sort | uniq --count >OUTPUT
>> +cat <<EOF >EXPECTED
>> +      1 Adrian Perez de Castro <aperez@igalia.com>
>> +      2 Alex Botero-Lowry <alex.boterolowry@gmail.com>
>> +      4 Alexander Botero-Lowry <alex.boterolowry@gmail.com>
>> +      1 Allan McRae <allan@archlinux.org>
>> +      1 Aron Griffis <agriffis@n01se.net>
>> +     12 Carl Worth <cworth@cworth.org>
>> +      1 Chris Wilson <chris@chris-wilson.co.uk>
>> +      1 Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
>> +      1 François Boulogne <boulogne.f@gmail.com>
>> +      1 Ingmar Vanhassel <ingmar@exherbo.org>
>> +      1 Israel Herraiz <isra@herraiz.org>
>> +      4 Jan Janak <jan@ryngle.com>
>> +      2 Jjgod Jiang <gzjjgod@gmail.com>
>> +      8 Keith Packard <keithp@keithp.com>
>> +      5 Lars Kellogg-Stedman <lars@seas.harvard.edu>
>> +      6 Mikhail Gusarov <dottedmag@dottedmag.net>
>> +      1 Olivier Berger <olivier.berger@it-sudparis.eu>
>> +      1 Rolland Santimano <rollandsantimano@yahoo.com>
>> +      3 Stewart Smith <stewart@flamingspork.com>
>> +      2 notmuch <notmuch@notmuchmail.org>
>> +     48 notmuch@notmuchmail.org
>> +      1 olivier.berger@it-sudparis.eu
>> +EOF
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>>  test_begin_subtest "sanitize output for quoted-printable line-breaks in author and subject"
>>  add_message "[subject]='two =?ISO-8859-1?Q?line=0A_subject?=
>>  	headers'"
>> -- 
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-23  9:41                     ` Mark Walters
  2014-10-24  9:38                       ` Tomi Ollila
@ 2014-10-24 10:23                       ` David Edmondson
  1 sibling, 0 replies; 31+ messages in thread
From: David Edmondson @ 2014-10-24 10:23 UTC (permalink / raw)
  To: Mark Walters, Michal Sojka, notmuch

On Thu, Oct 23 2014, Mark Walters wrote:
> Thinking about this some more how about printing the name and address as
> a structured pair/map (at least for all cases except text/text0 output):
> something like (in JSON)
> [name: "John Doe" address: "john.doe@example.com"]
>
> It seems wrong to me to go to the effort of separating them in the C and
> then combining them in the output.

Agreed, this would be convenient.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients}
  2014-10-22 10:51                     ` Mark Walters
@ 2014-10-24 10:57                       ` Michal Sojka
  0 siblings, 0 replies; 31+ messages in thread
From: Michal Sojka @ 2014-10-24 10:57 UTC (permalink / raw)
  To: Mark Walters, notmuch

Hi Mark,

I mostly agree with your points mentioned in this and other your emails.
I'll prepare v4 based on that.

On Wed, Oct 22 2014, Mark Walters wrote:
> On Sun, 12 Oct 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> The new outputs allow printing senders, recipients or both of matching
>> messages. The --output option is converted from "keyword" argument to
>> "flags" argument, which means that the user can use --output=sender and
>> --output=recipients simultaneously, to print both. Other combinations
>> produce an error.
>>
>> This code based on a patch from Jani Nikula.
>> ---
>>  completion/notmuch-completion.bash |   2 +-
>>  completion/notmuch-completion.zsh  |   3 +-
>>  doc/man1/notmuch-search.rst        |  22 +++++++-
>>  notmuch-search.c                   | 110 ++++++++++++++++++++++++++++++++++---
>>  test/T090-search-output.sh         |  64 +++++++++++++++++++++
>>  5 files changed, 189 insertions(+), 12 deletions(-)
>>
>> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
>> index 0571dc9..cfbd389 100644
>> --- a/completion/notmuch-completion.bash
>> +++ b/completion/notmuch-completion.bash
>> @@ -294,7 +294,7 @@ _notmuch_search()
>>  	    return
>>  	    ;;
>>  	--output)
>> -	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
>> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags sender recipients" -- "${cur}" ) )
>>  	    return
>>  	    ;;
>>  	--sort)
>> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
>> index 67a9aba..3e52a00 100644
>> --- a/completion/notmuch-completion.zsh
>> +++ b/completion/notmuch-completion.zsh
>> @@ -52,7 +52,8 @@ _notmuch_search()
>>    _arguments -s : \
>>      '--max-threads=[display only the first x threads from the search results]:number of threads to show: ' \
>>      '--first=[omit the first x threads from the search results]:number of threads to omit: ' \
>> -    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))'
>> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
>> +    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
>>  }
>>
>>  _notmuch()
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 90160f2..c9d38b1 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -35,7 +35,7 @@ Supported options for **search** include
>>          intended for programs that invoke **notmuch(1)** internally. If
>>          omitted, the latest supported version will be used.
>>
>> -    ``--output=(summary|threads|messages|files|tags)``
>> +    ``--output=(summary|threads|messages|files|tags|sender|recipients)``
>>
>>          **summary**
>>              Output a summary of each thread with any message matching
>> @@ -78,6 +78,26 @@ Supported options for **search** include
>>              by null characters (--format=text0), as a JSON array
>>              (--format=json), or as an S-Expression list (--format=sexp).
>>
>> +	**sender**
>> +            Output all addresses from the *From* header that appear on
>> +            any message matching the search terms, either one per line
>> +            (--format=text), separated by null characters
>> +            (--format=text0), as a JSON array (--format=json), or as
>> +            an S-Expression list (--format=sexp).
>> +
>> +	    Note: Searching for **sender** should be much faster than
>> +	    searching for **recipients**, because sender addresses are
>> +	    cached directly in the database whereas other addresses
>> +	    need to be fetched from message files.
>> +
>> +	**recipients**
>> +            Like **sender** but for addresses from *To*, *Cc* and
>> +	    *Bcc* headers.
>> +
>> +	This option can be given multiple times to combine different
>> +	outputs. Curently, this is only supported for **sender** and
>> +	**recipients** outputs.
>> +
>>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>>          This option can be used to present results in either
>>          chronological order (**oldest-first**) or reverse chronological
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index 5ac2a26..74588f8 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -23,11 +23,14 @@
>>  #include "string-util.h"
>>
>>  typedef enum {
>> -    OUTPUT_SUMMARY,
>> -    OUTPUT_THREADS,
>> -    OUTPUT_MESSAGES,
>> -    OUTPUT_FILES,
>> -    OUTPUT_TAGS
>> +    OUTPUT_SUMMARY	= 1 << 0,
>> +    OUTPUT_THREADS	= 1 << 1,
>> +    OUTPUT_MESSAGES	= 1 << 2,
>> +    OUTPUT_FILES	= 1 << 3,
>> +    OUTPUT_TAGS		= 1 << 4,
>> +    OUTPUT_SENDER	= 1 << 5,
>> +    OUTPUT_RECIPIENTS	= 1 << 6,
>> +    OUTPUT_ADDRESSES	= OUTPUT_SENDER | OUTPUT_RECIPIENTS,
>
> I think I would drop the OUTPUT_ADDRESSES enum as the parser no longer
> uses it (and replace the one use by OUTPUT_SENDER | OUTPUT_RECIPIENTS
> below).

As mentioned elsewhere, this is required to suppress the following warning.

notmuch-search.c:634:5: warning: case value ‘96’ not in enumerated type ‘output_t’ [-Wswitch]

>
>>  } output_t;
>>
>>  typedef struct {
>> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o)
>>      return 0;
>>  }
>>
>> +static void
>> +print_address_list (const search_options_t *o, InternetAddressList *list)
>> +{
>> +    InternetAddress *address;
>> +    int i;
>> +
>> +    for (i = 0; i < internet_address_list_length (list); i++) {
>> +	address = internet_address_list_get_address (list, i);
>> +	if (INTERNET_ADDRESS_IS_GROUP (address)) {
>> +	    InternetAddressGroup *group;
>> +	    InternetAddressList *group_list;
>> +
>> +	    group = INTERNET_ADDRESS_GROUP (address);
>> +	    group_list = internet_address_group_get_members (group);
>> +	    if (group_list == NULL)
>> +		continue;
>> +
>> +	    print_address_list (o, group_list);
>> +	} else {
>> +	    InternetAddressMailbox *mailbox;
>> +	    const char *name;
>> +	    const char *addr;
>> +	    char *full_address;
>> +
>> +	    mailbox = INTERNET_ADDRESS_MAILBOX (address);
>> +
>> +	    name = internet_address_get_name (address);
>> +	    addr = internet_address_mailbox_get_addr (mailbox);
>> +
>> +	    if (name && *name)
>> +		full_address = talloc_asprintf (o->format, "%s <%s>", name, addr);
>> +	    else
>> +		full_address = talloc_strdup (o->format, addr);
>> +
>> +	    if (!full_address) {
>> +		fprintf (stderr, "Error: out of memory\n");
>> +		break;
>> +	    }
>> +	    o->format->string (o->format, full_address);
>> +	    o->format->separator (o->format);
>> +
>> +	    talloc_free (full_address);
>> +	}
>> +    }
>> +}
>> +
>> +static void
>> +print_address_string (const search_options_t *o, const char *recipients)
>> +{
>> +    InternetAddressList *list;
>> +
>> +    if (recipients == NULL)
>> +	return;
>> +
>> +    list = internet_address_list_parse_string (recipients);
>> +    if (list == NULL)
>> +	return;
>> +
>> +    print_address_list (o, list);
>> +}
>> +
>>  static int
>>  do_search_messages (search_options_t *o)
>>  {
>> @@ -266,11 +330,29 @@ do_search_messages (search_options_t *o)
>>
>>  	    notmuch_filenames_destroy( filenames );
>>
>> -	} else { /* output == OUTPUT_MESSAGES */
>> +	} else if (o->output == OUTPUT_MESSAGES) {
>>  	    format->set_prefix (format, "id");
>>  	    format->string (format,
>>  			    notmuch_message_get_message_id (message));
>>  	    format->separator (format);
>> +	} else {
>> +	    if (o->output & OUTPUT_SENDER) {
>> +		const char *addrs;
>> +
>> +		addrs = notmuch_message_get_header (message, "from");
>> +		print_address_string (o, addrs);
>> +	    }
>> +
>> +	    if (o->output & OUTPUT_RECIPIENTS) {
>> +		const char *hdrs[] = { "to", "cc", "bcc" };
>> +		const char *addrs;
>> +		size_t j;
>> +
>> +		for (j = 0; j < ARRAY_SIZE (hdrs); j++) {
>> +		    addrs = notmuch_message_get_header (message, hdrs[j]);
>> +		    print_address_string (o, addrs);
>> +		}
>> +	    }
>>  	}
>>
>>  	notmuch_message_destroy (message);
>> @@ -337,7 +419,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      notmuch_database_t *notmuch;
>>      search_options_t o = {
>>  	.sort = NOTMUCH_SORT_NEWEST_FIRST,
>> -	.output = OUTPUT_SUMMARY,
>> +	.output = 0,
>>  	.offset = 0,
>>  	.limit = -1, /* unlimited */
>>  	.dupe = -1,
>
> It is slightly unfortunate that all the other defaults are defined here
> but the default output is after the option parsing but I can't see a
> nice way round that. I only mention it in case you (or anyone else) can
> see a nice way round this.

Solution would to have --output as NOTMUCH_OPT_KEYWORD rather than
NOTMUCH_OPT_KEYWORD_FLAGS as it was in v2.

>
>> @@ -366,10 +448,12 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>>  				  { 0, 0 } } },
>>  	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>> -	{ NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o',
>> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &o.output, "output", 'o',
>>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>>  				  { "threads", OUTPUT_THREADS },
>>  				  { "messages", OUTPUT_MESSAGES },
>> +				  { "sender", OUTPUT_SENDER },
>> +				  { "recipients", OUTPUT_RECIPIENTS },
>>  				  { "files", OUTPUT_FILES },
>>  				  { "tags", OUTPUT_TAGS },
>>  				  { 0, 0 } } },
>> @@ -389,6 +473,9 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      if (opt_index < 0)
>>  	return EXIT_FAILURE;
>>
>> +    if (! o.output)
>> +	o.output = OUTPUT_SUMMARY;
>> +
>>      switch (format_sel) {
>>      case NOTMUCH_FORMAT_TEXT:
>>  	o.format = sprinter_text_create (config, stdout);
>> @@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      }
>>
>>      switch (o.output) {
>> -    default:
>>      case OUTPUT_SUMMARY:
>>      case OUTPUT_THREADS:
>>  	ret = do_search_threads (&o);
>>  	break;
>>      case OUTPUT_MESSAGES:
>> +    case OUTPUT_SENDER:
>> +    case OUTPUT_RECIPIENTS:
>> +    case OUTPUT_ADDRESSES:
>
> This is the one use of OUTPUT_ADDRESSES that I would replace with
> OUTPUT_SENDER | OUTPUT_RECIPIENTS

See above.

-Michal

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2014-10-24 10:57 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-22  9:37 [PATCH 0/5] notmuch search --output=addresses Michal Sojka
2014-09-22  9:37 ` [PATCH 1/5] cli: Refactor option passing in the search command Michal Sojka
2014-09-25 19:13   ` Tomi Ollila
2014-09-25 20:48     ` Michal Sojka
2014-09-29 16:04       ` Tomi Ollila
2014-09-29 17:28       ` Jani Nikula
2014-10-05 20:51         ` [PATCH v2 0/4] notmuch search --output=addresses Michal Sojka
2014-10-05 20:51           ` [PATCH v2 1/4] cli: Refactor option passing in the search command Michal Sojka
2014-10-05 20:51           ` [PATCH v2 2/4] cli: Extend the search command for --output=addresses and similar Michal Sojka
2014-10-06 18:56             ` Tomi Ollila
2014-10-09 10:55               ` Michal Sojka
2014-10-12 21:41                 ` [PATCH v3 0/4] notmuch search --output=sender/recipients Michal Sojka
2014-10-12 21:41                   ` [PATCH v3 1/4] cli: Refactor option passing in the search command Michal Sojka
2014-10-22 10:44                     ` Mark Walters
2014-10-12 21:41                   ` [PATCH v3 2/4] cli: Add support for parsing multiple keyword arguments Michal Sojka
2014-10-12 21:41                   ` [PATCH v3 3/4] cli: Extend the search command for --output={sender, recipients} Michal Sojka
2014-10-13 19:00                     ` Tomi Ollila
2014-10-13 21:38                       ` Michal Sojka
2014-10-19 10:02                         ` Tomi Ollila
2014-10-22 10:51                     ` Mark Walters
2014-10-24 10:57                       ` Michal Sojka
2014-10-23  9:41                     ` Mark Walters
2014-10-24  9:38                       ` Tomi Ollila
2014-10-24 10:23                       ` David Edmondson
2014-10-12 21:41                   ` [PATCH v3 4/4] cli: Add an option to filter our duplicate addresses Michal Sojka
2014-10-05 20:51           ` [PATCH v2 3/4] cli: Add support for parsing multiple keyword arguments Michal Sojka
2014-10-05 20:51           ` [PATCH v2 4/4] cli: Add configurable address deduplication for --output=addresses Michal Sojka
2014-09-22  9:37 ` [PATCH 2/5] cli: Extend the search command for --output=addresses and similar Michal Sojka
2014-09-22  9:37 ` [PATCH 3/5] cli: Add support for parsing command line "flag" options Michal Sojka
2014-09-22  9:37 ` [PATCH 4/5] cli: Add configurable address deduplication for --output=addresses Michal Sojka
2014-09-22  9:37 ` [PATCH 5/5] cli: Add tests for 'search --output=addresses' and similar Michal Sojka

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).