[PATCH v5 0/4] indicate length of omitted body content

[PATCH v5 0/4] indicate length of omitted body content

[Peter Wang]

This obsoletes 1355057796-19260-1-git-send-email-markwalters1009@gmail.com

Peter Wang (4):
  test: normalize only message filenames in show json
  show: indicate charset for all omitted parts
  show: indicate length, encoding of omitted body content
  test: conform to content length, encoding fields

 devel/schemata   |  9 ++++++++-
 notmuch-show.c   | 36 ++++++++++++++++++++++++++++--------
 test/crypto      | 30 +++++++++++++++++++++---------
 test/json        |  4 +++-
 test/multipart   | 26 ++++++++++++++------------
 test/sexp        |  4 +++-
 test/test-lib.sh |  2 +-
 7 files changed, 78 insertions(+), 33 deletions(-)

-- 
1.7.12.1

[PATCH v5 1/4] test: normalize only message filenames in show json

[Peter Wang]

notmuch_json_show_sanitize replaced "filename" field values even in part
structures, where the value is predictable.  Make it only normalize the
filename value if it is an absolute path (begins with slash), which is
true of the Maildir filenames that were intended to be normalized away.
---
 test/multipart   | 2 +-
 test/test-lib.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/multipart b/test/multipart
index 0527f84..344ed81 100755
--- a/test/multipart
+++ b/test/multipart
@@ -630,7 +630,7 @@ cat <<EOF >EXPECTED
  "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]},
  {"id": 7,
  "content-type": "text/plain",
- "filename": "YYYYY",
+ "filename": "attachment",
  "content": "This is a text attachment.\n"},
  {"id": 8,
  "content-type": "text/plain",
diff --git a/test/test-lib.sh b/test/test-lib.sh
index fd64736..6ce3b31 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -600,7 +600,7 @@ notmuch_json_show_sanitize ()
 {
     sed \
 	-e 's|"id": "[^"]*",|"id": "XXXXX",|g' \
-	-e 's|"filename": "[^"]*",|"filename": "YYYYY",|g'
+	-e 's|"filename": "/[^"]*",|"filename": "YYYYY",|g'
 }
 
 # End of notmuch helper functions
-- 
1.7.12.1

[PATCH v5 2/4] show: indicate charset for all omitted parts

[Peter Wang]

Write a "charset" field for all omitted parts for which it is applicable,
not only text/html parts. Factor out the code to a separate function.
It will be extended with more fields next.
---
 notmuch-show.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index a83fef9..a781a49 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -599,6 +599,17 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
     return NOTMUCH_STATUS_SUCCESS;
 }
 
+static void
+format_omitted_part_meta (sprinter_t *sp, GMimeObject *meta)
+{
+    const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset");
+
+    if (content_charset != NULL) {
+	sp->map_key (sp, "content-charset");
+	sp->string (sp, content_charset);
+    }
+}
+
 void
 format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 		      notmuch_bool_t first, notmuch_bool_t output_body)
@@ -677,14 +688,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	 * makes charset decoding the responsibility on the caller, we
 	 * report the charset for text/html parts.
 	 */
-	if (g_mime_content_type_is_type (content_type, "text", "html")) {
-	    const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset");
-
-	    if (content_charset != NULL) {
-		sp->map_key (sp, "content-charset");
-		sp->string (sp, content_charset);
-	    }
-	} else if (g_mime_content_type_is_type (content_type, "text", "*")) {
+	if (g_mime_content_type_is_type (content_type, "text", "*") &&
+	    ! g_mime_content_type_is_type (content_type, "text", "html"))
+	{
 	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
 	    GByteArray *part_content;
 	    show_text_part_content (node->part, stream_memory, 0);
@@ -692,6 +698,8 @@ format_part_sprinter (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 {
+	    format_omitted_part_meta (sp, meta);
 	}
     } else if (GMIME_IS_MULTIPART (node->part)) {
 	sp->map_key (sp, "content");
-- 
1.7.12.1

[PATCH v5 3/4] show: indicate length, encoding of omitted body content

[Peter Wang]

If a leaf part's body content is omitted, return the encoded length and
transfer encoding 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.

Returning the _encoded_ content length is more efficient than returning
the _decoded_ content length.  Returning the transfer encoding allows
the consumer to estimate the decoded content length.
---
 devel/schemata |  9 ++++++++-
 notmuch-show.c | 16 ++++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index d1ab983..7d167be 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -75,7 +75,14 @@ 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 length of
+    # the encoded content (in bytes) may be given instead.
+    content-length?: int,
+    # If a leaf part's body content is not included, its transfer encoding
+    # may be given.  Using this and the encoded content length, it is
+    # possible for the consumer to estimate the decoded content length.
+    content-transfer-encoding?: string
 }
 
 # The headers of a message or part (format_headers_sprinter with reply = FALSE)
diff --git a/notmuch-show.c b/notmuch-show.c
index a781a49..a8a348d 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -600,14 +600,26 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 }
 
 static void
-format_omitted_part_meta (sprinter_t *sp, GMimeObject *meta)
+format_omitted_part_meta (sprinter_t *sp, GMimeObject *meta, GMimePart *part)
 {
     const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset");
+    const char *cte = g_mime_object_get_header (meta, "content-transfer-encoding");
+    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (part);
+    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
+    ssize_t content_length = g_mime_stream_length (stream);
 
     if (content_charset != NULL) {
 	sp->map_key (sp, "content-charset");
 	sp->string (sp, content_charset);
     }
+    if (cte != NULL) {
+	sp->map_key (sp, "content-transfer-encoding");
+	sp->string (sp, cte);
+    }
+    if (content_length >= 0) {
+	sp->map_key (sp, "content-length");
+	sp->integer (sp, content_length);
+    }
 }
 
 void
@@ -699,7 +711,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    sp->string_len (sp, (char *) part_content->data, part_content->len);
 	    g_object_unref (stream_memory);
 	} else {
-	    format_omitted_part_meta (sp, meta);
+	    format_omitted_part_meta (sp, meta, GMIME_PART (node->part));
 	}
     } else if (GMIME_IS_MULTIPART (node->part)) {
 	sp->map_key (sp, "content");
-- 
1.7.12.1

[PATCH v5 4/4] test: conform to content length, encoding fields

[Peter Wang]

Update tests to expect content-length and content-transfer-encoding
fields in show --format=json output, for leaf parts with omitted body
content.
---
 test/crypto    | 30 +++++++++++++++++++++---------
 test/json      |  4 +++-
 test/multipart | 24 +++++++++++++-----------
 test/sexp      |  4 +++-
 4 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/test/crypto b/test/crypto
index 5dd14c4..aa96ec2 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,8 @@ expected='[[[{"id": "XXXXX",
  "content": "This is a test encrypted message.\n"},
  {"id": 5,
  "content-type": "application/octet-stream",
+ "content-length": 28,
+ "content-transfer-encoding": "base64",
  "filename": "TESTATTACHMENT"}]}]}]},
  []]]]'
 test_expect_equal_json \
@@ -231,9 +237,11 @@ test_expect_equal_file OUTPUT TESTATTACHMENT
 
 test_begin_subtest "decryption failure with missing key"
 mv "${GNUPGHOME}"{,.bak}
+# The length of the encrypted attachment varies so must be normalized.
 output=$(notmuch show --format=json --decrypt subject:"test encrypted message 001" \
     | notmuch_json_show_sanitize \
-    | sed -e 's|"created": [1234567890]*|"created": 946728000|')
+    | sed -e 's|"created": [1234567890]*|"created": 946728000|' \
+    | sed -e 's|"content-length": 6[1234567890]*|"content-length": 652|')
 expected='[[[{"id": "XXXXX",
  "match": true,
  "excluded": false,
@@ -249,9 +257,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 +297,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 +353,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 bfafd55..ff7fbe1 100755
--- a/test/json
+++ b/test/json
@@ -45,7 +45,9 @@ 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\"}]}]}, []]]]"
+# Get length of README after base64-encoding, minus additional newline.
+attachment_length=$(( $(base64 $TEST_DIRECTORY/README | wc -c) - 1 ))
+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\": $attachment_length, \"content-transfer-encoding\": \"base64\", \"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 344ed81..c974226 100755
--- a/test/multipart
+++ b/test/multipart
@@ -326,11 +326,11 @@ cat <<EOF >EXPECTED
 {"id": 2, "content-type": "multipart/mixed", "content": [
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"Subject": "html message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
 {"id": 4, "content-type": "multipart/alternative", "content": [
-{"id": 5, "content-type": "text/html"}, 
+{"id": 5, "content-type": "text/html", "content-length": 71},
 {"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)"
 
@@ -341,11 +341,11 @@ cat <<EOF >EXPECTED
 {"id": 2, "content-type": "multipart/mixed", "content": [
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"Subject": "html message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
 {"id": 4, "content-type": "multipart/alternative", "content": [
-{"id": 5, "content-type": "text/html"}, 
+{"id": 5, "content-type": "text/html", "content-length": 71},
 {"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)"
 
@@ -355,7 +355,7 @@ cat <<EOF >EXPECTED
 {"id": 2, "content-type": "multipart/mixed", "content": [
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"Subject": "html message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
 {"id": 4, "content-type": "multipart/alternative", "content": [
-{"id": 5, "content-type": "text/html"}, 
+{"id": 5, "content-type": "text/html", "content-length": 71},
 {"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"}]}
@@ -367,7 +367,7 @@ notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OU
 cat <<EOF >EXPECTED
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"Subject": "html message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
 {"id": 4, "content-type": "multipart/alternative", "content": [
-{"id": 5, "content-type": "text/html"}, 
+{"id": 5, "content-type": "text/html", "content-length": 71},
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
@@ -376,7 +376,7 @@ test_begin_subtest "--format=json --part=4, rfc822's multipart/alternative"
 notmuch show --format=json --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
 {"id": 4, "content-type": "multipart/alternative", "content": [
-{"id": 5, "content-type": "text/html"}, 
+{"id": 5, "content-type": "text/html", "content-length": 71},
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
@@ -384,7 +384,7 @@ test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 test_begin_subtest "--format=json --part=5, rfc822's html part"
 notmuch show --format=json --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-{"id": 5, "content-type": "text/html"}
+{"id": 5, "content-type": "text/html", "content-length": 71}
 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)"
 
@@ -624,7 +624,8 @@ cat <<EOF >EXPECTED
  "body": [{"id": 4,
  "content-type": "multipart/alternative",
  "content": [{"id": 5,
- "content-type": "text/html"},
+ "content-type": "text/html",
+ "content-length": 71},
  {"id": 6,
  "content-type": "text/plain",
  "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]},
@@ -636,7 +637,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)"
 
diff --git a/test/sexp b/test/sexp
index b804942..492a82f 100755
--- a/test/sexp
+++ b/test/sexp
@@ -37,7 +37,9 @@ emacs_deliver_message \
      (insert \"Message-ID: <$id>\n\")"
 output=$(notmuch show --format=sexp "id:$id")
 filename=$(notmuch search --output=files "id:$id")
-test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename \"$filename\" :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\") :headers (:Subject \"sexp-show-inline-attachment-filename\" :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\"))))) ())))"
+# Get length of README after base64-encoding, minus additional newline.
+attachment_length=$(( $(base64 $TEST_DIRECTORY/README | wc -c) - 1 ))
+test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename \"$filename\" :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\") :headers (:Subject \"sexp-show-inline-attachment-filename\" :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\" :content-transfer-encoding \"base64\" :content-length $attachment_length))))) ())))"
 
 test_begin_subtest "Search message: sexp, utf-8"
 add_message "[subject]=\"sexp-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
-- 
1.7.12.1

Re: [PATCH v5 0/4] indicate length of omitted body content

[Mark Walters]

This version looks good to me with one very minor comments:

Perhaps the name format-omitted-part-meta should include sprinter (eg
format-omitted-part-meta-sprinter)?

Best wishes

Mark



On Sat, 15 Dec 2012, Peter Wang <novalazy@gmail.com> wrote:
> This obsoletes 1355057796-19260-1-git-send-email-markwalters1009@gmail.com
>
> Peter Wang (4):
>   test: normalize only message filenames in show json
>   show: indicate charset for all omitted parts
>   show: indicate length, encoding of omitted body content
>   test: conform to content length, encoding fields
>
>  devel/schemata   |  9 ++++++++-
>  notmuch-show.c   | 36 ++++++++++++++++++++++++++++--------
>  test/crypto      | 30 +++++++++++++++++++++---------
>  test/json        |  4 +++-
>  test/multipart   | 26 ++++++++++++++------------
>  test/sexp        |  4 +++-
>  test/test-lib.sh |  2 +-
>  7 files changed, 78 insertions(+), 33 deletions(-)
>
> -- 
> 1.7.12.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v5 0/4] indicate length of omitted body content

[Peter Wang]

On Sat, 15 Dec 2012 08:45:33 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> This version looks good to me with one very minor comments:
> 
> Perhaps the name format-omitted-part-meta should include sprinter (eg
> format-omitted-part-meta-sprinter)?

Seems so. I'll roll up another series if that's necessary.

Peter

Re: [PATCH v5 0/4] indicate length of omitted body content

[David Bremner]

Peter Wang <novalazy@gmail.com> writes:

> On Sat, 15 Dec 2012 08:45:33 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
>> 
>> This version looks good to me with one very minor comments:
>> 
>> Perhaps the name format-omitted-part-meta should include sprinter (eg
>> format-omitted-part-meta-sprinter)?
>
> Seems so. I'll roll up another series if that's necessary.

Maybe just post the one amended patch as a reply to the patch it's
amending?

d

Re: [PATCH v5 0/4] indicate length of omitted body content

[Tomi Ollila]

On Sat, Dec 15 2012, David Bremner wrote:

> Peter Wang <novalazy@gmail.com> writes:
>
>> On Sat, 15 Dec 2012 08:45:33 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
>>> 
>>> This version looks good to me with one very minor comments:
>>> 
>>> Perhaps the name format-omitted-part-meta should include sprinter (eg
>>> format-omitted-part-meta-sprinter)?
>>
>> Seems so. I'll roll up another series if that's necessary.
>
> Maybe just post the one amended patch as a reply to the patch it's
> amending?

The 'format_omitted_part_meta' in in context of 2 patches; Are you
planning to do the change sin one patch (3/4) only (or making 5/4
which changes naming) ?

And... The patch series looks good to me.

>
> d

Tomi

[PATCH v5b] show: indicate charset for all omitted parts

[Peter Wang]

Write a "charset" field for all omitted parts for which it is applicable,
not only text/html parts. Factor out the code to a separate function.
It will be extended with more fields next.
---
 notmuch-show.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index a83fef9..6a9278c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -599,6 +599,17 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
     return NOTMUCH_STATUS_SUCCESS;
 }
 
+static void
+format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta)
+{
+    const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset");
+
+    if (content_charset != NULL) {
+	sp->map_key (sp, "content-charset");
+	sp->string (sp, content_charset);
+    }
+}
+
 void
 format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 		      notmuch_bool_t first, notmuch_bool_t output_body)
@@ -677,14 +688,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	 * makes charset decoding the responsibility on the caller, we
 	 * report the charset for text/html parts.
 	 */
-	if (g_mime_content_type_is_type (content_type, "text", "html")) {
-	    const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset");
-
-	    if (content_charset != NULL) {
-		sp->map_key (sp, "content-charset");
-		sp->string (sp, content_charset);
-	    }
-	} else if (g_mime_content_type_is_type (content_type, "text", "*")) {
+	if (g_mime_content_type_is_type (content_type, "text", "*") &&
+	    ! g_mime_content_type_is_type (content_type, "text", "html"))
+	{
 	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
 	    GByteArray *part_content;
 	    show_text_part_content (node->part, stream_memory, 0);
@@ -692,6 +698,8 @@ format_part_sprinter (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 {
+	    format_omitted_part_meta_sprinter (sp, meta);
 	}
     } else if (GMIME_IS_MULTIPART (node->part)) {
 	sp->map_key (sp, "content");
-- 
1.7.12.1

[PATCH v5b] show: indicate length, encoding of omitted body content

[Peter Wang]

If a leaf part's body content is omitted, return the encoded length and
transfer encoding 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.

Returning the _encoded_ content length is more efficient than returning
the _decoded_ content length.  Returning the transfer encoding allows
the consumer to estimate the decoded content length.
---
 devel/schemata |  9 ++++++++-
 notmuch-show.c | 16 ++++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index d1ab983..7d167be 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -75,7 +75,14 @@ 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 length of
+    # the encoded content (in bytes) may be given instead.
+    content-length?: int,
+    # If a leaf part's body content is not included, its transfer encoding
+    # may be given.  Using this and the encoded content length, it is
+    # possible for the consumer to estimate the decoded content length.
+    content-transfer-encoding?: string
 }
 
 # The headers of a message or part (format_headers_sprinter with reply = FALSE)
diff --git a/notmuch-show.c b/notmuch-show.c
index 6a9278c..2d59865 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -600,14 +600,26 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 }
 
 static void
-format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta)
+format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta, GMimePart *part)
 {
     const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset");
+    const char *cte = g_mime_object_get_header (meta, "content-transfer-encoding");
+    GMimeDataWrapper *wrapper = g_mime_part_get_content_object (part);
+    GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper);
+    ssize_t content_length = g_mime_stream_length (stream);
 
     if (content_charset != NULL) {
 	sp->map_key (sp, "content-charset");
 	sp->string (sp, content_charset);
     }
+    if (cte != NULL) {
+	sp->map_key (sp, "content-transfer-encoding");
+	sp->string (sp, cte);
+    }
+    if (content_length >= 0) {
+	sp->map_key (sp, "content-length");
+	sp->integer (sp, content_length);
+    }
 }
 
 void
@@ -699,7 +711,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	    sp->string_len (sp, (char *) part_content->data, part_content->len);
 	    g_object_unref (stream_memory);
 	} else {
-	    format_omitted_part_meta_sprinter (sp, meta);
+	    format_omitted_part_meta_sprinter (sp, meta, GMIME_PART (node->part));
 	}
     } else if (GMIME_IS_MULTIPART (node->part)) {
 	sp->map_key (sp, "content");
-- 
1.7.12.1

Re: [PATCH v5 0/4] indicate length of omitted body content

[Mark Walters]

The b versions of this look good to me.

Best wishes

Mark

On Sat, 15 Dec 2012, David Bremner <david@tethera.net> wrote:
> Peter Wang <novalazy@gmail.com> writes:
>
>> On Sat, 15 Dec 2012 08:45:33 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
>>> 
>>> This version looks good to me with one very minor comments:
>>> 
>>> Perhaps the name format-omitted-part-meta should include sprinter (eg
>>> format-omitted-part-meta-sprinter)?
>>
>> Seems so. I'll roll up another series if that's necessary.
>
> Maybe just post the one amended patch as a reply to the patch it's
> amending?
>
> d
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v5 0/4] indicate length of omitted body content

[David Bremner]

Peter Wang <novalazy@gmail.com> writes:

> This obsoletes 1355057796-19260-1-git-send-email-markwalters1009@gmail.com
>
> Peter Wang (4):
>   test: normalize only message filenames in show json
>   show: indicate charset for all omitted parts
>   show: indicate length, encoding of omitted body content
>   test: conform to content length, encoding fields

Pushed,

d