[PATCH v2 00/10] "notmuch address" command

[PATCH v2 00/10] "notmuch address" command

[Michal Sojka]

Hi all,

this is v2 of "notmuch address" patchset. It obsoletes [1].

Don't be scared by the number of patches. Most of them are trivial
refactoring. Patches 1-4 refactor the code so that "notmuch search"
command is easier to split. Patch 5 is Jani's hierarchical command
line parsing patch. Patch 6 splits search functionality to new address
command. Patch 7 is minor refactoring. Patches 8-10 correspond to
patches 5-7 in the original "notmuch search
--output=sender/recipients" patch series [2].

Changes from v1:

- Rebased to current master (conflicted with Jani's "notmuch search
  --duplicate=N with --output=messages" patch)
- Fixed printing of false "Unrecognized option" error message in
  hierarchical command line parser.

Regards,
-Michal

[1] id:1414889400-30977-1-git-send-email-sojkam1@fel.cvut.cz
[2] id:1414792441-29555-1-git-send-email-sojkam1@fel.cvut.cz


Jani Nikula (1):
  cli: add support for hierarchical command line option arrays

Michal Sojka (9):
  cli: search: Rename options to context
  cli: search: Move more variables into search_context_t
  cli: search: Convert ctx. to ctx->
  cli: search: Split notmuch_search_command to smaller functions
  cli: Introduce "notmuch address" command
  cli: search: Convert --output to keyword argument
  cli: address: Do not output duplicate addresses
  cli: address: Add --output=count
  cli: address: Add --filter-by option to configure address filtering

 command-line-arguments.c           |  16 +-
 command-line-arguments.h           |   1 +
 completion/notmuch-completion.bash |  48 +++-
 completion/notmuch-completion.zsh  |  11 +-
 doc/man1/notmuch-address.rst       | 140 ++++++++++++
 doc/man1/notmuch-search.rst        |  21 +-
 doc/man1/notmuch.rst               |   7 +-
 notmuch-client.h                   |   3 +
 notmuch-search.c                   | 454 +++++++++++++++++++++++++------------
 notmuch.c                          |   2 +
 test/T095-address.sh               | 148 ++++++++++++
 test/T097-address-filter-by.sh     |  73 ++++++
 12 files changed, 751 insertions(+), 173 deletions(-)
 create mode 100644 doc/man1/notmuch-address.rst
 create mode 100755 test/T095-address.sh
 create mode 100755 test/T097-address-filter-by.sh

-- 
2.1.1

[PATCH v2 01/10] cli: search: Rename options to context

[Michal Sojka]

Just text replacement, no other changes.
---
 notmuch-search.c | 142 +++++++++++++++++++++++++++----------------------------
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 6345fb6..2c47b80 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -42,7 +42,7 @@ typedef struct {
     int offset;
     int limit;
     int dupe;
-} search_options_t;
+} search_context_t;
 
 typedef struct {
     const char *name;
@@ -89,39 +89,39 @@ get_thread_query (notmuch_thread_t *thread,
 }
 
 static int
-do_search_threads (search_options_t *opt)
+do_search_threads (search_context_t *ctx)
 {
     notmuch_thread_t *thread;
     notmuch_threads_t *threads;
     notmuch_tags_t *tags;
-    sprinter_t *format = opt->format;
+    sprinter_t *format = ctx->format;
     time_t date;
     int i;
 
-    if (opt->offset < 0) {
-	opt->offset += notmuch_query_count_threads (opt->query);
-	if (opt->offset < 0)
-	    opt->offset = 0;
+    if (ctx->offset < 0) {
+	ctx->offset += notmuch_query_count_threads (ctx->query);
+	if (ctx->offset < 0)
+	    ctx->offset = 0;
     }
 
-    threads = notmuch_query_search_threads (opt->query);
+    threads = notmuch_query_search_threads (ctx->query);
     if (threads == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_threads_valid (threads) && (opt->limit < 0 || i < opt->offset + opt->limit);
+	 notmuch_threads_valid (threads) && (ctx->limit < 0 || i < ctx->offset + ctx->limit);
 	 notmuch_threads_move_to_next (threads), i++)
     {
 	thread = notmuch_threads_get (threads);
 
-	if (i < opt->offset) {
+	if (i < ctx->offset) {
 	    notmuch_thread_destroy (thread);
 	    continue;
 	}
 
-	if (opt->output == OUTPUT_THREADS) {
+	if (ctx->output == OUTPUT_THREADS) {
 	    format->set_prefix (format, "thread");
 	    format->string (format,
 			    notmuch_thread_get_thread_id (thread));
@@ -138,7 +138,7 @@ do_search_threads (search_options_t *opt)
 
 	    format->begin_map (format);
 
-	    if (opt->sort == NOTMUCH_SORT_OLDEST_FIRST)
+	    if (ctx->sort == NOTMUCH_SORT_OLDEST_FIRST)
 		date = notmuch_thread_get_oldest_date (thread);
 	    else
 		date = notmuch_thread_get_newest_date (thread);
@@ -230,11 +230,11 @@ do_search_threads (search_options_t *opt)
 }
 
 static void
-print_mailbox (const search_options_t *opt, const mailbox_t *mailbox)
+print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
 {
     const char *name = mailbox->name;
     const char *addr = mailbox->addr;
-    sprinter_t *format = opt->format;
+    sprinter_t *format = ctx->format;
     InternetAddress *ia = internet_address_mailbox_new (name, addr);
     char *name_addr;
 
@@ -263,7 +263,7 @@ print_mailbox (const search_options_t *opt, const mailbox_t *mailbox)
 
 /* Print addresses from InternetAddressList.  */
 static void
-process_address_list (const search_options_t *opt, InternetAddressList *list)
+process_address_list (const search_context_t *ctx, InternetAddressList *list)
 {
     InternetAddress *address;
     int i;
@@ -279,7 +279,7 @@ process_address_list (const search_options_t *opt, InternetAddressList *list)
 	    if (group_list == NULL)
 		continue;
 
-	    process_address_list (opt, group_list);
+	    process_address_list (ctx, group_list);
 	} else {
 	    InternetAddressMailbox *mailbox = INTERNET_ADDRESS_MAILBOX (address);
 	    mailbox_t mbx = {
@@ -287,14 +287,14 @@ process_address_list (const search_options_t *opt, InternetAddressList *list)
 		.addr = internet_address_mailbox_get_addr (mailbox),
 	    };
 
-	    print_mailbox (opt, &mbx);
+	    print_mailbox (ctx, &mbx);
 	}
     }
 }
 
 /* Print addresses from a message header.  */
 static void
-process_address_header (const search_options_t *opt, const char *value)
+process_address_header (const search_context_t *ctx, const char *value)
 {
     InternetAddressList *list;
 
@@ -305,7 +305,7 @@ process_address_header (const search_options_t *opt, const char *value)
     if (list == NULL)
 	return;
 
-    process_address_list (opt, list);
+    process_address_list (ctx, list);
 
     g_object_unref (list);
 }
@@ -329,36 +329,36 @@ _count_filenames (notmuch_message_t *message)
 }
 
 static int
-do_search_messages (search_options_t *opt)
+do_search_messages (search_context_t *ctx)
 {
     notmuch_message_t *message;
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
-    sprinter_t *format = opt->format;
+    sprinter_t *format = ctx->format;
     int i;
 
-    if (opt->offset < 0) {
-	opt->offset += notmuch_query_count_messages (opt->query);
-	if (opt->offset < 0)
-	    opt->offset = 0;
+    if (ctx->offset < 0) {
+	ctx->offset += notmuch_query_count_messages (ctx->query);
+	if (ctx->offset < 0)
+	    ctx->offset = 0;
     }
 
-    messages = notmuch_query_search_messages (opt->query);
+    messages = notmuch_query_search_messages (ctx->query);
     if (messages == NULL)
 	return 1;
 
     format->begin_list (format);
 
     for (i = 0;
-	 notmuch_messages_valid (messages) && (opt->limit < 0 || i < opt->offset + opt->limit);
+	 notmuch_messages_valid (messages) && (ctx->limit < 0 || i < ctx->offset + ctx->limit);
 	 notmuch_messages_move_to_next (messages), i++)
     {
-	if (i < opt->offset)
+	if (i < ctx->offset)
 	    continue;
 
 	message = notmuch_messages_get (messages);
 
-	if (opt->output == OUTPUT_FILES) {
+	if (ctx->output == OUTPUT_FILES) {
 	    int j;
 	    filenames = notmuch_message_get_filenames (message);
 
@@ -366,7 +366,7 @@ do_search_messages (search_options_t *opt)
 		 notmuch_filenames_valid (filenames);
 		 notmuch_filenames_move_to_next (filenames), j++)
 	    {
-		if (opt->dupe < 0 || opt->dupe == j) {
+		if (ctx->dupe < 0 || ctx->dupe == j) {
 		    format->string (format, notmuch_filenames_get (filenames));
 		    format->separator (format);
 		}
@@ -374,30 +374,30 @@ do_search_messages (search_options_t *opt)
 	    
 	    notmuch_filenames_destroy( filenames );
 
-	} else if (opt->output == OUTPUT_MESSAGES) {
+	} else if (ctx->output == OUTPUT_MESSAGES) {
             /* special case 1 for speed */
-            if (opt->dupe <= 1 || opt->dupe <= _count_filenames (message)) {
+            if (ctx->dupe <= 1 || ctx->dupe <= _count_filenames (message)) {
                 format->set_prefix (format, "id");
                 format->string (format,
                                 notmuch_message_get_message_id (message));
                 format->separator (format);
             }
 	} else {
-	    if (opt->output & OUTPUT_SENDER) {
+	    if (ctx->output & OUTPUT_SENDER) {
 		const char *addrs;
 
 		addrs = notmuch_message_get_header (message, "from");
-		process_address_header (opt, addrs);
+		process_address_header (ctx, addrs);
 	    }
 
-	    if (opt->output & OUTPUT_RECIPIENTS) {
+	    if (ctx->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]);
-		    process_address_header (opt, addrs);
+		    process_address_header (ctx, addrs);
 		}
 	    }
 	}
@@ -414,13 +414,13 @@ do_search_messages (search_options_t *opt)
 
 static int
 do_search_tags (notmuch_database_t *notmuch,
-		const search_options_t *opt)
+		const search_context_t *ctx)
 {
     notmuch_messages_t *messages = NULL;
     notmuch_tags_t *tags;
     const char *tag;
-    sprinter_t *format = opt->format;
-    notmuch_query_t *query = opt->query;
+    sprinter_t *format = ctx->format;
+    notmuch_query_t *query = ctx->query;
 
     /* should the following only special case if no excluded terms
      * specified? */
@@ -465,7 +465,7 @@ int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
     notmuch_database_t *notmuch;
-    search_options_t opt = {
+    search_context_t ctx = {
 	.sort = NOTMUCH_SORT_NEWEST_FIRST,
 	.output = 0,
 	.offset = 0,
@@ -485,7 +485,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, &opt.sort, "sort", 's',
+	{ NOTMUCH_OPT_KEYWORD, &ctx.sort, "sort", 's',
 	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
 				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
 				  { 0, 0 } } },
@@ -496,7 +496,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_FLAGS, &opt.output, "output", 'o',
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx.output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
@@ -511,9 +511,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, &opt.offset, "offset", 'O', 0 },
-	{ NOTMUCH_OPT_INT, &opt.limit, "limit", 'L', 0  },
-	{ NOTMUCH_OPT_INT, &opt.dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_INT, &ctx.offset, "offset", 'O', 0 },
+	{ NOTMUCH_OPT_INT, &ctx.limit, "limit", 'L', 0  },
+	{ NOTMUCH_OPT_INT, &ctx.dupe, "duplicate", 'D', 0  },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -521,31 +521,31 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
-    if (! opt.output)
-	opt.output = OUTPUT_SUMMARY;
+    if (! ctx.output)
+	ctx.output = OUTPUT_SUMMARY;
 
-    if (opt.output != OUTPUT_FILES && opt.output != OUTPUT_MESSAGES &&
-	opt.dupe != -1) {
+    if (ctx.output != OUTPUT_FILES && ctx.output != OUTPUT_MESSAGES &&
+	ctx.dupe != -1) {
         fprintf (stderr, "Error: --duplicate=N is only supported with --output=files and --output=messages.\n");
         return EXIT_FAILURE;
     }
 
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	opt.format = sprinter_text_create (config, stdout);
+	ctx.format = sprinter_text_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_TEXT0:
-	if (opt.output == OUTPUT_SUMMARY) {
+	if (ctx.output == OUTPUT_SUMMARY) {
 	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
 	    return EXIT_FAILURE;
 	}
-	opt.format = sprinter_text0_create (config, stdout);
+	ctx.format = sprinter_text0_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	opt.format = sprinter_json_create (config, stdout);
+	ctx.format = sprinter_json_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_SEXP:
-	opt.format = sprinter_sexp_create (config, stdout);
+	ctx.format = sprinter_sexp_create (config, stdout);
 	break;
     default:
 	/* this should never happen */
@@ -568,15 +568,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    opt.query = notmuch_query_create (notmuch, query_str);
-    if (opt.query == NULL) {
+    ctx.query = notmuch_query_create (notmuch, query_str);
+    if (ctx.query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
     }
 
-    notmuch_query_set_sort (opt.query, opt.sort);
+    notmuch_query_set_sort (ctx.query, ctx.sort);
 
-    if (exclude == NOTMUCH_EXCLUDE_FLAG && opt.output != OUTPUT_SUMMARY) {
+    if (exclude == NOTMUCH_EXCLUDE_FLAG && ctx.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. */
@@ -591,28 +591,28 @@ 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 (opt.query, search_exclude_tags[i]);
-	notmuch_query_set_omit_excluded (opt.query, exclude);
+	    notmuch_query_add_tag_exclude (ctx.query, search_exclude_tags[i]);
+	notmuch_query_set_omit_excluded (ctx.query, exclude);
     }
 
-    if (opt.output == OUTPUT_SUMMARY ||
-	opt.output == OUTPUT_THREADS)
-	ret = do_search_threads (&opt);
-    else if (opt.output == OUTPUT_MESSAGES ||
-	     opt.output == OUTPUT_FILES ||
-	     (opt.output & OUTPUT_ADDRESS_FLAGS && !(opt.output & ~OUTPUT_ADDRESS_FLAGS)))
-	ret = do_search_messages (&opt);
-    else if (opt.output == OUTPUT_TAGS)
-	ret = do_search_tags (notmuch, &opt);
+    if (ctx.output == OUTPUT_SUMMARY ||
+	ctx.output == OUTPUT_THREADS)
+	ret = do_search_threads (&ctx);
+    else if (ctx.output == OUTPUT_MESSAGES ||
+	     ctx.output == OUTPUT_FILES ||
+	     (ctx.output & OUTPUT_ADDRESS_FLAGS && !(ctx.output & ~OUTPUT_ADDRESS_FLAGS)))
+	ret = do_search_messages (&ctx);
+    else if (ctx.output == OUTPUT_TAGS)
+	ret = do_search_tags (notmuch, &ctx);
     else {
 	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
 	ret = 1;
     }
 
-    notmuch_query_destroy (opt.query);
+    notmuch_query_destroy (ctx.query);
     notmuch_database_destroy (notmuch);
 
-    talloc_free (opt.format);
+    talloc_free (ctx.format);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.1.1

Re: [PATCH v2 01/10] cli: search: Rename options to context

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> Just text replacement, no other changes.

I agree that the actual change is trivial, but the commit message
could/should hint why it is being done.

[PATCH v2 02/10] cli: search: Move more variables into search_context_t

[Michal Sojka]

Just refactoring, no functional changes.
---
 notmuch-search.c | 49 ++++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 2c47b80..3d2012b 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -34,8 +34,18 @@ typedef enum {
 
 #define OUTPUT_ADDRESS_FLAGS (OUTPUT_SENDER | OUTPUT_RECIPIENTS)
 
+typedef enum {
+    NOTMUCH_FORMAT_JSON,
+    NOTMUCH_FORMAT_TEXT,
+    NOTMUCH_FORMAT_TEXT0,
+    NOTMUCH_FORMAT_SEXP
+} format_sel_t;
+
 typedef struct {
+    notmuch_database_t *notmuch;
+    format_sel_t format_sel;
     sprinter_t *format;
+    notmuch_exclude_t exclude;
     notmuch_query_t *query;
     notmuch_sort_t sort;
     output_t output;
@@ -413,14 +423,14 @@ do_search_messages (search_context_t *ctx)
 }
 
 static int
-do_search_tags (notmuch_database_t *notmuch,
-		const search_context_t *ctx)
+do_search_tags (const search_context_t *ctx)
 {
     notmuch_messages_t *messages = NULL;
     notmuch_tags_t *tags;
     const char *tag;
     sprinter_t *format = ctx->format;
     notmuch_query_t *query = ctx->query;
+    notmuch_database_t *notmuch = ctx->notmuch;
 
     /* should the following only special case if no excluded terms
      * specified? */
@@ -464,8 +474,9 @@ do_search_tags (notmuch_database_t *notmuch,
 int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    notmuch_database_t *notmuch;
     search_context_t ctx = {
+	.format_sel = NOTMUCH_FORMAT_TEXT,
+	.exclude = NOTMUCH_EXCLUDE_TRUE,
 	.sort = NOTMUCH_SORT_NEWEST_FIRST,
 	.output = 0,
 	.offset = 0,
@@ -474,22 +485,14 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     };
     char *query_str;
     int opt_index, ret;
-    notmuch_exclude_t exclude = NOTMUCH_EXCLUDE_TRUE;
     unsigned int i;
 
-    enum {
-	NOTMUCH_FORMAT_JSON,
-	NOTMUCH_FORMAT_TEXT,
-	NOTMUCH_FORMAT_TEXT0,
-	NOTMUCH_FORMAT_SEXP
-    } format_sel = NOTMUCH_FORMAT_TEXT;
-
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &ctx.sort, "sort", 's',
 	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
 				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
+	{ NOTMUCH_OPT_KEYWORD, &ctx.format_sel, "format", 'f',
 	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
 				  { "sexp", NOTMUCH_FORMAT_SEXP },
 				  { "text", NOTMUCH_FORMAT_TEXT },
@@ -505,7 +508,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "files", OUTPUT_FILES },
 				  { "tags", OUTPUT_TAGS },
 				  { 0, 0 } } },
-        { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
+        { NOTMUCH_OPT_KEYWORD, &ctx.exclude, "exclude", 'x',
           (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
                                   { "false", NOTMUCH_EXCLUDE_FALSE },
                                   { "flag", NOTMUCH_EXCLUDE_FLAG },
@@ -530,7 +533,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
         return EXIT_FAILURE;
     }
 
-    switch (format_sel) {
+    switch (ctx.format_sel) {
     case NOTMUCH_FORMAT_TEXT:
 	ctx.format = sprinter_text_create (config, stdout);
 	break;
@@ -555,10 +558,10 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_exit_if_unsupported_format ();
 
     if (notmuch_database_open (notmuch_config_get_database_path (config),
-			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx.notmuch))
 	return EXIT_FAILURE;
 
-    query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index);
+    query_str = query_string_from_args (ctx.notmuch, argc-opt_index, argv+opt_index);
     if (query_str == NULL) {
 	fprintf (stderr, "Out of memory.\n");
 	return EXIT_FAILURE;
@@ -568,7 +571,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    ctx.query = notmuch_query_create (notmuch, query_str);
+    ctx.query = notmuch_query_create (ctx.notmuch, query_str);
     if (ctx.query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
@@ -576,15 +579,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_query_set_sort (ctx.query, ctx.sort);
 
-    if (exclude == NOTMUCH_EXCLUDE_FLAG && ctx.output != OUTPUT_SUMMARY) {
+    if (ctx.exclude == NOTMUCH_EXCLUDE_FLAG && ctx.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. */
 	fprintf (stderr, "Warning: this output format cannot flag excluded messages.\n");
-	exclude = NOTMUCH_EXCLUDE_FALSE;
+	ctx.exclude = NOTMUCH_EXCLUDE_FALSE;
     }
 
-    if (exclude != NOTMUCH_EXCLUDE_FALSE) {
+    if (ctx.exclude != NOTMUCH_EXCLUDE_FALSE) {
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
 
@@ -592,7 +595,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	    (config, &search_exclude_tags_length);
 	for (i = 0; i < search_exclude_tags_length; i++)
 	    notmuch_query_add_tag_exclude (ctx.query, search_exclude_tags[i]);
-	notmuch_query_set_omit_excluded (ctx.query, exclude);
+	notmuch_query_set_omit_excluded (ctx.query, ctx.exclude);
     }
 
     if (ctx.output == OUTPUT_SUMMARY ||
@@ -603,14 +606,14 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	     (ctx.output & OUTPUT_ADDRESS_FLAGS && !(ctx.output & ~OUTPUT_ADDRESS_FLAGS)))
 	ret = do_search_messages (&ctx);
     else if (ctx.output == OUTPUT_TAGS)
-	ret = do_search_tags (notmuch, &ctx);
+	ret = do_search_tags (&ctx);
     else {
 	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
 	ret = 1;
     }
 
     notmuch_query_destroy (ctx.query);
-    notmuch_database_destroy (notmuch);
+    notmuch_database_destroy (ctx.notmuch);
 
     talloc_free (ctx.format);
 
-- 
2.1.1

[PATCH v2 03/10] cli: search: Convert ctx. to ctx->

[Michal Sojka]

Mostly text replacement.
---
 notmuch-search.c | 81 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 41 insertions(+), 40 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 3d2012b..6765a16 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -474,7 +474,7 @@ do_search_tags (const search_context_t *ctx)
 int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    search_context_t ctx = {
+    search_context_t search_context = {
 	.format_sel = NOTMUCH_FORMAT_TEXT,
 	.exclude = NOTMUCH_EXCLUDE_TRUE,
 	.sort = NOTMUCH_SORT_NEWEST_FIRST,
@@ -483,23 +483,24 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	.limit = -1, /* unlimited */
 	.dupe = -1,
     };
+    search_context_t *ctx = &search_context;
     char *query_str;
     int opt_index, ret;
     unsigned int i;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &ctx.sort, "sort", 's',
+	{ NOTMUCH_OPT_KEYWORD, &ctx->sort, "sort", 's',
 	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
 				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
 				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD, &ctx.format_sel, "format", 'f',
+	{ NOTMUCH_OPT_KEYWORD, &ctx->format_sel, "format", 'f',
 	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
 				  { "sexp", NOTMUCH_FORMAT_SEXP },
 				  { "text", NOTMUCH_FORMAT_TEXT },
 				  { "text0", NOTMUCH_FORMAT_TEXT0 },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
-	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx.output, "output", 'o',
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
@@ -508,15 +509,15 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "files", OUTPUT_FILES },
 				  { "tags", OUTPUT_TAGS },
 				  { 0, 0 } } },
-        { NOTMUCH_OPT_KEYWORD, &ctx.exclude, "exclude", 'x',
+        { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
           (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
                                   { "false", NOTMUCH_EXCLUDE_FALSE },
                                   { "flag", NOTMUCH_EXCLUDE_FLAG },
                                   { "all", NOTMUCH_EXCLUDE_ALL },
                                   { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &ctx.offset, "offset", 'O', 0 },
-	{ NOTMUCH_OPT_INT, &ctx.limit, "limit", 'L', 0  },
-	{ NOTMUCH_OPT_INT, &ctx.dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_INT, &ctx->offset, "offset", 'O', 0 },
+	{ NOTMUCH_OPT_INT, &ctx->limit, "limit", 'L', 0  },
+	{ NOTMUCH_OPT_INT, &ctx->dupe, "duplicate", 'D', 0  },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -524,31 +525,31 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
-    if (! ctx.output)
-	ctx.output = OUTPUT_SUMMARY;
+    if (! ctx->output)
+	ctx->output = OUTPUT_SUMMARY;
 
-    if (ctx.output != OUTPUT_FILES && ctx.output != OUTPUT_MESSAGES &&
-	ctx.dupe != -1) {
+    if (ctx->output != OUTPUT_FILES && ctx->output != OUTPUT_MESSAGES &&
+	ctx->dupe != -1) {
         fprintf (stderr, "Error: --duplicate=N is only supported with --output=files and --output=messages.\n");
         return EXIT_FAILURE;
     }
 
-    switch (ctx.format_sel) {
+    switch (ctx->format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	ctx.format = sprinter_text_create (config, stdout);
+	ctx->format = sprinter_text_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_TEXT0:
-	if (ctx.output == OUTPUT_SUMMARY) {
+	if (ctx->output == OUTPUT_SUMMARY) {
 	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
 	    return EXIT_FAILURE;
 	}
-	ctx.format = sprinter_text0_create (config, stdout);
+	ctx->format = sprinter_text0_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	ctx.format = sprinter_json_create (config, stdout);
+	ctx->format = sprinter_json_create (config, stdout);
 	break;
     case NOTMUCH_FORMAT_SEXP:
-	ctx.format = sprinter_sexp_create (config, stdout);
+	ctx->format = sprinter_sexp_create (config, stdout);
 	break;
     default:
 	/* this should never happen */
@@ -558,10 +559,10 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_exit_if_unsupported_format ();
 
     if (notmuch_database_open (notmuch_config_get_database_path (config),
-			       NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx.notmuch))
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch))
 	return EXIT_FAILURE;
 
-    query_str = query_string_from_args (ctx.notmuch, argc-opt_index, argv+opt_index);
+    query_str = query_string_from_args (ctx->notmuch, argc-opt_index, argv+opt_index);
     if (query_str == NULL) {
 	fprintf (stderr, "Out of memory.\n");
 	return EXIT_FAILURE;
@@ -571,51 +572,51 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    ctx.query = notmuch_query_create (ctx.notmuch, query_str);
-    if (ctx.query == NULL) {
+    ctx->query = notmuch_query_create (ctx->notmuch, query_str);
+    if (ctx->query == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
     }
 
-    notmuch_query_set_sort (ctx.query, ctx.sort);
+    notmuch_query_set_sort (ctx->query, ctx->sort);
 
-    if (ctx.exclude == NOTMUCH_EXCLUDE_FLAG && ctx.output != OUTPUT_SUMMARY) {
+    if (ctx->exclude == NOTMUCH_EXCLUDE_FLAG && ctx->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. */
 	fprintf (stderr, "Warning: this output format cannot flag excluded messages.\n");
-	ctx.exclude = NOTMUCH_EXCLUDE_FALSE;
+	ctx->exclude = NOTMUCH_EXCLUDE_FALSE;
     }
 
-    if (ctx.exclude != NOTMUCH_EXCLUDE_FALSE) {
+    if (ctx->exclude != NOTMUCH_EXCLUDE_FALSE) {
 	const char **search_exclude_tags;
 	size_t search_exclude_tags_length;
 
 	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 (ctx.query, search_exclude_tags[i]);
-	notmuch_query_set_omit_excluded (ctx.query, ctx.exclude);
+	    notmuch_query_add_tag_exclude (ctx->query, search_exclude_tags[i]);
+	notmuch_query_set_omit_excluded (ctx->query, ctx->exclude);
     }
 
-    if (ctx.output == OUTPUT_SUMMARY ||
-	ctx.output == OUTPUT_THREADS)
-	ret = do_search_threads (&ctx);
-    else if (ctx.output == OUTPUT_MESSAGES ||
-	     ctx.output == OUTPUT_FILES ||
-	     (ctx.output & OUTPUT_ADDRESS_FLAGS && !(ctx.output & ~OUTPUT_ADDRESS_FLAGS)))
-	ret = do_search_messages (&ctx);
-    else if (ctx.output == OUTPUT_TAGS)
-	ret = do_search_tags (&ctx);
+    if (ctx->output == OUTPUT_SUMMARY ||
+	ctx->output == OUTPUT_THREADS)
+	ret = do_search_threads (ctx);
+    else if (ctx->output == OUTPUT_MESSAGES ||
+	     ctx->output == OUTPUT_FILES ||
+	     (ctx->output & OUTPUT_ADDRESS_FLAGS && !(ctx->output & ~OUTPUT_ADDRESS_FLAGS)))
+	ret = do_search_messages (ctx);
+    else if (ctx->output == OUTPUT_TAGS)
+	ret = do_search_tags (ctx);
     else {
 	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
 	ret = 1;
     }
 
-    notmuch_query_destroy (ctx.query);
-    notmuch_database_destroy (ctx.notmuch);
+    notmuch_query_destroy (ctx->query);
+    notmuch_database_destroy (ctx->notmuch);
 
-    talloc_free (ctx.format);
+    talloc_free (ctx->format);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.1.1

Re: [PATCH v2 03/10] cli: search: Convert ctx. to ctx->

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> Mostly text replacement.

Here I'd like to know why we are moving from a struct to a pointer to
struct.

It would be a bonus to hightlight any non text-replacement.

[PATCH v2 04/10] cli: search: Split notmuch_search_command to smaller functions

[Michal Sojka]

In the next commit, these functions will be used to share some
functionality between search and address commands.
---
 notmuch-search.c | 155 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 86 insertions(+), 69 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 6765a16..f115359 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -471,6 +471,89 @@ do_search_tags (const search_context_t *ctx)
     return 0;
 }
 
+static int
+_notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int argc, char *argv[])
+{
+    char *query_str;
+    unsigned int i;
+
+    switch (ctx->format_sel) {
+    case NOTMUCH_FORMAT_TEXT:
+	ctx->format = sprinter_text_create (config, stdout);
+	break;
+    case NOTMUCH_FORMAT_TEXT0:
+	if (ctx->output == OUTPUT_SUMMARY) {
+	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
+	    return EXIT_FAILURE;
+	}
+	ctx->format = sprinter_text0_create (config, stdout);
+	break;
+    case NOTMUCH_FORMAT_JSON:
+	ctx->format = sprinter_json_create (config, stdout);
+	break;
+    case NOTMUCH_FORMAT_SEXP:
+	ctx->format = sprinter_sexp_create (config, stdout);
+	break;
+    default:
+	/* this should never happen */
+	INTERNAL_ERROR("no output format selected");
+    }
+
+    notmuch_exit_if_unsupported_format ();
+
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch))
+	return EXIT_FAILURE;
+
+    query_str = query_string_from_args (ctx->notmuch, argc, argv);
+    if (query_str == NULL) {
+	fprintf (stderr, "Out of memory.\n");
+	return EXIT_FAILURE;
+    }
+    if (*query_str == '\0') {
+	fprintf (stderr, "Error: notmuch search requires at least one search term.\n");
+	return EXIT_FAILURE;
+    }
+
+    ctx->query = notmuch_query_create (ctx->notmuch, query_str);
+    if (ctx->query == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return EXIT_FAILURE;
+    }
+
+    notmuch_query_set_sort (ctx->query, ctx->sort);
+
+    if (ctx->exclude == NOTMUCH_EXCLUDE_FLAG && ctx->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. */
+	fprintf (stderr, "Warning: this output format cannot flag excluded messages.\n");
+	ctx->exclude = NOTMUCH_EXCLUDE_FALSE;
+    }
+
+    if (ctx->exclude != NOTMUCH_EXCLUDE_FALSE) {
+	const char **search_exclude_tags;
+	size_t search_exclude_tags_length;
+
+	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 (ctx->query, search_exclude_tags[i]);
+	notmuch_query_set_omit_excluded (ctx->query, ctx->exclude);
+    }
+
+    return 0;
+}
+
+static void
+_notmuch_search_cleanup (search_context_t *ctx)
+{
+    notmuch_query_destroy (ctx->query);
+    notmuch_database_destroy (ctx->notmuch);
+
+    talloc_free (ctx->format);
+}
+
 int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
@@ -484,9 +567,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	.dupe = -1,
     };
     search_context_t *ctx = &search_context;
-    char *query_str;
     int opt_index, ret;
-    unsigned int i;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &ctx->sort, "sort", 's',
@@ -534,71 +615,10 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
         return EXIT_FAILURE;
     }
 
-    switch (ctx->format_sel) {
-    case NOTMUCH_FORMAT_TEXT:
-	ctx->format = sprinter_text_create (config, stdout);
-	break;
-    case NOTMUCH_FORMAT_TEXT0:
-	if (ctx->output == OUTPUT_SUMMARY) {
-	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
-	    return EXIT_FAILURE;
-	}
-	ctx->format = sprinter_text0_create (config, stdout);
-	break;
-    case NOTMUCH_FORMAT_JSON:
-	ctx->format = sprinter_json_create (config, stdout);
-	break;
-    case NOTMUCH_FORMAT_SEXP:
-	ctx->format = sprinter_sexp_create (config, stdout);
-	break;
-    default:
-	/* this should never happen */
-	INTERNAL_ERROR("no output format selected");
-    }
-
-    notmuch_exit_if_unsupported_format ();
-
-    if (notmuch_database_open (notmuch_config_get_database_path (config),
-			       NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch))
+    if (_notmuch_search_prepare (ctx, config,
+				 argc - opt_index, argv + opt_index))
 	return EXIT_FAILURE;
 
-    query_str = query_string_from_args (ctx->notmuch, argc-opt_index, argv+opt_index);
-    if (query_str == NULL) {
-	fprintf (stderr, "Out of memory.\n");
-	return EXIT_FAILURE;
-    }
-    if (*query_str == '\0') {
-	fprintf (stderr, "Error: notmuch search requires at least one search term.\n");
-	return EXIT_FAILURE;
-    }
-
-    ctx->query = notmuch_query_create (ctx->notmuch, query_str);
-    if (ctx->query == NULL) {
-	fprintf (stderr, "Out of memory\n");
-	return EXIT_FAILURE;
-    }
-
-    notmuch_query_set_sort (ctx->query, ctx->sort);
-
-    if (ctx->exclude == NOTMUCH_EXCLUDE_FLAG && ctx->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. */
-	fprintf (stderr, "Warning: this output format cannot flag excluded messages.\n");
-	ctx->exclude = NOTMUCH_EXCLUDE_FALSE;
-    }
-
-    if (ctx->exclude != NOTMUCH_EXCLUDE_FALSE) {
-	const char **search_exclude_tags;
-	size_t search_exclude_tags_length;
-
-	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 (ctx->query, search_exclude_tags[i]);
-	notmuch_query_set_omit_excluded (ctx->query, ctx->exclude);
-    }
-
     if (ctx->output == OUTPUT_SUMMARY ||
 	ctx->output == OUTPUT_THREADS)
 	ret = do_search_threads (ctx);
@@ -613,10 +633,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	ret = 1;
     }
 
-    notmuch_query_destroy (ctx->query);
-    notmuch_database_destroy (ctx->notmuch);
-
-    talloc_free (ctx->format);
+    _notmuch_search_cleanup (ctx);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.1.1

[PATCH v2 05/10] cli: add support for hierarchical command line option arrays

[Michal Sojka]

From: Jani Nikula <jani@nikula.org>

NOTMUCH_OPT_INHERIT expects a notmuch_opt_desc_t * pointer in
output_var.

The "Unrecognized option" message was moved out of parse_option() to
not be emitted twice or when parsing a non-inherited option.
---
 command-line-arguments.c | 16 +++++++++-------
 command-line-arguments.h |  1 +
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index c6f7269..de6b453 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -122,16 +122,18 @@ parse_position_arg (const char *arg_str, int pos_arg_index,
  */
 
 notmuch_bool_t
-parse_option (const char *arg,
-	      const notmuch_opt_desc_t *options) {
-
-    assert(arg);
+parse_option (const char *_arg, const notmuch_opt_desc_t *options)
+{
+    assert(_arg);
     assert(options);
 
-    arg += 2;
-
+    const char *arg = _arg + 2; /* _arg starts with -- */
     const notmuch_opt_desc_t *try;
     for (try = options; try->opt_type != NOTMUCH_OPT_END; try++) {
+	if (try->opt_type == NOTMUCH_OPT_INHERIT &&
+	    parse_option (_arg, try->output_var))
+	    return TRUE;
+
 	if (! try->name)
 	    continue;
 
@@ -170,7 +172,6 @@ parse_option (const char *arg,
 	    /*UNREACHED*/
 	}
     }
-    fprintf (stderr, "Unrecognized option: --%s\n", arg);
     return FALSE;
 }
 
@@ -201,6 +202,7 @@ parse_arguments (int argc, char **argv,
 	    if (more_args) {
 		opt_index++;
 	    } else {
+		fprintf (stderr, "Unrecognized option: %s\n", argv[opt_index]);
 		opt_index = -1;
 	    }
 
diff --git a/command-line-arguments.h b/command-line-arguments.h
index 6444129..309aaf2 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -5,6 +5,7 @@
 
 enum notmuch_opt_type {
     NOTMUCH_OPT_END = 0,
+    NOTMUCH_OPT_INHERIT,	/* another options table */
     NOTMUCH_OPT_BOOLEAN,	/* --verbose              */
     NOTMUCH_OPT_INT,		/* --frob=8               */
     NOTMUCH_OPT_KEYWORD,	/* --format=raw|json|text */
-- 
2.1.1

Re: [PATCH v2 05/10] cli: add support for hierarchical command line option arrays

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> From: Jani Nikula <jani@nikula.org>
>
> NOTMUCH_OPT_INHERIT expects a notmuch_opt_desc_t * pointer in
> output_var.

At the risk of bikeshedding, what about NOTMUCH_OPT_RECURSE instead of
_INHERIT?

Re: [PATCH v2 05/10] cli: add support for hierarchical command line option arrays

[David Bremner]

David Bremner <david@tethera.net> writes:

> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>> From: Jani Nikula <jani@nikula.org>
>>
>> NOTMUCH_OPT_INHERIT expects a notmuch_opt_desc_t * pointer in
>> output_var.
>
> At the risk of bikeshedding, what about NOTMUCH_OPT_RECURSE instead of
> _INHERIT?

Hrm. Now that I read the next patch, I retract my suggestion.

d

[PATCH v2 06/10] cli: Introduce "notmuch address" command

[Michal Sojka]

This moves address-related functionality from search command to the
new address command. The implementation shares almost all code and
some command line options.

Options --offset and --limit were intentionally not included in the
address command, because they refer to messages numbers, which users
do not see in the output. This could confuse users because, for
example, they could see more addresses in the output that what was
specified with --limit. This functionality can be correctly
reimplemented for addresses later.

This was inspired by a patch from Jani Nikula.
---
 completion/notmuch-completion.bash |  42 ++++++++++++++-
 completion/notmuch-completion.zsh  |  10 +++-
 doc/man1/notmuch-address.rst       |  99 ++++++++++++++++++++++++++++++++++++
 doc/man1/notmuch-search.rst        |  20 +-------
 doc/man1/notmuch.rst               |   7 +--
 notmuch-client.h                   |   3 ++
 notmuch-search.c                   | 101 +++++++++++++++++++++++++------------
 notmuch.c                          |   2 +
 8 files changed, 228 insertions(+), 56 deletions(-)
 create mode 100644 doc/man1/notmuch-address.rst

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index cfbd389..94ea2d5 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 sender recipients" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
 	    return
 	    ;;
 	--sort)
@@ -320,6 +320,44 @@ _notmuch_search()
     esac
 }
 
+_notmuch_address()
+{
+    local cur prev words cword split
+    _init_completion -s || return
+
+    $split &&
+    case "${prev}" in
+	--format)
+	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
+	    return
+	    ;;
+	--output)
+	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
+	    return
+	    ;;
+	--sort)
+	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
+	    return
+	    ;;
+	--exclude)
+	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
+	    return
+	    ;;
+    esac
+
+    ! $split &&
+    case "${cur}" in
+	-*)
+	    local options="--format= --output= --sort= --exclude="
+	    compopt -o nospace
+	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
+	    ;;
+	*)
+	    _notmuch_search_terms
+	    ;;
+    esac
+}
+
 _notmuch_show()
 {
     local cur prev words cword split
@@ -393,7 +431,7 @@ _notmuch_tag()
 
 _notmuch()
 {
-    local _notmuch_commands="compact config count dump help insert new reply restore search setup show tag"
+    local _notmuch_commands="compact config count dump help insert new reply restore search address setup show tag"
     local arg cur prev words cword split
 
     # require bash-completion with _init_completion
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index 3e52a00..c606b75 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -10,6 +10,7 @@ _notmuch_commands()
     'setup:interactively set up notmuch for first use'
     'new:find and import any new message to the database'
     'search:search for messages matching the search terms, display matching threads as results'
+    'address:get addresses from messages matching the given search terms'
     'reply:constructs a reply template for a set of messages'
     'show:show all messages matching the search terms'
     'tag:add or remove tags for all messages matching the search terms'
@@ -53,7 +54,14 @@ _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))'
+}
+
+_notmuch_address()
+{
+  _arguments -s : \
+    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
+    '--output=[select what to output]:output:((sender recipients))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
new file mode 100644
index 0000000..8109f11
--- /dev/null
+++ b/doc/man1/notmuch-address.rst
@@ -0,0 +1,99 @@
+===============
+notmuch-address
+===============
+
+SYNOPSIS
+========
+
+**notmuch** **address** [*option* ...] <*search-term*> ...
+
+DESCRIPTION
+===========
+
+Search for messages matching the given search terms, and display the
+addresses from them.
+
+See **notmuch-search-terms(7)** for details of the supported syntax for
+<search-terms>.
+
+Supported options for **address** include
+
+    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
+        Presents the results in either JSON, S-Expressions, newline
+        character separated plain-text (default), or null character
+        separated plain-text (compatible with **xargs(1)** -0 option
+        where available).
+
+    ``--format-version=N``
+        Use the specified structured output format version. This is
+        intended for programs that invoke **notmuch(1)** internally. If
+        omitted, the latest supported version will be used.
+
+    ``--output=(sender|recipients)``
+
+        Controls which information appears in the output. This option
+	can be given multiple times to combine different outputs.
+	Omitting this option is equivalent to
+	--output=sender --output=recipients.
+
+	**sender**
+            Output all addresses from the *From* header.
+
+	    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**
+            Output all addresses from the *To*, *Cc* and *Bcc*
+            headers.
+
+    ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
+        This option can be used to present results in either
+        chronological order (**oldest-first**) or reverse chronological
+        order (**newest-first**).
+
+        By default, results will be displayed in reverse chronological
+        order, (that is, the newest results will be displayed first).
+
+    ``--exclude=(true|false|all|flag)``
+        A message is called "excluded" if it matches at least one tag in
+        search.tag\_exclude that does not appear explicitly in the
+        search terms. This option specifies whether to omit excluded
+        messages in the search process.
+
+        The default value, **true**, prevents excluded messages from
+        matching the search terms.
+
+        **all** additionally prevents excluded messages from appearing
+        in displayed results, in effect behaving as though the excluded
+        messages do not exist.
+
+        **false** allows excluded messages to match search terms and
+        appear in displayed results. Excluded messages are still marked
+        in the relevant outputs.
+
+        **flag** only has an effect when ``--output=summary``. The
+        output is almost identical to **false**, but the "match count"
+        is the number of matching non-excluded messages in the thread,
+        rather than the number of matching messages.
+
+EXIT STATUS
+===========
+
+This command supports the following special exit status codes
+
+``20``
+    The requested format version is too old.
+
+``21``
+    The requested format version is too new.
+
+SEE ALSO
+========
+
+**notmuch(1)**, **notmuch-config(1)**, **notmuch-count(1)**,
+**notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
+**notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
+**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
+***notmuch-search(1)**
diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index 8110086..65df288 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -78,25 +78,8 @@ 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. Currently, this is only supported for **sender** and
-	**recipients** outputs.
+	outputs.
 
     ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
         This option can be used to present results in either
@@ -173,3 +156,4 @@ SEE ALSO
 **notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
 **notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
 **notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
+***notmuch-address(1)**
diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst
index 9710294..98590a4 100644
--- a/doc/man1/notmuch.rst
+++ b/doc/man1/notmuch.rst
@@ -88,8 +88,8 @@ Several of the notmuch commands accept search terms with a common
 syntax. See **notmuch-search-terms**\ (7) for more details on the
 supported syntax.
 
-The **search**, **show** and **count** commands are used to query the
-email database.
+The **search**, **show**, **address** and **count** commands are used
+to query the email database.
 
 The **reply** command is useful for preparing a template for an email
 reply.
@@ -128,7 +128,8 @@ SEE ALSO
 **notmuch-config(1)**, **notmuch-count(1)**, **notmuch-dump(1)**,
 **notmuch-hooks(5)**, **notmuch-insert(1)**, **notmuch-new(1)**,
 **notmuch-reply(1)**, **notmuch-restore(1)**, **notmuch-search(1)**,
-**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
+**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
+***notmuch-address(1)**
 
 The notmuch website: **http://notmuchmail.org**
 
diff --git a/notmuch-client.h b/notmuch-client.h
index e1efbe0..5e0d475 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -199,6 +199,9 @@ int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
+notmuch_address_command (notmuch_config_t *config, int argc, char *argv[]);
+
+int
 notmuch_setup_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
diff --git a/notmuch-search.c b/notmuch-search.c
index f115359..cbd84f5 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -23,17 +23,18 @@
 #include "string-util.h"
 
 typedef enum {
+    /* Search command */
     OUTPUT_SUMMARY	= 1 << 0,
     OUTPUT_THREADS	= 1 << 1,
     OUTPUT_MESSAGES	= 1 << 2,
     OUTPUT_FILES	= 1 << 3,
     OUTPUT_TAGS		= 1 << 4,
+
+    /* Address command */
     OUTPUT_SENDER	= 1 << 5,
     OUTPUT_RECIPIENTS	= 1 << 6,
 } output_t;
 
-#define OUTPUT_ADDRESS_FLAGS (OUTPUT_SENDER | OUTPUT_RECIPIENTS)
-
 typedef enum {
     NOTMUCH_FORMAT_JSON,
     NOTMUCH_FORMAT_TEXT,
@@ -554,51 +555,55 @@ _notmuch_search_cleanup (search_context_t *ctx)
     talloc_free (ctx->format);
 }
 
+static search_context_t search_context = {
+    .format_sel = NOTMUCH_FORMAT_TEXT,
+    .exclude = NOTMUCH_EXCLUDE_TRUE,
+    .sort = NOTMUCH_SORT_NEWEST_FIRST,
+    .output = 0,
+    .offset = 0,
+    .limit = -1, /* unlimited */
+    .dupe = -1,
+};
+
+static const notmuch_opt_desc_t common_options[] = {
+    { NOTMUCH_OPT_KEYWORD, &search_context.sort, "sort", 's',
+      (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
+			      { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
+			      { 0, 0 } } },
+    { NOTMUCH_OPT_KEYWORD, &search_context.format_sel, "format", 'f',
+      (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
+			      { "sexp", NOTMUCH_FORMAT_SEXP },
+			      { "text", NOTMUCH_FORMAT_TEXT },
+			      { "text0", NOTMUCH_FORMAT_TEXT0 },
+			      { 0, 0 } } },
+    { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
+    { NOTMUCH_OPT_KEYWORD, &search_context.exclude, "exclude", 'x',
+      (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
+			      { "false", NOTMUCH_EXCLUDE_FALSE },
+			      { "flag", NOTMUCH_EXCLUDE_FLAG },
+			      { "all", NOTMUCH_EXCLUDE_ALL },
+			      { 0, 0 } } },
+    { 0, 0, 0, 0, 0 }
+};
+
 int
 notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 {
-    search_context_t search_context = {
-	.format_sel = NOTMUCH_FORMAT_TEXT,
-	.exclude = NOTMUCH_EXCLUDE_TRUE,
-	.sort = NOTMUCH_SORT_NEWEST_FIRST,
-	.output = 0,
-	.offset = 0,
-	.limit = -1, /* unlimited */
-	.dupe = -1,
-    };
     search_context_t *ctx = &search_context;
     int opt_index, ret;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD, &ctx->sort, "sort", 's',
-	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
-				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
-				  { 0, 0 } } },
-	{ NOTMUCH_OPT_KEYWORD, &ctx->format_sel, "format", 'f',
-	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
-				  { "sexp", NOTMUCH_FORMAT_SEXP },
-				  { "text", NOTMUCH_FORMAT_TEXT },
-				  { "text0", NOTMUCH_FORMAT_TEXT0 },
-				  { 0, 0 } } },
-	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
 	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->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 } } },
-        { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
-          (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
-                                  { "false", NOTMUCH_EXCLUDE_FALSE },
-                                  { "flag", NOTMUCH_EXCLUDE_FLAG },
-                                  { "all", NOTMUCH_EXCLUDE_ALL },
-                                  { 0, 0 } } },
 	{ NOTMUCH_OPT_INT, &ctx->offset, "offset", 'O', 0 },
 	{ NOTMUCH_OPT_INT, &ctx->limit, "limit", 'L', 0  },
 	{ NOTMUCH_OPT_INT, &ctx->dupe, "duplicate", 'D', 0  },
+	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -623,8 +628,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	ctx->output == OUTPUT_THREADS)
 	ret = do_search_threads (ctx);
     else if (ctx->output == OUTPUT_MESSAGES ||
-	     ctx->output == OUTPUT_FILES ||
-	     (ctx->output & OUTPUT_ADDRESS_FLAGS && !(ctx->output & ~OUTPUT_ADDRESS_FLAGS)))
+	     ctx->output == OUTPUT_FILES)
 	ret = do_search_messages (ctx);
     else if (ctx->output == OUTPUT_TAGS)
 	ret = do_search_tags (ctx);
@@ -637,3 +641,36 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
 }
+
+int
+notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
+{
+    search_context_t *ctx = &search_context;
+    int opt_index, ret;
+
+    notmuch_opt_desc_t options[] = {
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
+	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
+				  { "recipients", OUTPUT_RECIPIENTS },
+				  { 0, 0 } } },
+	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
+	{ 0, 0, 0, 0, 0 }
+    };
+
+    opt_index = parse_arguments (argc, argv, options, 1);
+    if (opt_index < 0)
+	return EXIT_FAILURE;
+
+    if (! ctx->output)
+	search_context.output = OUTPUT_SENDER | OUTPUT_RECIPIENTS;
+
+    if (_notmuch_search_prepare (ctx, config,
+				 argc - opt_index, argv + opt_index))
+	return EXIT_FAILURE;
+
+    ret = do_search_messages (ctx);
+
+    _notmuch_search_cleanup (ctx);
+
+    return ret ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/notmuch.c b/notmuch.c
index dcda039..0fac099 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -54,6 +54,8 @@ static command_t commands[] = {
       "Add a new message into the maildir and notmuch database." },
     { "search", notmuch_search_command, FALSE,
       "Search for messages matching the given search terms." },
+    { "address", notmuch_address_command, FALSE,
+      "Get addresses from messages matching the given search terms." },
     { "show", notmuch_show_command, FALSE,
       "Show all messages matching the search terms." },
     { "count", notmuch_count_command, FALSE,
-- 
2.1.1

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> +{
> +    local cur prev words cword split
> +    _init_completion -s || return
> +
> +    $split &&
> +    case "${prev}" in
> +	--format)
> +	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
> +	    return
> +	    ;;
> +	--output)
> +	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
> +	    return
> +	    ;;
> +	--sort)
> +	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
> +	    return
> +	    ;;
> +	--exclude)
> +	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
> +	    return
> +	    ;;
> +    esac
> +
> +    ! $split &&
> +    case "${cur}" in
> +	-*)
> +	    local options="--format= --output= --sort= --exclude="
> +	    compopt -o nospace
> +	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
> +	    ;;
> +	*)
> +	    _notmuch_search_terms
> +	    ;;
> +    esac
> +}
> +

I am reminded that we have no tests for shell completion stuff, which
seems pretty fragile.

> +
> +    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
> +        Presents the results in either JSON, S-Expressions, newline
> +        character separated plain-text (default), or null character
> +        separated plain-text (compatible with **xargs(1)** -0 option
> +        where available).
> +
> +    ``--format-version=N``
> +        Use the specified structured output format version. This is
> +        intended for programs that invoke **notmuch(1)** internally. If
> +        omitted, the latest supported version will be used.
> +


I wonder if at some point we should have a notmuch-output-formats.7 page.


> +    ``--exclude=(true|false|all|flag)``
> +        A message is called "excluded" if it matches at least one tag in
> +        search.tag\_exclude that does not appear explicitly in the
> +        search terms. This option specifies whether to omit excluded
> +        messages in the search process.

Similarly for excludes.  I'm ok with the duplication for now, and I can
see an argument for not making the user chase references.

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[Tomi Ollila]

On Tue, Nov 04 2014, David Bremner <david@tethera.net> wrote:

> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>> +{
>> +    local cur prev words cword split
>> +    _init_completion -s || return
>> +
>> +    $split &&
>> +    case "${prev}" in
>> +	--format)
>> +	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--output)
>> +	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--sort)
>> +	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--exclude)
>> +	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +    esac
>> +
>> +    ! $split &&
>> +    case "${cur}" in
>> +	-*)
>> +	    local options="--format= --output= --sort= --exclude="
>> +	    compopt -o nospace
>> +	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
>> +	    ;;
>> +	*)
>> +	    _notmuch_search_terms
>> +	    ;;
>> +    esac
>> +}
>> +
>
> I am reminded that we have no tests for shell completion stuff, which
> seems pretty fragile.

That is what users are for -- i.e. the completion testing monkies ! ;D
>
>> +
>> +    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
>> +        Presents the results in either JSON, S-Expressions, newline
>> +        character separated plain-text (default), or null character
>> +        separated plain-text (compatible with **xargs(1)** -0 option
>> +        where available).
>> +
>> +    ``--format-version=N``
>> +        Use the specified structured output format version. This is
>> +        intended for programs that invoke **notmuch(1)** internally. If
>> +        omitted, the latest supported version will be used.
>> +
>
>
> I wonder if at some point we should have a notmuch-output-formats.7 page.
>
>
>> +    ``--exclude=(true|false|all|flag)``
>> +        A message is called "excluded" if it matches at least one tag in
>> +        search.tag\_exclude that does not appear explicitly in the
>> +        search terms. This option specifies whether to omit excluded
>> +        messages in the search process.
>
> Similarly for excludes.  I'm ok with the duplication for now, and I can
> see an argument for not making the user chase references.
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[Michal Sojka]

On Tue, Nov 04 2014, David Bremner wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>> +{
>> +    local cur prev words cword split
>> +    _init_completion -s || return
>> +
>> +    $split &&
>> +    case "${prev}" in
>> +	--format)
>> +	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--output)
>> +	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--sort)
>> +	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--exclude)
>> +	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +    esac
>> +
>> +    ! $split &&
>> +    case "${cur}" in
>> +	-*)
>> +	    local options="--format= --output= --sort= --exclude="
>> +	    compopt -o nospace
>> +	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
>> +	    ;;
>> +	*)
>> +	    _notmuch_search_terms
>> +	    ;;
>> +    esac
>> +}
>> +
>
> I am reminded that we have no tests for shell completion stuff, which
> seems pretty fragile.
>
>> +
>> +    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
>> +        Presents the results in either JSON, S-Expressions, newline
>> +        character separated plain-text (default), or null character
>> +        separated plain-text (compatible with **xargs(1)** -0 option
>> +        where available).
>> +
>> +    ``--format-version=N``
>> +        Use the specified structured output format version. This is
>> +        intended for programs that invoke **notmuch(1)** internally. If
>> +        omitted, the latest supported version will be used.
>> +
>
>
> I wonder if at some point we should have a notmuch-output-formats.7 page.
>
>
>> +    ``--exclude=(true|false|all|flag)``
>> +        A message is called "excluded" if it matches at least one tag in
>> +        search.tag\_exclude that does not appear explicitly in the
>> +        search terms. This option specifies whether to omit excluded
>> +        messages in the search process.
>
> Similarly for excludes.  I'm ok with the duplication for now, and I can
> see an argument for not making the user chase references.

What about using RST include directive [1] to include shared parts into
more documents?

[1] http://docutils.sourceforge.net/docs/ref/rst/directives.html#including-an-external-document-fragment

-Michal

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

>> Similarly for excludes.  I'm ok with the duplication for now, and I can
>> see an argument for not making the user chase references.
>
> What about using RST include directive [1] to include shared parts into
> more documents?
>
> [1] http://docutils.sourceforge.net/docs/ref/rst/directives.html#including-an-external-document-fragment

Probably a good idea, but let's leave it until after 0.19 to deal with
the ensuing storm of "This doesn't work on my PDP-11/70 running RSX/11"
type complaints.

d

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[Mark Walters]

On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> This moves address-related functionality from search command to the
> new address command. The implementation shares almost all code and
> some command line options.
>
> Options --offset and --limit were intentionally not included in the
> address command, because they refer to messages numbers, which users
> do not see in the output. This could confuse users because, for
> example, they could see more addresses in the output that what was
> specified with --limit. This functionality can be correctly
> reimplemented for addresses later.

I am not sure about this: we already have this anomaly for output=files
say. Also I can imagine calling notmuch address --limit=1000 ... to get
a bunch of recent addresses quickly and I really am wanting to look at
1000 messages, not collect 1000 addresses.

Additionally, the 1000 message approach makes sense when we start
deduping whereas 1000 authors becomes unclear.

>
> This was inspired by a patch from Jani Nikula.
> ---
>  completion/notmuch-completion.bash |  42 ++++++++++++++-
>  completion/notmuch-completion.zsh  |  10 +++-
>  doc/man1/notmuch-address.rst       |  99 ++++++++++++++++++++++++++++++++++++
>  doc/man1/notmuch-search.rst        |  20 +-------
>  doc/man1/notmuch.rst               |   7 +--
>  notmuch-client.h                   |   3 ++
>  notmuch-search.c                   | 101 +++++++++++++++++++++++++------------
>  notmuch.c                          |   2 +
>  8 files changed, 228 insertions(+), 56 deletions(-)
>  create mode 100644 doc/man1/notmuch-address.rst
>
> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
> index cfbd389..94ea2d5 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 sender recipients" -- "${cur}" ) )
> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
>  	    return
>  	    ;;
>  	--sort)
> @@ -320,6 +320,44 @@ _notmuch_search()
>      esac
>  }
>  
> +_notmuch_address()
> +{
> +    local cur prev words cword split
> +    _init_completion -s || return
> +
> +    $split &&
> +    case "${prev}" in
> +	--format)
> +	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
> +	    return
> +	    ;;
> +	--output)
> +	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
> +	    return
> +	    ;;
> +	--sort)
> +	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
> +	    return
> +	    ;;
> +	--exclude)
> +	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
> +	    return
> +	    ;;
> +    esac
> +
> +    ! $split &&
> +    case "${cur}" in
> +	-*)
> +	    local options="--format= --output= --sort= --exclude="
> +	    compopt -o nospace
> +	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
> +	    ;;
> +	*)
> +	    _notmuch_search_terms
> +	    ;;
> +    esac
> +}
> +
>  _notmuch_show()
>  {
>      local cur prev words cword split
> @@ -393,7 +431,7 @@ _notmuch_tag()
>  
>  _notmuch()
>  {
> -    local _notmuch_commands="compact config count dump help insert new reply restore search setup show tag"
> +    local _notmuch_commands="compact config count dump help insert new reply restore search address setup show tag"
>      local arg cur prev words cword split
>  
>      # require bash-completion with _init_completion
> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
> index 3e52a00..c606b75 100644
> --- a/completion/notmuch-completion.zsh
> +++ b/completion/notmuch-completion.zsh
> @@ -10,6 +10,7 @@ _notmuch_commands()
>      'setup:interactively set up notmuch for first use'
>      'new:find and import any new message to the database'
>      'search:search for messages matching the search terms, display matching threads as results'
> +    'address:get addresses from messages matching the given search terms'
>      'reply:constructs a reply template for a set of messages'
>      'show:show all messages matching the search terms'
>      'tag:add or remove tags for all messages matching the search terms'
> @@ -53,7 +54,14 @@ _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))'
> +}
> +
> +_notmuch_address()
> +{
> +  _arguments -s : \
> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
> +    '--output=[select what to output]:output:((sender recipients))'
>  }
>  
>  _notmuch()
> diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
> new file mode 100644
> index 0000000..8109f11
> --- /dev/null
> +++ b/doc/man1/notmuch-address.rst
> @@ -0,0 +1,99 @@
> +===============
> +notmuch-address
> +===============
> +
> +SYNOPSIS
> +========
> +
> +**notmuch** **address** [*option* ...] <*search-term*> ...
> +
> +DESCRIPTION
> +===========
> +
> +Search for messages matching the given search terms, and display the
> +addresses from them.
> +
> +See **notmuch-search-terms(7)** for details of the supported syntax for
> +<search-terms>.
> +
> +Supported options for **address** include
> +
> +    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
> +        Presents the results in either JSON, S-Expressions, newline
> +        character separated plain-text (default), or null character
> +        separated plain-text (compatible with **xargs(1)** -0 option
> +        where available).
> +
> +    ``--format-version=N``
> +        Use the specified structured output format version. This is
> +        intended for programs that invoke **notmuch(1)** internally. If
> +        omitted, the latest supported version will be used.
> +
> +    ``--output=(sender|recipients)``
> +
> +        Controls which information appears in the output. This option
> +	can be given multiple times to combine different outputs.
> +	Omitting this option is equivalent to
> +	--output=sender --output=recipients.
> +
> +	**sender**
> +            Output all addresses from the *From* header.
> +
> +	    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**
> +            Output all addresses from the *To*, *Cc* and *Bcc*
> +            headers.
> +
> +    ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
> +        This option can be used to present results in either
> +        chronological order (**oldest-first**) or reverse chronological
> +        order (**newest-first**).
> +
> +        By default, results will be displayed in reverse chronological
> +        order, (that is, the newest results will be displayed first).
> +
> +    ``--exclude=(true|false|all|flag)``
> +        A message is called "excluded" if it matches at least one tag in
> +        search.tag\_exclude that does not appear explicitly in the
> +        search terms. This option specifies whether to omit excluded
> +        messages in the search process.
> +
> +        The default value, **true**, prevents excluded messages from
> +        matching the search terms.
> +
> +        **all** additionally prevents excluded messages from appearing
> +        in displayed results, in effect behaving as though the excluded
> +        messages do not exist.
> +
> +        **false** allows excluded messages to match search terms and
> +        appear in displayed results. Excluded messages are still marked
> +        in the relevant outputs.
> +
> +        **flag** only has an effect when ``--output=summary``. The
> +        output is almost identical to **false**, but the "match count"
> +        is the number of matching non-excluded messages in the thread,
> +        rather than the number of matching messages.

The exclude text needs updating as flag makes no sense for the address
command. 

Best wishes 

Mark

> +EXIT STATUS
> +===========
> +
> +This command supports the following special exit status codes
> +
> +``20``
> +    The requested format version is too old.
> +
> +``21``
> +    The requested format version is too new.
> +
> +SEE ALSO
> +========
> +
> +**notmuch(1)**, **notmuch-config(1)**, **notmuch-count(1)**,
> +**notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
> +**notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
> +***notmuch-search(1)**
> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
> index 8110086..65df288 100644
> --- a/doc/man1/notmuch-search.rst
> +++ b/doc/man1/notmuch-search.rst
> @@ -78,25 +78,8 @@ 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. Currently, this is only supported for **sender** and
> -	**recipients** outputs.
> +	outputs.
>  
>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>          This option can be used to present results in either
> @@ -173,3 +156,4 @@ SEE ALSO
>  **notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
>  **notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
>  **notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
> +***notmuch-address(1)**
> diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst
> index 9710294..98590a4 100644
> --- a/doc/man1/notmuch.rst
> +++ b/doc/man1/notmuch.rst
> @@ -88,8 +88,8 @@ Several of the notmuch commands accept search terms with a common
>  syntax. See **notmuch-search-terms**\ (7) for more details on the
>  supported syntax.
>  
> -The **search**, **show** and **count** commands are used to query the
> -email database.
> +The **search**, **show**, **address** and **count** commands are used
> +to query the email database.
>  
>  The **reply** command is useful for preparing a template for an email
>  reply.
> @@ -128,7 +128,8 @@ SEE ALSO
>  **notmuch-config(1)**, **notmuch-count(1)**, **notmuch-dump(1)**,
>  **notmuch-hooks(5)**, **notmuch-insert(1)**, **notmuch-new(1)**,
>  **notmuch-reply(1)**, **notmuch-restore(1)**, **notmuch-search(1)**,
> -**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
> +***notmuch-address(1)**
>  
>  The notmuch website: **http://notmuchmail.org**
>  
> diff --git a/notmuch-client.h b/notmuch-client.h
> index e1efbe0..5e0d475 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -199,6 +199,9 @@ int
>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]);
>  
>  int
> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[]);
> +
> +int
>  notmuch_setup_command (notmuch_config_t *config, int argc, char *argv[]);
>  
>  int
> diff --git a/notmuch-search.c b/notmuch-search.c
> index f115359..cbd84f5 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -23,17 +23,18 @@
>  #include "string-util.h"
>  
>  typedef enum {
> +    /* Search command */
>      OUTPUT_SUMMARY	= 1 << 0,
>      OUTPUT_THREADS	= 1 << 1,
>      OUTPUT_MESSAGES	= 1 << 2,
>      OUTPUT_FILES	= 1 << 3,
>      OUTPUT_TAGS		= 1 << 4,
> +
> +    /* Address command */
>      OUTPUT_SENDER	= 1 << 5,
>      OUTPUT_RECIPIENTS	= 1 << 6,
>  } output_t;
>  
> -#define OUTPUT_ADDRESS_FLAGS (OUTPUT_SENDER | OUTPUT_RECIPIENTS)
> -
>  typedef enum {
>      NOTMUCH_FORMAT_JSON,
>      NOTMUCH_FORMAT_TEXT,
> @@ -554,51 +555,55 @@ _notmuch_search_cleanup (search_context_t *ctx)
>      talloc_free (ctx->format);
>  }
>  
> +static search_context_t search_context = {
> +    .format_sel = NOTMUCH_FORMAT_TEXT,
> +    .exclude = NOTMUCH_EXCLUDE_TRUE,
> +    .sort = NOTMUCH_SORT_NEWEST_FIRST,
> +    .output = 0,
> +    .offset = 0,
> +    .limit = -1, /* unlimited */
> +    .dupe = -1,
> +};
> +
> +static const notmuch_opt_desc_t common_options[] = {
> +    { NOTMUCH_OPT_KEYWORD, &search_context.sort, "sort", 's',
> +      (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
> +			      { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
> +			      { 0, 0 } } },
> +    { NOTMUCH_OPT_KEYWORD, &search_context.format_sel, "format", 'f',
> +      (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
> +			      { "sexp", NOTMUCH_FORMAT_SEXP },
> +			      { "text", NOTMUCH_FORMAT_TEXT },
> +			      { "text0", NOTMUCH_FORMAT_TEXT0 },
> +			      { 0, 0 } } },
> +    { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
> +    { NOTMUCH_OPT_KEYWORD, &search_context.exclude, "exclude", 'x',
> +      (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
> +			      { "false", NOTMUCH_EXCLUDE_FALSE },
> +			      { "flag", NOTMUCH_EXCLUDE_FLAG },
> +			      { "all", NOTMUCH_EXCLUDE_ALL },
> +			      { 0, 0 } } },
> +    { 0, 0, 0, 0, 0 }
> +};
> +
>  int
>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  {
> -    search_context_t search_context = {
> -	.format_sel = NOTMUCH_FORMAT_TEXT,
> -	.exclude = NOTMUCH_EXCLUDE_TRUE,
> -	.sort = NOTMUCH_SORT_NEWEST_FIRST,
> -	.output = 0,
> -	.offset = 0,
> -	.limit = -1, /* unlimited */
> -	.dupe = -1,
> -    };
>      search_context_t *ctx = &search_context;
>      int opt_index, ret;
>  
>      notmuch_opt_desc_t options[] = {
> -	{ NOTMUCH_OPT_KEYWORD, &ctx->sort, "sort", 's',
> -	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
> -				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
> -				  { 0, 0 } } },
> -	{ NOTMUCH_OPT_KEYWORD, &ctx->format_sel, "format", 'f',
> -	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
> -				  { "sexp", NOTMUCH_FORMAT_SEXP },
> -				  { "text", NOTMUCH_FORMAT_TEXT },
> -				  { "text0", NOTMUCH_FORMAT_TEXT0 },
> -				  { 0, 0 } } },
> -	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>  	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->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 } } },
> -        { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
> -          (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
> -                                  { "false", NOTMUCH_EXCLUDE_FALSE },
> -                                  { "flag", NOTMUCH_EXCLUDE_FLAG },
> -                                  { "all", NOTMUCH_EXCLUDE_ALL },
> -                                  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &ctx->offset, "offset", 'O', 0 },
>  	{ NOTMUCH_OPT_INT, &ctx->limit, "limit", 'L', 0  },
>  	{ NOTMUCH_OPT_INT, &ctx->dupe, "duplicate", 'D', 0  },
> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -623,8 +628,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  	ctx->output == OUTPUT_THREADS)
>  	ret = do_search_threads (ctx);
>      else if (ctx->output == OUTPUT_MESSAGES ||
> -	     ctx->output == OUTPUT_FILES ||
> -	     (ctx->output & OUTPUT_ADDRESS_FLAGS && !(ctx->output & ~OUTPUT_ADDRESS_FLAGS)))
> +	     ctx->output == OUTPUT_FILES)
>  	ret = do_search_messages (ctx);
>      else if (ctx->output == OUTPUT_TAGS)
>  	ret = do_search_tags (ctx);
> @@ -637,3 +641,36 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  
>      return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>  }
> +
> +int
> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
> +{
> +    search_context_t *ctx = &search_context;
> +    int opt_index, ret;
> +
> +    notmuch_opt_desc_t options[] = {
> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
> +	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
> +				  { "recipients", OUTPUT_RECIPIENTS },
> +				  { 0, 0 } } },
> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
> +	{ 0, 0, 0, 0, 0 }
> +    };
> +
> +    opt_index = parse_arguments (argc, argv, options, 1);
> +    if (opt_index < 0)
> +	return EXIT_FAILURE;
> +
> +    if (! ctx->output)
> +	search_context.output = OUTPUT_SENDER | OUTPUT_RECIPIENTS;
> +
> +    if (_notmuch_search_prepare (ctx, config,
> +				 argc - opt_index, argv + opt_index))
> +	return EXIT_FAILURE;
> +
> +    ret = do_search_messages (ctx);
> +
> +    _notmuch_search_cleanup (ctx);
> +
> +    return ret ? EXIT_FAILURE : EXIT_SUCCESS;
> +}
> diff --git a/notmuch.c b/notmuch.c
> index dcda039..0fac099 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -54,6 +54,8 @@ static command_t commands[] = {
>        "Add a new message into the maildir and notmuch database." },
>      { "search", notmuch_search_command, FALSE,
>        "Search for messages matching the given search terms." },
> +    { "address", notmuch_address_command, FALSE,
> +      "Get addresses from messages matching the given search terms." },
>      { "show", notmuch_show_command, FALSE,
>        "Show all messages matching the search terms." },
>      { "count", notmuch_count_command, FALSE,
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[Michal Sojka]

On Tue, Nov 04 2014, Mark Walters wrote:
> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> This moves address-related functionality from search command to the
>> new address command. The implementation shares almost all code and
>> some command line options.
>>
>> Options --offset and --limit were intentionally not included in the
>> address command, because they refer to messages numbers, which users
>> do not see in the output. This could confuse users because, for
>> example, they could see more addresses in the output that what was
>> specified with --limit. This functionality can be correctly
>> reimplemented for addresses later.
>
> I am not sure about this: we already have this anomaly for output=files
> say. Also I can imagine calling notmuch address --limit=1000 ... to get
> a bunch of recent addresses quickly and I really am wanting to look at
> 1000 messages, not collect 1000 addresses.

I think that one of the reasons for having the new "address" command is
to have cleaner user interface. And including "anomalies" doesn't sound
like a way to achieve this. I think that now you can use "date:" query
to limit the search.

I volunteer to implement "address --limit" properly after 0.19. This
should be easy.

-Michal

> Additionally, the 1000 message approach makes sense when we start
> deduping whereas 1000 authors becomes unclear.
>
>>
>> This was inspired by a patch from Jani Nikula.
>> ---
>>  completion/notmuch-completion.bash |  42 ++++++++++++++-
>>  completion/notmuch-completion.zsh  |  10 +++-
>>  doc/man1/notmuch-address.rst       |  99 ++++++++++++++++++++++++++++++++++++
>>  doc/man1/notmuch-search.rst        |  20 +-------
>>  doc/man1/notmuch.rst               |   7 +--
>>  notmuch-client.h                   |   3 ++
>>  notmuch-search.c                   | 101 +++++++++++++++++++++++++------------
>>  notmuch.c                          |   2 +
>>  8 files changed, 228 insertions(+), 56 deletions(-)
>>  create mode 100644 doc/man1/notmuch-address.rst
>>
>> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
>> index cfbd389..94ea2d5 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 sender recipients" -- "${cur}" ) )
>> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
>>  	    return
>>  	    ;;
>>  	--sort)
>> @@ -320,6 +320,44 @@ _notmuch_search()
>>      esac
>>  }
>>  
>> +_notmuch_address()
>> +{
>> +    local cur prev words cword split
>> +    _init_completion -s || return
>> +
>> +    $split &&
>> +    case "${prev}" in
>> +	--format)
>> +	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--output)
>> +	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--sort)
>> +	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--exclude)
>> +	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +    esac
>> +
>> +    ! $split &&
>> +    case "${cur}" in
>> +	-*)
>> +	    local options="--format= --output= --sort= --exclude="
>> +	    compopt -o nospace
>> +	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
>> +	    ;;
>> +	*)
>> +	    _notmuch_search_terms
>> +	    ;;
>> +    esac
>> +}
>> +
>>  _notmuch_show()
>>  {
>>      local cur prev words cword split
>> @@ -393,7 +431,7 @@ _notmuch_tag()
>>  
>>  _notmuch()
>>  {
>> -    local _notmuch_commands="compact config count dump help insert new reply restore search setup show tag"
>> +    local _notmuch_commands="compact config count dump help insert new reply restore search address setup show tag"
>>      local arg cur prev words cword split
>>  
>>      # require bash-completion with _init_completion
>> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
>> index 3e52a00..c606b75 100644
>> --- a/completion/notmuch-completion.zsh
>> +++ b/completion/notmuch-completion.zsh
>> @@ -10,6 +10,7 @@ _notmuch_commands()
>>      'setup:interactively set up notmuch for first use'
>>      'new:find and import any new message to the database'
>>      'search:search for messages matching the search terms, display matching threads as results'
>> +    'address:get addresses from messages matching the given search terms'
>>      'reply:constructs a reply template for a set of messages'
>>      'show:show all messages matching the search terms'
>>      'tag:add or remove tags for all messages matching the search terms'
>> @@ -53,7 +54,14 @@ _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))'
>> +}
>> +
>> +_notmuch_address()
>> +{
>> +  _arguments -s : \
>> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
>> +    '--output=[select what to output]:output:((sender recipients))'
>>  }
>>  
>>  _notmuch()
>> diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
>> new file mode 100644
>> index 0000000..8109f11
>> --- /dev/null
>> +++ b/doc/man1/notmuch-address.rst
>> @@ -0,0 +1,99 @@
>> +===============
>> +notmuch-address
>> +===============
>> +
>> +SYNOPSIS
>> +========
>> +
>> +**notmuch** **address** [*option* ...] <*search-term*> ...
>> +
>> +DESCRIPTION
>> +===========
>> +
>> +Search for messages matching the given search terms, and display the
>> +addresses from them.
>> +
>> +See **notmuch-search-terms(7)** for details of the supported syntax for
>> +<search-terms>.
>> +
>> +Supported options for **address** include
>> +
>> +    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
>> +        Presents the results in either JSON, S-Expressions, newline
>> +        character separated plain-text (default), or null character
>> +        separated plain-text (compatible with **xargs(1)** -0 option
>> +        where available).
>> +
>> +    ``--format-version=N``
>> +        Use the specified structured output format version. This is
>> +        intended for programs that invoke **notmuch(1)** internally. If
>> +        omitted, the latest supported version will be used.
>> +
>> +    ``--output=(sender|recipients)``
>> +
>> +        Controls which information appears in the output. This option
>> +	can be given multiple times to combine different outputs.
>> +	Omitting this option is equivalent to
>> +	--output=sender --output=recipients.
>> +
>> +	**sender**
>> +            Output all addresses from the *From* header.
>> +
>> +	    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**
>> +            Output all addresses from the *To*, *Cc* and *Bcc*
>> +            headers.
>> +
>> +    ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>> +        This option can be used to present results in either
>> +        chronological order (**oldest-first**) or reverse chronological
>> +        order (**newest-first**).
>> +
>> +        By default, results will be displayed in reverse chronological
>> +        order, (that is, the newest results will be displayed first).
>> +
>> +    ``--exclude=(true|false|all|flag)``
>> +        A message is called "excluded" if it matches at least one tag in
>> +        search.tag\_exclude that does not appear explicitly in the
>> +        search terms. This option specifies whether to omit excluded
>> +        messages in the search process.
>> +
>> +        The default value, **true**, prevents excluded messages from
>> +        matching the search terms.
>> +
>> +        **all** additionally prevents excluded messages from appearing
>> +        in displayed results, in effect behaving as though the excluded
>> +        messages do not exist.
>> +
>> +        **false** allows excluded messages to match search terms and
>> +        appear in displayed results. Excluded messages are still marked
>> +        in the relevant outputs.
>> +
>> +        **flag** only has an effect when ``--output=summary``. The
>> +        output is almost identical to **false**, but the "match count"
>> +        is the number of matching non-excluded messages in the thread,
>> +        rather than the number of matching messages.
>
> The exclude text needs updating as flag makes no sense for the address
> command. 
>
> Best wishes 
>
> Mark
>
>> +EXIT STATUS
>> +===========
>> +
>> +This command supports the following special exit status codes
>> +
>> +``20``
>> +    The requested format version is too old.
>> +
>> +``21``
>> +    The requested format version is too new.
>> +
>> +SEE ALSO
>> +========
>> +
>> +**notmuch(1)**, **notmuch-config(1)**, **notmuch-count(1)**,
>> +**notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
>> +**notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
>> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
>> +***notmuch-search(1)**
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 8110086..65df288 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -78,25 +78,8 @@ 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. Currently, this is only supported for **sender** and
>> -	**recipients** outputs.
>> +	outputs.
>>  
>>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>>          This option can be used to present results in either
>> @@ -173,3 +156,4 @@ SEE ALSO
>>  **notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
>>  **notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
>>  **notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
>> +***notmuch-address(1)**
>> diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst
>> index 9710294..98590a4 100644
>> --- a/doc/man1/notmuch.rst
>> +++ b/doc/man1/notmuch.rst
>> @@ -88,8 +88,8 @@ Several of the notmuch commands accept search terms with a common
>>  syntax. See **notmuch-search-terms**\ (7) for more details on the
>>  supported syntax.
>>  
>> -The **search**, **show** and **count** commands are used to query the
>> -email database.
>> +The **search**, **show**, **address** and **count** commands are used
>> +to query the email database.
>>  
>>  The **reply** command is useful for preparing a template for an email
>>  reply.
>> @@ -128,7 +128,8 @@ SEE ALSO
>>  **notmuch-config(1)**, **notmuch-count(1)**, **notmuch-dump(1)**,
>>  **notmuch-hooks(5)**, **notmuch-insert(1)**, **notmuch-new(1)**,
>>  **notmuch-reply(1)**, **notmuch-restore(1)**, **notmuch-search(1)**,
>> -**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
>> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
>> +***notmuch-address(1)**
>>  
>>  The notmuch website: **http://notmuchmail.org**
>>  
>> diff --git a/notmuch-client.h b/notmuch-client.h
>> index e1efbe0..5e0d475 100644
>> --- a/notmuch-client.h
>> +++ b/notmuch-client.h
>> @@ -199,6 +199,9 @@ int
>>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]);
>>  
>>  int
>> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[]);
>> +
>> +int
>>  notmuch_setup_command (notmuch_config_t *config, int argc, char *argv[]);
>>  
>>  int
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index f115359..cbd84f5 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -23,17 +23,18 @@
>>  #include "string-util.h"
>>  
>>  typedef enum {
>> +    /* Search command */
>>      OUTPUT_SUMMARY	= 1 << 0,
>>      OUTPUT_THREADS	= 1 << 1,
>>      OUTPUT_MESSAGES	= 1 << 2,
>>      OUTPUT_FILES	= 1 << 3,
>>      OUTPUT_TAGS		= 1 << 4,
>> +
>> +    /* Address command */
>>      OUTPUT_SENDER	= 1 << 5,
>>      OUTPUT_RECIPIENTS	= 1 << 6,
>>  } output_t;
>>  
>> -#define OUTPUT_ADDRESS_FLAGS (OUTPUT_SENDER | OUTPUT_RECIPIENTS)
>> -
>>  typedef enum {
>>      NOTMUCH_FORMAT_JSON,
>>      NOTMUCH_FORMAT_TEXT,
>> @@ -554,51 +555,55 @@ _notmuch_search_cleanup (search_context_t *ctx)
>>      talloc_free (ctx->format);
>>  }
>>  
>> +static search_context_t search_context = {
>> +    .format_sel = NOTMUCH_FORMAT_TEXT,
>> +    .exclude = NOTMUCH_EXCLUDE_TRUE,
>> +    .sort = NOTMUCH_SORT_NEWEST_FIRST,
>> +    .output = 0,
>> +    .offset = 0,
>> +    .limit = -1, /* unlimited */
>> +    .dupe = -1,
>> +};
>> +
>> +static const notmuch_opt_desc_t common_options[] = {
>> +    { NOTMUCH_OPT_KEYWORD, &search_context.sort, "sort", 's',
>> +      (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
>> +			      { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
>> +			      { 0, 0 } } },
>> +    { NOTMUCH_OPT_KEYWORD, &search_context.format_sel, "format", 'f',
>> +      (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
>> +			      { "sexp", NOTMUCH_FORMAT_SEXP },
>> +			      { "text", NOTMUCH_FORMAT_TEXT },
>> +			      { "text0", NOTMUCH_FORMAT_TEXT0 },
>> +			      { 0, 0 } } },
>> +    { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>> +    { NOTMUCH_OPT_KEYWORD, &search_context.exclude, "exclude", 'x',
>> +      (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
>> +			      { "false", NOTMUCH_EXCLUDE_FALSE },
>> +			      { "flag", NOTMUCH_EXCLUDE_FLAG },
>> +			      { "all", NOTMUCH_EXCLUDE_ALL },
>> +			      { 0, 0 } } },
>> +    { 0, 0, 0, 0, 0 }
>> +};
>> +
>>  int
>>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  {
>> -    search_context_t search_context = {
>> -	.format_sel = NOTMUCH_FORMAT_TEXT,
>> -	.exclude = NOTMUCH_EXCLUDE_TRUE,
>> -	.sort = NOTMUCH_SORT_NEWEST_FIRST,
>> -	.output = 0,
>> -	.offset = 0,
>> -	.limit = -1, /* unlimited */
>> -	.dupe = -1,
>> -    };
>>      search_context_t *ctx = &search_context;
>>      int opt_index, ret;
>>  
>>      notmuch_opt_desc_t options[] = {
>> -	{ NOTMUCH_OPT_KEYWORD, &ctx->sort, "sort", 's',
>> -	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
>> -				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
>> -				  { 0, 0 } } },
>> -	{ NOTMUCH_OPT_KEYWORD, &ctx->format_sel, "format", 'f',
>> -	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
>> -				  { "sexp", NOTMUCH_FORMAT_SEXP },
>> -				  { "text", NOTMUCH_FORMAT_TEXT },
>> -				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>> -				  { 0, 0 } } },
>> -	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>>  	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->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 } } },
>> -        { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
>> -          (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
>> -                                  { "false", NOTMUCH_EXCLUDE_FALSE },
>> -                                  { "flag", NOTMUCH_EXCLUDE_FLAG },
>> -                                  { "all", NOTMUCH_EXCLUDE_ALL },
>> -                                  { 0, 0 } } },
>>  	{ NOTMUCH_OPT_INT, &ctx->offset, "offset", 'O', 0 },
>>  	{ NOTMUCH_OPT_INT, &ctx->limit, "limit", 'L', 0  },
>>  	{ NOTMUCH_OPT_INT, &ctx->dupe, "duplicate", 'D', 0  },
>> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>>  	{ 0, 0, 0, 0, 0 }
>>      };
>>  
>> @@ -623,8 +628,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  	ctx->output == OUTPUT_THREADS)
>>  	ret = do_search_threads (ctx);
>>      else if (ctx->output == OUTPUT_MESSAGES ||
>> -	     ctx->output == OUTPUT_FILES ||
>> -	     (ctx->output & OUTPUT_ADDRESS_FLAGS && !(ctx->output & ~OUTPUT_ADDRESS_FLAGS)))
>> +	     ctx->output == OUTPUT_FILES)
>>  	ret = do_search_messages (ctx);
>>      else if (ctx->output == OUTPUT_TAGS)
>>  	ret = do_search_tags (ctx);
>> @@ -637,3 +641,36 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  
>>      return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>>  }
>> +
>> +int
>> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
>> +{
>> +    search_context_t *ctx = &search_context;
>> +    int opt_index, ret;
>> +
>> +    notmuch_opt_desc_t options[] = {
>> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
>> +	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
>> +				  { "recipients", OUTPUT_RECIPIENTS },
>> +				  { 0, 0 } } },
>> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>> +	{ 0, 0, 0, 0, 0 }
>> +    };
>> +
>> +    opt_index = parse_arguments (argc, argv, options, 1);
>> +    if (opt_index < 0)
>> +	return EXIT_FAILURE;
>> +
>> +    if (! ctx->output)
>> +	search_context.output = OUTPUT_SENDER | OUTPUT_RECIPIENTS;
>> +
>> +    if (_notmuch_search_prepare (ctx, config,
>> +				 argc - opt_index, argv + opt_index))
>> +	return EXIT_FAILURE;
>> +
>> +    ret = do_search_messages (ctx);
>> +
>> +    _notmuch_search_cleanup (ctx);
>> +
>> +    return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>> +}
>> diff --git a/notmuch.c b/notmuch.c
>> index dcda039..0fac099 100644
>> --- a/notmuch.c
>> +++ b/notmuch.c
>> @@ -54,6 +54,8 @@ static command_t commands[] = {
>>        "Add a new message into the maildir and notmuch database." },
>>      { "search", notmuch_search_command, FALSE,
>>        "Search for messages matching the given search terms." },
>> +    { "address", notmuch_address_command, FALSE,
>> +      "Get addresses from messages matching the given search terms." },
>>      { "show", notmuch_show_command, FALSE,
>>        "Show all messages matching the search terms." },
>>      { "count", notmuch_count_command, FALSE,
>> -- 
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[Mark Walters]

On Tue, 04 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> On Tue, Nov 04 2014, Mark Walters wrote:
>> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>>> This moves address-related functionality from search command to the
>>> new address command. The implementation shares almost all code and
>>> some command line options.
>>>
>>> Options --offset and --limit were intentionally not included in the
>>> address command, because they refer to messages numbers, which users
>>> do not see in the output. This could confuse users because, for
>>> example, they could see more addresses in the output that what was
>>> specified with --limit. This functionality can be correctly
>>> reimplemented for addresses later.
>>
>> I am not sure about this: we already have this anomaly for output=files
>> say. Also I can imagine calling notmuch address --limit=1000 ... to get
>> a bunch of recent addresses quickly and I really am wanting to look at
>> 1000 messages, not collect 1000 addresses.
>
> I think that one of the reasons for having the new "address" command is
> to have cleaner user interface. And including "anomalies" doesn't sound
> like a way to achieve this. I think that now you can use "date:" query
> to limit the search.
>
> I volunteer to implement "address --limit" properly after 0.19. This
> should be easy.

I think this depends on how you view limit: is it to limit the output
(roughly to run "head" on the output), or is to bound the amount of work
notmuch has to do (eg to make sure you don't get a long delay). Your
suggestion is definitely the former, whereas I am more worried about the
latter: limit in your definition could take an essentially unbounded
amount of time.

Best wishes

Mark

>
> -Michal
>
>> Additionally, the 1000 message approach makes sense when we start
>> deduping whereas 1000 authors becomes unclear.
>>
>>>
>>> This was inspired by a patch from Jani Nikula.
>>> ---
>>>  completion/notmuch-completion.bash |  42 ++++++++++++++-
>>>  completion/notmuch-completion.zsh  |  10 +++-
>>>  doc/man1/notmuch-address.rst       |  99 ++++++++++++++++++++++++++++++++++++
>>>  doc/man1/notmuch-search.rst        |  20 +-------
>>>  doc/man1/notmuch.rst               |   7 +--
>>>  notmuch-client.h                   |   3 ++
>>>  notmuch-search.c                   | 101 +++++++++++++++++++++++++------------
>>>  notmuch.c                          |   2 +
>>>  8 files changed, 228 insertions(+), 56 deletions(-)
>>>  create mode 100644 doc/man1/notmuch-address.rst
>>>
>>> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
>>> index cfbd389..94ea2d5 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 sender recipients" -- "${cur}" ) )
>>> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
>>>  	    return
>>>  	    ;;
>>>  	--sort)
>>> @@ -320,6 +320,44 @@ _notmuch_search()
>>>      esac
>>>  }
>>>  
>>> +_notmuch_address()
>>> +{
>>> +    local cur prev words cword split
>>> +    _init_completion -s || return
>>> +
>>> +    $split &&
>>> +    case "${prev}" in
>>> +	--format)
>>> +	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
>>> +	    return
>>> +	    ;;
>>> +	--output)
>>> +	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
>>> +	    return
>>> +	    ;;
>>> +	--sort)
>>> +	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
>>> +	    return
>>> +	    ;;
>>> +	--exclude)
>>> +	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
>>> +	    return
>>> +	    ;;
>>> +    esac
>>> +
>>> +    ! $split &&
>>> +    case "${cur}" in
>>> +	-*)
>>> +	    local options="--format= --output= --sort= --exclude="
>>> +	    compopt -o nospace
>>> +	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
>>> +	    ;;
>>> +	*)
>>> +	    _notmuch_search_terms
>>> +	    ;;
>>> +    esac
>>> +}
>>> +
>>>  _notmuch_show()
>>>  {
>>>      local cur prev words cword split
>>> @@ -393,7 +431,7 @@ _notmuch_tag()
>>>  
>>>  _notmuch()
>>>  {
>>> -    local _notmuch_commands="compact config count dump help insert new reply restore search setup show tag"
>>> +    local _notmuch_commands="compact config count dump help insert new reply restore search address setup show tag"
>>>      local arg cur prev words cword split
>>>  
>>>      # require bash-completion with _init_completion
>>> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
>>> index 3e52a00..c606b75 100644
>>> --- a/completion/notmuch-completion.zsh
>>> +++ b/completion/notmuch-completion.zsh
>>> @@ -10,6 +10,7 @@ _notmuch_commands()
>>>      'setup:interactively set up notmuch for first use'
>>>      'new:find and import any new message to the database'
>>>      'search:search for messages matching the search terms, display matching threads as results'
>>> +    'address:get addresses from messages matching the given search terms'
>>>      'reply:constructs a reply template for a set of messages'
>>>      'show:show all messages matching the search terms'
>>>      'tag:add or remove tags for all messages matching the search terms'
>>> @@ -53,7 +54,14 @@ _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))'
>>> +}
>>> +
>>> +_notmuch_address()
>>> +{
>>> +  _arguments -s : \
>>> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
>>> +    '--output=[select what to output]:output:((sender recipients))'
>>>  }
>>>  
>>>  _notmuch()
>>> diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
>>> new file mode 100644
>>> index 0000000..8109f11
>>> --- /dev/null
>>> +++ b/doc/man1/notmuch-address.rst
>>> @@ -0,0 +1,99 @@
>>> +===============
>>> +notmuch-address
>>> +===============
>>> +
>>> +SYNOPSIS
>>> +========
>>> +
>>> +**notmuch** **address** [*option* ...] <*search-term*> ...
>>> +
>>> +DESCRIPTION
>>> +===========
>>> +
>>> +Search for messages matching the given search terms, and display the
>>> +addresses from them.
>>> +
>>> +See **notmuch-search-terms(7)** for details of the supported syntax for
>>> +<search-terms>.
>>> +
>>> +Supported options for **address** include
>>> +
>>> +    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
>>> +        Presents the results in either JSON, S-Expressions, newline
>>> +        character separated plain-text (default), or null character
>>> +        separated plain-text (compatible with **xargs(1)** -0 option
>>> +        where available).
>>> +
>>> +    ``--format-version=N``
>>> +        Use the specified structured output format version. This is
>>> +        intended for programs that invoke **notmuch(1)** internally. If
>>> +        omitted, the latest supported version will be used.
>>> +
>>> +    ``--output=(sender|recipients)``
>>> +
>>> +        Controls which information appears in the output. This option
>>> +	can be given multiple times to combine different outputs.
>>> +	Omitting this option is equivalent to
>>> +	--output=sender --output=recipients.
>>> +
>>> +	**sender**
>>> +            Output all addresses from the *From* header.
>>> +
>>> +	    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**
>>> +            Output all addresses from the *To*, *Cc* and *Bcc*
>>> +            headers.
>>> +
>>> +    ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>>> +        This option can be used to present results in either
>>> +        chronological order (**oldest-first**) or reverse chronological
>>> +        order (**newest-first**).
>>> +
>>> +        By default, results will be displayed in reverse chronological
>>> +        order, (that is, the newest results will be displayed first).
>>> +
>>> +    ``--exclude=(true|false|all|flag)``
>>> +        A message is called "excluded" if it matches at least one tag in
>>> +        search.tag\_exclude that does not appear explicitly in the
>>> +        search terms. This option specifies whether to omit excluded
>>> +        messages in the search process.
>>> +
>>> +        The default value, **true**, prevents excluded messages from
>>> +        matching the search terms.
>>> +
>>> +        **all** additionally prevents excluded messages from appearing
>>> +        in displayed results, in effect behaving as though the excluded
>>> +        messages do not exist.
>>> +
>>> +        **false** allows excluded messages to match search terms and
>>> +        appear in displayed results. Excluded messages are still marked
>>> +        in the relevant outputs.
>>> +
>>> +        **flag** only has an effect when ``--output=summary``. The
>>> +        output is almost identical to **false**, but the "match count"
>>> +        is the number of matching non-excluded messages in the thread,
>>> +        rather than the number of matching messages.
>>
>> The exclude text needs updating as flag makes no sense for the address
>> command. 
>>
>> Best wishes 
>>
>> Mark
>>
>>> +EXIT STATUS
>>> +===========
>>> +
>>> +This command supports the following special exit status codes
>>> +
>>> +``20``
>>> +    The requested format version is too old.
>>> +
>>> +``21``
>>> +    The requested format version is too new.
>>> +
>>> +SEE ALSO
>>> +========
>>> +
>>> +**notmuch(1)**, **notmuch-config(1)**, **notmuch-count(1)**,
>>> +**notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
>>> +**notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
>>> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
>>> +***notmuch-search(1)**
>>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>>> index 8110086..65df288 100644
>>> --- a/doc/man1/notmuch-search.rst
>>> +++ b/doc/man1/notmuch-search.rst
>>> @@ -78,25 +78,8 @@ 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. Currently, this is only supported for **sender** and
>>> -	**recipients** outputs.
>>> +	outputs.
>>>  
>>>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>>>          This option can be used to present results in either
>>> @@ -173,3 +156,4 @@ SEE ALSO
>>>  **notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
>>>  **notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
>>>  **notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
>>> +***notmuch-address(1)**
>>> diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst
>>> index 9710294..98590a4 100644
>>> --- a/doc/man1/notmuch.rst
>>> +++ b/doc/man1/notmuch.rst
>>> @@ -88,8 +88,8 @@ Several of the notmuch commands accept search terms with a common
>>>  syntax. See **notmuch-search-terms**\ (7) for more details on the
>>>  supported syntax.
>>>  
>>> -The **search**, **show** and **count** commands are used to query the
>>> -email database.
>>> +The **search**, **show**, **address** and **count** commands are used
>>> +to query the email database.
>>>  
>>>  The **reply** command is useful for preparing a template for an email
>>>  reply.
>>> @@ -128,7 +128,8 @@ SEE ALSO
>>>  **notmuch-config(1)**, **notmuch-count(1)**, **notmuch-dump(1)**,
>>>  **notmuch-hooks(5)**, **notmuch-insert(1)**, **notmuch-new(1)**,
>>>  **notmuch-reply(1)**, **notmuch-restore(1)**, **notmuch-search(1)**,
>>> -**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
>>> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
>>> +***notmuch-address(1)**
>>>  
>>>  The notmuch website: **http://notmuchmail.org**
>>>  
>>> diff --git a/notmuch-client.h b/notmuch-client.h
>>> index e1efbe0..5e0d475 100644
>>> --- a/notmuch-client.h
>>> +++ b/notmuch-client.h
>>> @@ -199,6 +199,9 @@ int
>>>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]);
>>>  
>>>  int
>>> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[]);
>>> +
>>> +int
>>>  notmuch_setup_command (notmuch_config_t *config, int argc, char *argv[]);
>>>  
>>>  int
>>> diff --git a/notmuch-search.c b/notmuch-search.c
>>> index f115359..cbd84f5 100644
>>> --- a/notmuch-search.c
>>> +++ b/notmuch-search.c
>>> @@ -23,17 +23,18 @@
>>>  #include "string-util.h"
>>>  
>>>  typedef enum {
>>> +    /* Search command */
>>>      OUTPUT_SUMMARY	= 1 << 0,
>>>      OUTPUT_THREADS	= 1 << 1,
>>>      OUTPUT_MESSAGES	= 1 << 2,
>>>      OUTPUT_FILES	= 1 << 3,
>>>      OUTPUT_TAGS		= 1 << 4,
>>> +
>>> +    /* Address command */
>>>      OUTPUT_SENDER	= 1 << 5,
>>>      OUTPUT_RECIPIENTS	= 1 << 6,
>>>  } output_t;
>>>  
>>> -#define OUTPUT_ADDRESS_FLAGS (OUTPUT_SENDER | OUTPUT_RECIPIENTS)
>>> -
>>>  typedef enum {
>>>      NOTMUCH_FORMAT_JSON,
>>>      NOTMUCH_FORMAT_TEXT,
>>> @@ -554,51 +555,55 @@ _notmuch_search_cleanup (search_context_t *ctx)
>>>      talloc_free (ctx->format);
>>>  }
>>>  
>>> +static search_context_t search_context = {
>>> +    .format_sel = NOTMUCH_FORMAT_TEXT,
>>> +    .exclude = NOTMUCH_EXCLUDE_TRUE,
>>> +    .sort = NOTMUCH_SORT_NEWEST_FIRST,
>>> +    .output = 0,
>>> +    .offset = 0,
>>> +    .limit = -1, /* unlimited */
>>> +    .dupe = -1,
>>> +};
>>> +
>>> +static const notmuch_opt_desc_t common_options[] = {
>>> +    { NOTMUCH_OPT_KEYWORD, &search_context.sort, "sort", 's',
>>> +      (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
>>> +			      { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
>>> +			      { 0, 0 } } },
>>> +    { NOTMUCH_OPT_KEYWORD, &search_context.format_sel, "format", 'f',
>>> +      (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
>>> +			      { "sexp", NOTMUCH_FORMAT_SEXP },
>>> +			      { "text", NOTMUCH_FORMAT_TEXT },
>>> +			      { "text0", NOTMUCH_FORMAT_TEXT0 },
>>> +			      { 0, 0 } } },
>>> +    { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>>> +    { NOTMUCH_OPT_KEYWORD, &search_context.exclude, "exclude", 'x',
>>> +      (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
>>> +			      { "false", NOTMUCH_EXCLUDE_FALSE },
>>> +			      { "flag", NOTMUCH_EXCLUDE_FLAG },
>>> +			      { "all", NOTMUCH_EXCLUDE_ALL },
>>> +			      { 0, 0 } } },
>>> +    { 0, 0, 0, 0, 0 }
>>> +};
>>> +
>>>  int
>>>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>>  {
>>> -    search_context_t search_context = {
>>> -	.format_sel = NOTMUCH_FORMAT_TEXT,
>>> -	.exclude = NOTMUCH_EXCLUDE_TRUE,
>>> -	.sort = NOTMUCH_SORT_NEWEST_FIRST,
>>> -	.output = 0,
>>> -	.offset = 0,
>>> -	.limit = -1, /* unlimited */
>>> -	.dupe = -1,
>>> -    };
>>>      search_context_t *ctx = &search_context;
>>>      int opt_index, ret;
>>>  
>>>      notmuch_opt_desc_t options[] = {
>>> -	{ NOTMUCH_OPT_KEYWORD, &ctx->sort, "sort", 's',
>>> -	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
>>> -				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
>>> -				  { 0, 0 } } },
>>> -	{ NOTMUCH_OPT_KEYWORD, &ctx->format_sel, "format", 'f',
>>> -	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
>>> -				  { "sexp", NOTMUCH_FORMAT_SEXP },
>>> -				  { "text", NOTMUCH_FORMAT_TEXT },
>>> -				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>>> -				  { 0, 0 } } },
>>> -	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>>>  	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->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 } } },
>>> -        { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
>>> -          (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
>>> -                                  { "false", NOTMUCH_EXCLUDE_FALSE },
>>> -                                  { "flag", NOTMUCH_EXCLUDE_FLAG },
>>> -                                  { "all", NOTMUCH_EXCLUDE_ALL },
>>> -                                  { 0, 0 } } },
>>>  	{ NOTMUCH_OPT_INT, &ctx->offset, "offset", 'O', 0 },
>>>  	{ NOTMUCH_OPT_INT, &ctx->limit, "limit", 'L', 0  },
>>>  	{ NOTMUCH_OPT_INT, &ctx->dupe, "duplicate", 'D', 0  },
>>> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>>>  	{ 0, 0, 0, 0, 0 }
>>>      };
>>>  
>>> @@ -623,8 +628,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>>  	ctx->output == OUTPUT_THREADS)
>>>  	ret = do_search_threads (ctx);
>>>      else if (ctx->output == OUTPUT_MESSAGES ||
>>> -	     ctx->output == OUTPUT_FILES ||
>>> -	     (ctx->output & OUTPUT_ADDRESS_FLAGS && !(ctx->output & ~OUTPUT_ADDRESS_FLAGS)))
>>> +	     ctx->output == OUTPUT_FILES)
>>>  	ret = do_search_messages (ctx);
>>>      else if (ctx->output == OUTPUT_TAGS)
>>>  	ret = do_search_tags (ctx);
>>> @@ -637,3 +641,36 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>>  
>>>      return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>>>  }
>>> +
>>> +int
>>> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
>>> +{
>>> +    search_context_t *ctx = &search_context;
>>> +    int opt_index, ret;
>>> +
>>> +    notmuch_opt_desc_t options[] = {
>>> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
>>> +	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
>>> +				  { "recipients", OUTPUT_RECIPIENTS },
>>> +				  { 0, 0 } } },
>>> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>>> +	{ 0, 0, 0, 0, 0 }
>>> +    };
>>> +
>>> +    opt_index = parse_arguments (argc, argv, options, 1);
>>> +    if (opt_index < 0)
>>> +	return EXIT_FAILURE;
>>> +
>>> +    if (! ctx->output)
>>> +	search_context.output = OUTPUT_SENDER | OUTPUT_RECIPIENTS;
>>> +
>>> +    if (_notmuch_search_prepare (ctx, config,
>>> +				 argc - opt_index, argv + opt_index))
>>> +	return EXIT_FAILURE;
>>> +
>>> +    ret = do_search_messages (ctx);
>>> +
>>> +    _notmuch_search_cleanup (ctx);
>>> +
>>> +    return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>>> +}
>>> diff --git a/notmuch.c b/notmuch.c
>>> index dcda039..0fac099 100644
>>> --- a/notmuch.c
>>> +++ b/notmuch.c
>>> @@ -54,6 +54,8 @@ static command_t commands[] = {
>>>        "Add a new message into the maildir and notmuch database." },
>>>      { "search", notmuch_search_command, FALSE,
>>>        "Search for messages matching the given search terms." },
>>> +    { "address", notmuch_address_command, FALSE,
>>> +      "Get addresses from messages matching the given search terms." },
>>>      { "show", notmuch_show_command, FALSE,
>>>        "Show all messages matching the search terms." },
>>>      { "count", notmuch_count_command, FALSE,
>>> -- 
>>> 2.1.1
>>>
>>> _______________________________________________
>>> notmuch mailing list
>>> notmuch@notmuchmail.org
>>> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[Michal Sojka]

On Wed, Nov 05 2014, Mark Walters wrote:
> On Tue, 04 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> On Tue, Nov 04 2014, Mark Walters wrote:
>>> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>>>> This moves address-related functionality from search command to the
>>>> new address command. The implementation shares almost all code and
>>>> some command line options.
>>>>
>>>> Options --offset and --limit were intentionally not included in the
>>>> address command, because they refer to messages numbers, which users
>>>> do not see in the output. This could confuse users because, for
>>>> example, they could see more addresses in the output that what was
>>>> specified with --limit. This functionality can be correctly
>>>> reimplemented for addresses later.
>>>
>>> I am not sure about this: we already have this anomaly for output=files
>>> say. Also I can imagine calling notmuch address --limit=1000 ... to get
>>> a bunch of recent addresses quickly and I really am wanting to look at
>>> 1000 messages, not collect 1000 addresses.
>>
>> I think that one of the reasons for having the new "address" command is
>> to have cleaner user interface. And including "anomalies" doesn't sound
>> like a way to achieve this. I think that now you can use "date:" query
>> to limit the search.
>>
>> I volunteer to implement "address --limit" properly after 0.19. This
>> should be easy.
>
> I think this depends on how you view limit: is it to limit the output
> (roughly to run "head" on the output), or is to bound the amount of work
> notmuch has to do (eg to make sure you don't get a long delay). Your
> suggestion is definitely the former, whereas I am more worried about the
> latter: limit in your definition could take an essentially unbounded
> amount of time.

Why? If I understand you correctly, you think of limit in terms of
messages. There is 1:N mapping between messages and addresses, where
N >= 1. If I limit the number of printed addresses, I limit the number
of messages as well. Only if N is zero (which probably can be the case
with Bcc and --output=recipients) then it can result in unbounded work
(provided you have infinite number of Bcc only messages in your
database :-)).

Do I miss something?

-Michal

Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command

[Mark Walters]

On Wed, 05 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> On Wed, Nov 05 2014, Mark Walters wrote:
>> On Tue, 04 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>>> On Tue, Nov 04 2014, Mark Walters wrote:
>>>> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>>>>> This moves address-related functionality from search command to the
>>>>> new address command. The implementation shares almost all code and
>>>>> some command line options.
>>>>>
>>>>> Options --offset and --limit were intentionally not included in the
>>>>> address command, because they refer to messages numbers, which users
>>>>> do not see in the output. This could confuse users because, for
>>>>> example, they could see more addresses in the output that what was
>>>>> specified with --limit. This functionality can be correctly
>>>>> reimplemented for addresses later.
>>>>
>>>> I am not sure about this: we already have this anomaly for output=files
>>>> say. Also I can imagine calling notmuch address --limit=1000 ... to get
>>>> a bunch of recent addresses quickly and I really am wanting to look at
>>>> 1000 messages, not collect 1000 addresses.
>>>
>>> I think that one of the reasons for having the new "address" command is
>>> to have cleaner user interface. And including "anomalies" doesn't sound
>>> like a way to achieve this. I think that now you can use "date:" query
>>> to limit the search.
>>>
>>> I volunteer to implement "address --limit" properly after 0.19. This
>>> should be easy.
>>
>> I think this depends on how you view limit: is it to limit the output
>> (roughly to run "head" on the output), or is to bound the amount of work
>> notmuch has to do (eg to make sure you don't get a long delay). Your
>> suggestion is definitely the former, whereas I am more worried about the
>> latter: limit in your definition could take an essentially unbounded
>> amount of time.
>
> Why? If I understand you correctly, you think of limit in terms of
> messages. There is 1:N mapping between messages and addresses, where
> N >= 1. If I limit the number of printed addresses, I limit the number
> of messages as well. Only if N is zero (which probably can be the case
> with Bcc and --output=recipients) then it can result in unbounded work
> (provided you have infinite number of Bcc only messages in your
> database :-)).

Hi 

I was assuming the limit in your scheme would come after the
deduplication: so notmuch would have to find "limit" distinct
addresses. If the limit is applied before the deduping then I agree work
is bounded (in any sane case).

If limit is applied before the deduping then that seems fine.

Best wishes 

Mark

>
> Do I miss something?
>
> -Michal

[PATCH v2 07/10] cli: search: Convert --output to keyword argument

[Michal Sojka]

Now, when address related outputs are in a separate command, it makes
no sense to combine multiple --output options in search command line.
Using switch statement to handle different outputs is more readable
than a series of if statements.
---
 doc/man1/notmuch-search.rst |  3 ---
 notmuch-search.c            | 25 +++++++++++++------------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
index 65df288..0cc2911 100644
--- a/doc/man1/notmuch-search.rst
+++ b/doc/man1/notmuch-search.rst
@@ -78,9 +78,6 @@ Supported options for **search** include
             by null characters (--format=text0), as a JSON array
             (--format=json), or as an S-Expression list (--format=sexp).
 
-	This option can be given multiple times to combine different
-	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 cbd84f5..402e860 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -593,7 +593,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index, ret;
 
     notmuch_opt_desc_t options[] = {
-	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
+	{ NOTMUCH_OPT_KEYWORD, &ctx->output, "output", 'o',
 	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
 				  { "threads", OUTPUT_THREADS },
 				  { "messages", OUTPUT_MESSAGES },
@@ -607,13 +607,11 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	{ 0, 0, 0, 0, 0 }
     };
 
+    ctx->output = OUTPUT_SUMMARY;
     opt_index = parse_arguments (argc, argv, options, 1);
     if (opt_index < 0)
 	return EXIT_FAILURE;
 
-    if (! ctx->output)
-	ctx->output = OUTPUT_SUMMARY;
-
     if (ctx->output != OUTPUT_FILES && ctx->output != OUTPUT_MESSAGES &&
 	ctx->dupe != -1) {
         fprintf (stderr, "Error: --duplicate=N is only supported with --output=files and --output=messages.\n");
@@ -624,17 +622,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 				 argc - opt_index, argv + opt_index))
 	return EXIT_FAILURE;
 
-    if (ctx->output == OUTPUT_SUMMARY ||
-	ctx->output == OUTPUT_THREADS)
+    switch (ctx->output) {
+    case OUTPUT_SUMMARY:
+    case OUTPUT_THREADS:
 	ret = do_search_threads (ctx);
-    else if (ctx->output == OUTPUT_MESSAGES ||
-	     ctx->output == OUTPUT_FILES)
+	break;
+    case OUTPUT_MESSAGES:
+    case OUTPUT_FILES:
 	ret = do_search_messages (ctx);
-    else if (ctx->output == OUTPUT_TAGS)
+	break;
+    case OUTPUT_TAGS:
 	ret = do_search_tags (ctx);
-    else {
-	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
-	ret = 1;
+	break;
+    default:
+	INTERNAL_ERROR ("Unexpected output");
     }
 
     _notmuch_search_cleanup (ctx);
-- 
2.1.1

Re: [PATCH v2 07/10] cli: search: Convert --output to keyword argument

[Mark Walters]

Hi

On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> Now, when address related outputs are in a separate command, it makes
> no sense to combine multiple --output options in search command line.
> Using switch statement to handle different outputs is more readable
> than a series of if statements.

I am not keen on this change: I think the user should always be able to
force the default output by setting command line options (which should
protect against future changes in the default). Thus I would like to
continue to allow  --output=sender --output=recipients. 

I do approve of making the default do something useful but whether it
should be both or just sender (which is much faster) is unclear to me.

Best wishes

Mark

 
> ---
>  doc/man1/notmuch-search.rst |  3 ---
>  notmuch-search.c            | 25 +++++++++++++------------
>  2 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
> index 65df288..0cc2911 100644
> --- a/doc/man1/notmuch-search.rst
> +++ b/doc/man1/notmuch-search.rst
> @@ -78,9 +78,6 @@ Supported options for **search** include
>              by null characters (--format=text0), as a JSON array
>              (--format=json), or as an S-Expression list (--format=sexp).
>  
> -	This option can be given multiple times to combine different
> -	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 cbd84f5..402e860 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -593,7 +593,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>      int opt_index, ret;
>  
>      notmuch_opt_desc_t options[] = {
> -	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
> +	{ NOTMUCH_OPT_KEYWORD, &ctx->output, "output", 'o',
>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>  				  { "threads", OUTPUT_THREADS },
>  				  { "messages", OUTPUT_MESSAGES },
> @@ -607,13 +607,11 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> +    ctx->output = OUTPUT_SUMMARY;
>      opt_index = parse_arguments (argc, argv, options, 1);
>      if (opt_index < 0)
>  	return EXIT_FAILURE;
>  
> -    if (! ctx->output)
> -	ctx->output = OUTPUT_SUMMARY;
> -
>      if (ctx->output != OUTPUT_FILES && ctx->output != OUTPUT_MESSAGES &&
>  	ctx->dupe != -1) {
>          fprintf (stderr, "Error: --duplicate=N is only supported with --output=files and --output=messages.\n");
> @@ -624,17 +622,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>  				 argc - opt_index, argv + opt_index))
>  	return EXIT_FAILURE;
>  
> -    if (ctx->output == OUTPUT_SUMMARY ||
> -	ctx->output == OUTPUT_THREADS)
> +    switch (ctx->output) {
> +    case OUTPUT_SUMMARY:
> +    case OUTPUT_THREADS:
>  	ret = do_search_threads (ctx);
> -    else if (ctx->output == OUTPUT_MESSAGES ||
> -	     ctx->output == OUTPUT_FILES)
> +	break;
> +    case OUTPUT_MESSAGES:
> +    case OUTPUT_FILES:
>  	ret = do_search_messages (ctx);
> -    else if (ctx->output == OUTPUT_TAGS)
> +	break;
> +    case OUTPUT_TAGS:
>  	ret = do_search_tags (ctx);
> -    else {
> -	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
> -	ret = 1;
> +	break;
> +    default:
> +	INTERNAL_ERROR ("Unexpected output");
>      }
>  
>      _notmuch_search_cleanup (ctx);
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 07/10] cli: search: Convert --output to keyword argument

[Mark Walters]

> Hi
>
> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> Now, when address related outputs are in a separate command, it makes
>> no sense to combine multiple --output options in search command line.
>> Using switch statement to handle different outputs is more readable
>> than a series of if statements.
>
> I am not keen on this change: I think the user should always be able to
> force the default output by setting command line options (which should
> protect against future changes in the default). Thus I would like to
> continue to allow  --output=sender --output=recipients. 
>
> I do approve of making the default do something useful but whether it
> should be both or just sender (which is much faster) is unclear to me.

Sorry my stupidity: you said "search" not "address" so this is fine.

Best wishes

Mark

>
> Best wishes
>
> Mark
>
>  
>> ---
>>  doc/man1/notmuch-search.rst |  3 ---
>>  notmuch-search.c            | 25 +++++++++++++------------
>>  2 files changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 65df288..0cc2911 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -78,9 +78,6 @@ Supported options for **search** include
>>              by null characters (--format=text0), as a JSON array
>>              (--format=json), or as an S-Expression list (--format=sexp).
>>  
>> -	This option can be given multiple times to combine different
>> -	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 cbd84f5..402e860 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -593,7 +593,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      int opt_index, ret;
>>  
>>      notmuch_opt_desc_t options[] = {
>> -	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
>> +	{ NOTMUCH_OPT_KEYWORD, &ctx->output, "output", 'o',
>>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>>  				  { "threads", OUTPUT_THREADS },
>>  				  { "messages", OUTPUT_MESSAGES },
>> @@ -607,13 +607,11 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  	{ 0, 0, 0, 0, 0 }
>>      };
>>  
>> +    ctx->output = OUTPUT_SUMMARY;
>>      opt_index = parse_arguments (argc, argv, options, 1);
>>      if (opt_index < 0)
>>  	return EXIT_FAILURE;
>>  
>> -    if (! ctx->output)
>> -	ctx->output = OUTPUT_SUMMARY;
>> -
>>      if (ctx->output != OUTPUT_FILES && ctx->output != OUTPUT_MESSAGES &&
>>  	ctx->dupe != -1) {
>>          fprintf (stderr, "Error: --duplicate=N is only supported with --output=files and --output=messages.\n");
>> @@ -624,17 +622,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  				 argc - opt_index, argv + opt_index))
>>  	return EXIT_FAILURE;
>>  
>> -    if (ctx->output == OUTPUT_SUMMARY ||
>> -	ctx->output == OUTPUT_THREADS)
>> +    switch (ctx->output) {
>> +    case OUTPUT_SUMMARY:
>> +    case OUTPUT_THREADS:
>>  	ret = do_search_threads (ctx);
>> -    else if (ctx->output == OUTPUT_MESSAGES ||
>> -	     ctx->output == OUTPUT_FILES)
>> +	break;
>> +    case OUTPUT_MESSAGES:
>> +    case OUTPUT_FILES:
>>  	ret = do_search_messages (ctx);
>> -    else if (ctx->output == OUTPUT_TAGS)
>> +	break;
>> +    case OUTPUT_TAGS:
>>  	ret = do_search_tags (ctx);
>> -    else {
>> -	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
>> -	ret = 1;
>> +	break;
>> +    default:
>> +	INTERNAL_ERROR ("Unexpected output");
>>      }
>>  
>>      _notmuch_search_cleanup (ctx);
>> -- 
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 07/10] cli: search: Convert --output to keyword argument

[Michal Sojka]

On Tue, Nov 04 2014, Mark Walters wrote:
> Hi
>
> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> Now, when address related outputs are in a separate command, it makes
>> no sense to combine multiple --output options in search command line.
>> Using switch statement to handle different outputs is more readable
>> than a series of if statements.
>
> I am not keen on this change: I think the user should always be able to
> force the default output by setting command line options (which should
> protect against future changes in the default). 

You can do this even with this patch.

> Thus I would like to continue to allow --output=sender
> --output=recipients.

This is allowed in notmuch address. This patch modifies only notmuch
search. Currently it makes no sense to run --output=files --output=tags
or other output combinations.

> I do approve of making the default do something useful but whether it
> should be both or just sender (which is much faster) is unclear to me.

Does this comment refer to 7/10 or to 6/10?

I'd say it refers to the following line from 6/10.

search_context.output = OUTPUT_SENDER | OUTPUT_RECIPIENTS;

Even there you can override the default by command line option. This get
executed only if no --output appears on the command line.

Thanks
-Michal


>
> Best wishes
>
> Mark
>
>  
>> ---
>>  doc/man1/notmuch-search.rst |  3 ---
>>  notmuch-search.c            | 25 +++++++++++++------------
>>  2 files changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 65df288..0cc2911 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -78,9 +78,6 @@ Supported options for **search** include
>>              by null characters (--format=text0), as a JSON array
>>              (--format=json), or as an S-Expression list (--format=sexp).
>>  
>> -	This option can be given multiple times to combine different
>> -	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 cbd84f5..402e860 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -593,7 +593,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>      int opt_index, ret;
>>  
>>      notmuch_opt_desc_t options[] = {
>> -	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
>> +	{ NOTMUCH_OPT_KEYWORD, &ctx->output, "output", 'o',
>>  	  (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>>  				  { "threads", OUTPUT_THREADS },
>>  				  { "messages", OUTPUT_MESSAGES },
>> @@ -607,13 +607,11 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  	{ 0, 0, 0, 0, 0 }
>>      };
>>  
>> +    ctx->output = OUTPUT_SUMMARY;
>>      opt_index = parse_arguments (argc, argv, options, 1);
>>      if (opt_index < 0)
>>  	return EXIT_FAILURE;
>>  
>> -    if (! ctx->output)
>> -	ctx->output = OUTPUT_SUMMARY;
>> -
>>      if (ctx->output != OUTPUT_FILES && ctx->output != OUTPUT_MESSAGES &&
>>  	ctx->dupe != -1) {
>>          fprintf (stderr, "Error: --duplicate=N is only supported with --output=files and --output=messages.\n");
>> @@ -624,17 +622,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  				 argc - opt_index, argv + opt_index))
>>  	return EXIT_FAILURE;
>>  
>> -    if (ctx->output == OUTPUT_SUMMARY ||
>> -	ctx->output == OUTPUT_THREADS)
>> +    switch (ctx->output) {
>> +    case OUTPUT_SUMMARY:
>> +    case OUTPUT_THREADS:
>>  	ret = do_search_threads (ctx);
>> -    else if (ctx->output == OUTPUT_MESSAGES ||
>> -	     ctx->output == OUTPUT_FILES)
>> +	break;
>> +    case OUTPUT_MESSAGES:
>> +    case OUTPUT_FILES:
>>  	ret = do_search_messages (ctx);
>> -    else if (ctx->output == OUTPUT_TAGS)
>> +	break;
>> +    case OUTPUT_TAGS:
>>  	ret = do_search_tags (ctx);
>> -    else {
>> -	fprintf (stderr, "Error: the combination of outputs is not supported.\n");
>> -	ret = 1;
>> +	break;
>> +    default:
>> +	INTERNAL_ERROR ("Unexpected output");
>>      }
>>  
>>      _notmuch_search_cleanup (ctx);
>> -- 
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 08/10] cli: address: Do not output duplicate addresses

[Michal Sojka]

This filters out duplicate addresses from address command output.

It also also adds tests for the address command.

The code here is an extended version of a patch from Jani Nikula.
---
 doc/man1/notmuch-address.rst |   2 +-
 notmuch-search.c             |  40 ++++++++++++++++-
 test/T095-address.sh         | 100 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+), 2 deletions(-)
 create mode 100755 test/T095-address.sh

diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index 8109f11..96512b7 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -11,7 +11,7 @@ DESCRIPTION
 ===========
 
 Search for messages matching the given search terms, and display the
-addresses from them.
+addresses from them. Duplicate addresses are filtered out.
 
 See **notmuch-search-terms(7)** for details of the supported syntax for
 <search-terms>.
diff --git a/notmuch-search.c b/notmuch-search.c
index 402e860..741702a 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -53,6 +53,7 @@ typedef struct {
     int offset;
     int limit;
     int dupe;
+    GHashTable *addresses;
 } search_context_t;
 
 typedef struct {
@@ -240,6 +241,27 @@ do_search_threads (search_context_t *ctx)
     return 0;
 }
 
+/* Returns TRUE iff name and addr is duplicate. */
+static notmuch_bool_t
+is_duplicate (const search_context_t *ctx, const char *name, const char *addr)
+{
+    notmuch_bool_t duplicate;
+    char *key;
+
+    key = talloc_asprintf (ctx->format, "%s <%s>", name, addr);
+    if (! key)
+	return FALSE;
+
+    duplicate = g_hash_table_lookup_extended (ctx->addresses, key, NULL, NULL);
+
+    if (! duplicate)
+	g_hash_table_insert (ctx->addresses, key, NULL);
+    else
+	talloc_free (key);
+
+    return duplicate;
+}
+
 static void
 print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
 {
@@ -274,7 +296,8 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
 
 /* Print addresses from InternetAddressList.  */
 static void
-process_address_list (const search_context_t *ctx, InternetAddressList *list)
+process_address_list (const search_context_t *ctx,
+		      InternetAddressList *list)
 {
     InternetAddress *address;
     int i;
@@ -298,6 +321,9 @@ process_address_list (const search_context_t *ctx, InternetAddressList *list)
 		.addr = internet_address_mailbox_get_addr (mailbox),
 	    };
 
+	    if (is_duplicate (ctx, mbx.name, mbx.addr))
+		continue;
+
 	    print_mailbox (ctx, &mbx);
 	}
     }
@@ -321,6 +347,12 @@ process_address_header (const search_context_t *ctx, const char *value)
     g_object_unref (list);
 }
 
+static void
+_my_talloc_free_for_g_hash (void *ptr)
+{
+    talloc_free (ptr);
+}
+
 static int
 _count_filenames (notmuch_message_t *message)
 {
@@ -669,8 +701,14 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
 				 argc - opt_index, argv + opt_index))
 	return EXIT_FAILURE;
 
+    ctx->addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
+					    _my_talloc_free_for_g_hash, NULL);
+
     ret = do_search_messages (ctx);
 
+    g_hash_table_unref (ctx->addresses);
+
+
     _notmuch_search_cleanup (ctx);
 
     return ret ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/test/T095-address.sh b/test/T095-address.sh
new file mode 100755
index 0000000..8a256d2
--- /dev/null
+++ b/test/T095-address.sh
@@ -0,0 +1,100 @@
+#!/usr/bin/env bash
+test_description='"notmuch address" in several variants'
+. ./test-lib.sh
+
+add_email_corpus
+
+test_begin_subtest "--output=sender"
+notmuch address --output=sender '*' >OUTPUT
+cat <<EOF >EXPECTED
+François Boulogne <boulogne.f@gmail.com>
+Olivier Berger <olivier.berger@it-sudparis.eu>
+Chris Wilson <chris@chris-wilson.co.uk>
+Carl Worth <cworth@cworth.org>
+Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+Keith Packard <keithp@keithp.com>
+Jjgod Jiang <gzjjgod@gmail.com>
+Rolland Santimano <rollandsantimano@yahoo.com>
+Jan Janak <jan@ryngle.com>
+Stewart Smith <stewart@flamingspork.com>
+Lars Kellogg-Stedman <lars@seas.harvard.edu>
+Alex Botero-Lowry <alex.boterolowry@gmail.com>
+Ingmar Vanhassel <ingmar@exherbo.org>
+Aron Griffis <agriffis@n01se.net>
+Adrian Perez de Castro <aperez@igalia.com>
+Israel Herraiz <isra@herraiz.org>
+Mikhail Gusarov <dottedmag@dottedmag.net>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=sender --format=json"
+notmuch address --output=sender --format=json '*' >OUTPUT
+cat <<EOF >EXPECTED
+[{"name": "François Boulogne", "address": "boulogne.f@gmail.com", "name-addr": "François Boulogne <boulogne.f@gmail.com>"},
+{"name": "Olivier Berger", "address": "olivier.berger@it-sudparis.eu", "name-addr": "Olivier Berger <olivier.berger@it-sudparis.eu>"},
+{"name": "Chris Wilson", "address": "chris@chris-wilson.co.uk", "name-addr": "Chris Wilson <chris@chris-wilson.co.uk>"},
+{"name": "Carl Worth", "address": "cworth@cworth.org", "name-addr": "Carl Worth <cworth@cworth.org>"},
+{"name": "Alexander Botero-Lowry", "address": "alex.boterolowry@gmail.com", "name-addr": "Alexander Botero-Lowry <alex.boterolowry@gmail.com>"},
+{"name": "Keith Packard", "address": "keithp@keithp.com", "name-addr": "Keith Packard <keithp@keithp.com>"},
+{"name": "Jjgod Jiang", "address": "gzjjgod@gmail.com", "name-addr": "Jjgod Jiang <gzjjgod@gmail.com>"},
+{"name": "Rolland Santimano", "address": "rollandsantimano@yahoo.com", "name-addr": "Rolland Santimano <rollandsantimano@yahoo.com>"},
+{"name": "Jan Janak", "address": "jan@ryngle.com", "name-addr": "Jan Janak <jan@ryngle.com>"},
+{"name": "Stewart Smith", "address": "stewart@flamingspork.com", "name-addr": "Stewart Smith <stewart@flamingspork.com>"},
+{"name": "Lars Kellogg-Stedman", "address": "lars@seas.harvard.edu", "name-addr": "Lars Kellogg-Stedman <lars@seas.harvard.edu>"},
+{"name": "Alex Botero-Lowry", "address": "alex.boterolowry@gmail.com", "name-addr": "Alex Botero-Lowry <alex.boterolowry@gmail.com>"},
+{"name": "Ingmar Vanhassel", "address": "ingmar@exherbo.org", "name-addr": "Ingmar Vanhassel <ingmar@exherbo.org>"},
+{"name": "Aron Griffis", "address": "agriffis@n01se.net", "name-addr": "Aron Griffis <agriffis@n01se.net>"},
+{"name": "Adrian Perez de Castro", "address": "aperez@igalia.com", "name-addr": "Adrian Perez de Castro <aperez@igalia.com>"},
+{"name": "Israel Herraiz", "address": "isra@herraiz.org", "name-addr": "Israel Herraiz <isra@herraiz.org>"},
+{"name": "Mikhail Gusarov", "address": "dottedmag@dottedmag.net", "name-addr": "Mikhail Gusarov <dottedmag@dottedmag.net>"}]
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients"
+notmuch address --output=recipients '*' >OUTPUT
+cat <<EOF >EXPECTED
+Allan McRae <allan@archlinux.org>
+"Discussion about the Arch User Repository (AUR)" <aur-general@archlinux.org>
+olivier.berger@it-sudparis.eu
+notmuch@notmuchmail.org
+notmuch <notmuch@notmuchmail.org>
+Keith Packard <keithp@keithp.com>
+Mikhail Gusarov <dottedmag@dottedmag.net>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=sender --output=recipients"
+notmuch address --output=sender --output=recipients '*' >OUTPUT
+cat <<EOF >EXPECTED
+François Boulogne <boulogne.f@gmail.com>
+Allan McRae <allan@archlinux.org>
+"Discussion about the Arch User Repository (AUR)" <aur-general@archlinux.org>
+Olivier Berger <olivier.berger@it-sudparis.eu>
+olivier.berger@it-sudparis.eu
+Chris Wilson <chris@chris-wilson.co.uk>
+notmuch@notmuchmail.org
+Carl Worth <cworth@cworth.org>
+Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+Keith Packard <keithp@keithp.com>
+Jjgod Jiang <gzjjgod@gmail.com>
+Rolland Santimano <rollandsantimano@yahoo.com>
+Jan Janak <jan@ryngle.com>
+Stewart Smith <stewart@flamingspork.com>
+Lars Kellogg-Stedman <lars@seas.harvard.edu>
+notmuch <notmuch@notmuchmail.org>
+Alex Botero-Lowry <alex.boterolowry@gmail.com>
+Ingmar Vanhassel <ingmar@exherbo.org>
+Aron Griffis <agriffis@n01se.net>
+Adrian Perez de Castro <aperez@igalia.com>
+Israel Herraiz <isra@herraiz.org>
+Mikhail Gusarov <dottedmag@dottedmag.net>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "No --output"
+notmuch address --output=sender --output=recipients '*' >OUTPUT
+# Use EXPECTED from previous subtest
+test_expect_equal_file OUTPUT EXPECTED
+
+
+test_done
-- 
2.1.1

Re: [PATCH v2 08/10] cli: address: Do not output duplicate addresses

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

>  
> +/* Returns TRUE iff name and addr is duplicate. */

If you're revising this patch, it would be good to mention the side
effect of this function.

> -process_address_list (const search_context_t *ctx, InternetAddressList *list)
> +process_address_list (const search_context_t *ctx,
> +		      InternetAddressList *list)

It probably doesn't make any difference, but this looks like a needless
whitespace change.

This function definitely needs some comment / pointer to
documention. And probably not to have _my in the name.

> +static void
> +_my_talloc_free_for_g_hash (void *ptr)
> +{
> +    talloc_free (ptr);
> +}
> +

I don't understand the name of the next subtest

> +test_begin_subtest "No --output"
> +notmuch address --output=sender --output=recipients '*' >OUTPUT
> +# Use EXPECTED from previous subtest
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +
> +test_done

nitpick, extra blank lines

So, AIUI, this is all of the series proposed for 0.19. It looks close to
OK to me, modulo some minor style nits. One anonymous commentator on
IRC mentioned the use of module scope variables, I guess in patch
6/10. I'm not sure of a better solution, but it's true in a perfect
world we wouldn't have module local state.

d

Re: [PATCH v2 08/10] cli: address: Do not output duplicate addresses

[Michal Sojka]

On Tue, Nov 04 2014, David Bremner wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>>  
>> +/* Returns TRUE iff name and addr is duplicate. */
>
> If you're revising this patch, it would be good to mention the side
> effect of this function.
>
>> -process_address_list (const search_context_t *ctx, InternetAddressList *list)
>> +process_address_list (const search_context_t *ctx,
>> +		      InternetAddressList *list)
>
> It probably doesn't make any difference, but this looks like a needless
> whitespace change.
>
> This function definitely needs some comment / pointer to
> documention. And probably not to have _my in the name.
>
>> +static void
>> +_my_talloc_free_for_g_hash (void *ptr)
>> +{
>> +    talloc_free (ptr);
>> +}
>> +
>
> I don't understand the name of the next subtest
>
>> +test_begin_subtest "No --output"
>> +notmuch address --output=sender --output=recipients '*' >OUTPUT

This should be "notmuch address '*' >OUTPUT". I'll fix that.

>> +# Use EXPECTED from previous subtest
>> +test_expect_equal_file OUTPUT EXPECTED
>> +
>> +
>> +test_done
>
> nitpick, extra blank lines
>
> So, AIUI, this is all of the series proposed for 0.19. 

Agreed.

> It looks close to OK to me, modulo some minor style nits. One
> anonymous commentator on IRC mentioned the use of module scope
> variables, I guess in patch 6/10. I'm not sure of a better solution,
> but it's true in a perfect world we wouldn't have module local state.

A possible solution would be fill in common_options structure
programmatically, but this would make the code much less readable. I can
think of a few other solutions but none of them would fit into "perfect
world" :)

I'll send updated patches in the evening (CET timezone).

Thanks
-Michal

[PATCH v2 09/10] cli: address: Add --output=count

[Michal Sojka]

This output prints how many times was each address encountered during
search.
---
 completion/notmuch-completion.bash |  2 +-
 completion/notmuch-completion.zsh  |  2 +-
 doc/man1/notmuch-address.rst       |  7 ++++++
 notmuch-search.c                   | 49 ++++++++++++++++++++++++++++++++------
 test/T095-address.sh               | 48 +++++++++++++++++++++++++++++++++++++
 5 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index 94ea2d5..db152f3 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -332,7 +332,7 @@ _notmuch_address()
 	    return
 	    ;;
 	--output)
-	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
+	    COMPREPLY=( $( compgen -W "sender recipients count" -- "${cur}" ) )
 	    return
 	    ;;
 	--sort)
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index c606b75..8968562 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -61,7 +61,7 @@ _notmuch_address()
 {
   _arguments -s : \
     '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
-    '--output=[select what to output]:output:((sender recipients))'
+    '--output=[select what to output]:output:((sender recipients count))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index 96512b7..18473a7 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -48,6 +48,13 @@ Supported options for **address** include
             Output all addresses from the *To*, *Cc* and *Bcc*
             headers.
 
+	**count**
+	    Print the count of how many times was the address
+	    encountered during search.
+
+	    Note: With this option, addresses are printed only after
+	    the whole search is finished. This may take long time.
+
     ``--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 741702a..d99e530 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -33,6 +33,7 @@ typedef enum {
     /* Address command */
     OUTPUT_SENDER	= 1 << 5,
     OUTPUT_RECIPIENTS	= 1 << 6,
+    OUTPUT_COUNT	= 1 << 7,
 } output_t;
 
 typedef enum {
@@ -59,6 +60,7 @@ typedef struct {
 typedef struct {
     const char *name;
     const char *addr;
+    int count;
 } mailbox_t;
 
 /* Return two stable query strings that identify exactly the matched
@@ -247,17 +249,24 @@ is_duplicate (const search_context_t *ctx, const char *name, const char *addr)
 {
     notmuch_bool_t duplicate;
     char *key;
+    mailbox_t *mailbox;
 
     key = talloc_asprintf (ctx->format, "%s <%s>", name, addr);
     if (! key)
 	return FALSE;
 
-    duplicate = g_hash_table_lookup_extended (ctx->addresses, key, NULL, NULL);
+    duplicate = g_hash_table_lookup_extended (ctx->addresses, key, NULL, (gpointer)&mailbox);
 
-    if (! duplicate)
-	g_hash_table_insert (ctx->addresses, key, NULL);
-    else
+    if (! duplicate) {
+	mailbox = talloc (ctx->format, mailbox_t);
+	mailbox->name = talloc_strdup (mailbox, name);
+	mailbox->addr = talloc_strdup (mailbox, addr);
+	mailbox->count = 1;
+	g_hash_table_insert (ctx->addresses, key, mailbox);
+    } else {
+	mailbox->count++;
 	talloc_free (key);
+    }
 
     return duplicate;
 }
@@ -267,6 +276,7 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
 {
     const char *name = mailbox->name;
     const char *addr = mailbox->addr;
+    int count = mailbox->count;
     sprinter_t *format = ctx->format;
     InternetAddress *ia = internet_address_mailbox_new (name, addr);
     char *name_addr;
@@ -276,6 +286,10 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
     name_addr = internet_address_to_string (ia, FALSE);
 
     if (format->is_text_printer) {
+	if (count > 0) {
+	    format->integer (format, count);
+	    format->string (format, "\t");
+	}
 	format->string (format, name_addr);
 	format->separator (format);
     } else {
@@ -286,6 +300,10 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
 	format->string (format, addr);
 	format->map_key (format, "name-addr");
 	format->string (format, name_addr);
+	if (count > 0) {
+	    format->map_key (format, "count");
+	    format->integer (format, count);
+	}
 	format->end (format);
 	format->separator (format);
     }
@@ -294,7 +312,7 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
     g_free (name_addr);
 }
 
-/* Print addresses from InternetAddressList.  */
+/* Print or prepare for printing addresses from InternetAddressList. */
 static void
 process_address_list (const search_context_t *ctx,
 		      InternetAddressList *list)
@@ -319,17 +337,21 @@ process_address_list (const search_context_t *ctx,
 	    mailbox_t mbx = {
 		.name = internet_address_get_name (address),
 		.addr = internet_address_mailbox_get_addr (mailbox),
+		.count = 0,
 	    };
 
 	    if (is_duplicate (ctx, mbx.name, mbx.addr))
 		continue;
 
+	    if (ctx->output & OUTPUT_COUNT)
+		continue;
+
 	    print_mailbox (ctx, &mbx);
 	}
     }
 }
 
-/* Print addresses from a message header.  */
+/* Print or prepare for printing addresses from a message header. */
 static void
 process_address_header (const search_context_t *ctx, const char *value)
 {
@@ -353,6 +375,15 @@ _my_talloc_free_for_g_hash (void *ptr)
     talloc_free (ptr);
 }
 
+static void
+print_hash_value (unused (gpointer key), gpointer value, gpointer user_data)
+{
+    const mailbox_t *mailbox = value;
+    search_context_t *ctx = user_data;
+
+    print_mailbox (ctx, mailbox);
+}
+
 static int
 _count_filenames (notmuch_message_t *message)
 {
@@ -448,6 +479,9 @@ do_search_messages (search_context_t *ctx)
 	notmuch_message_destroy (message);
     }
 
+    if (ctx->addresses && ctx->output & OUTPUT_COUNT)
+	g_hash_table_foreach (ctx->addresses, print_hash_value, ctx);
+
     notmuch_messages_destroy (messages);
 
     format->end (format);
@@ -685,6 +719,7 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
 	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
 	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
 				  { "recipients", OUTPUT_RECIPIENTS },
+				  { "count", OUTPUT_COUNT },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
 	{ 0, 0, 0, 0, 0 }
@@ -702,7 +737,7 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
 
     ctx->addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
-					    _my_talloc_free_for_g_hash, NULL);
+					    _my_talloc_free_for_g_hash, _my_talloc_free_for_g_hash);
 
     ret = do_search_messages (ctx);
 
diff --git a/test/T095-address.sh b/test/T095-address.sh
index 8a256d2..92e17b0 100755
--- a/test/T095-address.sh
+++ b/test/T095-address.sh
@@ -96,5 +96,53 @@ notmuch address --output=sender --output=recipients '*' >OUTPUT
 # Use EXPECTED from previous subtest
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "--output=sender --output=count"
+notmuch address --output=sender --output=count '*' | sort -n >OUTPUT
+cat <<EOF >EXPECTED
+1	Adrian Perez de Castro <aperez@igalia.com>
+1	Aron Griffis <agriffis@n01se.net>
+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>
+1	Olivier Berger <olivier.berger@it-sudparis.eu>
+1	Rolland Santimano <rollandsantimano@yahoo.com>
+2	Alex Botero-Lowry <alex.boterolowry@gmail.com>
+2	Jjgod Jiang <gzjjgod@gmail.com>
+3	Stewart Smith <stewart@flamingspork.com>
+4	Alexander Botero-Lowry <alex.boterolowry@gmail.com>
+4	Jan Janak <jan@ryngle.com>
+5	Lars Kellogg-Stedman <lars@seas.harvard.edu>
+5	Mikhail Gusarov <dottedmag@dottedmag.net>
+7	Keith Packard <keithp@keithp.com>
+12	Carl Worth <cworth@cworth.org>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=sender --output=count --format=json"
+# Since the iteration order of GHashTable is not specified, we
+# preprocess and sort the results to keep the order stable here.
+notmuch address --output=sender --output=count --format=json '*' | \
+    sed -e 's/^\[//' -e 's/]$//' -e 's/,$//' | sort >OUTPUT
+cat <<EOF >EXPECTED
+{"name": "Adrian Perez de Castro", "address": "aperez@igalia.com", "name-addr": "Adrian Perez de Castro <aperez@igalia.com>", "count": 1}
+{"name": "Alex Botero-Lowry", "address": "alex.boterolowry@gmail.com", "name-addr": "Alex Botero-Lowry <alex.boterolowry@gmail.com>", "count": 2}
+{"name": "Alexander Botero-Lowry", "address": "alex.boterolowry@gmail.com", "name-addr": "Alexander Botero-Lowry <alex.boterolowry@gmail.com>", "count": 4}
+{"name": "Aron Griffis", "address": "agriffis@n01se.net", "name-addr": "Aron Griffis <agriffis@n01se.net>", "count": 1}
+{"name": "Carl Worth", "address": "cworth@cworth.org", "name-addr": "Carl Worth <cworth@cworth.org>", "count": 12}
+{"name": "Chris Wilson", "address": "chris@chris-wilson.co.uk", "name-addr": "Chris Wilson <chris@chris-wilson.co.uk>", "count": 1}
+{"name": "François Boulogne", "address": "boulogne.f@gmail.com", "name-addr": "François Boulogne <boulogne.f@gmail.com>", "count": 1}
+{"name": "Ingmar Vanhassel", "address": "ingmar@exherbo.org", "name-addr": "Ingmar Vanhassel <ingmar@exherbo.org>", "count": 1}
+{"name": "Israel Herraiz", "address": "isra@herraiz.org", "name-addr": "Israel Herraiz <isra@herraiz.org>", "count": 1}
+{"name": "Jan Janak", "address": "jan@ryngle.com", "name-addr": "Jan Janak <jan@ryngle.com>", "count": 4}
+{"name": "Jjgod Jiang", "address": "gzjjgod@gmail.com", "name-addr": "Jjgod Jiang <gzjjgod@gmail.com>", "count": 2}
+{"name": "Keith Packard", "address": "keithp@keithp.com", "name-addr": "Keith Packard <keithp@keithp.com>", "count": 7}
+{"name": "Lars Kellogg-Stedman", "address": "lars@seas.harvard.edu", "name-addr": "Lars Kellogg-Stedman <lars@seas.harvard.edu>", "count": 5}
+{"name": "Mikhail Gusarov", "address": "dottedmag@dottedmag.net", "name-addr": "Mikhail Gusarov <dottedmag@dottedmag.net>", "count": 5}
+{"name": "Olivier Berger", "address": "olivier.berger@it-sudparis.eu", "name-addr": "Olivier Berger <olivier.berger@it-sudparis.eu>", "count": 1}
+{"name": "Rolland Santimano", "address": "rollandsantimano@yahoo.com", "name-addr": "Rolland Santimano <rollandsantimano@yahoo.com>", "count": 1}
+{"name": "Stewart Smith", "address": "stewart@flamingspork.com", "name-addr": "Stewart Smith <stewart@flamingspork.com>", "count": 3}
+EOF
+test_expect_equal_file OUTPUT EXPECTED
 
 test_done
-- 
2.1.1

Re: [PATCH v2 09/10] cli: address: Add --output=count

[Mark Walters]

On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> This output prints how many times was each address encountered during
> search.
> ---
>  completion/notmuch-completion.bash |  2 +-
>  completion/notmuch-completion.zsh  |  2 +-
>  doc/man1/notmuch-address.rst       |  7 ++++++
>  notmuch-search.c                   | 49 ++++++++++++++++++++++++++++++++------
>  test/T095-address.sh               | 48 +++++++++++++++++++++++++++++++++++++
>  5 files changed, 99 insertions(+), 9 deletions(-)
>
> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
> index 94ea2d5..db152f3 100644
> --- a/completion/notmuch-completion.bash
> +++ b/completion/notmuch-completion.bash
> @@ -332,7 +332,7 @@ _notmuch_address()
>  	    return
>  	    ;;
>  	--output)
> -	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
> +	    COMPREPLY=( $( compgen -W "sender recipients count" -- "${cur}" ) )
>  	    return
>  	    ;;
>  	--sort)
> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
> index c606b75..8968562 100644
> --- a/completion/notmuch-completion.zsh
> +++ b/completion/notmuch-completion.zsh
> @@ -61,7 +61,7 @@ _notmuch_address()
>  {
>    _arguments -s : \
>      '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
> -    '--output=[select what to output]:output:((sender recipients))'
> +    '--output=[select what to output]:output:((sender recipients count))'
>  }
>  
>  _notmuch()
> diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
> index 96512b7..18473a7 100644
> --- a/doc/man1/notmuch-address.rst
> +++ b/doc/man1/notmuch-address.rst
> @@ -48,6 +48,13 @@ Supported options for **address** include
>              Output all addresses from the *To*, *Cc* and *Bcc*
>              headers.
>  
> +	**count**
> +	    Print the count of how many times was the address
> +	    encountered during search.
> +
> +	    Note: With this option, addresses are printed only after
> +	    the whole search is finished. This may take long time.
> +

Hi

I think count should be added to the --output=(sender|recipients) line a
little above this.

Also it should say that count ignores the sort order (or possibly that
should go with sort?)

Best wishes

Mark



>      ``--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 741702a..d99e530 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -33,6 +33,7 @@ typedef enum {
>      /* Address command */
>      OUTPUT_SENDER	= 1 << 5,
>      OUTPUT_RECIPIENTS	= 1 << 6,
> +    OUTPUT_COUNT	= 1 << 7,
>  } output_t;
>  
>  typedef enum {
> @@ -59,6 +60,7 @@ typedef struct {
>  typedef struct {
>      const char *name;
>      const char *addr;
> +    int count;
>  } mailbox_t;
>  
>  /* Return two stable query strings that identify exactly the matched
> @@ -247,17 +249,24 @@ is_duplicate (const search_context_t *ctx, const char *name, const char *addr)
>  {
>      notmuch_bool_t duplicate;
>      char *key;
> +    mailbox_t *mailbox;
>  
>      key = talloc_asprintf (ctx->format, "%s <%s>", name, addr);
>      if (! key)
>  	return FALSE;
>  
> -    duplicate = g_hash_table_lookup_extended (ctx->addresses, key, NULL, NULL);
> +    duplicate = g_hash_table_lookup_extended (ctx->addresses, key, NULL, (gpointer)&mailbox);
>  
> -    if (! duplicate)
> -	g_hash_table_insert (ctx->addresses, key, NULL);
> -    else
> +    if (! duplicate) {
> +	mailbox = talloc (ctx->format, mailbox_t);
> +	mailbox->name = talloc_strdup (mailbox, name);
> +	mailbox->addr = talloc_strdup (mailbox, addr);
> +	mailbox->count = 1;
> +	g_hash_table_insert (ctx->addresses, key, mailbox);
> +    } else {
> +	mailbox->count++;
>  	talloc_free (key);
> +    }
>  
>      return duplicate;
>  }
> @@ -267,6 +276,7 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
>  {
>      const char *name = mailbox->name;
>      const char *addr = mailbox->addr;
> +    int count = mailbox->count;
>      sprinter_t *format = ctx->format;
>      InternetAddress *ia = internet_address_mailbox_new (name, addr);
>      char *name_addr;
> @@ -276,6 +286,10 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
>      name_addr = internet_address_to_string (ia, FALSE);
>  
>      if (format->is_text_printer) {
> +	if (count > 0) {
> +	    format->integer (format, count);
> +	    format->string (format, "\t");
> +	}
>  	format->string (format, name_addr);
>  	format->separator (format);
>      } else {
> @@ -286,6 +300,10 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
>  	format->string (format, addr);
>  	format->map_key (format, "name-addr");
>  	format->string (format, name_addr);
> +	if (count > 0) {
> +	    format->map_key (format, "count");
> +	    format->integer (format, count);
> +	}
>  	format->end (format);
>  	format->separator (format);
>      }
> @@ -294,7 +312,7 @@ print_mailbox (const search_context_t *ctx, const mailbox_t *mailbox)
>      g_free (name_addr);
>  }
>  
> -/* Print addresses from InternetAddressList.  */
> +/* Print or prepare for printing addresses from InternetAddressList. */
>  static void
>  process_address_list (const search_context_t *ctx,
>  		      InternetAddressList *list)
> @@ -319,17 +337,21 @@ process_address_list (const search_context_t *ctx,
>  	    mailbox_t mbx = {
>  		.name = internet_address_get_name (address),
>  		.addr = internet_address_mailbox_get_addr (mailbox),
> +		.count = 0,
>  	    };
>  
>  	    if (is_duplicate (ctx, mbx.name, mbx.addr))
>  		continue;
>  
> +	    if (ctx->output & OUTPUT_COUNT)
> +		continue;
> +
>  	    print_mailbox (ctx, &mbx);
>  	}
>      }
>  }
>  
> -/* Print addresses from a message header.  */
> +/* Print or prepare for printing addresses from a message header. */
>  static void
>  process_address_header (const search_context_t *ctx, const char *value)
>  {
> @@ -353,6 +375,15 @@ _my_talloc_free_for_g_hash (void *ptr)
>      talloc_free (ptr);
>  }
>  
> +static void
> +print_hash_value (unused (gpointer key), gpointer value, gpointer user_data)
> +{
> +    const mailbox_t *mailbox = value;
> +    search_context_t *ctx = user_data;
> +
> +    print_mailbox (ctx, mailbox);
> +}
> +
>  static int
>  _count_filenames (notmuch_message_t *message)
>  {
> @@ -448,6 +479,9 @@ do_search_messages (search_context_t *ctx)
>  	notmuch_message_destroy (message);
>      }
>  
> +    if (ctx->addresses && ctx->output & OUTPUT_COUNT)
> +	g_hash_table_foreach (ctx->addresses, print_hash_value, ctx);
> +
>      notmuch_messages_destroy (messages);
>  
>      format->end (format);
> @@ -685,6 +719,7 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
>  	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
>  	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
>  				  { "recipients", OUTPUT_RECIPIENTS },
> +				  { "count", OUTPUT_COUNT },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>  	{ 0, 0, 0, 0, 0 }
> @@ -702,7 +737,7 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
>  	return EXIT_FAILURE;
>  
>      ctx->addresses = g_hash_table_new_full (g_str_hash, g_str_equal,
> -					    _my_talloc_free_for_g_hash, NULL);
> +					    _my_talloc_free_for_g_hash, _my_talloc_free_for_g_hash);
>  
>      ret = do_search_messages (ctx);
>  
> diff --git a/test/T095-address.sh b/test/T095-address.sh
> index 8a256d2..92e17b0 100755
> --- a/test/T095-address.sh
> +++ b/test/T095-address.sh
> @@ -96,5 +96,53 @@ notmuch address --output=sender --output=recipients '*' >OUTPUT
>  # Use EXPECTED from previous subtest
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "--output=sender --output=count"
> +notmuch address --output=sender --output=count '*' | sort -n >OUTPUT
> +cat <<EOF >EXPECTED
> +1	Adrian Perez de Castro <aperez@igalia.com>
> +1	Aron Griffis <agriffis@n01se.net>
> +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>
> +1	Olivier Berger <olivier.berger@it-sudparis.eu>
> +1	Rolland Santimano <rollandsantimano@yahoo.com>
> +2	Alex Botero-Lowry <alex.boterolowry@gmail.com>
> +2	Jjgod Jiang <gzjjgod@gmail.com>
> +3	Stewart Smith <stewart@flamingspork.com>
> +4	Alexander Botero-Lowry <alex.boterolowry@gmail.com>
> +4	Jan Janak <jan@ryngle.com>
> +5	Lars Kellogg-Stedman <lars@seas.harvard.edu>
> +5	Mikhail Gusarov <dottedmag@dottedmag.net>
> +7	Keith Packard <keithp@keithp.com>
> +12	Carl Worth <cworth@cworth.org>
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=sender --output=count --format=json"
> +# Since the iteration order of GHashTable is not specified, we
> +# preprocess and sort the results to keep the order stable here.
> +notmuch address --output=sender --output=count --format=json '*' | \
> +    sed -e 's/^\[//' -e 's/]$//' -e 's/,$//' | sort >OUTPUT
> +cat <<EOF >EXPECTED
> +{"name": "Adrian Perez de Castro", "address": "aperez@igalia.com", "name-addr": "Adrian Perez de Castro <aperez@igalia.com>", "count": 1}
> +{"name": "Alex Botero-Lowry", "address": "alex.boterolowry@gmail.com", "name-addr": "Alex Botero-Lowry <alex.boterolowry@gmail.com>", "count": 2}
> +{"name": "Alexander Botero-Lowry", "address": "alex.boterolowry@gmail.com", "name-addr": "Alexander Botero-Lowry <alex.boterolowry@gmail.com>", "count": 4}
> +{"name": "Aron Griffis", "address": "agriffis@n01se.net", "name-addr": "Aron Griffis <agriffis@n01se.net>", "count": 1}
> +{"name": "Carl Worth", "address": "cworth@cworth.org", "name-addr": "Carl Worth <cworth@cworth.org>", "count": 12}
> +{"name": "Chris Wilson", "address": "chris@chris-wilson.co.uk", "name-addr": "Chris Wilson <chris@chris-wilson.co.uk>", "count": 1}
> +{"name": "François Boulogne", "address": "boulogne.f@gmail.com", "name-addr": "François Boulogne <boulogne.f@gmail.com>", "count": 1}
> +{"name": "Ingmar Vanhassel", "address": "ingmar@exherbo.org", "name-addr": "Ingmar Vanhassel <ingmar@exherbo.org>", "count": 1}
> +{"name": "Israel Herraiz", "address": "isra@herraiz.org", "name-addr": "Israel Herraiz <isra@herraiz.org>", "count": 1}
> +{"name": "Jan Janak", "address": "jan@ryngle.com", "name-addr": "Jan Janak <jan@ryngle.com>", "count": 4}
> +{"name": "Jjgod Jiang", "address": "gzjjgod@gmail.com", "name-addr": "Jjgod Jiang <gzjjgod@gmail.com>", "count": 2}
> +{"name": "Keith Packard", "address": "keithp@keithp.com", "name-addr": "Keith Packard <keithp@keithp.com>", "count": 7}
> +{"name": "Lars Kellogg-Stedman", "address": "lars@seas.harvard.edu", "name-addr": "Lars Kellogg-Stedman <lars@seas.harvard.edu>", "count": 5}
> +{"name": "Mikhail Gusarov", "address": "dottedmag@dottedmag.net", "name-addr": "Mikhail Gusarov <dottedmag@dottedmag.net>", "count": 5}
> +{"name": "Olivier Berger", "address": "olivier.berger@it-sudparis.eu", "name-addr": "Olivier Berger <olivier.berger@it-sudparis.eu>", "count": 1}
> +{"name": "Rolland Santimano", "address": "rollandsantimano@yahoo.com", "name-addr": "Rolland Santimano <rollandsantimano@yahoo.com>", "count": 1}
> +{"name": "Stewart Smith", "address": "stewart@flamingspork.com", "name-addr": "Stewart Smith <stewart@flamingspork.com>", "count": 3}
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
>  
>  test_done
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 10/10] cli: address: Add --filter-by option to configure address filtering

[Michal Sojka]

This option allows to configure the criterion for duplicate address
filtering. Without this option, all unique combinations of name and
address parts are printed. This option allows to filter the output
more, for example to only contain unique address parts.
---
 completion/notmuch-completion.bash |  6 +++-
 completion/notmuch-completion.zsh  |  1 +
 doc/man1/notmuch-address.rst       | 36 ++++++++++++++++++-
 notmuch-search.c                   | 48 +++++++++++++++++++++++--
 test/T097-address-filter-by.sh     | 73 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 160 insertions(+), 4 deletions(-)
 create mode 100755 test/T097-address-filter-by.sh

diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
index db152f3..2cb1586 100644
--- a/completion/notmuch-completion.bash
+++ b/completion/notmuch-completion.bash
@@ -310,7 +310,7 @@ _notmuch_search()
     ! $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}) )
 	    ;;
@@ -343,6 +343,10 @@ _notmuch_address()
 	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
 	    return
 	    ;;
+	--filter-by)
+	    COMPREPLY=( $( compgen -W "nameaddr name addr addrfold nameaddrfold" -- "${cur}" ) )
+	    return
+	    ;;
     esac
 
     ! $split &&
diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
index 8968562..3758f1a 100644
--- a/completion/notmuch-completion.zsh
+++ b/completion/notmuch-completion.zsh
@@ -62,6 +62,7 @@ _notmuch_address()
   _arguments -s : \
     '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
     '--output=[select what to output]:output:((sender recipients count))'
+    '--filter-by=[filter out duplicate addresses]:filter-by:((nameaddr\:"both name and address part" name\:"name part" addr\:"address part" addrfold\:"case-insensitive address part" nameaddrfold\:"name and case-insensitive address part"))'
 }
 
 _notmuch()
diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
index 18473a7..524ab91 100644
--- a/doc/man1/notmuch-address.rst
+++ b/doc/man1/notmuch-address.rst
@@ -11,7 +11,8 @@ DESCRIPTION
 ===========
 
 Search for messages matching the given search terms, and display the
-addresses from them. Duplicate addresses are filtered out.
+addresses from them. Duplicate addresses are filtered out. Filtering
+can be configured with the --filter-by option.
 
 See **notmuch-search-terms(7)** for details of the supported syntax for
 <search-terms>.
@@ -85,6 +86,39 @@ Supported options for **address** include
         is the number of matching non-excluded messages in the thread,
         rather than the number of matching messages.
 
+    ``--filter-by=``\ (**nameaddr**\ \|\ **name** \|\ **addr**\ \|\ **addrfold**\ \|\ **nameaddrfold**\)
+
+	Controls how 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 the two addresses are compared:
+
+	**nameaddr** means that both name and address parts are
+	compared in case-sensitive manner. Therefore, all same looking
+	addresses strings are considered duplicate. This is the
+	default.
+
+	**name** means that only 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.
+
+	**addr** means that only 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 like **addr**, but comparison is done in
+	canse-insensitive manner. For example, the addresses "John Doe
+	<john@example.com>" and "Dr. John Doe <JOHN@EXAMPLE.COM>" will
+	be considered duplicate.
+
+	**nameaddrfold** is like **nameaddr**, but address comparison
+	is done in canse-insensitive manner. For example, the
+	addresses "John Doe <john@example.com>" and "John Doe
+	<JOHN@EXAMPLE.COM>" will be considered duplicate.
+
 EXIT STATUS
 ===========
 
diff --git a/notmuch-search.c b/notmuch-search.c
index d99e530..04e33c6 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -43,6 +43,14 @@ typedef enum {
     NOTMUCH_FORMAT_SEXP
 } format_sel_t;
 
+typedef enum {
+    FILTER_BY_NAMEADDR = 0,
+    FILTER_BY_NAME,
+    FILTER_BY_ADDR,
+    FILTER_BY_ADDRFOLD,
+    FILTER_BY_NAMEADDRFOLD,
+} filter_by_t;
+
 typedef struct {
     notmuch_database_t *notmuch;
     format_sel_t format_sel;
@@ -55,6 +63,7 @@ typedef struct {
     int limit;
     int dupe;
     GHashTable *addresses;
+    filter_by_t filter_by;
 } search_context_t;
 
 typedef struct {
@@ -243,15 +252,42 @@ do_search_threads (search_context_t *ctx)
     return 0;
 }
 
-/* Returns TRUE iff name and addr is duplicate. */
+/* Returns TRUE iff name and/or addr is considered duplicate. */
 static notmuch_bool_t
 is_duplicate (const search_context_t *ctx, const char *name, const char *addr)
 {
     notmuch_bool_t duplicate;
     char *key;
+    gchar *addrfold = NULL;
     mailbox_t *mailbox;
 
-    key = talloc_asprintf (ctx->format, "%s <%s>", name, addr);
+    if (ctx->filter_by == FILTER_BY_ADDRFOLD ||
+	ctx->filter_by == FILTER_BY_NAMEADDRFOLD)
+	addrfold = g_utf8_casefold (addr, -1);
+
+    switch (ctx->filter_by) {
+    case FILTER_BY_NAMEADDR:
+	key = talloc_asprintf (ctx->format, "%s <%s>", name, addr);
+	break;
+    case FILTER_BY_NAMEADDRFOLD:
+	key = talloc_asprintf (ctx->format, "%s <%s>", name, addrfold);
+	break;
+    case FILTER_BY_NAME:
+	key = talloc_strdup (ctx->format, name); /* !name results in !key */
+	break;
+    case FILTER_BY_ADDR:
+	key = talloc_strdup (ctx->format, addr);
+	break;
+    case FILTER_BY_ADDRFOLD:
+	key = talloc_strdup (ctx->format, addrfold);
+	break;
+    default:
+	INTERNAL_ERROR("invalid --filter-by flags");
+    }
+
+    if (addrfold)
+	g_free (addrfold);
+
     if (! key)
 	return FALSE;
 
@@ -721,10 +757,18 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
 				  { "recipients", OUTPUT_RECIPIENTS },
 				  { "count", OUTPUT_COUNT },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD, &ctx->filter_by, "filter-by", 'b',
+	  (notmuch_keyword_t []){ { "nameaddr", FILTER_BY_NAMEADDR },
+				  { "name", FILTER_BY_NAME },
+				  { "addr", FILTER_BY_ADDR },
+				  { "addrfold", FILTER_BY_ADDRFOLD },
+				  { "nameaddrfold", FILTER_BY_NAMEADDRFOLD },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
+    ctx->filter_by = FILTER_BY_NAMEADDR,
     opt_index = parse_arguments (argc, argv, options, 1);
     if (opt_index < 0)
 	return EXIT_FAILURE;
diff --git a/test/T097-address-filter-by.sh b/test/T097-address-filter-by.sh
new file mode 100755
index 0000000..544d8e8
--- /dev/null
+++ b/test/T097-address-filter-by.sh
@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+test_description='duplicite address filtering in "notmuch address"'
+. ./test-lib.sh
+
+add_message '[to]="John Doe <foo@example.com>, John Doe <bar@example.com>"'
+add_message '[to]="\"Doe, John\" <foo@example.com>"' '[cc]="John Doe <Bar@Example.COM>"'
+add_message '[to]="\"Doe, John\" <foo@example.com>"' '[bcc]="John Doe <Bar@Example.COM>"'
+
+test_begin_subtest "--output=recipients"
+notmuch address --output=recipients "*" >OUTPUT
+cat <<EOF >EXPECTED
+John Doe <foo@example.com>
+John Doe <bar@example.com>
+"Doe, John" <foo@example.com>
+John Doe <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=nameaddr"
+notmuch address --output=recipients --filter-by=nameaddr "*" >OUTPUT
+# The same as above
+cat <<EOF >EXPECTED
+John Doe <foo@example.com>
+John Doe <bar@example.com>
+"Doe, John" <foo@example.com>
+John Doe <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=name"
+notmuch address --output=recipients --filter-by=name "*" >OUTPUT
+cat <<EOF >EXPECTED
+John Doe <foo@example.com>
+"Doe, John" <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=addr"
+notmuch address --output=recipients --filter-by=addr "*" >OUTPUT
+cat <<EOF >EXPECTED
+John Doe <foo@example.com>
+John Doe <bar@example.com>
+John Doe <Bar@Example.COM>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=addrfold"
+notmuch address --output=recipients --filter-by=addrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+John Doe <foo@example.com>
+John Doe <bar@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=nameaddrfold"
+notmuch address --output=recipients --filter-by=nameaddrfold "*" >OUTPUT
+cat <<EOF >EXPECTED
+John Doe <foo@example.com>
+John Doe <bar@example.com>
+"Doe, John" <foo@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "--output=recipients --filter-by=nameaddrfold --output=count"
+notmuch address --output=recipients --filter-by=nameaddrfold --output=count "*" | sort -n >OUTPUT
+cat <<EOF >EXPECTED
+1	John Doe <foo@example.com>
+2	"Doe, John" <foo@example.com>
+3	John Doe <bar@example.com>
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
-- 
2.1.1

Re: [PATCH v2 00/10] "notmuch address" command

[Mark Walters]

On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> Hi all,
>
> this is v2 of "notmuch address" patchset. It obsoletes [1].
>
> Don't be scared by the number of patches. Most of them are trivial
> refactoring. Patches 1-4 refactor the code so that "notmuch search"
> command is easier to split. Patch 5 is Jani's hierarchical command
> line parsing patch. Patch 6 splits search functionality to new address
> command. Patch 7 is minor refactoring. Patches 8-10 correspond to
> patches 5-7 in the original "notmuch search
> --output=sender/recipients" patch series [2].
>
> Changes from v1:
>
> - Rebased to current master (conflicted with Jani's "notmuch search
>   --duplicate=N with --output=messages" patch)
> - Fixed printing of false "Unrecognized option" error message in
>   hierarchical command line parser.

Overall this series looks good. My inclination is to leave patch 10
until after the freeze but I am happy either way. Tests all work and my
testing is fine. 

A couple of other comments:

you should add something to devel/schemata detailing the fields in the
structured address output.

Please check the new manpage: I think some of it is slightly outdated. I
mentioned in my comments the exclude=false, but missed exclude=all. Also
the manpage could say what actually gets printed in the default text
case (in particular to emphasise that it includes the real name)

Best wishes

Mark


>
> Regards,
> -Michal
>
> [1] id:1414889400-30977-1-git-send-email-sojkam1@fel.cvut.cz
> [2] id:1414792441-29555-1-git-send-email-sojkam1@fel.cvut.cz
>
>
> Jani Nikula (1):
>   cli: add support for hierarchical command line option arrays
>
> Michal Sojka (9):
>   cli: search: Rename options to context
>   cli: search: Move more variables into search_context_t
>   cli: search: Convert ctx. to ctx->
>   cli: search: Split notmuch_search_command to smaller functions
>   cli: Introduce "notmuch address" command
>   cli: search: Convert --output to keyword argument
>   cli: address: Do not output duplicate addresses
>   cli: address: Add --output=count
>   cli: address: Add --filter-by option to configure address filtering
>
>  command-line-arguments.c           |  16 +-
>  command-line-arguments.h           |   1 +
>  completion/notmuch-completion.bash |  48 +++-
>  completion/notmuch-completion.zsh  |  11 +-
>  doc/man1/notmuch-address.rst       | 140 ++++++++++++
>  doc/man1/notmuch-search.rst        |  21 +-
>  doc/man1/notmuch.rst               |   7 +-
>  notmuch-client.h                   |   3 +
>  notmuch-search.c                   | 454 +++++++++++++++++++++++++------------
>  notmuch.c                          |   2 +
>  test/T095-address.sh               | 148 ++++++++++++
>  test/T097-address-filter-by.sh     |  73 ++++++
>  12 files changed, 751 insertions(+), 173 deletions(-)
>  create mode 100644 doc/man1/notmuch-address.rst
>  create mode 100755 test/T095-address.sh
>  create mode 100755 test/T097-address-filter-by.sh
>
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 00/10] "notmuch address" command

[Tomi Ollila]

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

> Hi all,
>
> this is v2 of "notmuch address" patchset. It obsoletes [1].
>
> Don't be scared by the number of patches. Most of them are trivial
> refactoring. Patches 1-4 refactor the code so that "notmuch search"
> command is easier to split. Patch 5 is Jani's hierarchical command
> line parsing patch. Patch 6 splits search functionality to new address
> command. Patch 7 is minor refactoring. Patches 8-10 correspond to
> patches 5-7 in the original "notmuch search
> --output=sender/recipients" patch series [2].

Haven't got time to test these (will do tomorrow). Feature-wise I support
having patches 1-9 applied now(*). (and leave 10 to be bikeshedded post
0.19)

Tomi

(*) while testing currennt HEAD i found nomuc cearch --output=sender
date:1d.. | uniq useful, have to see how to have this later...

(Sent using N9)

>
> Changes from v1:
>
> - Rebased to current master (conflicted with Jani's "notmuch search
>   --duplicate=N with --output=messages" patch)
> - Fixed printing of false "Unrecognized option" error message in
>   hierarchical command line parser.
>
> Regards,
> -Michal
>
> [1] id:1414889400-30977-1-git-send-email-sojkam1@fel.cvut.cz
> [2] id:1414792441-29555-1-git-send-email-sojkam1@fel.cvut.cz
>
>
> Jani Nikula (1):
>   cli: add support for hierarchical command line option arrays
>
> Michal Sojka (9):
>   cli: search: Rename options to context
>   cli: search: Move more variables into search_context_t
>   cli: search: Convert ctx. to ctx->
>   cli: search: Split notmuch_search_command to smaller functions
>   cli: Introduce "notmuch address" command
>   cli: search: Convert --output to keyword argument
>   cli: address: Do not output duplicate addresses
>   cli: address: Add --output=count
>   cli: address: Add --filter-by option to configure address filtering
>
>  command-line-arguments.c           |  16 +-
>  command-line-arguments.h           |   1 +
>  completion/notmuch-completion.bash |  48 +++-
>  completion/notmuch-completion.zsh  |  11 +-
>  doc/man1/notmuch-address.rst       | 140 ++++++++++++
>  doc/man1/notmuch-search.rst        |  21 +-
>  doc/man1/notmuch.rst               |   7 +-
>  notmuch-client.h                   |   3 +
>  notmuch-search.c                   | 454 +++++++++++++++++++++++++------------
>  notmuch.c                          |   2 +
>  test/T095-address.sh               | 148 ++++++++++++
>  test/T097-address-filter-by.sh     |  73 ++++++
>  12 files changed, 751 insertions(+), 173 deletions(-)
>  create mode 100644 doc/man1/notmuch-address.rst
>  create mode 100755 test/T095-address.sh
>  create mode 100755 test/T097-address-filter-by.sh
>
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch