[PATCH 1/2] cli/show: abstract get content disposition

[PATCH 1/2] cli/show: abstract get content disposition

[Jani Nikula]

Reduce duplication in follow-up work. As a side effect, handle error
returns from g_mime_content_disposition_get_disposition() without
segfaulting.
---
 notmuch-show.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 22fa655ad20d..8b38fe6db136 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -110,6 +110,17 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
 			    from, relative_date, tags);
 }
 
+static const char *_get_disposition(GMimeObject *meta)
+{
+    GMimeContentDisposition *disposition;
+
+    disposition = g_mime_object_get_content_disposition (meta);
+    if (!disposition)
+	return NULL;
+
+    return g_mime_content_disposition_get_disposition (disposition);
+}
+
 /* Emit a sequence of key/value pairs for the metadata of message.
  * The caller should begin a map before calling this. */
 static void
@@ -450,14 +461,13 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
 		notmuch_message_get_filename (message));
     } else {
-	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
+	const char *disposition = _get_disposition (meta);
 	const char *cid = g_mime_object_get_content_id (meta);
 	const char *filename = leaf ?
 	    g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
 
 	if (disposition &&
-	    strcasecmp (g_mime_content_disposition_get_disposition (disposition),
-			GMIME_DISPOSITION_ATTACHMENT) == 0)
+	    strcasecmp (disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
 	    part_type = "attachment";
 	else
 	    part_type = "part";
-- 
2.11.0

[PATCH 2/2] cli/show: add content-disposition to structured output message parts

[Jani Nikula]

Help the clients decide how to display parts.

---

I'm not sure if this should bump the version in schemata, because the
comment in notmuch-client.h says new map fields can be added without
increasing NOTMUCH_FORMAT_CUR. Yet the schemata version history says
thread_summary.query was added in v2... which looks like a change that
shouldn't need a version bump. Confused.

The problem could be avoided by conflating both this change and
id:20170110201929.21875-1-jani@nikula.org into v3... ;)
---
 devel/schemata | 1 +
 notmuch-show.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/devel/schemata b/devel/schemata
index 41dc4a60fff3..c94459eb783a 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -76,6 +76,7 @@ part = {
     sigstatus?:     sigstatus,
 
     content-type:   string,
+    content-disposition?:       string,
     content-id?:    string,
     # if content-type starts with "multipart/":
     content:        [part*],
diff --git a/notmuch-show.c b/notmuch-show.c
index 8b38fe6db136..8e69b3465886 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -582,6 +582,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
     GMimeObject *meta = node->envelope_part ?
 	GMIME_OBJECT (node->envelope_part) : node->part;
     GMimeContentType *content_type = g_mime_object_get_content_type (meta);
+    const char *disposition = _get_disposition (meta);
     const char *cid = g_mime_object_get_content_id (meta);
     const char *filename = GMIME_IS_PART (node->part) ?
 	g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
@@ -611,6 +612,11 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
     sp->map_key (sp, "content-type");
     sp->string (sp, g_mime_content_type_to_string (content_type));
 
+    if (disposition) {
+	sp->map_key (sp, "content-disposition");
+	sp->string (sp, disposition);
+    }
+
     if (cid) {
 	sp->map_key (sp, "content-id");
 	sp->string (sp, cid);
-- 
2.11.0

Re: [PATCH 2/2] cli/show: add content-disposition to structured output message parts

[Mark Walters]

On Sat, 28 Jan 2017, Jani Nikula <jani@nikula.org> wrote:
> Help the clients decide how to display parts.

This series looks good to me. I would say +1, but I am not sure my
knowledge of C is really up to that.

There are a bunch of tests to update; I will send a patch to do that in
shortly.

> I'm not sure if this should bump the version in schemata, because the
> comment in notmuch-client.h says new map fields can be added without
> increasing NOTMUCH_FORMAT_CUR. Yet the schemata version history says
> thread_summary.query was added in v2... which looks like a change that
> shouldn't need a version bump. Confused.

I think the reason for the bump before was that the client needed to
know it had got the new format, so it could either bail out, or at least
work around it, if it got the old format.

In this case though, I think any sane program has to treat
content-disposition nil as being "do whatever the default is" (since I
don't think all parts have a content-disposition).

Thus, I don't think we need a schemata bump, as the client can just use
the extra information if present, and otherwise do what it normally would.

Best wishes

Mark

>
> The problem could be avoided by conflating both this change and
> id:20170110201929.21875-1-jani@nikula.org into v3... ;)
> ---
>  devel/schemata | 1 +
>  notmuch-show.c | 6 ++++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/devel/schemata b/devel/schemata
> index 41dc4a60fff3..c94459eb783a 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -76,6 +76,7 @@ part = {
>      sigstatus?:     sigstatus,
>  
>      content-type:   string,
> +    content-disposition?:       string,
>      content-id?:    string,
>      # if content-type starts with "multipart/":
>      content:        [part*],
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8b38fe6db136..8e69b3465886 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -582,6 +582,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
>      GMimeObject *meta = node->envelope_part ?
>  	GMIME_OBJECT (node->envelope_part) : node->part;
>      GMimeContentType *content_type = g_mime_object_get_content_type (meta);
> +    const char *disposition = _get_disposition (meta);
>      const char *cid = g_mime_object_get_content_id (meta);
>      const char *filename = GMIME_IS_PART (node->part) ?
>  	g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
> @@ -611,6 +612,11 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
>      sp->map_key (sp, "content-type");
>      sp->string (sp, g_mime_content_type_to_string (content_type));
>  
> +    if (disposition) {
> +	sp->map_key (sp, "content-disposition");
> +	sp->string (sp, disposition);
> +    }
> +
>      if (cid) {
>  	sp->map_key (sp, "content-id");
>  	sp->string (sp, cid);
> -- 
> 2.11.0

[PATCH] test: fix tests for content-disposition

[Mark Walters]

This fixes all tests for the recent addition of content-disposition to
the structured format outputs of notmuch-show.
---

Since Jani added the content-disposition as I wanted to use it from emacs (thanks Jani!)
I thought I should fix the tests.

I don't know if it should be folded in to the main patch or applied
separately. Either is fine with me.

Best wishes

Mark



test/T160-json.sh      |  2 +-
 test/T170-sexp.sh      |  2 +-
 test/T190-multipart.sh | 22 ++++++++++++++--------
 test/T350-crypto.sh    |  1 +
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/test/T160-json.sh b/test/T160-json.sh
index b346f37..74e0d8f 100755
--- a/test/T160-json.sh
+++ b/test/T160-json.sh
@@ -48,7 +48,7 @@ output=$(notmuch show --format=json "id:$id")
 filename=$(notmuch search --output=files "id:$id")
 # 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_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\", \"content-disposition\": \"inline\", \"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/T170-sexp.sh b/test/T170-sexp.sh
index 800ebc6..2088972 100755
--- a/test/T170-sexp.sh
+++ b/test/T170-sexp.sh
@@ -39,7 +39,7 @@ output=$(notmuch show --format=sexp "id:$id")
 filename=$(notmuch search --output=files "id:$id")
 # 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_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\" :content-disposition \"inline\" :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\""
diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
index 3567890..b631c5e 100755
--- a/test/T190-multipart.sh
+++ b/test/T190-multipart.sh
@@ -348,11 +348,11 @@ cat <<EOF >EXPECTED
 {"id": "87liy5ap00.fsf@yoom.home.cworth.org", "match": true, "excluded": false, "filename": "${MAIL_DIR}/multipart", "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["attachment","inbox","signed","unread"], "headers": {"Subject": "Multipart message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:43:57 +0000"}, "body": [
 {"id": 1, "content-type": "multipart/signed", "content": [
 {"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": 3, "content-type": "message/rfc822", "content-disposition": "inline", "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", "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": 7, "content-type": "text/plain", "content-disposition": "attachment", "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", "content-length": 197}]}]}
 EOF
@@ -363,11 +363,11 @@ notmuch show --format=json --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OU
 cat <<EOF >EXPECTED
 {"id": 1, "content-type": "multipart/signed", "content": [
 {"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": 3, "content-type": "message/rfc822", "content-disposition": "inline", "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", "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": 7, "content-type": "text/plain", "content-disposition": "attachment", "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", "content-length": 197}]}
 EOF
@@ -377,11 +377,11 @@ test_begin_subtest "--format=json --part=2, multipart/mixed"
 notmuch show --format=json --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 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": 3, "content-type": "message/rfc822", "content-disposition": "inline", "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", "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": 7, "content-type": "text/plain", "content-disposition": "attachment", "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"}]}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
@@ -389,7 +389,7 @@ test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 test_begin_subtest "--format=json --part=3, rfc822 part"
 notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 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": 3, "content-type": "message/rfc822", "content-disposition": "inline", "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", "content-length": 71},
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]}
@@ -422,7 +422,11 @@ test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 test_begin_subtest "--format=json --part=7, inline attachment"
 notmuch show --format=json --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-{"id": 7, "content-type": "text/plain", "filename": "attachment", "content": "This is a text attachment.\n"}
+{"id": 7,
+ "content-type": "text/plain",
+ "filename": "attachment",
+ "content": "This is a text attachment.\n",
+ "content-disposition": "attachment"}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
@@ -641,6 +645,7 @@ notmuch_json_show_sanitize <<EOF >EXPECTED
  "content-type": "multipart/mixed",
  "content": [{"id": 3,
  "content-type": "message/rfc822",
+ "content-disposition": "inline",
  "content": [{"headers": {"Subject": "html message",
  "From": "Carl Worth <cworth@cworth.org>",
  "To": "cworth@cworth.org",
@@ -655,6 +660,7 @@ notmuch_json_show_sanitize <<EOF >EXPECTED
  "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]},
  {"id": 7,
  "content-type": "text/plain",
+ "content-disposition": "attachment",
  "filename": "attachment",
  "content": "This is a text attachment.\n"},
  {"id": 8,
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index a1e5e20..c79ce38 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -205,6 +205,7 @@ expected='[[[{"id": "XXXXX",
  "content": "This is a test encrypted message.\n"},
  {"id": 5,
  "content-type": "application/octet-stream",
+ "content-disposition": "attachment",
  "content-length": "NONZERO",
  "content-transfer-encoding": "base64",
  "filename": "TESTATTACHMENT"}]}]}]},
-- 
2.1.4

Re: [PATCH] test: fix tests for content-disposition

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> This fixes all tests for the recent addition of content-disposition to
> the structured format outputs of notmuch-show.
> ---
>
> Since Jani added the content-disposition as I wanted to use it from emacs (thanks Jani!)
> I thought I should fix the tests.
>
> I don't know if it should be folded in to the main patch or applied
> separately. Either is fine with me.

Unfortunately this does not apply since 75bb23f74874e3820b0295d0a618045fc5f33417

Re: [PATCH] test: fix tests for content-disposition

[Jani Nikula]

On Sun, 26 Feb 2017, David Bremner <david@tethera.net> wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
>
>> This fixes all tests for the recent addition of content-disposition to
>> the structured format outputs of notmuch-show.
>> ---
>>
>> Since Jani added the content-disposition as I wanted to use it from emacs (thanks Jani!)
>> I thought I should fix the tests.
>>
>> I don't know if it should be folded in to the main patch or applied
>> separately. Either is fine with me.
>
> Unfortunately this does not apply since 75bb23f74874e3820b0295d0a618045fc5f33417

Rebased version at id:20170226183348.30470-1-jani@nikula.org with Mark's
test updates squashed in (I dislike having commits where tests fail).

BR,
Jani.