[PATCH 1/2] Add part filename and content-id in notmuch show output if available.

[PATCH 1/2] Add part filename and content-id in notmuch show output if available.

[Dmitry Kurochkin]

Before the change, notmuch show output had filename only for
parts with "Content-Disposition: attachment".  But parts with
inline disposition may have filename as well.

The patch makes notmuch show always output filename if available,
independent of Content-Disposition.  Both JSON and text output
formats are changed.

Also, the patch adds Content-id to text output format of notmuch
show.

The main goal of these changes is to have filenames on Emacs
buttons for inline attachments.  In particular, this is very
helpful for inline patches.

Note: text format changes may require updates in clients that use
it.  The changes are:

* text part header format changed from:

    ^Lpart{ ID: 2, Content-type: text/x-diff

  to:

    ^Lpart{ ID: 2, Filename: cool-feature.patch, Content-type: text/x-diff

* attachment format changed from:

    ^Lattachment{ ID: 4, Content-type: application/octet-stream
    Attachment: data.tar.bz2 (application/octet-stream)
    Non-text part: application/octet-stream
    ^Lattachment}

  to:

    ^Lattachment{ ID: 4, Filename: data.tar.bz2, Content-type: application/octet-stream
    Non-text part: application/octet-stream
    ^Lattachment}
---
 notmuch-show.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index e90f07e..8f275d7 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -445,20 +445,22 @@ format_part_start_text (GMimeObject *part, int *part_count)
 static void
 format_part_content_text (GMimeObject *part)
 {
-    GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
+    const char *cid = g_mime_object_get_content_id (part);
     GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
     GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
 
-    printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type));
-
-    if (disposition &&
-	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+    if (GMIME_IS_PART (part))
     {
 	const char *filename = g_mime_part_get_filename (GMIME_PART (part));
-	printf ("Attachment: %s (%s)\n", filename,
-		g_mime_content_type_to_string (content_type));
+	if (filename)
+	    printf (", Filename: %s", filename);
     }
 
+    if (cid)
+	printf (", Content-id: %s", cid);
+
+    printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type));
+
     if (g_mime_content_type_is_type (content_type, "text", "*") &&
 	!g_mime_content_type_is_type (content_type, "text", "html"))
     {
@@ -580,7 +582,6 @@ format_part_content_json (GMimeObject *part)
     GMimeStream *stream_memory = g_mime_stream_mem_new ();
     const char *cid = g_mime_object_get_content_id (part);
     void *ctx = talloc_new (NULL);
-    GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
     GByteArray *part_content;
 
     printf (", \"content-type\": %s",
@@ -589,12 +590,11 @@ format_part_content_json (GMimeObject *part)
     if (cid != NULL)
 	    printf(", \"content-id\": %s", json_quote_str (ctx, cid));
 
-    if (disposition &&
-	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+    if (GMIME_IS_PART (part))
     {
 	const char *filename = g_mime_part_get_filename (GMIME_PART (part));
-
-	printf (", \"filename\": %s", json_quote_str (ctx, filename));
+	if (filename)
+	    printf (", \"filename\": %s", json_quote_str (ctx, filename));
     }
 
     if (g_mime_content_type_is_type (content_type, "text", "*") &&
-- 
1.7.5.3

[PATCH 2/2] test: fix tests after notmuch show output changes related to filenames

[Dmitry Kurochkin]

---
 test/crypto    |    3 +--
 test/multipart |   12 ++++--------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/test/crypto b/test/crypto
index 8e92016..796c4ca 100755
--- a/test/crypto
+++ b/test/crypto
@@ -168,8 +168,7 @@ Non-text part: application/pgp-encrypted
 \fpart{ ID: 4, Content-type: text/plain
 This is a test encrypted message.
 \fpart}
-\fattachment{ ID: 5, Content-type: application/octet-stream
-Attachment: TESTATTACHMENT (application/octet-stream)
+\fattachment{ ID: 5, Filename: TESTATTACHMENT, Content-type: application/octet-stream
 Non-text part: application/octet-stream
 \fattachment}
 \fpart}
diff --git a/test/multipart b/test/multipart
index 62efd84..9348fb6 100755
--- a/test/multipart
+++ b/test/multipart
@@ -77,8 +77,7 @@ Date: Tue, 05 Jan 2001 15:43:57 -0000
 Non-text part: text/html
 \fpart}
 \fpart}
-\fattachment{ ID: 5, Content-type: text/plain
-Attachment: attachment (text/plain)
+\fattachment{ ID: 5, Filename: attachment, Content-type: text/plain
 This is a text attachment.
 \fattachment}
 \fpart{ ID: 6, Content-type: text/plain
@@ -104,8 +103,7 @@ test_expect_equal "$output" \
 Non-text part: text/html
 \fpart}
 \fpart}
-\fattachment{ ID: 5, Content-type: text/plain
-Attachment: attachment (text/plain)
+\fattachment{ ID: 5, Filename: attachment, Content-type: text/plain
 This is a text attachment.
 \fattachment}
 \fpart{ ID: 6, Content-type: text/plain
@@ -128,8 +126,7 @@ test_expect_equal "$output" \
 Non-text part: text/html
 \fpart}
 \fpart}
-\fattachment{ ID: 5, Content-type: text/plain
-Attachment: attachment (text/plain)
+\fattachment{ ID: 5, Filename: attachment, Content-type: text/plain
 This is a text attachment.
 \fattachment}
 \fpart{ ID: 6, Content-type: text/plain
@@ -158,8 +155,7 @@ Non-text part: text/html
 test_begin_subtest "--format=text --part=5, inline attachement"
 output=$(notmuch show --format=text --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org')
 test_expect_equal "$output" \
-"\fattachment{ ID: 5, Content-type: text/plain
-Attachment: attachment (text/plain)
+"\fattachment{ ID: 5, Filename: attachment, Content-type: text/plain
 This is a text attachment.
 \fattachment}"
 
-- 
1.7.5.3

Re: [PATCH 1/2] Add part filename and content-id in notmuch show output if available.

[Carl Worth]

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

On Sun, 29 May 2011 02:03:47 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Also, the patch adds Content-id to text output format of notmuch
> show.

I don't think I've raised this point before, but I've been tempted to
setup a git hook that rejects any commits with a paragraph beginning
with the word "also" or the word "additionally". These words are clues
to me that the commit is doing multiple independent things and should
really be split up.

> The patch makes notmuch show always output filename if available,
> independent of Content-Disposition.  Both JSON and text output
> formats are changed.

The commit message claims to change both JSON and text output, (and the
patch bears that out). But the test suite only gets changes to text
output.

This suggests to me that the test suite needs an additional test here
(for a "Content-Disposition: inline" attachment with a filename).

What can I say, I must be feeling soft today, because in spite of both
of those issues I've just pushed these two commits, (I did augment the
commit message of the second commit to be more than a single line—I know
that I've discussed that issue before).

Thanks, Dmitry, for the improvements.

-Carl

-- 
carl.d.worth@intel.com

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