unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/3] Control notmuch show output
@ 2012-07-07 15:12 Mark Walters
  2012-07-07 15:12 ` [PATCH 1/3] cli: allow keyword lists in argument parser Mark Walters
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Mark Walters @ 2012-07-07 15:12 UTC (permalink / raw)
  To: notmuch

This is a first draft of a patch set allowing the caller to control
the output of notmuch show. This option should subsume the
--headers-only option (see
id:"1341041595-5858-1-git-send-email-markwalters1009@gmail.com") and
the reply-to patch
(id:"1340508470-16606-2-git-send-email-novalazy@gmail.com") as well as
being extensible.

It follows the suggestions made in
id:"20120704182459.GI2342@hili.localdomain" and
id:"CAB+hUn_sxy=QP1+OzGwKOcSYGi13Q61m4-bq+PGnYxCMPd0fvg@mail.gmail.com"

and modifies the cli parser to allow keyword lists of the form --output=from,cc,body

The first patch adds this functionality to the option parser. It uses
a bitfield to pass the flags specified so we are limited to 32 (or
perhaps 31) possible options.

The second patch implements this for the selection of which headers to
return. Since notmuch-reply.c wanted exactly this functionality (since
it wants in-reply-to headers and reference headers but not the date
header) it is converted to use this style.

The third patch uses this functionality to implement the
--headers-only functionality by adding a "body" option to the list of
things the user can choose to output or not. 

Currently, I have not written any tests for the new functionality,
updated the man page, or implemented for any format except JSON.

Do people have any comments in the current form?

Best wishes

Mark




Mark Walters (3):
  cli: allow keyword lists in argument parser.
  cli: show allow the caller to specify the headers output.
  cli: allow show to omit message bodies.

 command-line-arguments.c |   47 +++++++++++++++++++++++
 command-line-arguments.h |    3 +-
 notmuch-client.h         |   26 ++++++++++++-
 notmuch-reply.c          |   12 +++++-
 notmuch-show.c           |   92 +++++++++++++++++++++++++++++++--------------
 5 files changed, 146 insertions(+), 34 deletions(-)

-- 
1.7.9.1

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

* [PATCH 1/3] cli: allow keyword lists in argument parser.
  2012-07-07 15:12 [PATCH 0/3] Control notmuch show output Mark Walters
@ 2012-07-07 15:12 ` Mark Walters
  2012-07-07 15:12 ` [PATCH 2/3] cli: show allow the caller to specify the headers output Mark Walters
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2012-07-07 15:12 UTC (permalink / raw)
  To: notmuch

We allow keyword lists in the argument parser. The parser returns a
bitfield of the arguments present which is the union of the specified
bitfields for each option. Since it is the union the parser could
allow things like

--output-headers=default,reply-to

to get the default headers with reply-to added.
---
 command-line-arguments.c |   47 ++++++++++++++++++++++++++++++++++++++++++++++
 command-line-arguments.h |    3 +-
 2 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index b0a0dab..d322f9e 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -37,6 +37,50 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
 }
 
 static notmuch_bool_t
+_process_keyword_list (const notmuch_opt_desc_t *arg_desc, const char *arg_str) {
+
+    char *key_str, *final;
+    unsigned int matched_keys = 0;
+    notmuch_bool_t matched;
+    const notmuch_keyword_t *keywords;
+
+    key_str = strdup (arg_str);
+
+    do {
+	keywords = arg_desc->keywords;
+	matched = FALSE;
+
+	final = strrchr (key_str, ',');
+
+	if (final) {
+	    *final = '\0';
+	    final++;
+	} else {
+	    final = key_str;
+	}
+
+	while (keywords->name && !matched) {
+	    if (strcmp (final, keywords->name) == 0) {
+		matched_keys |= keywords->value;
+		matched = TRUE;
+	    }
+	    keywords++;
+	}
+	if (!matched) {
+	    fprintf (stderr, "unknown keyword: \'%s\' in list %s\n", final, arg_str);
+	    goto DONE;
+	}
+    } while (final != key_str);
+
+    if (arg_desc->output_var) {
+	*((unsigned int *)arg_desc->output_var) = matched_keys;
+    }
+DONE:
+    free (key_str);
+    return matched;
+}
+
+static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
 
     if (next == 0) {
@@ -121,6 +165,9 @@ parse_option (const char *arg,
 	    case NOTMUCH_OPT_KEYWORD:
 		return _process_keyword_arg (try, next, value);
 		break;
+	    case NOTMUCH_OPT_KEYWORD_LIST:
+		return _process_keyword_list (try, value);
+		break;
 	    case NOTMUCH_OPT_BOOLEAN:
 		return _process_boolean_arg (try, next, value);
 		break;
diff --git a/command-line-arguments.h b/command-line-arguments.h
index de1734a..f2b2275 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -9,7 +9,8 @@ enum notmuch_opt_type {
     NOTMUCH_OPT_INT,		/* --frob=8               */
     NOTMUCH_OPT_KEYWORD,	/* --format=raw|json|text */
     NOTMUCH_OPT_STRING,		/* --file=/tmp/gnarf.txt  */
-    NOTMUCH_OPT_POSITION	/* notmuch dump pos_arg   */
+    NOTMUCH_OPT_POSITION,	/* notmuch dump pos_arg   */
+    NOTMUCH_OPT_KEYWORD_LIST	/* --output=header,body   */
 };
 
 /*
-- 
1.7.9.1

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

* [PATCH 2/3] cli: show allow the caller to specify the headers output.
  2012-07-07 15:12 [PATCH 0/3] Control notmuch show output Mark Walters
  2012-07-07 15:12 ` [PATCH 1/3] cli: allow keyword lists in argument parser Mark Walters
@ 2012-07-07 15:12 ` Mark Walters
  2012-07-13 19:33   ` Jameson Graef Rollins
  2012-07-07 15:12 ` [PATCH 3/3] cli: allow show to omit message bodies Mark Walters
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2012-07-07 15:12 UTC (permalink / raw)
  To: notmuch

This uses the new parsing functionality to allow the caller of notmuch
show to specify which headers they want. Callers of format_part_json
should pass a bitfield specifiying the headers they want.

Since the functionality includes that contained in the previous
"reply" boolean passed notmuch-reply.c is converted to use this new
style.
---
 notmuch-client.h |   24 +++++++++++++-
 notmuch-reply.c  |   12 ++++++-
 notmuch-show.c   |   90 ++++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 93 insertions(+), 33 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 0c17b79..c241a7d 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -68,6 +68,24 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 typedef struct mime_node mime_node_t;
 struct notmuch_show_params;
 
+
+typedef enum {
+    NOTMUCH_SHOW_OUTPUT_HDR_DATE = 0x1,
+    NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT = 0x2,
+    NOTMUCH_SHOW_OUTPUT_HDR_FROM = 0x4,
+    NOTMUCH_SHOW_OUTPUT_HDR_TO = 0x8,
+    NOTMUCH_SHOW_OUTPUT_HDR_CC = 0x10,
+    NOTMUCH_SHOW_OUTPUT_HDR_REPLY_TO = 0x20,
+    NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO = 0x40,
+    NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES = 0x80,
+    NOTMUCH_SHOW_OUTPUT_DEFAULT =
+	    NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT |
+	    NOTMUCH_SHOW_OUTPUT_HDR_FROM |
+	    NOTMUCH_SHOW_OUTPUT_HDR_TO |
+	    NOTMUCH_SHOW_OUTPUT_HDR_CC |
+	    NOTMUCH_SHOW_OUTPUT_HDR_DATE
+} notmuch_show_output_t;
+
 typedef struct notmuch_show_format {
     const char *message_set_start;
     notmuch_status_t (*part) (const void *ctx,
@@ -90,6 +108,7 @@ typedef struct notmuch_show_params {
     notmuch_bool_t raw;
     int part;
     notmuch_crypto_t crypto;
+    notmuch_show_output_t output;
 } notmuch_show_params_t;
 
 /* There's no point in continuing when we've detected that we've done
@@ -176,10 +195,11 @@ notmuch_status_t
 show_one_part (const char *filename, int part);
 
 void
-format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first);
+format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first,
+		  notmuch_show_output_t output);
 
 void
-format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
+format_headers_json (const void *ctx, GMimeMessage *message, notmuch_show_output_t output);
 
 typedef enum {
     NOTMUCH_SHOW_TEXT_PART_REPLY = 1 << 0,
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 3a038ed..f4bf70d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -611,16 +611,24 @@ notmuch_reply_format_json(void *ctx,
     if (!reply)
 	return 1;
 
+    notmuch_show_output_t NOTMUCH_REPLY_OUTPUT =
+	NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT |
+	NOTMUCH_SHOW_OUTPUT_HDR_FROM |
+	NOTMUCH_SHOW_OUTPUT_HDR_TO |
+	NOTMUCH_SHOW_OUTPUT_HDR_CC |
+	NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO |
+	NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES;
+
     /* The headers of the reply message we've created */
     printf ("{\"reply-headers\": ");
-    format_headers_json (ctx, reply, TRUE);
+    format_headers_json (ctx, reply, NOTMUCH_REPLY_OUTPUT);
     g_object_unref (G_OBJECT (reply));
     reply = NULL;
 
     /* Start the original */
     printf (", \"original\": ");
 
-    format_part_json (ctx, node, TRUE);
+    format_part_json (ctx, node, TRUE, NOTMUCH_SHOW_OUTPUT_DEFAULT);
 
     /* End */
     printf ("}\n");
diff --git a/notmuch-show.c b/notmuch-show.c
index 8f3c60e..242e8e0 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -194,44 +194,58 @@ _is_from_line (const char *line)
 }
 
 void
-format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply)
+format_headers_json (const void *ctx, GMimeMessage *message, notmuch_show_output_t output)
 {
     void *local = talloc_new (ctx);
     InternetAddressList *recipients;
     const char *recipients_string;
+    const char *reply_to_string;
 
-    printf ("{%s: %s",
-	    json_quote_str (local, "Subject"),
-	    json_quote_str (local, g_mime_message_get_subject (message)));
-    printf (", %s: %s",
-	    json_quote_str (local, "From"),
-	    json_quote_str (local, g_mime_message_get_sender (message)));
-    recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
-    recipients_string = internet_address_list_to_string (recipients, 0);
-    if (recipients_string)
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT)
+	printf ("{%s: %s",
+		json_quote_str (local, "Subject"),
+		json_quote_str (local, g_mime_message_get_subject (message)));
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_FROM)
 	printf (", %s: %s",
-		json_quote_str (local, "To"),
-		json_quote_str (local, recipients_string));
-    recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
-    recipients_string = internet_address_list_to_string (recipients, 0);
-    if (recipients_string)
-	printf (", %s: %s",
-		json_quote_str (local, "Cc"),
-		json_quote_str (local, recipients_string));
-
-    if (reply) {
+		json_quote_str (local, "From"),
+		json_quote_str (local, g_mime_message_get_sender (message)));
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_TO) {
+	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
+	recipients_string = internet_address_list_to_string (recipients, 0);
+	if (recipients_string)
+	    printf (", %s: %s",
+		    json_quote_str (local, "To"),
+		    json_quote_str (local, recipients_string));
+    }
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_CC) {
+	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
+	recipients_string = internet_address_list_to_string (recipients, 0);
+	if (recipients_string)
+	    printf (", %s: %s",
+		    json_quote_str (local, "Cc"),
+		    json_quote_str (local, recipients_string));
+    }
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_REPLY_TO) {
+	reply_to_string = g_mime_message_get_reply_to (message);
+	if (reply_to_string)
+	    printf (", %s: %s",
+		    json_quote_str (local, "Reply-To"),
+		    json_quote_str (local, reply_to_string));
+    }
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO)
 	printf (", %s: %s",
 		json_quote_str (local, "In-reply-to"),
 		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "In-reply-to")));
 
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES)
 	printf (", %s: %s",
 		json_quote_str (local, "References"),
 		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "References")));
-    } else {
+
+    if (output & NOTMUCH_SHOW_OUTPUT_HDR_DATE)
 	printf (", %s: %s",
 		json_quote_str (local, "Date"),
 		json_quote_str (local, g_mime_message_get_date_as_string (message)));
-    }
 
     printf ("}");
 
@@ -559,7 +573,7 @@ format_part_text (const void *ctx, mime_node_t *node,
 }
 
 void
-format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
+format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first, notmuch_show_output_t output)
 {
     /* Any changes to the JSON format should be reflected in the file
      * devel/schemata. */
@@ -569,10 +583,10 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
 	format_message_json (ctx, node->envelope_file);
 
 	printf ("\"headers\": ");
-	format_headers_json (ctx, GMIME_MESSAGE (node->part), FALSE);
+	format_headers_json (ctx, GMIME_MESSAGE (node->part), output);
 
 	printf (", \"body\": [");
-	format_part_json (ctx, mime_node_child (node, 0), first);
+	format_part_json (ctx, mime_node_child (node, 0), first, output);
 
 	printf ("]}");
 	return;
@@ -643,7 +657,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
     } else if (GMIME_IS_MESSAGE (node->part)) {
 	printf (", \"content\": [{");
 	printf ("\"headers\": ");
-	format_headers_json (local, GMIME_MESSAGE (node->part), FALSE);
+	format_headers_json (local, GMIME_MESSAGE (node->part), output);
 
 	printf (", \"body\": [");
 	terminator = "]}]";
@@ -652,16 +666,16 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
     talloc_free (local);
 
     for (i = 0; i < node->nchildren; i++)
-	format_part_json (ctx, mime_node_child (node, i), i == 0);
+	format_part_json (ctx, mime_node_child (node, i), i == 0, output);
 
     printf ("%s}", terminator);
 }
 
 static notmuch_status_t
 format_part_json_entry (const void *ctx, mime_node_t *node, unused (int indent),
-			unused (const notmuch_show_params_t *params))
+			const notmuch_show_params_t *params)
 {
-    format_part_json (ctx, node, TRUE);
+    format_part_json (ctx, node, TRUE, params->output);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
@@ -1020,6 +1034,17 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 				  { "mbox", NOTMUCH_FORMAT_MBOX },
 				  { "raw", NOTMUCH_FORMAT_RAW },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD_LIST, &params.output, "output", 'o',
+	  (notmuch_keyword_t []){ { "default", NOTMUCH_SHOW_OUTPUT_DEFAULT },
+				  { "date", NOTMUCH_SHOW_OUTPUT_HDR_DATE },
+				  { "subject", NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT },
+				  { "from", NOTMUCH_SHOW_OUTPUT_HDR_FROM },
+				  { "to", NOTMUCH_SHOW_OUTPUT_HDR_TO },
+				  { "cc", NOTMUCH_SHOW_OUTPUT_HDR_CC },
+				  { "reply-to", NOTMUCH_SHOW_OUTPUT_HDR_REPLY_TO },
+				  { "in-reply-to", NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO },
+				  { "references", NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES },
+				  { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
 	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
 				  { "false", EXCLUDE_FALSE },
@@ -1053,6 +1078,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	    format_sel = NOTMUCH_FORMAT_TEXT;
     }
 
+    if (params.output) {
+	if (format_sel != NOTMUCH_FORMAT_JSON)
+	    fprintf (stderr, "Error: specifying output is only implemented for json format.\n");
+    } else {
+	params.output = NOTMUCH_SHOW_OUTPUT_DEFAULT;
+    }
+
     switch (format_sel) {
     case NOTMUCH_FORMAT_JSON:
 	format = &format_json;
-- 
1.7.9.1

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

* [PATCH 3/3] cli: allow show to omit message bodies.
  2012-07-07 15:12 [PATCH 0/3] Control notmuch show output Mark Walters
  2012-07-07 15:12 ` [PATCH 1/3] cli: allow keyword lists in argument parser Mark Walters
  2012-07-07 15:12 ` [PATCH 2/3] cli: show allow the caller to specify the headers output Mark Walters
@ 2012-07-07 15:12 ` Mark Walters
  2012-07-11 12:29 ` [PATCH 0/3] Control notmuch show output Peter Wang
  2012-07-14 16:49 ` Mark Walters
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2012-07-07 15:12 UTC (permalink / raw)
  To: notmuch

The new keyword parsing makes this a trivial addition. It replaces
the previously proposed "--headers-only" functionality.
---
 notmuch-client.h |    4 +++-
 notmuch-show.c   |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index c241a7d..8e2d9da 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -78,12 +78,14 @@ typedef enum {
     NOTMUCH_SHOW_OUTPUT_HDR_REPLY_TO = 0x20,
     NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO = 0x40,
     NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES = 0x80,
+    NOTMUCH_SHOW_OUTPUT_BODY = 0x100,
     NOTMUCH_SHOW_OUTPUT_DEFAULT =
 	    NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT |
 	    NOTMUCH_SHOW_OUTPUT_HDR_FROM |
 	    NOTMUCH_SHOW_OUTPUT_HDR_TO |
 	    NOTMUCH_SHOW_OUTPUT_HDR_CC |
-	    NOTMUCH_SHOW_OUTPUT_HDR_DATE
+	    NOTMUCH_SHOW_OUTPUT_HDR_DATE |
+	    NOTMUCH_SHOW_OUTPUT_BODY
 } notmuch_show_output_t;
 
 typedef struct notmuch_show_format {
diff --git a/notmuch-show.c b/notmuch-show.c
index 242e8e0..5c3d5c3 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -586,7 +586,8 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first, notm
 	format_headers_json (ctx, GMIME_MESSAGE (node->part), output);
 
 	printf (", \"body\": [");
-	format_part_json (ctx, mime_node_child (node, 0), first, output);
+	if (output & NOTMUCH_SHOW_OUTPUT_BODY)
+	    format_part_json (ctx, mime_node_child (node, 0), first, output);
 
 	printf ("]}");
 	return;
@@ -1044,6 +1045,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 				  { "reply-to", NOTMUCH_SHOW_OUTPUT_HDR_REPLY_TO },
 				  { "in-reply-to", NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO },
 				  { "references", NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES },
+				  { "body", NOTMUCH_SHOW_OUTPUT_BODY },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
 	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
-- 
1.7.9.1

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

* Re: [PATCH 0/3] Control notmuch show output
  2012-07-07 15:12 [PATCH 0/3] Control notmuch show output Mark Walters
                   ` (2 preceding siblings ...)
  2012-07-07 15:12 ` [PATCH 3/3] cli: allow show to omit message bodies Mark Walters
@ 2012-07-11 12:29 ` Peter Wang
  2012-07-14 16:49 ` Mark Walters
  4 siblings, 0 replies; 9+ messages in thread
From: Peter Wang @ 2012-07-11 12:29 UTC (permalink / raw)
  To: notmuch

On Sat,  7 Jul 2012 16:12:55 +0100, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> Do people have any comments in the current form?

Very nice!

Peter

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

* Re: [PATCH 2/3] cli: show allow the caller to specify the headers output.
  2012-07-07 15:12 ` [PATCH 2/3] cli: show allow the caller to specify the headers output Mark Walters
@ 2012-07-13 19:33   ` Jameson Graef Rollins
  2012-07-13 19:50     ` Mark Walters
  0 siblings, 1 reply; 9+ messages in thread
From: Jameson Graef Rollins @ 2012-07-13 19:33 UTC (permalink / raw)
  To: Mark Walters, notmuch

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

On Sat, Jul 07 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> +typedef enum {
> +    NOTMUCH_SHOW_OUTPUT_HDR_DATE = 0x1,
> +    NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT = 0x2,
> +    NOTMUCH_SHOW_OUTPUT_HDR_FROM = 0x4,
> +    NOTMUCH_SHOW_OUTPUT_HDR_TO = 0x8,
> +    NOTMUCH_SHOW_OUTPUT_HDR_CC = 0x10,
> +    NOTMUCH_SHOW_OUTPUT_HDR_REPLY_TO = 0x20,
> +    NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO = 0x40,
> +    NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES = 0x80,
> +    NOTMUCH_SHOW_OUTPUT_DEFAULT =
> +	    NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT |
> +	    NOTMUCH_SHOW_OUTPUT_HDR_FROM |
> +	    NOTMUCH_SHOW_OUTPUT_HDR_TO |
> +	    NOTMUCH_SHOW_OUTPUT_HDR_CC |
> +	    NOTMUCH_SHOW_OUTPUT_HDR_DATE
> +} notmuch_show_output_t;

Is there a reason we need to limit this to some pre-defined subset of
headers?  Wouldn't it be nice if you could just specify arbitrary
headers?

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

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

* Re: [PATCH 2/3] cli: show allow the caller to specify the headers output.
  2012-07-13 19:33   ` Jameson Graef Rollins
@ 2012-07-13 19:50     ` Mark Walters
  2012-07-13 20:37       ` Jameson Graef Rollins
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2012-07-13 19:50 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

On Fri, 13 Jul 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sat, Jul 07 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>> +typedef enum {
>> +    NOTMUCH_SHOW_OUTPUT_HDR_DATE = 0x1,
>> +    NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT = 0x2,
>> +    NOTMUCH_SHOW_OUTPUT_HDR_FROM = 0x4,
>> +    NOTMUCH_SHOW_OUTPUT_HDR_TO = 0x8,
>> +    NOTMUCH_SHOW_OUTPUT_HDR_CC = 0x10,
>> +    NOTMUCH_SHOW_OUTPUT_HDR_REPLY_TO = 0x20,
>> +    NOTMUCH_SHOW_OUTPUT_HDR_IN_REPLY_TO = 0x40,
>> +    NOTMUCH_SHOW_OUTPUT_HDR_REFERENCES = 0x80,
>> +    NOTMUCH_SHOW_OUTPUT_DEFAULT =
>> +	    NOTMUCH_SHOW_OUTPUT_HDR_SUBJECT |
>> +	    NOTMUCH_SHOW_OUTPUT_HDR_FROM |
>> +	    NOTMUCH_SHOW_OUTPUT_HDR_TO |
>> +	    NOTMUCH_SHOW_OUTPUT_HDR_CC |
>> +	    NOTMUCH_SHOW_OUTPUT_HDR_DATE
>> +} notmuch_show_output_t;
>
> Is there a reason we need to limit this to some pre-defined subset of
> headers?  Wouldn't it be nice if you could just specify arbitrary
> headers?

I basically agree but it looked like it would be relatively ugly to pass
around. 

However, perhaps this is all being too general: the caller probably
cares that all the headers it wants are output and perhaps not too many
others (eg over ssh or to android etc). Would something more like
headers=brief or headers=full be enough? And we would still want an
option allowing the body to be omitted.

What do you think?

Best wishes

Mark

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

* Re: [PATCH 2/3] cli: show allow the caller to specify the headers output.
  2012-07-13 19:50     ` Mark Walters
@ 2012-07-13 20:37       ` Jameson Graef Rollins
  0 siblings, 0 replies; 9+ messages in thread
From: Jameson Graef Rollins @ 2012-07-13 20:37 UTC (permalink / raw)
  To: Mark Walters, notmuch

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

On Fri, Jul 13 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> I basically agree but it looked like it would be relatively ugly to pass
> around. 
>
> However, perhaps this is all being too general: the caller probably
> cares that all the headers it wants are output and perhaps not too many
> others (eg over ssh or to android etc). Would something more like
> headers=brief or headers=full be enough? And we would still want an
> option allowing the body to be omitted.
>
> What do you think?

I think different people are going to want different headers, and I know
that people want headers that are not included in the default output.
It just seems to me we would be better served with a more general
solution.  Just being able to specify from the headers we are already
outputting doesn't seem like much of an improvement to me.

As for body omission: I still think it would be a nice clean separation
if the contents of body parts were retrieved separately.  If there
really are situations where that constitutes a performance hit, I guess
we'll want the option.  Although I would argue that bodies should be
omitted by default, and the option should include them, rather than
vice-versa.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

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

* Re: [PATCH 0/3] Control notmuch show output
  2012-07-07 15:12 [PATCH 0/3] Control notmuch show output Mark Walters
                   ` (3 preceding siblings ...)
  2012-07-11 12:29 ` [PATCH 0/3] Control notmuch show output Peter Wang
@ 2012-07-14 16:49 ` Mark Walters
  4 siblings, 0 replies; 9+ messages in thread
From: Mark Walters @ 2012-07-14 16:49 UTC (permalink / raw)
  To: notmuch


I wonder if we need as many options as this: I implemented a simpler set
of possibilities: 
no-body:          just from, to, cc, subject and date headers and no bodies
brief:            from, to, cc, subject and date header with bodies
most:             all headers except "received" with bodies
all:              all headers and bodies

I then benchmarked this by timing the command line run. I saved the
output and ran it in a simple emacs lisp script which just ran (and
timed) json-read on the output. I also noted the size of the data.

All were done on the query 

notmuch show --format=json <relevant output> tag:notmuch 

(which contains 11 775 messages in 1 672 threads)

             Cli time      JSON time      Data MB
no-body:      10.0            7               7    
brief:        11.9           55              70
most:         12.6           73              91          
all:          13.0           85             107

(note the cli was actually run on a different (slower) machine than the
json)

So do we want all sorts of header options, or would the above be
sufficient? Note that for notmuch show itself the difference is probably
negligible in most cases as the data size is *much* smaller. The
datasize may be significant for some uses though.

Best wishes

Mark

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

end of thread, other threads:[~2012-07-14 16:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-07 15:12 [PATCH 0/3] Control notmuch show output Mark Walters
2012-07-07 15:12 ` [PATCH 1/3] cli: allow keyword lists in argument parser Mark Walters
2012-07-07 15:12 ` [PATCH 2/3] cli: show allow the caller to specify the headers output Mark Walters
2012-07-13 19:33   ` Jameson Graef Rollins
2012-07-13 19:50     ` Mark Walters
2012-07-13 20:37       ` Jameson Graef Rollins
2012-07-07 15:12 ` [PATCH 3/3] cli: allow show to omit message bodies Mark Walters
2012-07-11 12:29 ` [PATCH 0/3] Control notmuch show output Peter Wang
2012-07-14 16:49 ` Mark Walters

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

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

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