[RFC PATCH 0/5] Convert show text format to new style

[RFC PATCH 0/5] Convert show text format to new style

[Austin Clements]

I'm starting to convert the show formats to the new mime_node_t
self-recursive style.  Before I go through all of them, I wanted to
get some feedback.

Besides general feedback, I'm interested in knowing if this is a good
way to structure the patches for review.  Patches 1 through 3 just get
requirements out of the way, but the actual conversion is done in two
steps: patch 4 just moves the existing code into the new code
structure without changing any of the logic and patch 5 simplifies the
restructured code to actually benefit from the new structure.  The
hope is that patch 4 is easy to review even though it's big and that
patch 5 makes the real (and interesting) changes obvious.

[RFC PATCH 1/5] mime node: Record depth-first part numbers

[Austin Clements]

This makes the part numbers readily accessible to formatters.
Hierarchical part numbering would be a more natural and efficient fit
for MIME and may be the way to go in the future, but depth-first
numbering maintains compatibility with what we currently do.
---
 mime-node.c      |   33 ++++++++++++++++++++++++++++++++-
 notmuch-client.h |   11 +++++++++++
 2 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index d26bb44..30b542f 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -104,6 +104,10 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
     root->nchildren = 1;
     root->ctx = mctx;
 
+    root->part_num = 0;
+    root->next_child = 0;
+    root->next_part_num = 1;
+
     *root_out = root;
     return NOTMUCH_STATUS_SUCCESS;
 
@@ -133,6 +137,8 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 	talloc_free (node);
 	return NULL;
     }
+    node->parent = parent;
+    node->part_num = node->next_part_num = -1;
 
     /* Deal with the different types of parts */
     if (GMIME_IS_PART (part)) {
@@ -217,6 +223,7 @@ mime_node_t *
 mime_node_child (const mime_node_t *parent, int child)
 {
     GMimeObject *sub;
+    mime_node_t *node;
 
     if (!parent || child < 0 || child >= parent->nchildren)
 	return NULL;
@@ -234,7 +241,31 @@ mime_node_child (const mime_node_t *parent, int child)
 	INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
 			g_type_name (G_OBJECT_TYPE (parent->part)));
     }
-    return _mime_node_create (parent, sub);
+    node = _mime_node_create (parent, sub);
+
+    if (child == parent->next_child && parent->next_part_num != -1) {
+	/* We're traversing in depth-first order.  Record the child's
+	 * depth-first numbering. */
+	node->part_num = parent->next_part_num;
+	node->next_part_num = node->part_num + 1;
+
+	/* Drop the const qualifier because these are internal fields
+	 * whose mutability doesn't affect the interface. */
+	((mime_node_t*)parent)->next_child++;
+	((mime_node_t*)parent)->next_part_num = -1;
+
+	if (node->nchildren == 0) {
+	    /* We've reached a leaf, so find the parent that has more
+	     * children and set it up to number its next child. */
+	    const mime_node_t *it = node;
+	    while (it && it->next_child == it->nchildren)
+		it = it->parent;
+	    if (it)
+		((mime_node_t*)it)->next_part_num = node->part_num + 1;
+	}
+    }
+
+    return node;
 }
 
 static mime_node_t *
diff --git a/notmuch-client.h b/notmuch-client.h
index 517c010..8832242 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -273,6 +273,13 @@ typedef struct mime_node {
     /* The number of children of this part. */
     int nchildren;
 
+    /* The parent of this node or NULL if this is the root node. */
+    const struct mime_node *parent;
+
+    /* The depth-first part number of this child if the MIME tree is
+     * being traversed in depth-first order, or -1 otherwise. */
+    int part_num;
+
     /* True if decryption of this part was attempted. */
     notmuch_bool_t decrypt_attempted;
     /* True if decryption of this part's child succeeded.  In this
@@ -294,6 +301,10 @@ typedef struct mime_node {
     /* Internal: For successfully decrypted multipart parts, the
      * decrypted part to substitute for the second child. */
     GMimeObject *decrypted_child;
+
+    /* Internal: The next child for depth-first traversal and the part
+     * number to assign it (or -1 if unknown). */
+    int next_child, next_part_num;
 } mime_node_t;
 
 /* Construct a new MIME node pointing to the root message part of
-- 
1.7.7.3

[RFC PATCH 2/5] show: Introduce mime_node formatter callback

[Austin Clements]

This callback is the gateway to the new mime_node_t-based formatters.
This maintains backwards compatibility so the formatters can be
transitioned one at a time.  Once this is done, the formatter
structure can be safely gutted.
---
 notmuch-client.h |    6 ++++++
 notmuch-reply.c  |    2 +-
 notmuch-show.c   |   22 ++++++++++++++++++----
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 8832242..52448ef 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -54,8 +54,14 @@
 #define STRINGIFY(s) STRINGIFY_(s)
 #define STRINGIFY_(s) #s
 
+struct mime_node;
+struct notmuch_show_params;
+
 typedef struct notmuch_show_format {
     const char *message_set_start;
+    void (*part) (const void *ctx,
+		  struct mime_node *node, int indent,
+		  struct notmuch_show_params *params);
     const char *message_start;
     void (*message) (const void *ctx,
 		     notmuch_message_t *message,
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 000f6da..33c4ee8 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -31,7 +31,7 @@ static void
 reply_part_content (GMimeObject *part);
 
 static const notmuch_show_format_t format_reply = {
-    "",
+    "", NULL,
 	"", NULL,
 	    "", NULL, reply_headers_message_part, ">\n",
 	    "",
diff --git a/notmuch-show.c b/notmuch-show.c
index 0200b9c..45c3569 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -42,7 +42,7 @@ static void
 format_part_end_text (GMimeObject *part);
 
 static const notmuch_show_format_t format_text = {
-    "",
+    "", NULL,
 	"\fmessage{ ", format_message_text,
 	    "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n",
 	    "\fbody{\n",
@@ -85,7 +85,7 @@ static void
 format_part_end_json (GMimeObject *part);
 
 static const notmuch_show_format_t format_json = {
-    "[",
+    "[", NULL,
 	"{", format_message_json,
 	    "\"headers\": {", format_headers_json, format_headers_message_part_json, "}",
 	    ", \"body\": [",
@@ -106,7 +106,7 @@ format_message_mbox (const void *ctx,
 		     unused (int indent));
 
 static const notmuch_show_format_t format_mbox = {
-    "",
+    "", NULL,
         "", format_message_mbox,
             "", NULL, NULL, "",
             "",
@@ -125,7 +125,7 @@ static void
 format_part_content_raw (GMimeObject *part);
 
 static const notmuch_show_format_t format_raw = {
-    "",
+    "", NULL,
 	"", NULL,
 	    "", NULL, format_headers_message_part_text, "\n",
             "",
@@ -744,6 +744,20 @@ show_message (void *ctx,
 	      int indent,
 	      notmuch_show_params_t *params)
 {
+    if (format->part) {
+	void *local = talloc_new (ctx);
+	mime_node_t *root, *part;
+
+	if (mime_node_open (local, message, params->cryptoctx, params->decrypt,
+			    &root) != NOTMUCH_STATUS_SUCCESS)
+	    return;
+	part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part);
+	if (part)
+	    format->part (local, part, indent, params);
+	talloc_free (local);
+	return;
+    }
+
     if (params->part <= 0) {
 	fputs (format->message_start, stdout);
 	if (format->message)
-- 
1.7.7.3

[RFC PATCH 3/5] show: Use consistent header ordering in the text format

[Austin Clements]

Previously, top-level message headers were printed as Subject, From,
To, Date, while embedded message headers were printed From, To,
Subject, Date.  This makes both cases use the former order and updates
the tests accordingly.

Strangely, the raw format also uses this function, so this also fixes
the two raw format tests affected by this change.
---
 notmuch-show.c |    2 +-
 test/multipart |   12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 45c3569..ad54d69 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -360,6 +360,7 @@ format_headers_message_part_text (GMimeMessage *message)
     InternetAddressList *recipients;
     const char *recipients_string;
 
+    printf ("Subject: %s\n", g_mime_message_get_subject (message));
     printf ("From: %s\n", g_mime_message_get_sender (message));
     recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
     recipients_string = internet_address_list_to_string (recipients, 0);
@@ -371,7 +372,6 @@ format_headers_message_part_text (GMimeMessage *message)
     if (recipients_string)
 	printf ("Cc: %s\n",
 		recipients_string);
-    printf ("Subject: %s\n", g_mime_message_get_subject (message));
     printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
 }
 
diff --git a/test/multipart b/test/multipart
index f83526b..2dd73f5 100755
--- a/test/multipart
+++ b/test/multipart
@@ -121,9 +121,9 @@ Date: Fri, 05 Jan 2001 15:43:57 +0000
 \fpart{ ID: 2, Content-type: multipart/mixed
 \fpart{ ID: 3, Content-type: message/rfc822
 \fheader{
+Subject: html message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: html message
 Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
@@ -162,9 +162,9 @@ cat <<EOF >EXPECTED
 \fpart{ ID: 2, Content-type: multipart/mixed
 \fpart{ ID: 3, Content-type: message/rfc822
 \fheader{
+Subject: html message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: html message
 Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
@@ -200,9 +200,9 @@ cat <<EOF >EXPECTED
 \fpart{ ID: 2, Content-type: multipart/mixed
 \fpart{ ID: 3, Content-type: message/rfc822
 \fheader{
+Subject: html message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: html message
 Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
@@ -233,9 +233,9 @@ notmuch show --format=text --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OU
 cat <<EOF >EXPECTED
 \fpart{ ID: 3, Content-type: message/rfc822
 \fheader{
+Subject: html message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: html message
 Date: Fri, 05 Jan 2001 15:42:57 +0000
 \fheader}
 \fbody{
@@ -452,9 +452,9 @@ notmuch show --format=raw --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUT
 # output should *not* include newline
 echo >>OUTPUT
 cat <<EOF >EXPECTED
+Subject: html message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: html message
 Date: Fri, 05 Jan 2001 15:42:57 +0000
 
 <p>This is an embedded message, with a multipart/alternative part.</p>
@@ -476,9 +476,9 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "--format=raw --part=2, multipart/mixed"
 notmuch show --format=raw --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
+Subject: html message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: html message
 Date: Fri, 05 Jan 2001 15:42:57 +0000
 
 <p>This is an embedded message, with a multipart/alternative part.</p>
-- 
1.7.7.3

[RFC PATCH 4/5] show: Convert text format to the new self-recursive style

[Austin Clements]

This is all code movement and a smidgen of glue.  This moves the
existing text formatter code into one self-recursive function, but
doesn't change any of the logic.  The next patch will actually take
advantage of what the new structure has to offer.
---
 notmuch-show.c |  267 +++++++++++++++++++++++++++++---------------------------
 1 files changed, 140 insertions(+), 127 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index ad54d69..3241965 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -21,40 +21,17 @@
 #include "notmuch-client.h"
 
 static void
-format_message_text (unused (const void *ctx),
-		     notmuch_message_t *message,
-		     int indent);
-static void
-format_headers_text (const void *ctx,
-		     notmuch_message_t *message);
-
-static void
 format_headers_message_part_text (GMimeMessage *message);
 
 static void
-format_part_start_text (GMimeObject *part,
-			int *part_count);
-
-static void
-format_part_content_text (GMimeObject *part);
-
-static void
-format_part_end_text (GMimeObject *part);
+format_part_text (const void *ctx, mime_node_t *node,
+		  int indent, notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_text = {
-    "", NULL,
-	"\fmessage{ ", format_message_text,
-	    "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n",
-	    "\fbody{\n",
-	        format_part_start_text,
-	        NULL,
-	        NULL,
-	        format_part_content_text,
-	        format_part_end_text,
-	        "",
-	    "\fbody}\n",
-	"\fmessage}\n", "",
-    ""
+    .message_set_start = "",
+    .part = format_part_text,
+    .message_set_sep = "",
+    .message_set_end = ""
 };
 
 static void
@@ -140,6 +117,9 @@ static const notmuch_show_format_t format_raw = {
     ""
 };
 
+static void
+show_text_part_content (GMimeObject *part, GMimeStream *stream_out);
+
 static const char *
 _get_tags_as_string (const void *ctx, notmuch_message_t *message)
 {
@@ -187,13 +167,138 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
 }
 
 static void
-format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent)
+format_part_text (const void *ctx, mime_node_t *node,
+		  int indent, notmuch_show_params_t *params)
 {
-    printf ("id:%s depth:%d match:%d filename:%s\n",
-	    notmuch_message_get_message_id (message),
-	    indent,
-	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
-	    notmuch_message_get_filename (message));
+    /* The disposition and content-type metadata are associated with
+     * the envelope for message parts */
+    GMimeObject *meta = node->envelope_part ?
+	GMIME_OBJECT (node->envelope_part) : node->part;
+    GMimeContentType *content_type = g_mime_object_get_content_type (meta);
+    int i;
+
+    if (node->envelope_file) {
+	notmuch_message_t *message = node->envelope_file;
+	const char *headers[] = {
+	    "Subject", "From", "To", "Cc", "Bcc", "Date"
+	};
+	const char *name, *value;
+	unsigned int i;
+
+	printf ("\fmessage{ ");
+	printf ("id:%s depth:%d match:%d filename:%s\n",
+		notmuch_message_get_message_id (message),
+		indent,
+		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
+		notmuch_message_get_filename (message));
+
+	printf ("\fheader{\n");
+
+	printf ("%s\n", _get_one_line_summary (ctx, message));
+
+	for (i = 0; i < ARRAY_SIZE (headers); i++) {
+	    name = headers[i];
+	    value = notmuch_message_get_header (message, name);
+	    if (value && strlen (value))
+		printf ("%s: %s\n", name, value);
+	}
+	printf ("\fheader}\n");
+    } else {
+	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
+	const char *cid = g_mime_object_get_content_id (meta);
+
+	if (disposition &&
+	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+	{
+	    printf ("\fattachment{ ID: %d", node->part_num);
+
+	} else {
+
+	    printf ("\fpart{ ID: %d", node->part_num);
+	}
+
+	if (GMIME_IS_PART (node->part))
+	{
+	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
+	    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 (node->envelope_part) {
+	GMimeMessage *message = GMIME_MESSAGE (node->part);
+	InternetAddressList *recipients;
+	const char *recipients_string;
+
+	printf ("\fheader{\n");
+	printf ("Subject: %s\n", g_mime_message_get_subject (message));
+	printf ("From: %s\n", g_mime_message_get_sender (message));
+	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
+	recipients_string = internet_address_list_to_string (recipients, 0);
+	if (recipients_string)
+	    printf ("To: %s\n", recipients_string);
+	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
+	recipients_string = internet_address_list_to_string (recipients, 0);
+	if (recipients_string)
+	    printf ("Cc: %s\n", recipients_string);
+	printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
+	printf ("\fheader}\n");
+    }
+
+    if (!node->envelope_file) {
+	if (g_mime_content_type_is_type (content_type, "text", "*") &&
+	    !g_mime_content_type_is_type (content_type, "text", "html"))
+	{
+	    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);
+	    g_object_unref(stream_stdout);
+	}
+	else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
+		 g_mime_content_type_is_type (content_type, "message", "rfc822"))
+	{
+	    /* Do nothing for multipart since its content will be printed
+	     * when recursing. */
+	}
+	else
+	{
+	    printf ("Non-text part: %s\n",
+		    g_mime_content_type_to_string (content_type));
+	}
+    }
+
+    if (GMIME_IS_MESSAGE (node->part)) {
+	printf ("\fbody{\n");
+    }
+
+    for (i = 0; i < node->nchildren; i++)
+	format_part_text (ctx, mime_node_child (node, i), indent, params);
+
+    if (GMIME_IS_MESSAGE (node->part)) {
+	printf ("\fbody}\n");
+    }
+
+    if (node->envelope_file) {
+	printf ("\fmessage}\n");
+    } else {
+	GMimeContentDisposition *disposition;
+
+	disposition = g_mime_object_get_content_disposition (meta);
+	if (disposition &&
+	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+	{
+	    printf ("\fattachment}\n");
+	}
+	else
+	{
+	    printf ("\fpart}\n");
+	}
+    }
 }
 
 static void
@@ -334,26 +439,6 @@ format_message_mbox (const void *ctx,
     fclose (file);
 }
 
-
-static void
-format_headers_text (const void *ctx, notmuch_message_t *message)
-{
-    const char *headers[] = {
-	"Subject", "From", "To", "Cc", "Bcc", "Date"
-    };
-    const char *name, *value;
-    unsigned int i;
-
-    printf ("%s\n", _get_one_line_summary (ctx, message));
-
-    for (i = 0; i < ARRAY_SIZE (headers); i++) {
-	name = headers[i];
-	value = notmuch_message_get_header (message, name);
-	if (value && strlen (value))
-	    printf ("%s: %s\n", name, value);
-    }
-}
-
 static void
 format_headers_message_part_text (GMimeMessage *message)
 {
@@ -503,78 +588,6 @@ signer_status_to_string (GMimeSignerStatus x)
 }
 
 static void
-format_part_start_text (GMimeObject *part, int *part_count)
-{
-    GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
-
-    if (disposition &&
-	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-    {
-	printf ("\fattachment{ ID: %d", *part_count);
-
-    } else {
-
-	printf ("\fpart{ ID: %d", *part_count);
-    }
-}
-
-static void
-format_part_content_text (GMimeObject *part)
-{
-    const char *cid = g_mime_object_get_content_id (part);
-    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
-
-    if (GMIME_IS_PART (part))
-    {
-	const char *filename = g_mime_part_get_filename (GMIME_PART (part));
-	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"))
-    {
-	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 (part, stream_stdout);
-	g_object_unref(stream_stdout);
-    }
-    else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
-	     g_mime_content_type_is_type (content_type, "message", "rfc822"))
-    {
-	/* Do nothing for multipart since its content will be printed
-	 * when recursing. */
-    }
-    else
-    {
-	printf ("Non-text part: %s\n",
-		g_mime_content_type_to_string (content_type));
-    }
-}
-
-static void
-format_part_end_text (GMimeObject *part)
-{
-    GMimeContentDisposition *disposition;
-
-    disposition = g_mime_object_get_content_disposition (part);
-    if (disposition &&
-	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-    {
-	printf ("\fattachment}\n");
-    }
-    else
-    {
-	printf ("\fpart}\n");
-    }
-}
-
-static void
 format_part_start_json (unused (GMimeObject *part), int *part_count)
 {
     printf ("{\"id\": %d", *part_count);
-- 
1.7.7.3

Re: [RFC PATCH 4/5] show: Convert text format to the new self-recursive style

[Dmitry Kurochkin]

On Wed, 11 Jan 2012 20:49:32 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> This is all code movement and a smidgen of glue.  This moves the
> existing text formatter code into one self-recursive function, but
> doesn't change any of the logic.  The next patch will actually take
> advantage of what the new structure has to offer.

The patch looks good to me.

Regards,
  Dmitry

> ---
>  notmuch-show.c |  267 +++++++++++++++++++++++++++++---------------------------
>  1 files changed, 140 insertions(+), 127 deletions(-)
> 
> diff --git a/notmuch-show.c b/notmuch-show.c
> index ad54d69..3241965 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -21,40 +21,17 @@
>  #include "notmuch-client.h"
>  
>  static void
> -format_message_text (unused (const void *ctx),
> -		     notmuch_message_t *message,
> -		     int indent);
> -static void
> -format_headers_text (const void *ctx,
> -		     notmuch_message_t *message);
> -
> -static void
>  format_headers_message_part_text (GMimeMessage *message);
>  
>  static void
> -format_part_start_text (GMimeObject *part,
> -			int *part_count);
> -
> -static void
> -format_part_content_text (GMimeObject *part);
> -
> -static void
> -format_part_end_text (GMimeObject *part);
> +format_part_text (const void *ctx, mime_node_t *node,
> +		  int indent, notmuch_show_params_t *params);
>  
>  static const notmuch_show_format_t format_text = {
> -    "", NULL,
> -	"\fmessage{ ", format_message_text,
> -	    "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n",
> -	    "\fbody{\n",
> -	        format_part_start_text,
> -	        NULL,
> -	        NULL,
> -	        format_part_content_text,
> -	        format_part_end_text,
> -	        "",
> -	    "\fbody}\n",
> -	"\fmessage}\n", "",
> -    ""
> +    .message_set_start = "",
> +    .part = format_part_text,
> +    .message_set_sep = "",
> +    .message_set_end = ""
>  };
>  
>  static void
> @@ -140,6 +117,9 @@ static const notmuch_show_format_t format_raw = {
>      ""
>  };
>  
> +static void
> +show_text_part_content (GMimeObject *part, GMimeStream *stream_out);
> +
>  static const char *
>  _get_tags_as_string (const void *ctx, notmuch_message_t *message)
>  {
> @@ -187,13 +167,138 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
>  }
>  
>  static void
> -format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent)
> +format_part_text (const void *ctx, mime_node_t *node,
> +		  int indent, notmuch_show_params_t *params)
>  {
> -    printf ("id:%s depth:%d match:%d filename:%s\n",
> -	    notmuch_message_get_message_id (message),
> -	    indent,
> -	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> -	    notmuch_message_get_filename (message));
> +    /* The disposition and content-type metadata are associated with
> +     * the envelope for message parts */
> +    GMimeObject *meta = node->envelope_part ?
> +	GMIME_OBJECT (node->envelope_part) : node->part;
> +    GMimeContentType *content_type = g_mime_object_get_content_type (meta);
> +    int i;
> +
> +    if (node->envelope_file) {
> +	notmuch_message_t *message = node->envelope_file;
> +	const char *headers[] = {
> +	    "Subject", "From", "To", "Cc", "Bcc", "Date"
> +	};
> +	const char *name, *value;
> +	unsigned int i;
> +
> +	printf ("\fmessage{ ");
> +	printf ("id:%s depth:%d match:%d filename:%s\n",
> +		notmuch_message_get_message_id (message),
> +		indent,
> +		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> +		notmuch_message_get_filename (message));
> +
> +	printf ("\fheader{\n");
> +
> +	printf ("%s\n", _get_one_line_summary (ctx, message));
> +
> +	for (i = 0; i < ARRAY_SIZE (headers); i++) {
> +	    name = headers[i];
> +	    value = notmuch_message_get_header (message, name);
> +	    if (value && strlen (value))
> +		printf ("%s: %s\n", name, value);
> +	}
> +	printf ("\fheader}\n");
> +    } else {
> +	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
> +	const char *cid = g_mime_object_get_content_id (meta);
> +
> +	if (disposition &&
> +	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> +	{
> +	    printf ("\fattachment{ ID: %d", node->part_num);
> +
> +	} else {
> +
> +	    printf ("\fpart{ ID: %d", node->part_num);
> +	}
> +
> +	if (GMIME_IS_PART (node->part))
> +	{
> +	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
> +	    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 (node->envelope_part) {
> +	GMimeMessage *message = GMIME_MESSAGE (node->part);
> +	InternetAddressList *recipients;
> +	const char *recipients_string;
> +
> +	printf ("\fheader{\n");
> +	printf ("Subject: %s\n", g_mime_message_get_subject (message));
> +	printf ("From: %s\n", g_mime_message_get_sender (message));
> +	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
> +	recipients_string = internet_address_list_to_string (recipients, 0);
> +	if (recipients_string)
> +	    printf ("To: %s\n", recipients_string);
> +	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
> +	recipients_string = internet_address_list_to_string (recipients, 0);
> +	if (recipients_string)
> +	    printf ("Cc: %s\n", recipients_string);
> +	printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
> +	printf ("\fheader}\n");
> +    }
> +
> +    if (!node->envelope_file) {
> +	if (g_mime_content_type_is_type (content_type, "text", "*") &&
> +	    !g_mime_content_type_is_type (content_type, "text", "html"))
> +	{
> +	    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);
> +	    g_object_unref(stream_stdout);
> +	}
> +	else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
> +		 g_mime_content_type_is_type (content_type, "message", "rfc822"))
> +	{
> +	    /* Do nothing for multipart since its content will be printed
> +	     * when recursing. */
> +	}
> +	else
> +	{
> +	    printf ("Non-text part: %s\n",
> +		    g_mime_content_type_to_string (content_type));
> +	}
> +    }
> +
> +    if (GMIME_IS_MESSAGE (node->part)) {
> +	printf ("\fbody{\n");
> +    }
> +
> +    for (i = 0; i < node->nchildren; i++)
> +	format_part_text (ctx, mime_node_child (node, i), indent, params);
> +
> +    if (GMIME_IS_MESSAGE (node->part)) {
> +	printf ("\fbody}\n");
> +    }
> +
> +    if (node->envelope_file) {
> +	printf ("\fmessage}\n");
> +    } else {
> +	GMimeContentDisposition *disposition;
> +
> +	disposition = g_mime_object_get_content_disposition (meta);
> +	if (disposition &&
> +	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> +	{
> +	    printf ("\fattachment}\n");
> +	}
> +	else
> +	{
> +	    printf ("\fpart}\n");
> +	}
> +    }
>  }
>  
>  static void
> @@ -334,26 +439,6 @@ format_message_mbox (const void *ctx,
>      fclose (file);
>  }
>  
> -
> -static void
> -format_headers_text (const void *ctx, notmuch_message_t *message)
> -{
> -    const char *headers[] = {
> -	"Subject", "From", "To", "Cc", "Bcc", "Date"
> -    };
> -    const char *name, *value;
> -    unsigned int i;
> -
> -    printf ("%s\n", _get_one_line_summary (ctx, message));
> -
> -    for (i = 0; i < ARRAY_SIZE (headers); i++) {
> -	name = headers[i];
> -	value = notmuch_message_get_header (message, name);
> -	if (value && strlen (value))
> -	    printf ("%s: %s\n", name, value);
> -    }
> -}
> -
>  static void
>  format_headers_message_part_text (GMimeMessage *message)
>  {
> @@ -503,78 +588,6 @@ signer_status_to_string (GMimeSignerStatus x)
>  }
>  
>  static void
> -format_part_start_text (GMimeObject *part, int *part_count)
> -{
> -    GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
> -
> -    if (disposition &&
> -	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> -    {
> -	printf ("\fattachment{ ID: %d", *part_count);
> -
> -    } else {
> -
> -	printf ("\fpart{ ID: %d", *part_count);
> -    }
> -}
> -
> -static void
> -format_part_content_text (GMimeObject *part)
> -{
> -    const char *cid = g_mime_object_get_content_id (part);
> -    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
> -
> -    if (GMIME_IS_PART (part))
> -    {
> -	const char *filename = g_mime_part_get_filename (GMIME_PART (part));
> -	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"))
> -    {
> -	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 (part, stream_stdout);
> -	g_object_unref(stream_stdout);
> -    }
> -    else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
> -	     g_mime_content_type_is_type (content_type, "message", "rfc822"))
> -    {
> -	/* Do nothing for multipart since its content will be printed
> -	 * when recursing. */
> -    }
> -    else
> -    {
> -	printf ("Non-text part: %s\n",
> -		g_mime_content_type_to_string (content_type));
> -    }
> -}
> -
> -static void
> -format_part_end_text (GMimeObject *part)
> -{
> -    GMimeContentDisposition *disposition;
> -
> -    disposition = g_mime_object_get_content_disposition (part);
> -    if (disposition &&
> -	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> -    {
> -	printf ("\fattachment}\n");
> -    }
> -    else
> -    {
> -	printf ("\fpart}\n");
> -    }
> -}
> -
> -static void
>  format_part_start_json (unused (GMimeObject *part), int *part_count)
>  {
>      printf ("{\"id\": %d", *part_count);
> -- 
> 1.7.7.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[RFC PATCH 5/5] show: Simplify new text formatter code

[Austin Clements]

This makes the text formatter take advantage of the new code
structure.  The previously duplicated header logic is now unified,
several things that we used to compute repeatedly across different
callbacks are now computed once, and the code is generally simplified.

Unifying the header logic causes this to format some dates slightly
differently, so the two affected test cases are updated.
---
 notmuch-show.c     |   84 ++++++++++++----------------------------------------
 test/crypto        |    2 +-
 test/thread-naming |   16 +++++-----
 3 files changed, 28 insertions(+), 74 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 3241965..1689222 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -175,67 +175,42 @@ format_part_text (const void *ctx, 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);
+    GMimeContentDisposition *disposition =
+	g_mime_object_get_content_disposition (meta);
+    notmuch_bool_t attachment = disposition &&
+	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0;
+    notmuch_bool_t leaf = GMIME_IS_PART (node->part);
     int i;
 
     if (node->envelope_file) {
 	notmuch_message_t *message = node->envelope_file;
-	const char *headers[] = {
-	    "Subject", "From", "To", "Cc", "Bcc", "Date"
-	};
-	const char *name, *value;
-	unsigned int i;
 
-	printf ("\fmessage{ ");
-	printf ("id:%s depth:%d match:%d filename:%s\n",
+	printf ("\fmessage{ id:%s depth:%d match:%d filename:%s\n",
 		notmuch_message_get_message_id (message),
 		indent,
 		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
 		notmuch_message_get_filename (message));
-
-	printf ("\fheader{\n");
-
-	printf ("%s\n", _get_one_line_summary (ctx, message));
-
-	for (i = 0; i < ARRAY_SIZE (headers); i++) {
-	    name = headers[i];
-	    value = notmuch_message_get_header (message, name);
-	    if (value && strlen (value))
-		printf ("%s: %s\n", name, value);
-	}
-	printf ("\fheader}\n");
     } else {
-	GMimeContentDisposition *disposition = g_mime_object_get_content_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 &&
-	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-	{
-	    printf ("\fattachment{ ID: %d", node->part_num);
-
-	} else {
-
-	    printf ("\fpart{ ID: %d", node->part_num);
-	}
-
-	if (GMIME_IS_PART (node->part))
-	{
-	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
-	    if (filename)
-		printf (", Filename: %s", filename);
-	}
-
+	printf ("\f%s{ ID: %d", attachment ? "attachment" : "part", node->part_num);
+	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 (node->envelope_part) {
+    if (GMIME_IS_MESSAGE (node->part)) {
 	GMimeMessage *message = GMIME_MESSAGE (node->part);
 	InternetAddressList *recipients;
 	const char *recipients_string;
 
 	printf ("\fheader{\n");
+	if (node->envelope_file)
+	    printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
 	printf ("Subject: %s\n", g_mime_message_get_subject (message));
 	printf ("From: %s\n", g_mime_message_get_sender (message));
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
@@ -248,9 +223,11 @@ format_part_text (const void *ctx, mime_node_t *node,
 	    printf ("Cc: %s\n", recipients_string);
 	printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
 	printf ("\fheader}\n");
+
+	printf ("\fbody{\n");
     }
 
-    if (!node->envelope_file) {
+    if (leaf) {
 	if (g_mime_content_type_is_type (content_type, "text", "*") &&
 	    !g_mime_content_type_is_type (content_type, "text", "html"))
 	{
@@ -258,24 +235,12 @@ format_part_text (const void *ctx, mime_node_t *node,
 	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
 	    show_text_part_content (node->part, stream_stdout);
 	    g_object_unref(stream_stdout);
-	}
-	else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
-		 g_mime_content_type_is_type (content_type, "message", "rfc822"))
-	{
-	    /* Do nothing for multipart since its content will be printed
-	     * when recursing. */
-	}
-	else
-	{
+	} else {
 	    printf ("Non-text part: %s\n",
 		    g_mime_content_type_to_string (content_type));
 	}
     }
 
-    if (GMIME_IS_MESSAGE (node->part)) {
-	printf ("\fbody{\n");
-    }
-
     for (i = 0; i < node->nchildren; i++)
 	format_part_text (ctx, mime_node_child (node, i), indent, params);
 
@@ -286,18 +251,7 @@ format_part_text (const void *ctx, mime_node_t *node,
     if (node->envelope_file) {
 	printf ("\fmessage}\n");
     } else {
-	GMimeContentDisposition *disposition;
-
-	disposition = g_mime_object_get_content_disposition (meta);
-	if (disposition &&
-	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-	{
-	    printf ("\fattachment}\n");
-	}
-	else
-	{
-	    printf ("\fpart}\n");
-	}
+	printf ("\f%s}\n", attachment ? "attachment" : "part");
     }
 }
 
diff --git a/test/crypto b/test/crypto
index 0af4aa8..6723ef8 100755
--- a/test/crypto
+++ b/test/crypto
@@ -157,7 +157,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2000-01-01) (encrypted inbox)
 Subject: test encrypted message 001
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: test_suite@notmuchmail.org
-Date: 01 Jan 2000 12:00:00 -0000
+Date: Sat, 01 Jan 2000 12:00:00 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: multipart/encrypted
diff --git a/test/thread-naming b/test/thread-naming
index 41b97d9..b7c96f2 100755
--- a/test/thread-naming
+++ b/test/thread-naming
@@ -71,7 +71,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (unread)
 Subject: thread-naming: Initial thread subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 05 Jan 2001 15:43:56 -0000
+Date: Fri, 05 Jan 2001 15:43:56 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
@@ -85,7 +85,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-06) (inbox unread)
 Subject: thread-naming: Older changed subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Sat, 06 Jan 2001 15:43:56 -0000
+Date: Sat, 06 Jan 2001 15:43:56 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
@@ -99,7 +99,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-07) (inbox unread)
 Subject: thread-naming: Newer changed subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Sun, 07 Jan 2001 15:43:56 -0000
+Date: Sun, 07 Jan 2001 15:43:56 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
@@ -113,7 +113,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-08) (unread)
 Subject: thread-naming: Final thread subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Mon, 08 Jan 2001 15:43:56 -0000
+Date: Mon, 08 Jan 2001 15:43:56 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
@@ -127,7 +127,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-09) (inbox unread)
 Subject: Re: thread-naming: Initial thread subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Tue, 09 Jan 2001 15:43:45 -0000
+Date: Tue, 09 Jan 2001 15:43:45 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
@@ -141,7 +141,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-10) (inbox unread)
 Subject: Aw: thread-naming: Initial thread subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Wed, 10 Jan 2001 15:43:45 -0000
+Date: Wed, 10 Jan 2001 15:43:45 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
@@ -155,7 +155,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-11) (inbox unread)
 Subject: Vs: thread-naming: Initial thread subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Thu, 11 Jan 2001 15:43:45 -0000
+Date: Thu, 11 Jan 2001 15:43:45 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
@@ -169,7 +169,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-12) (inbox unread)
 Subject: Sv: thread-naming: Initial thread subject
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 12 Jan 2001 15:43:45 -0000
+Date: Fri, 12 Jan 2001 15:43:45 +0000
 \fheader}
 \fbody{
 \fpart{ ID: 1, Content-type: text/plain
-- 
1.7.7.3

Re: [RFC PATCH 5/5] show: Simplify new text formatter code

[Dmitry Kurochkin]

On Wed, 11 Jan 2012 20:49:33 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> This makes the text formatter take advantage of the new code
> structure.  The previously duplicated header logic is now unified,
> several things that we used to compute repeatedly across different
> callbacks are now computed once, and the code is generally simplified.
> 
> Unifying the header logic causes this to format some dates slightly
> differently, so the two affected test cases are updated.

Thanks for these patches, Austin.  They are a definite improvement for
notmuch show code.  I hope it would soon get pushed to master.  And I
hope more patches would follow :)

Few minor comments below.

> ---
>  notmuch-show.c     |   84 ++++++++++++----------------------------------------
>  test/crypto        |    2 +-
>  test/thread-naming |   16 +++++-----
>  3 files changed, 28 insertions(+), 74 deletions(-)
> 
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 3241965..1689222 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -175,67 +175,42 @@ format_part_text (const void *ctx, 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);
> +    GMimeContentDisposition *disposition =
> +	g_mime_object_get_content_disposition (meta);
> +    notmuch_bool_t attachment = disposition &&
> +	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0;

If I did not miss anything, attachment is used only as following:

  attachment ? "attachment" : "part"

Please make it const char[] and set to "attachment" or "part".

> +    notmuch_bool_t leaf = GMIME_IS_PART (node->part);

Please add const where possible to local variables (e.g. attachment, leaf).

>      int i;
>  
>      if (node->envelope_file) {
>  	notmuch_message_t *message = node->envelope_file;
> -	const char *headers[] = {
> -	    "Subject", "From", "To", "Cc", "Bcc", "Date"
> -	};
> -	const char *name, *value;
> -	unsigned int i;
>  
> -	printf ("\fmessage{ ");
> -	printf ("id:%s depth:%d match:%d filename:%s\n",
> +	printf ("\fmessage{ id:%s depth:%d match:%d filename:%s\n",
>  		notmuch_message_get_message_id (message),
>  		indent,
>  		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
>  		notmuch_message_get_filename (message));
> -
> -	printf ("\fheader{\n");
> -
> -	printf ("%s\n", _get_one_line_summary (ctx, message));
> -
> -	for (i = 0; i < ARRAY_SIZE (headers); i++) {
> -	    name = headers[i];
> -	    value = notmuch_message_get_header (message, name);
> -	    if (value && strlen (value))
> -		printf ("%s: %s\n", name, value);
> -	}
> -	printf ("\fheader}\n");

Yay!  Only one header-printing code left :)

>      } else {
> -	GMimeContentDisposition *disposition = g_mime_object_get_content_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 &&
> -	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> -	{
> -	    printf ("\fattachment{ ID: %d", node->part_num);
> -
> -	} else {
> -
> -	    printf ("\fpart{ ID: %d", node->part_num);
> -	}
> -
> -	if (GMIME_IS_PART (node->part))
> -	{
> -	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
> -	    if (filename)
> -		printf (", Filename: %s", filename);
> -	}
> -
> +	printf ("\f%s{ ID: %d", attachment ? "attachment" : "part", node->part_num);
> +	if (filename)

I always forget about it, can we declare variables inside if condition
like in C++?  I.e.:

  if (const char *filename = leaf ? g_mime_part_get_filename (GMIME_PART (node->part)) : NULL)

If yes, I would prefer to use this style where possible.

> +	    printf (", Filename: %s", filename);
>  	if (cid)
>  	    printf (", Content-id: %s", cid);
> -

I would revert blank line changes.  But I do not insist :)

>  	printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type));
>      }
>  
> -    if (node->envelope_part) {
> +    if (GMIME_IS_MESSAGE (node->part)) {

This condition is repeated at least twice.  Please consider moving the
message variable below to the top level and using it in the conditions.

Regards,
  Dmitry

>  	GMimeMessage *message = GMIME_MESSAGE (node->part);
>  	InternetAddressList *recipients;
>  	const char *recipients_string;
>  
>  	printf ("\fheader{\n");
> +	if (node->envelope_file)
> +	    printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
>  	printf ("Subject: %s\n", g_mime_message_get_subject (message));
>  	printf ("From: %s\n", g_mime_message_get_sender (message));
>  	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
> @@ -248,9 +223,11 @@ format_part_text (const void *ctx, mime_node_t *node,
>  	    printf ("Cc: %s\n", recipients_string);
>  	printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
>  	printf ("\fheader}\n");
> +
> +	printf ("\fbody{\n");
>      }
>  
> -    if (!node->envelope_file) {
> +    if (leaf) {
>  	if (g_mime_content_type_is_type (content_type, "text", "*") &&
>  	    !g_mime_content_type_is_type (content_type, "text", "html"))
>  	{
> @@ -258,24 +235,12 @@ format_part_text (const void *ctx, mime_node_t *node,
>  	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
>  	    show_text_part_content (node->part, stream_stdout);
>  	    g_object_unref(stream_stdout);
> -	}
> -	else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
> -		 g_mime_content_type_is_type (content_type, "message", "rfc822"))
> -	{
> -	    /* Do nothing for multipart since its content will be printed
> -	     * when recursing. */
> -	}
> -	else
> -	{
> +	} else {
>  	    printf ("Non-text part: %s\n",
>  		    g_mime_content_type_to_string (content_type));
>  	}
>      }
>  
> -    if (GMIME_IS_MESSAGE (node->part)) {
> -	printf ("\fbody{\n");
> -    }
> -
>      for (i = 0; i < node->nchildren; i++)
>  	format_part_text (ctx, mime_node_child (node, i), indent, params);
>  
> @@ -286,18 +251,7 @@ format_part_text (const void *ctx, mime_node_t *node,
>      if (node->envelope_file) {
>  	printf ("\fmessage}\n");
>      } else {
> -	GMimeContentDisposition *disposition;
> -
> -	disposition = g_mime_object_get_content_disposition (meta);
> -	if (disposition &&
> -	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> -	{
> -	    printf ("\fattachment}\n");
> -	}
> -	else
> -	{
> -	    printf ("\fpart}\n");
> -	}
> +	printf ("\f%s}\n", attachment ? "attachment" : "part");
>      }
>  }
>  
> diff --git a/test/crypto b/test/crypto
> index 0af4aa8..6723ef8 100755
> --- a/test/crypto
> +++ b/test/crypto
> @@ -157,7 +157,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2000-01-01) (encrypted inbox)
>  Subject: test encrypted message 001
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: test_suite@notmuchmail.org
> -Date: 01 Jan 2000 12:00:00 -0000
> +Date: Sat, 01 Jan 2000 12:00:00 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: multipart/encrypted
> diff --git a/test/thread-naming b/test/thread-naming
> index 41b97d9..b7c96f2 100755
> --- a/test/thread-naming
> +++ b/test/thread-naming
> @@ -71,7 +71,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (unread)
>  Subject: thread-naming: Initial thread subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Fri, 05 Jan 2001 15:43:56 -0000
> +Date: Fri, 05 Jan 2001 15:43:56 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> @@ -85,7 +85,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-06) (inbox unread)
>  Subject: thread-naming: Older changed subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Sat, 06 Jan 2001 15:43:56 -0000
> +Date: Sat, 06 Jan 2001 15:43:56 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> @@ -99,7 +99,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-07) (inbox unread)
>  Subject: thread-naming: Newer changed subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Sun, 07 Jan 2001 15:43:56 -0000
> +Date: Sun, 07 Jan 2001 15:43:56 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> @@ -113,7 +113,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-08) (unread)
>  Subject: thread-naming: Final thread subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Mon, 08 Jan 2001 15:43:56 -0000
> +Date: Mon, 08 Jan 2001 15:43:56 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> @@ -127,7 +127,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-09) (inbox unread)
>  Subject: Re: thread-naming: Initial thread subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Tue, 09 Jan 2001 15:43:45 -0000
> +Date: Tue, 09 Jan 2001 15:43:45 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> @@ -141,7 +141,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-10) (inbox unread)
>  Subject: Aw: thread-naming: Initial thread subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Wed, 10 Jan 2001 15:43:45 -0000
> +Date: Wed, 10 Jan 2001 15:43:45 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> @@ -155,7 +155,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-11) (inbox unread)
>  Subject: Vs: thread-naming: Initial thread subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Thu, 11 Jan 2001 15:43:45 -0000
> +Date: Thu, 11 Jan 2001 15:43:45 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> @@ -169,7 +169,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-12) (inbox unread)
>  Subject: Sv: thread-naming: Initial thread subject
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Fri, 12 Jan 2001 15:43:45 -0000
> +Date: Fri, 12 Jan 2001 15:43:45 +0000
>  \fheader}
>  \fbody{
>  \fpart{ ID: 1, Content-type: text/plain
> -- 
> 1.7.7.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [RFC PATCH 5/5] show: Simplify new text formatter code

[Austin Clements]

Quoth Dmitry Kurochkin on Jan 24 at  5:49 am:
> On Wed, 11 Jan 2012 20:49:33 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > This makes the text formatter take advantage of the new code
> > structure.  The previously duplicated header logic is now unified,
> > several things that we used to compute repeatedly across different
> > callbacks are now computed once, and the code is generally simplified.
> > 
> > Unifying the header logic causes this to format some dates slightly
> > differently, so the two affected test cases are updated.
> 
> Thanks for these patches, Austin.  They are a definite improvement for
> notmuch show code.  I hope it would soon get pushed to master.  And I
> hope more patches would follow :)
> 
> Few minor comments below.
> 
> > ---
> >  notmuch-show.c     |   84 ++++++++++++----------------------------------------
> >  test/crypto        |    2 +-
> >  test/thread-naming |   16 +++++-----
> >  3 files changed, 28 insertions(+), 74 deletions(-)
> > 
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index 3241965..1689222 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -175,67 +175,42 @@ format_part_text (const void *ctx, 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);
> > +    GMimeContentDisposition *disposition =
> > +	g_mime_object_get_content_disposition (meta);
> > +    notmuch_bool_t attachment = disposition &&
> > +	strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0;
> 
> If I did not miss anything, attachment is used only as following:
> 
>   attachment ? "attachment" : "part"
> 
> Please make it const char[] and set to "attachment" or "part".

Ooh, good catch.  But I'll do you one better.  I introduced a const
char *part_type that's set to "attachment", "part", *or* "message" in
the if structure below.  That eliminates the attachment variable,
moves the disposition variable back down into one of the early if
bodies, and eliminates the entire conditional the figures out what to
print to close the block.

> > +    notmuch_bool_t leaf = GMIME_IS_PART (node->part);
> 
> Please add const where possible to local variables (e.g. attachment, leaf).

Generally I consider these overkill on local non-pointer variables,
but okay.  leaf wound up being the only one I had to add it to.

> >      int i;
> >  
> >      if (node->envelope_file) {
> >  	notmuch_message_t *message = node->envelope_file;
> > -	const char *headers[] = {
> > -	    "Subject", "From", "To", "Cc", "Bcc", "Date"
> > -	};
> > -	const char *name, *value;
> > -	unsigned int i;
> >  
> > -	printf ("\fmessage{ ");
> > -	printf ("id:%s depth:%d match:%d filename:%s\n",
> > +	printf ("\fmessage{ id:%s depth:%d match:%d filename:%s\n",
> >  		notmuch_message_get_message_id (message),
> >  		indent,
> >  		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> >  		notmuch_message_get_filename (message));
> > -
> > -	printf ("\fheader{\n");
> > -
> > -	printf ("%s\n", _get_one_line_summary (ctx, message));
> > -
> > -	for (i = 0; i < ARRAY_SIZE (headers); i++) {
> > -	    name = headers[i];
> > -	    value = notmuch_message_get_header (message, name);
> > -	    if (value && strlen (value))
> > -		printf ("%s: %s\n", name, value);
> > -	}
> > -	printf ("\fheader}\n");
> 
> Yay!  Only one header-printing code left :)
> 
> >      } else {
> > -	GMimeContentDisposition *disposition = g_mime_object_get_content_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 &&
> > -	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> > -	{
> > -	    printf ("\fattachment{ ID: %d", node->part_num);
> > -
> > -	} else {
> > -
> > -	    printf ("\fpart{ ID: %d", node->part_num);
> > -	}
> > -
> > -	if (GMIME_IS_PART (node->part))
> > -	{
> > -	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
> > -	    if (filename)
> > -		printf (", Filename: %s", filename);
> > -	}
> > -
> > +	printf ("\f%s{ ID: %d", attachment ? "attachment" : "part", node->part_num);
> > +	if (filename)
> 
> I always forget about it, can we declare variables inside if condition
> like in C++?  I.e.:
> 
>   if (const char *filename = leaf ? g_mime_part_get_filename (GMIME_PART (node->part)) : NULL)
> 
> If yes, I would prefer to use this style where possible.

You can do that in C++?  You definitely can't do that in C.

> > +	    printf (", Filename: %s", filename);
> >  	if (cid)
> >  	    printf (", Content-id: %s", cid);
> > -
> 
> I would revert blank line changes.  But I do not insist :)

Eh.  I'm generally opposed to drive-by fixes, but this code inherited
a lot of funny formatting that just interrupts the whole flow when the
code is all in one place.

> >  	printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type));
> >      }
> >  
> > -    if (node->envelope_part) {
> > +    if (GMIME_IS_MESSAGE (node->part)) {
> 
> This condition is repeated at least twice.  Please consider moving the
> message variable below to the top level and using it in the conditions.

If the downcast had to be done in both places I would, but since the
conditional itself is the only repetition, I prefer to minimize the
scope of the message variable.

Thanks for the review!  I'll send a new version shortly.

> Regards,
>   Dmitry
> 
> >  	GMimeMessage *message = GMIME_MESSAGE (node->part);
> >  	InternetAddressList *recipients;
> >  	const char *recipients_string;
> >  
> >  	printf ("\fheader{\n");
> > +	if (node->envelope_file)
> > +	    printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
> >  	printf ("Subject: %s\n", g_mime_message_get_subject (message));
> >  	printf ("From: %s\n", g_mime_message_get_sender (message));
> >  	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
> > @@ -248,9 +223,11 @@ format_part_text (const void *ctx, mime_node_t *node,
> >  	    printf ("Cc: %s\n", recipients_string);
> >  	printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
> >  	printf ("\fheader}\n");
> > +
> > +	printf ("\fbody{\n");
> >      }
> >  
> > -    if (!node->envelope_file) {
> > +    if (leaf) {
> >  	if (g_mime_content_type_is_type (content_type, "text", "*") &&
> >  	    !g_mime_content_type_is_type (content_type, "text", "html"))
> >  	{
> > @@ -258,24 +235,12 @@ format_part_text (const void *ctx, mime_node_t *node,
> >  	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> >  	    show_text_part_content (node->part, stream_stdout);
> >  	    g_object_unref(stream_stdout);
> > -	}
> > -	else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
> > -		 g_mime_content_type_is_type (content_type, "message", "rfc822"))
> > -	{
> > -	    /* Do nothing for multipart since its content will be printed
> > -	     * when recursing. */
> > -	}
> > -	else
> > -	{
> > +	} else {
> >  	    printf ("Non-text part: %s\n",
> >  		    g_mime_content_type_to_string (content_type));
> >  	}
> >      }
> >  
> > -    if (GMIME_IS_MESSAGE (node->part)) {
> > -	printf ("\fbody{\n");
> > -    }
> > -
> >      for (i = 0; i < node->nchildren; i++)
> >  	format_part_text (ctx, mime_node_child (node, i), indent, params);
> >  
> > @@ -286,18 +251,7 @@ format_part_text (const void *ctx, mime_node_t *node,
> >      if (node->envelope_file) {
> >  	printf ("\fmessage}\n");
> >      } else {
> > -	GMimeContentDisposition *disposition;
> > -
> > -	disposition = g_mime_object_get_content_disposition (meta);
> > -	if (disposition &&
> > -	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> > -	{
> > -	    printf ("\fattachment}\n");
> > -	}
> > -	else
> > -	{
> > -	    printf ("\fpart}\n");
> > -	}
> > +	printf ("\f%s}\n", attachment ? "attachment" : "part");
> >      }
> >  }
> >  
> > diff --git a/test/crypto b/test/crypto
> > index 0af4aa8..6723ef8 100755
> > --- a/test/crypto
> > +++ b/test/crypto
> > @@ -157,7 +157,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2000-01-01) (encrypted inbox)
> >  Subject: test encrypted message 001
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: test_suite@notmuchmail.org
> > -Date: 01 Jan 2000 12:00:00 -0000
> > +Date: Sat, 01 Jan 2000 12:00:00 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: multipart/encrypted
> > diff --git a/test/thread-naming b/test/thread-naming
> > index 41b97d9..b7c96f2 100755
> > --- a/test/thread-naming
> > +++ b/test/thread-naming
> > @@ -71,7 +71,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (unread)
> >  Subject: thread-naming: Initial thread subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Fri, 05 Jan 2001 15:43:56 -0000
> > +Date: Fri, 05 Jan 2001 15:43:56 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> > @@ -85,7 +85,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-06) (inbox unread)
> >  Subject: thread-naming: Older changed subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Sat, 06 Jan 2001 15:43:56 -0000
> > +Date: Sat, 06 Jan 2001 15:43:56 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> > @@ -99,7 +99,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-07) (inbox unread)
> >  Subject: thread-naming: Newer changed subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Sun, 07 Jan 2001 15:43:56 -0000
> > +Date: Sun, 07 Jan 2001 15:43:56 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> > @@ -113,7 +113,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-08) (unread)
> >  Subject: thread-naming: Final thread subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Mon, 08 Jan 2001 15:43:56 -0000
> > +Date: Mon, 08 Jan 2001 15:43:56 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> > @@ -127,7 +127,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-09) (inbox unread)
> >  Subject: Re: thread-naming: Initial thread subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Tue, 09 Jan 2001 15:43:45 -0000
> > +Date: Tue, 09 Jan 2001 15:43:45 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> > @@ -141,7 +141,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-10) (inbox unread)
> >  Subject: Aw: thread-naming: Initial thread subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Wed, 10 Jan 2001 15:43:45 -0000
> > +Date: Wed, 10 Jan 2001 15:43:45 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> > @@ -155,7 +155,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-11) (inbox unread)
> >  Subject: Vs: thread-naming: Initial thread subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Thu, 11 Jan 2001 15:43:45 -0000
> > +Date: Thu, 11 Jan 2001 15:43:45 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> > @@ -169,7 +169,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-12) (inbox unread)
> >  Subject: Sv: thread-naming: Initial thread subject
> >  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> >  To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Fri, 12 Jan 2001 15:43:45 -0000
> > +Date: Fri, 12 Jan 2001 15:43:45 +0000
> >  \fheader}
> >  \fbody{
> >  \fpart{ ID: 1, Content-type: text/plain
> 

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.