[PATCH 0/4] Add a --body option to notmuch show

[PATCH 0/4] Add a --body option to notmuch show

[Mark Walters]

After discussion on irc today here is another version of the omit body
patches.  The conclusion on irc was that the option should just make
the body: field in the JSON output optional. The --body=false option
omits this field. The default remains --body=true including the body:
field in the output.

This version includes tests for the new functionality and updates the
manpage and the schemata.

The motivation for this patch is that some callers do not need the
body and omitting it gives a substantial speed up and substantial
reduction in amount of data output.

Previous related patches are at
id:"1342967879-20453-1-git-send-email-markwalters1009@gmail.com" and
id:"1341041595-5858-1-git-send-email-markwalters1009@gmail.com"

Best wishes

Mark



Mark Walters (4):
  cli: add --body=true|false option to notmuch-show.c
  test: add tests for the new --body=true|false option
  man: update man page for the new --body=true|false option
  schemata: update for --body=true|false option

 devel/schemata          |    2 +-
 man/man1/notmuch-show.1 |   16 ++++++++++++++++
 notmuch-client.h        |    3 ++-
 notmuch-reply.c         |    2 +-
 notmuch-show.c          |   23 +++++++++++++++--------
 test/json               |    9 +++++++++
 6 files changed, 44 insertions(+), 11 deletions(-)

-- 
1.7.9.1

[PATCH 1/4] cli: add --body=true|false option to notmuch-show.c

[Mark Walters]

This option allows the caller to suppress the output of the bodies of
the messages. Currently this is only implemented for format=json.

This is used by notmuch-pick.el (although not needed) because it gives
a speed-up of at least a factor of a two (and in some cases a speed up
of more than a factor of 8); moreover it reduces the memory usage in
emacs hugely.
---
 notmuch-client.h |    3 ++-
 notmuch-reply.c  |    2 +-
 notmuch-show.c   |   23 +++++++++++++++--------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 0c17b79..f930798 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -87,6 +87,7 @@ typedef struct notmuch_crypto {
 typedef struct notmuch_show_params {
     notmuch_bool_t entire_thread;
     notmuch_bool_t omit_excluded;
+    notmuch_bool_t output_body;
     notmuch_bool_t raw;
     int part;
     notmuch_crypto_t crypto;
@@ -176,7 +177,7 @@ 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_bool_t output_body);
 
 void
 format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 3a038ed..de21f3b 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -620,7 +620,7 @@ notmuch_reply_format_json(void *ctx,
     /* Start the original */
     printf (", \"original\": ");
 
-    format_part_json (ctx, node, TRUE);
+    format_part_json (ctx, node, TRUE, TRUE);
 
     /* End */
     printf ("}\n");
diff --git a/notmuch-show.c b/notmuch-show.c
index 8f3c60e..c2ad4fb 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -559,7 +559,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_bool_t output_body)
 {
     /* Any changes to the JSON format should be reflected in the file
      * devel/schemata. */
@@ -571,10 +571,12 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
 	printf ("\"headers\": ");
 	format_headers_json (ctx, GMIME_MESSAGE (node->part), FALSE);
 
-	printf (", \"body\": [");
-	format_part_json (ctx, mime_node_child (node, 0), first);
-
-	printf ("]}");
+	if (output_body) {
+	    printf (", \"body\": [");
+	    format_part_json (ctx, mime_node_child (node, 0), first, TRUE);
+	    printf ("]");
+	}
+	printf ("}");
 	return;
     }
 
@@ -652,16 +654,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, TRUE);
 
     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_body);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
@@ -1004,6 +1006,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     notmuch_show_params_t params = {
 	.part = -1,
 	.omit_excluded = TRUE,
+	.output_body = TRUE,
 	.crypto = {
 	    .verify = FALSE,
 	    .decrypt = FALSE
@@ -1032,6 +1035,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.verify, "verify", 'v', 0 },
+	{ NOTMUCH_OPT_BOOLEAN, &params.output_body, "body", 'h', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -1086,6 +1090,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	    entire_thread = ENTIRE_THREAD_FALSE;
     }
 
+    if (!params.output_body && format != &format_json)
+	fprintf (stderr,"Warning: --body=false only implemented for format=json\n");
+
     if (entire_thread == ENTIRE_THREAD_TRUE)
 	params.entire_thread = TRUE;
     else
-- 
1.7.9.1

[PATCH 2/4] test: add tests for the new --body=true|false option

[Mark Walters]

---
 test/json |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/test/json b/test/json
index 6439788..709b34f 100755
--- a/test/json
+++ b/test/json
@@ -7,6 +7,15 @@ add_message "[subject]=\"json-show-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:0
 output=$(notmuch show --format=json "json-show-message")
 test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
 
+# This should be the same output as above.
+test_begin_subtest "Show message: json --body=true"
+output=$(notmuch show --format=json --body=true "json-show-message")
+test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
+
+test_begin_subtest "Show message: json --body=false"
+output=$(notmuch show --format=json --body=false "json-show-message")
+test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}}, []]]]"
+
 test_begin_subtest "Search message: json"
 add_message "[subject]=\"json-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"json-search-message\""
 output=$(notmuch search --format=json "json-search-message" | notmuch_search_sanitize)
-- 
1.7.9.1

[PATCH 3/4] man: update man page for the new --body=true|false option

[Mark Walters]

---
 man/man1/notmuch-show.1 |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index 5fa590e..d781c47 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -154,6 +154,22 @@ The default is
 
 .RE
 
+.RS 4
+.TP 4
+.B \-\-body=(true|false)
+
+If true,
+.B notmuch show
+includes the bodies of the messages in the output whereas, if false,
+they are omitted. It defaults to true.  Note that
+.B --body=false
+is only implemented for the json format.
+
+This is useful if the caller is only interested in the headers as it
+is much faster and the amount of output data is substantially
+reduced.
+.RE
+
 A common use of
 .B notmuch show
 is to display a single thread of email messages. For this, use a
-- 
1.7.9.1

[PATCH 4/4] schemata: update for --body=true|false option

[Mark Walters]

Previously body: was a compulsory field in a message. The new
--body=false option causes notmuch show to omit this field so update
schemata to reflect this.
---
 devel/schemata |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 6677a1c..9cb25f5 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -47,7 +47,7 @@ message = {
     tags:           [string*],
 
     headers:        headers,
-    body:           [part]
+    body?:          [part]    # omitted if --body=false
 }
 
 # A MIME part (format_part_json)
-- 
1.7.9.1

Re: [PATCH 1/4] cli: add --body=true|false option to notmuch-show.c

[Austin Clements]

Quoth Mark Walters on Jul 23 at  5:17 pm:
> This option allows the caller to suppress the output of the bodies of
> the messages. Currently this is only implemented for format=json.
> 
> This is used by notmuch-pick.el (although not needed) because it gives
> a speed-up of at least a factor of a two (and in some cases a speed up
> of more than a factor of 8); moreover it reduces the memory usage in
> emacs hugely.
> ---
>  notmuch-client.h |    3 ++-
>  notmuch-reply.c  |    2 +-
>  notmuch-show.c   |   23 +++++++++++++++--------
>  3 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 0c17b79..f930798 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -87,6 +87,7 @@ typedef struct notmuch_crypto {
>  typedef struct notmuch_show_params {
>      notmuch_bool_t entire_thread;
>      notmuch_bool_t omit_excluded;
> +    notmuch_bool_t output_body;
>      notmuch_bool_t raw;
>      int part;
>      notmuch_crypto_t crypto;
> @@ -176,7 +177,7 @@ 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_bool_t output_body);
>  
>  void
>  format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 3a038ed..de21f3b 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -620,7 +620,7 @@ notmuch_reply_format_json(void *ctx,
>      /* Start the original */
>      printf (", \"original\": ");
>  
> -    format_part_json (ctx, node, TRUE);
> +    format_part_json (ctx, node, TRUE, TRUE);
>  
>      /* End */
>      printf ("}\n");
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8f3c60e..c2ad4fb 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -559,7 +559,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_bool_t output_body)
>  {
>      /* Any changes to the JSON format should be reflected in the file
>       * devel/schemata. */
> @@ -571,10 +571,12 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
>  	printf ("\"headers\": ");
>  	format_headers_json (ctx, GMIME_MESSAGE (node->part), FALSE);
>  
> -	printf (", \"body\": [");
> -	format_part_json (ctx, mime_node_child (node, 0), first);
> -
> -	printf ("]}");
> +	if (output_body) {
> +	    printf (", \"body\": [");
> +	    format_part_json (ctx, mime_node_child (node, 0), first, TRUE);
> +	    printf ("]");
> +	}
> +	printf ("}");
>  	return;
>      }
>  
> @@ -652,16 +654,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, TRUE);
>  
>      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_body);
>  
>      return NOTMUCH_STATUS_SUCCESS;
>  }
> @@ -1004,6 +1006,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>      notmuch_show_params_t params = {
>  	.part = -1,
>  	.omit_excluded = TRUE,
> +	.output_body = TRUE,
>  	.crypto = {
>  	    .verify = FALSE,
>  	    .decrypt = FALSE
> @@ -1032,6 +1035,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  	{ NOTMUCH_OPT_INT, &params.part, "part", 'p', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.decrypt, "decrypt", 'd', 0 },
>  	{ NOTMUCH_OPT_BOOLEAN, &params.crypto.verify, "verify", 'v', 0 },
> +	{ NOTMUCH_OPT_BOOLEAN, &params.output_body, "body", 'h', 0 },

Why 'h' for the short option?

>  	{ 0, 0, 0, 0, 0 }
>      };
>  
> @@ -1086,6 +1090,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  	    entire_thread = ENTIRE_THREAD_FALSE;
>      }
>  
> +    if (!params.output_body && format != &format_json)
> +	fprintf (stderr,"Warning: --body=false only implemented for format=json\n");

Missing space after comma.

Should we consider how --part and --body interact?  Maybe specifying
--part is incompatible with --body=false?  Or specifying a part > 0 is
incompatible with --body=false?

> +
>      if (entire_thread == ENTIRE_THREAD_TRUE)
>  	params.entire_thread = TRUE;
>      else

Re: [PATCH 3/4] man: update man page for the new --body=true|false option

[Austin Clements]

Quoth Mark Walters on Jul 23 at  5:17 pm:
> ---
>  man/man1/notmuch-show.1 |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
> index 5fa590e..d781c47 100644
> --- a/man/man1/notmuch-show.1
> +++ b/man/man1/notmuch-show.1
> @@ -154,6 +154,22 @@ The default is
>  
>  .RE
>  
> +.RS 4
> +.TP 4
> +.B \-\-body=(true|false)
> +
> +If true,

"If true (the default)"?

> +.B notmuch show
> +includes the bodies of the messages in the output whereas, if false,

The "whereas" is a bit awkward.  Perhaps just a semicolon?  "... in
the output; if false, bodies are omitted."?

> +they are omitted. It defaults to true.  Note that

"Note that" is unnecessary here.

> +.B --body=false
> +is only implemented for the json format.
> +
> +This is useful if the caller is only interested in the headers as it

s/is only interested in/only needs/?

The "it" is a bit awkward, since it's not the option that's much
faster, but the output.  "as body-less output is much faster and
substantially reduces the amount of output"?

> +is much faster and the amount of output data is substantially
> +reduced.
> +.RE
> +
>  A common use of
>  .B notmuch show
>  is to display a single thread of email messages. For this, use a