[PATCH 1/4] show: indicate length of omitted body content (json)

[PATCH 1/4] show: indicate length of omitted body content (json)

[Peter Wang]

If a leaf part's body content is omitted, return the content length in
--format=json output.  This information may be used by the consumer,
e.g. to decide whether to download a large attachment over a slow link.
---
 devel/schemata |    5 ++++-
 notmuch-show.c |    8 ++++++++
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 9cb25f5..3df2764 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -69,7 +69,10 @@ part = {
     # A leaf part's body content is optional, but may be included if
     # it can be correctly encoded as a string.  Consumers should use
     # this in preference to fetching the part content separately.
-    content?:       string
+    content?:       string,
+    # If a leaf part's body content is not included, the content-length
+    # may be included instead.
+    content-length?: int
 }
 
 # The headers of a message or part (format_headers_json with reply = FALSE)
diff --git a/notmuch-show.c b/notmuch-show.c
index 3556293..5c54257 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    sp->map_key (sp, "content");
 	    sp->string_len (sp, (char *) part_content->data, part_content->len);
 	    g_object_unref (stream_memory);
+	} else {
+	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part));
+	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
+	    ssize_t length = g_mime_stream_length (stream);
+	    if (length >= 0) {
+		sp->map_key (sp, "content-length");
+		sp->integer (sp, length);
+	    }
 	}
     } else if (GMIME_IS_MULTIPART (node->part)) {
 	sp->map_key (sp, "content");
-- 
1.7.4.4

[PATCH 2/4] test: conform to content-length fields (json)

[Peter Wang]

Update tests to expect content-length fields in show --format=json
output, for leaf parts with omitted body content.
---
 test/crypto    |   25 +++++++++++++++++--------
 test/json      |    2 +-
 test/multipart |    9 +++++----
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/test/crypto b/test/crypto
index 5dd14c4..4a73dcc 100755
--- a/test/crypto
+++ b/test/crypto
@@ -61,7 +61,8 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is a test signed message.\n"},
  {"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
  []]]]'
 test_expect_equal_json \
     "$output" \
@@ -95,7 +96,8 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is a test signed message.\n"},
  {"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
  []]]]'
 test_expect_equal_json \
     "$output" \
@@ -127,7 +129,8 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is a test signed message.\n"},
  {"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
  []]]]'
 test_expect_equal_json \
     "$output" \
@@ -196,7 +199,8 @@ expected='[[[{"id": "XXXXX",
  "sigstatus": [],
  "content-type": "multipart/encrypted",
  "content": [{"id": 2,
- "content-type": "application/pgp-encrypted"},
+ "content-type": "application/pgp-encrypted",
+ "content-length": 11},
  {"id": 3,
  "content-type": "multipart/mixed",
  "content": [{"id": 4,
@@ -204,6 +208,7 @@ expected='[[[{"id": "XXXXX",
  "content": "This is a test encrypted message.\n"},
  {"id": 5,
  "content-type": "application/octet-stream",
+ "content-length": 28,
  "filename": "TESTATTACHMENT"}]}]}]},
  []]]]'
 test_expect_equal_json \
@@ -249,9 +254,11 @@ expected='[[[{"id": "XXXXX",
  "encstatus": [{"status": "bad"}],
  "content-type": "multipart/encrypted",
  "content": [{"id": 2,
- "content-type": "application/pgp-encrypted"},
+ "content-type": "application/pgp-encrypted",
+ "content-length": 11},
  {"id": 3,
- "content-type": "application/octet-stream"}]}]},
+ "content-type": "application/octet-stream",
+ "content-length": 652}]}]},
  []]]]'
 test_expect_equal_json \
     "$output" \
@@ -287,7 +294,8 @@ expected='[[[{"id": "XXXXX",
  "userid": " Notmuch Test Suite <test_suite@notmuchmail.org> (INSECURE!)"}],
  "content-type": "multipart/encrypted",
  "content": [{"id": 2,
- "content-type": "application/pgp-encrypted"},
+ "content-type": "application/pgp-encrypted",
+ "content-length": 11},
  {"id": 3,
  "content-type": "text/plain",
  "content": "This is another test encrypted message.\n"}]}]},
@@ -342,7 +350,8 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is a test signed message.\n"},
  {"id": 3,
- "content-type": "application/pgp-signature"}]}]},
+ "content-type": "application/pgp-signature",
+ "content-length": 315}]}]},
  []]]]'
 test_expect_equal_json \
     "$output" \
diff --git a/test/json b/test/json
index ac8fa8e..ff2287e 100755
--- a/test/json
+++ b/test/json
@@ -45,7 +45,7 @@ emacs_deliver_message \
      (insert \"Message-ID: <$id>\n\")"
 output=$(notmuch show --format=json "id:$id")
 filename=$(notmuch search --output=files "id:$id")
-test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"test_suite@notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"filename\": \"README\"}]}]}, []]]]"
+test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"test_suite@notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"content-length\": 12392, \"filename\": \"README\"}]}]}, []]]]"
 
 test_begin_subtest "Search message: json, utf-8"
 add_message "[subject]=\"json-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
diff --git a/test/multipart b/test/multipart
index 0527f84..0f9e113 100755
--- a/test/multipart
+++ b/test/multipart
@@ -330,7 +330,7 @@ cat <<EOF >EXPECTED
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]}, 
 {"id": 7, "content-type": "text/plain", "filename": "attachment", "content": "This is a text attachment.\n"}, 
 {"id": 8, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}]}, 
-{"id": 9, "content-type": "application/pgp-signature"}]}]}
+{"id": 9, "content-type": "application/pgp-signature", "content-length": 197}]}]}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
@@ -345,7 +345,7 @@ cat <<EOF >EXPECTED
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]}, 
 {"id": 7, "content-type": "text/plain", "filename": "attachment", "content": "This is a text attachment.\n"}, 
 {"id": 8, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}]}, 
-{"id": 9, "content-type": "application/pgp-signature"}]}
+{"id": 9, "content-type": "application/pgp-signature", "content-length": 197}]}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
@@ -412,7 +412,7 @@ test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 test_begin_subtest "--format=json --part=9, pgp signature (unverified)"
 notmuch show --format=json --part=9 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-{"id": 9, "content-type": "application/pgp-signature"}
+{"id": 9, "content-type": "application/pgp-signature", "content-length": 197}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
@@ -636,7 +636,8 @@ cat <<EOF >EXPECTED
  "content-type": "text/plain",
  "content": "And this message is signed.\n\n-Carl\n"}]},
  {"id": 9,
- "content-type": "application/pgp-signature"}]}]}}
+ "content-type": "application/pgp-signature",
+ "content-length": 197}]}]}}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
-- 
1.7.4.4

[PATCH 3/4] show: indicate length of omitted body content (text)

[Peter Wang]

If a leaf part's body content is omitted, return the content length in
--format=text output, for parity with --format=json output.
---
 notmuch-show.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 5c54257..cde8a1e 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -488,9 +488,17 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	GMIME_OBJECT (node->envelope_part) : node->part;
     GMimeContentType *content_type = g_mime_object_get_content_type (meta);
     const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
+    notmuch_bool_t leaf_text_part = FALSE;
     const char *part_type;
     int i;
 
+    if (leaf &&
+	g_mime_content_type_is_type (content_type, "text", "*") &&
+	!g_mime_content_type_is_type (content_type, "text", "html"))
+    {
+	leaf_text_part = TRUE;
+    }
+
     if (node->envelope_file) {
 	notmuch_message_t *message = node->envelope_file;
 
@@ -519,7 +527,16 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    printf (", Filename: %s", filename);
 	if (cid)
 	    printf (", Content-id: %s", cid);
-	printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type));
+	printf (", Content-type: %s", g_mime_content_type_to_string (content_type));
+	if (leaf && !leaf_text_part) {
+	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part));
+	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
+	    ssize_t length = g_mime_stream_length (stream);
+	    if (length >= 0) {
+		printf (", Content-length: %ld", length);
+	    }
+	}
+	printf ("\n");
     }
 
     if (GMIME_IS_MESSAGE (node->part)) {
@@ -547,9 +564,7 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
     }
 
     if (leaf) {
-	if (g_mime_content_type_is_type (content_type, "text", "*") &&
-	    !g_mime_content_type_is_type (content_type, "text", "html"))
-	{
+	if (leaf_text_part) {
 	    GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
 	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
 	    show_text_part_content (node->part, stream_stdout, 0);
-- 
1.7.4.4

Re: [PATCH 3/4] show: indicate length of omitted body content (text)

[Austin Clements]

I'm not sure it's worth updating the text format.  There's already
plenty of disparity between the JSON and text formats, we're
considering deprecating the text format, and, from what I understand,
this might actually break consumers of the text format (the vim
frontend?) since the text format isn't particularly extensible.

Quoth Peter Wang on Aug 05 at  5:22 pm:
> If a leaf part's body content is omitted, return the content length in
> --format=text output, for parity with --format=json output.
> ---
>  notmuch-show.c |   23 +++++++++++++++++++----
>  1 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 5c54257..cde8a1e 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -488,9 +488,17 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  	GMIME_OBJECT (node->envelope_part) : node->part;
>      GMimeContentType *content_type = g_mime_object_get_content_type (meta);
>      const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
> +    notmuch_bool_t leaf_text_part = FALSE;
>      const char *part_type;
>      int i;
>  
> +    if (leaf &&
> +	g_mime_content_type_is_type (content_type, "text", "*") &&
> +	!g_mime_content_type_is_type (content_type, "text", "html"))
> +    {
> +	leaf_text_part = TRUE;
> +    }
> +
>      if (node->envelope_file) {
>  	notmuch_message_t *message = node->envelope_file;
>  
> @@ -519,7 +527,16 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  	    printf (", Filename: %s", filename);
>  	if (cid)
>  	    printf (", Content-id: %s", cid);
> -	printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type));
> +	printf (", Content-type: %s", g_mime_content_type_to_string (content_type));
> +	if (leaf && !leaf_text_part) {
> +	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part));
> +	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
> +	    ssize_t length = g_mime_stream_length (stream);
> +	    if (length >= 0) {
> +		printf (", Content-length: %ld", length);
> +	    }

Same comment about possibly unref'ing wrapper and/or stream.

> +	}
> +	printf ("\n");
>      }
>  
>      if (GMIME_IS_MESSAGE (node->part)) {
> @@ -547,9 +564,7 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
>      }
>  
>      if (leaf) {
> -	if (g_mime_content_type_is_type (content_type, "text", "*") &&
> -	    !g_mime_content_type_is_type (content_type, "text", "html"))
> -	{
> +	if (leaf_text_part) {
>  	    GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
>  	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
>  	    show_text_part_content (node->part, stream_stdout, 0);

[PATCH 4/4] test: conform to content-length fields (text)

[Peter Wang]

Update tests to expect Content-length fields in show --format=text
output, for leaf parts with omitted body content.
---
 test/crypto    |    4 ++--
 test/multipart |   18 +++++++++---------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/test/crypto b/test/crypto
index 4a73dcc..8dfbfa6 100755
--- a/test/crypto
+++ b/test/crypto
@@ -161,14 +161,14 @@ Date: Sat, 01 Jan 2000 12:00:00 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: multipart/encrypted
-\fpart{ ID: 2, Content-type: application/pgp-encrypted
+\fpart{ ID: 2, Content-type: application/pgp-encrypted, Content-length: 11
 Non-text part: application/pgp-encrypted
 \fpart}
 \fpart{ ID: 3, Content-type: multipart/mixed
 \fpart{ ID: 4, Content-type: text/plain
 This is a test encrypted message.
 \fpart}
-\fattachment{ ID: 5, Filename: TESTATTACHMENT, Content-type: application/octet-stream
+\fattachment{ ID: 5, Filename: TESTATTACHMENT, Content-type: application/octet-stream, Content-length: 28
 Non-text part: application/octet-stream
 \fattachment}
 \fpart}
diff --git a/test/multipart b/test/multipart
index 0f9e113..064e91a 100755
--- a/test/multipart
+++ b/test/multipart
@@ -129,7 +129,7 @@ Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 4, Content-type: multipart/alternative
-\fpart{ ID: 5, Content-type: text/html
+\fpart{ ID: 5, Content-type: text/html, Content-length: 71
 Non-text part: text/html
 \fpart}
 \fpart{ ID: 6, Content-type: text/plain
@@ -147,7 +147,7 @@ And this message is signed.
 -Carl
 \fpart}
 \fpart}
-\fpart{ ID: 9, Content-type: application/pgp-signature
+\fpart{ ID: 9, Content-type: application/pgp-signature, Content-length: 197
 Non-text part: application/pgp-signature
 \fpart}
 \fpart}
@@ -170,7 +170,7 @@ Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 4, Content-type: multipart/alternative
-\fpart{ ID: 5, Content-type: text/html
+\fpart{ ID: 5, Content-type: text/html, Content-length: 71
 Non-text part: text/html
 \fpart}
 \fpart{ ID: 6, Content-type: text/plain
@@ -188,7 +188,7 @@ And this message is signed.
 -Carl
 \fpart}
 \fpart}
-\fpart{ ID: 9, Content-type: application/pgp-signature
+\fpart{ ID: 9, Content-type: application/pgp-signature, Content-length: 197
 Non-text part: application/pgp-signature
 \fpart}
 \fpart}
@@ -208,7 +208,7 @@ Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 4, Content-type: multipart/alternative
-\fpart{ ID: 5, Content-type: text/html
+\fpart{ ID: 5, Content-type: text/html, Content-length: 71
 Non-text part: text/html
 \fpart}
 \fpart{ ID: 6, Content-type: text/plain
@@ -241,7 +241,7 @@ Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 4, Content-type: multipart/alternative
-\fpart{ ID: 5, Content-type: text/html
+\fpart{ ID: 5, Content-type: text/html, Content-length: 71
 Non-text part: text/html
 \fpart}
 \fpart{ ID: 6, Content-type: text/plain
@@ -257,7 +257,7 @@ test_begin_subtest "--format=text --part=4, rfc822's multipart"
 notmuch show --format=text --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
 \fpart{ ID: 4, Content-type: multipart/alternative
-\fpart{ ID: 5, Content-type: text/html
+\fpart{ ID: 5, Content-type: text/html, Content-length: 71
 Non-text part: text/html
 \fpart}
 \fpart{ ID: 6, Content-type: text/plain
@@ -270,7 +270,7 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "--format=text --part=5, rfc822's html part"
 notmuch show --format=text --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-\fpart{ ID: 5, Content-type: text/html
+\fpart{ ID: 5, Content-type: text/html, Content-length: 71
 Non-text part: text/html
 \fpart}
 EOF
@@ -308,7 +308,7 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "--format=text --part=9, pgp signature (unverified)"
 notmuch show --format=text --part=9 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-\fpart{ ID: 9, Content-type: application/pgp-signature
+\fpart{ ID: 9, Content-type: application/pgp-signature, Content-length: 197
 Non-text part: application/pgp-signature
 \fpart}
 EOF
-- 
1.7.4.4

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Tomi Ollila]

On Sun, Aug 05 2012, Peter Wang <novalazy@gmail.com> wrote:

> If a leaf part's body content is omitted, return the content length in
> --format=json output.  This information may be used by the consumer,
> e.g. to decide whether to download a large attachment over a slow link.
> ---

Code looks good to me and tests pass. So I am not against pushing...

Tomi


>  devel/schemata |    5 ++++-
>  notmuch-show.c |    8 ++++++++
>  2 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/devel/schemata b/devel/schemata
> index 9cb25f5..3df2764 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -69,7 +69,10 @@ part = {
>      # A leaf part's body content is optional, but may be included if
>      # it can be correctly encoded as a string.  Consumers should use
>      # this in preference to fetching the part content separately.
> -    content?:       string
> +    content?:       string,
> +    # If a leaf part's body content is not included, the content-length
> +    # may be included instead.
> +    content-length?: int
>  }
>  
>  # The headers of a message or part (format_headers_json with reply = FALSE)
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 3556293..5c54257 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  	    sp->map_key (sp, "content");
>  	    sp->string_len (sp, (char *) part_content->data, part_content->len);
>  	    g_object_unref (stream_memory);
> +	} else {
> +	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part));
> +	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
> +	    ssize_t length = g_mime_stream_length (stream);
> +	    if (length >= 0) {
> +		sp->map_key (sp, "content-length");
> +		sp->integer (sp, length);
> +	    }
>  	}
>      } else if (GMIME_IS_MULTIPART (node->part)) {
>  	sp->map_key (sp, "content");
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Jameson Graef Rollins]

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

On Sun, Aug 05 2012, Peter Wang <novalazy@gmail.com> wrote:
> If a leaf part's body content is omitted, return the content length in
> --format=json output.  This information may be used by the consumer,
> e.g. to decide whether to download a large attachment over a slow link.
> ---
>  devel/schemata |    5 ++++-
>  notmuch-show.c |    8 ++++++++
>  2 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/devel/schemata b/devel/schemata
> index 9cb25f5..3df2764 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -69,7 +69,10 @@ part = {
>      # A leaf part's body content is optional, but may be included if
>      # it can be correctly encoded as a string.  Consumers should use
>      # this in preference to fetching the part content separately.
> -    content?:       string
> +    content?:       string,
> +    # If a leaf part's body content is not included, the content-length
> +    # may be included instead.
> +    content-length?: int

Hey, Peter.  Something somewhere, and probably at least here in the
schemata, should mention what the uids are (b? kB? KiB? YiB?)

jamie.

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

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Peter Wang]

On Sun, 05 Aug 2012 14:37:02 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sun, Aug 05 2012, Peter Wang <novalazy@gmail.com> wrote:
> > diff --git a/devel/schemata b/devel/schemata
> > index 9cb25f5..3df2764 100644
> > --- a/devel/schemata
> > +++ b/devel/schemata
> > @@ -69,7 +69,10 @@ part = {
> >      # A leaf part's body content is optional, but may be included if
> >      # it can be correctly encoded as a string.  Consumers should use
> >      # this in preference to fetching the part content separately.
> > -    content?:       string
> > +    content?:       string,
> > +    # If a leaf part's body content is not included, the content-length
> > +    # may be included instead.
> > +    content-length?: int
> 
> Hey, Peter.  Something somewhere, and probably at least here in the
> schemata, should mention what the uids are (b? kB? KiB? YiB?)

I thought content-length was a MIME header, but it's not.
Anyway, it's the length of the encoded content in bytes.  GMime
doesn't provide an easy way to return the length of the decoded content.

I actually only need an _estimate_ of the decoded size to present to the
user.  Strictly speaking, that requires knowledge of the transfer encoding.
Previously I planned on guessing it from the content-type, but I think
it's better to return the transfer encoding as well (if any).

Alternatively, notmuch could put in more effort to return the exact
length of the decoded content.  But it's a waste of time if no consumers
will use it.

Peter

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Austin Clements]

What's the overall goal of adding this?  Are you planning to add size
information to one of the frontends?

Quoth Peter Wang on Aug 05 at  5:22 pm:
> If a leaf part's body content is omitted, return the content length in
> --format=json output.  This information may be used by the consumer,
> e.g. to decide whether to download a large attachment over a slow link.
> ---
>  devel/schemata |    5 ++++-
>  notmuch-show.c |    8 ++++++++
>  2 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/devel/schemata b/devel/schemata
> index 9cb25f5..3df2764 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -69,7 +69,10 @@ part = {
>      # A leaf part's body content is optional, but may be included if
>      # it can be correctly encoded as a string.  Consumers should use
>      # this in preference to fetching the part content separately.
> -    content?:       string
> +    content?:       string,
> +    # If a leaf part's body content is not included, the content-length
> +    # may be included instead.

You mentioned elsewhere that the content-length returned is an
estimate.  If that's the case, this comment should say as much.  Is it
actually the case, though?  g_mime_part_get_content_object is
remarkably poorly documented for such an important function, but based
on format_part_raw, it seems like the content-length your code returns
will be exactly the number of bytes returned by the raw format for a
leaf part.

> +    content-length?: int
>  }
>  
>  # The headers of a message or part (format_headers_json with reply = FALSE)
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 3556293..5c54257 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  	    sp->map_key (sp, "content");
>  	    sp->string_len (sp, (char *) part_content->data, part_content->len);
>  	    g_object_unref (stream_memory);
> +	} else {
> +	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part));
> +	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
> +	    ssize_t length = g_mime_stream_length (stream);
> +	    if (length >= 0) {
> +		sp->map_key (sp, "content-length");
> +		sp->integer (sp, length);
> +	    }

Do wrapper or stream need to be g_object_unref'd?

Any idea what the performance overhead of this is?  I'm just curious.
It might be approximately nothing, since GMime's parser is eager.

>  	}
>      } else if (GMIME_IS_MULTIPART (node->part)) {
>  	sp->map_key (sp, "content");

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Peter Wang]

On Mon, 6 Aug 2012 12:47:10 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> What's the overall goal of adding this?  Are you planning to add size
> information to one of the frontends?

Yes, to my frontend.

>> > diff --git a/devel/schemata b/devel/schemata
> > index 9cb25f5..3df2764 100644
> > --- a/devel/schemata
> > +++ b/devel/schemata
> > @@ -69,7 +69,10 @@ part = {
> >      # A leaf part's body content is optional, but may be included if
> >      # it can be correctly encoded as a string.  Consumers should use
> >      # this in preference to fetching the part content separately.
> > -    content?:       string
> > +    content?:       string,
> > +    # If a leaf part's body content is not included, the content-length
> > +    # may be included instead.
> 
> You mentioned elsewhere that the content-length returned is an
> estimate.  If that's the case, this comment should say as much.  Is it
> actually the case, though?  g_mime_part_get_content_object is
> remarkably poorly documented for such an important function, but based
> on format_part_raw, it seems like the content-length your code returns
> will be exactly the number of bytes returned by the raw format for a
> leaf part.

It's the exact length of the _encoded_ content.  If the transfer
encoding is base64, multiplying by 3/4 will get a close estimate of the
decoded content length.  I assume quoted-printable encoding would only
be used if the content is mostly ASCII, so the encoded length can serve
as the estimated decoded length then.

> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index 3556293..5c54257 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
> >  	    sp->map_key (sp, "content");
> >  	    sp->string_len (sp, (char *) part_content->data, part_content->len);
> >  	    g_object_unref (stream_memory);
> > +	} else {
> > +	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part));
> > +	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
> > +	    ssize_t length = g_mime_stream_length (stream);
> > +	    if (length >= 0) {
> > +		sp->map_key (sp, "content-length");
> > +		sp->integer (sp, length);
> > +	    }
> 
> Do wrapper or stream need to be g_object_unref'd?

No.

> Any idea what the performance overhead of this is?  I'm just curious.
> It might be approximately nothing, since GMime's parser is eager.

The start and end bounds of the stream are already known so there's
approximately nothing for g_mime_stream_length to do.  The other
functions simply return field values.

I'll drop the changes for text output.

Peter

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Austin Clements]

Quoting Peter Wang <novalazy@gmail.com>:
> On Mon, 6 Aug 2012 12:47:10 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
>> What's the overall goal of adding this?  Are you planning to add size
>> information to one of the frontends?
>
> Yes, to my frontend.
>
>>> > diff --git a/devel/schemata b/devel/schemata
>> > index 9cb25f5..3df2764 100644
>> > --- a/devel/schemata
>> > +++ b/devel/schemata
>> > @@ -69,7 +69,10 @@ part = {
>> >      # A leaf part's body content is optional, but may be included if
>> >      # it can be correctly encoded as a string.  Consumers should use
>> >      # this in preference to fetching the part content separately.
>> > -    content?:       string
>> > +    content?:       string,
>> > +    # If a leaf part's body content is not included, the content-length
>> > +    # may be included instead.
>>
>> You mentioned elsewhere that the content-length returned is an
>> estimate.  If that's the case, this comment should say as much.  Is it
>> actually the case, though?  g_mime_part_get_content_object is
>> remarkably poorly documented for such an important function, but based
>> on format_part_raw, it seems like the content-length your code returns
>> will be exactly the number of bytes returned by the raw format for a
>> leaf part.
>
> It's the exact length of the _encoded_ content.  If the transfer
> encoding is base64, multiplying by 3/4 will get a close estimate of the
> decoded content length.  I assume quoted-printable encoding would only
> be used if the content is mostly ASCII, so the encoded length can serve
> as the estimated decoded length then.

Ah, I see.  format_part_raw misled me; apparently the
g_mime_data_wrapper_write_to_stream is key there, since *that* decodes
the transfer encoding of the data wrapper's underlying, raw stream.

In that case, the comment could either mention that this is the length
of the transfer encoded content or it could say it's an approximation
of the decoded length.  The advantage of only claiming the latter is
that it would leave open the possibility of, say, multiplying by .75
for base64 transfer encoding to get a better decoded estimate (your
assumption about quoted-printable sounds completely reasonable).
Alternatively, we could add the transfer encoding in the future and
let the caller do such approximations.

>> > diff --git a/notmuch-show.c b/notmuch-show.c
>> > index 3556293..5c54257 100644
>> > --- a/notmuch-show.c
>> > +++ b/notmuch-show.c
>> > @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t 
>> *sp, mime_node_t *node,
>> >  	    sp->map_key (sp, "content");
>> >  	    sp->string_len (sp, (char *) part_content->data, part_content->len);
>> >  	    g_object_unref (stream_memory);
>> > +	} else {
>> > +	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object 
>> (GMIME_PART (node->part));
>> > +	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
>> > +	    ssize_t length = g_mime_stream_length (stream);
>> > +	    if (length >= 0) {
>> > +		sp->map_key (sp, "content-length");
>> > +		sp->integer (sp, length);
>> > +	    }
>>
>> Do wrapper or stream need to be g_object_unref'd?
>
> No.
>
>> Any idea what the performance overhead of this is?  I'm just curious.
>> It might be approximately nothing, since GMime's parser is eager.
>
> The start and end bounds of the stream are already known so there's
> approximately nothing for g_mime_stream_length to do.  The other
> functions simply return field values.

Sounds good.

> I'll drop the changes for text output.
>
> Peter

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Mark Walters]

I like this (and agree with Austin and you that text and json can
diverge). I just hacked something together which uses this and makes the
emacs front-end display the content-length on part buttons and as
someone who uses notmuch over ssh that is nice.

I have two minor queries: do you think omitted text/html parts should
also have the length given? Or even should it always be sent? But I am
happy with it as is.

I haven't checked the test patch (or the text ones as they are being
dropped).

Best wishes

Mark

On Sun, 05 Aug 2012, Peter Wang <novalazy@gmail.com> wrote:
> If a leaf part's body content is omitted, return the content length in
> --format=json output.  This information may be used by the consumer,
> e.g. to decide whether to download a large attachment over a slow link.
> ---
>  devel/schemata |    5 ++++-
>  notmuch-show.c |    8 ++++++++
>  2 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/devel/schemata b/devel/schemata
> index 9cb25f5..3df2764 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -69,7 +69,10 @@ part = {
>      # A leaf part's body content is optional, but may be included if
>      # it can be correctly encoded as a string.  Consumers should use
>      # this in preference to fetching the part content separately.
> -    content?:       string
> +    content?:       string,
> +    # If a leaf part's body content is not included, the content-length
> +    # may be included instead.
> +    content-length?: int
>  }
>  
>  # The headers of a message or part (format_headers_json with reply = FALSE)
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 3556293..5c54257 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  	    sp->map_key (sp, "content");
>  	    sp->string_len (sp, (char *) part_content->data, part_content->len);
>  	    g_object_unref (stream_memory);
> +	} else {
> +	    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part));
> +	    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
> +	    ssize_t length = g_mime_stream_length (stream);
> +	    if (length >= 0) {
> +		sp->map_key (sp, "content-length");
> +		sp->integer (sp, length);
> +	    }
>  	}
>      } else if (GMIME_IS_MULTIPART (node->part)) {
>  	sp->map_key (sp, "content");
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Peter Wang]

On Wed, 08 Aug 2012 00:01:06 +0100, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> I have two minor queries: do you think omitted text/html parts should
> also have the length given? Or even should it always be sent? But I am
> happy with it as is.

It could be added.  I wouldn't have much use for it myself.
text/html parts should usually be small enough that it doesn't matter,
and I never retrieve them when there is a text/plain alternative.

Peter

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Ethan Glasser-Camp]

Peter Wang <novalazy@gmail.com> writes:

> If a leaf part's body content is omitted, return the content length in
> --format=json output.  This information may be used by the consumer,
> e.g. to decide whether to download a large attachment over a slow link.

It looks like this patch series was thoroughly reviewed and then
obsoleted by the series in
id:"1344428872-12374-1-git-send-email-novalazy@gmail.com". I'm therefore
marking it notmuch::obsolete, and will review the other patch set.

Ethan