[PATCH 0/3] Rewrite default reply format

[PATCH 0/3] Rewrite default reply format

[Austin Clements]

The default reply format is the last bastion of the old message
formatter style.  This series converts it to the new self-recursive
style.  After this, there will be one last series to rip out the
compatibility code and do final cleanup.

[PATCH 1/3] show/reply: Unify the code that extracts text parts

[Austin Clements]

Previously, show and reply had separate implementations of decoding
and printing text parts.  Now both use show's implementation, which
was more complete.  Show's implementation has been extended with an
option to add reply quoting to the extracted part (this is implemented
as a named flag to avoid naked booleans, even though it's the only
flag it can take).
---
 notmuch-client.h |    8 ++++++++
 notmuch-reply.c  |   28 ++++------------------------
 notmuch-show.c   |   23 +++++++++++++++++++----
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index fa04fa2..e24d6b6 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -197,6 +197,14 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first);
 void
 format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
 
+typedef enum {
+    NOTMUCH_SHOW_TEXT_PART_REPLY = 1<<0,
+} notmuch_show_text_part_flags;
+
+void
+show_text_part_content (GMimeObject *part, GMimeStream *stream_out,
+			notmuch_show_text_part_flags flags);
+
 char *
 json_quote_chararray (const void *ctx, const char *str, const size_t len);
 
diff --git a/notmuch-reply.c b/notmuch-reply.c
index e2b6c25..2f5ed3d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -21,7 +21,6 @@
  */
 
 #include "notmuch-client.h"
-#include "gmime-filter-reply.h"
 #include "gmime-filter-headers.h"
 
 static void
@@ -106,29 +105,10 @@ reply_part_content (GMimeObject *part)
     else if (g_mime_content_type_is_type (content_type, "text", "*") &&
 	!g_mime_content_type_is_type (content_type, "text", "html"))
     {
-	GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
-	GMimeDataWrapper *wrapper;
-	const char *charset;
-
-	charset = g_mime_object_get_content_type_parameter (part, "charset");
-	stream_stdout = g_mime_stream_file_new (stdout);
-	if (stream_stdout) {
-	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    stream_filter = g_mime_stream_filter_new(stream_stdout);
-	    if (charset) {
-		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-					 g_mime_filter_charset_new(charset, "UTF-8"));
-	    }
-	}
-	g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-				 g_mime_filter_reply_new(TRUE));
-	wrapper = g_mime_part_get_content_object (GMIME_PART (part));
-	if (wrapper && stream_filter)
-	    g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
-	if (stream_filter)
-	    g_object_unref(stream_filter);
-	if (stream_stdout)
-	    g_object_unref(stream_stdout);
+	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, NOTMUCH_SHOW_TEXT_PART_REPLY);
+	g_object_unref(stream_stdout);
     }
     else
     {
diff --git a/notmuch-show.c b/notmuch-show.c
index ff9d427..9944791 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -19,6 +19,7 @@
  */
 
 #include "notmuch-client.h"
+#include "gmime-filter-reply.h"
 
 static notmuch_status_t
 format_part_text (const void *ctx, mime_node_t *node,
@@ -247,13 +248,17 @@ format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t repl
 
 /* Write a MIME text part out to the given stream.
  *
+ * If flags&NOTMUCH_SHOW_TEXT_PART_REPLY, this prepends "> " to each
+ * output line.
+ *
  * Both line-ending conversion (CRLF->LF) and charset conversion ( ->
  * UTF-8) will be performed, so it is inappropriate to call this
  * function with a non-text part. Doing so will trigger an internal
  * error.
  */
-static void
-show_text_part_content (GMimeObject *part, GMimeStream *stream_out)
+void
+show_text_part_content (GMimeObject *part, GMimeStream *stream_out,
+			notmuch_show_text_part_flags flags)
 {
     GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
     GMimeStream *stream_filter = NULL;
@@ -286,6 +291,16 @@ show_text_part_content (GMimeObject *part, GMimeStream *stream_out)
 
     }
 
+    if (flags & NOTMUCH_SHOW_TEXT_PART_REPLY) {
+	GMimeFilter *reply_filter;
+	reply_filter = g_mime_filter_reply_new (TRUE);
+	if (reply_filter) {
+	    g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
+				      reply_filter);
+	    g_object_unref (reply_filter);
+	}
+    }
+
     wrapper = g_mime_part_get_content_object (GMIME_PART (part));
     if (wrapper && stream_filter)
 	g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
@@ -532,7 +547,7 @@ format_part_text (const void *ctx, mime_node_t *node,
 	{
 	    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);
+	    show_text_part_content (node->part, stream_stdout, 0);
 	    g_object_unref(stream_stdout);
 	} else {
 	    printf ("Non-text part: %s\n",
@@ -624,7 +639,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
 	} else if (g_mime_content_type_is_type (content_type, "text", "*")) {
 	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
 	    GByteArray *part_content;
-	    show_text_part_content (node->part, stream_memory);
+	    show_text_part_content (node->part, stream_memory, 0);
 	    part_content = g_mime_stream_mem_get_byte_array (GMIME_STREAM_MEM (stream_memory));
 
 	    printf (", \"content\": %s", json_quote_chararray (local, (char *) part_content->data, part_content->len));
-- 
1.7.7.3

[PATCH 2/3] reply: Convert default reply format to self-recursive style

[Austin Clements]

This re-arranges the default reply formatter code to use the
mime_node_t abstraction.  There are no semantic changes.
---
 notmuch-reply.c |  119 +++++++++++++++++++++----------------------------------
 1 files changed, 45 insertions(+), 74 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2f5ed3d..84a1220 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -24,28 +24,6 @@
 #include "gmime-filter-headers.h"
 
 static void
-reply_headers_message_part (GMimeMessage *message);
-
-static void
-reply_part_content (GMimeObject *part);
-
-static const notmuch_show_format_t format_reply = {
-    "", NULL,
-	"", NULL,
-	    "", NULL, reply_headers_message_part, ">\n",
-	    "",
-	        NULL,
-	        NULL,
-	        NULL,
-	        reply_part_content,
-	        NULL,
-	        "",
-	    "",
-	"", "",
-    ""
-};
-
-static void
 show_reply_headers (GMimeMessage *message)
 {
     GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
@@ -65,66 +43,55 @@ show_reply_headers (GMimeMessage *message)
 }
 
 static void
-reply_headers_message_part (GMimeMessage *message)
+format_part_reply (mime_node_t *node)
 {
-    InternetAddressList *recipients;
-    const char *recipients_string;
+    int i;
 
-    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 ("> Subject: %s\n", g_mime_message_get_subject (message));
-    printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
-}
+    if (GMIME_IS_MESSAGE (node->part)) {
+	GMimeMessage *message = GMIME_MESSAGE (node->part);
+	InternetAddressList *recipients;
+	const char *recipients_string;
 
+	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 ("> Subject: %s\n", g_mime_message_get_subject (message));
+	printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
+	printf (">\n");
+    } else if (GMIME_IS_PART (node->part)) {
+	GMimeContentType *content_type = g_mime_object_get_content_type (node->part);
+	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (node->part);
 
-static void
-reply_part_content (GMimeObject *part)
-{
-    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
-    GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
-
-    if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
-	g_mime_content_type_is_type (content_type, "message", "rfc822"))
-    {
-	/* Output nothing, since multipart subparts will be handled individually. */
-    }
-    else if (g_mime_content_type_is_type (content_type, "application", "pgp-encrypted") ||
-	     g_mime_content_type_is_type (content_type, "application", "pgp-signature"))
-    {
-	/* Ignore PGP/MIME cruft parts */
-    }
-    else 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, NOTMUCH_SHOW_TEXT_PART_REPLY);
-	g_object_unref(stream_stdout);
-    }
-    else
-    {
-	if (disposition &&
-	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-	{
-	    const char *filename = g_mime_part_get_filename (GMIME_PART (part));
+	if (g_mime_content_type_is_type (content_type, "application", "pgp-encrypted") ||
+	    g_mime_content_type_is_type (content_type, "application", "pgp-signature")) {
+	    /* Ignore PGP/MIME cruft parts */
+	} else 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, NOTMUCH_SHOW_TEXT_PART_REPLY);
+	    g_object_unref(stream_stdout);
+	} else if (disposition &&
+		   strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0) {
+	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
 	    printf ("Attachment: %s (%s)\n", filename,
 		    g_mime_content_type_to_string (content_type));
-	}
-	else
-	{
+	} else {
 	    printf ("Non-text part: %s\n",
 		    g_mime_content_type_to_string (content_type));
 	}
     }
+
+    for (i = 0; i < node->nchildren; i++)
+	format_part_reply (mime_node_child (node, i));
 }
 
 /* Is the given address configured as one of the user's "personal" or
@@ -550,7 +517,7 @@ notmuch_reply_format_default(void *ctx,
     GMimeMessage *reply;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    const notmuch_show_format_t *format = &format_reply;
+    mime_node_t *root;
 
     for (messages = notmuch_query_search_messages (query);
 	 notmuch_messages_valid (messages);
@@ -577,7 +544,11 @@ notmuch_reply_format_default(void *ctx,
 		notmuch_message_get_header (message, "date"),
 		notmuch_message_get_header (message, "from"));
 
-	show_message_body (message, format, params);
+	if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
+			    &root) == NOTMUCH_STATUS_SUCCESS) {
+	    format_part_reply (mime_node_child (root, 0));
+	    talloc_free (root);
+	}
 
 	notmuch_message_destroy (message);
     }
-- 
1.7.7.3

[PATCH 3/3] reply: Move reply citation printing to the recursive MIME walk

[Austin Clements]

This makes more logical sense, since it makes the recursive printer
responsible for the entire reply body and lets it start at the root of
the MIME tree instead of the first child.  (We could move reply header
creation in there, too, but if we ever support proper reply to
multiple messages, we'll want just one set of reply headers computed
from the entire message set and many bodies.)
---
 notmuch-reply.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 84a1220..0949d9f 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -47,7 +47,11 @@ format_part_reply (mime_node_t *node)
 {
     int i;
 
-    if (GMIME_IS_MESSAGE (node->part)) {
+    if (node->envelope_file) {
+	printf ("On %s, %s wrote:\n",
+		notmuch_message_get_header (node->envelope_file, "date"),
+		notmuch_message_get_header (node->envelope_file, "from"));
+    } else if (GMIME_IS_MESSAGE (node->part)) {
 	GMimeMessage *message = GMIME_MESSAGE (node->part);
 	InternetAddressList *recipients;
 	const char *recipients_string;
@@ -540,13 +544,9 @@ notmuch_reply_format_default(void *ctx,
 	g_object_unref (G_OBJECT (reply));
 	reply = NULL;
 
-	printf ("On %s, %s wrote:\n",
-		notmuch_message_get_header (message, "date"),
-		notmuch_message_get_header (message, "from"));
-
 	if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
 			    &root) == NOTMUCH_STATUS_SUCCESS) {
-	    format_part_reply (mime_node_child (root, 0));
+	    format_part_reply (root);
 	    talloc_free (root);
 	}
 
-- 
1.7.7.3

Re: [PATCH 0/3] Rewrite default reply format

[Tomi Ollila]

Austin Clements <amdragon@MIT.EDU> writes:

> The default reply format is the last bastion of the old message
> formatter style.  This series converts it to the new self-recursive
> style.  After this, there will be one last series to rip out the
> compatibility code and do final cleanup.

Works fine, patches look good... just 2 "spacing" questions:

in id:"1332473647-9133-2-git-send-email-amdragon@mit.edu"

+ typedef enum {
+     NOTMUCH_SHOW_TEXT_PART_REPLY = 1<<0,
+ } notmuch_show_text_part_flags;

Should this be like: NOTMUCH_SHOW_TEXT_PART_REPLY = (1 << 0),

and this

+ * If flags&NOTMUCH_SHOW_TEXT_PART_REPLY, this prepends "> " to each
+ * output line.
+ *

like:

+ * If flags & NOTMUCH_SHOW_TEXT_PART_REPLY, this prepends "> " to each


Tomi

Re: [PATCH 0/3] Rewrite default reply format

[Austin Clements]

Thanks for the review.  New version coming shortly...

Quoth Tomi Ollila on Mar 24 at 12:06 pm:
> Austin Clements <amdragon@MIT.EDU> writes:
> 
> > The default reply format is the last bastion of the old message
> > formatter style.  This series converts it to the new self-recursive
> > style.  After this, there will be one last series to rip out the
> > compatibility code and do final cleanup.
> 
> Works fine, patches look good... just 2 "spacing" questions:
> 
> in id:"1332473647-9133-2-git-send-email-amdragon@mit.edu"
> 
> + typedef enum {
> +     NOTMUCH_SHOW_TEXT_PART_REPLY = 1<<0,
> + } notmuch_show_text_part_flags;
> 
> Should this be like: NOTMUCH_SHOW_TEXT_PART_REPLY = (1 << 0),

Changed to NOTMUCH_SHOW_TEXT_PART_REPLY = 1 << 0 to be consistent with
operator spacing.  I left out the parens since they aren't necessary.

> and this
> 
> + * If flags&NOTMUCH_SHOW_TEXT_PART_REPLY, this prepends "> " to each
> + * output line.
> + *
> 
> like:
> 
> + * If flags & NOTMUCH_SHOW_TEXT_PART_REPLY, this prepends "> " to each

Changed.  On this one I put in parens to better distinguish it from
the surrounding prose.

> Tomi

[PATCH v2 0/3] Rewrite default reply format

[Austin Clements]

This version fixes two minor formatting issues that Tomi pointed out
[1].  There are no other changes.

[1] id:"m2d382ia9d.fsf@guru.guru-group.fi"

[PATCH v2 1/3] show/reply: Unify the code that extracts text parts

[Austin Clements]

Previously, show and reply had separate implementations of decoding
and printing text parts.  Now both use show's implementation, which
was more complete.  Show's implementation has been extended with an
option to add reply quoting to the extracted part (this is implemented
as a named flag to avoid naked booleans, even though it's the only
flag it can take).
---
 notmuch-client.h |    8 ++++++++
 notmuch-reply.c  |   28 ++++------------------------
 notmuch-show.c   |   23 +++++++++++++++++++----
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index fa04fa2..203ac49 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -197,6 +197,14 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first);
 void
 format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
 
+typedef enum {
+    NOTMUCH_SHOW_TEXT_PART_REPLY = 1 << 0,
+} notmuch_show_text_part_flags;
+
+void
+show_text_part_content (GMimeObject *part, GMimeStream *stream_out,
+			notmuch_show_text_part_flags flags);
+
 char *
 json_quote_chararray (const void *ctx, const char *str, const size_t len);
 
diff --git a/notmuch-reply.c b/notmuch-reply.c
index e2b6c25..2f5ed3d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -21,7 +21,6 @@
  */
 
 #include "notmuch-client.h"
-#include "gmime-filter-reply.h"
 #include "gmime-filter-headers.h"
 
 static void
@@ -106,29 +105,10 @@ reply_part_content (GMimeObject *part)
     else if (g_mime_content_type_is_type (content_type, "text", "*") &&
 	!g_mime_content_type_is_type (content_type, "text", "html"))
     {
-	GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
-	GMimeDataWrapper *wrapper;
-	const char *charset;
-
-	charset = g_mime_object_get_content_type_parameter (part, "charset");
-	stream_stdout = g_mime_stream_file_new (stdout);
-	if (stream_stdout) {
-	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    stream_filter = g_mime_stream_filter_new(stream_stdout);
-	    if (charset) {
-		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-					 g_mime_filter_charset_new(charset, "UTF-8"));
-	    }
-	}
-	g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-				 g_mime_filter_reply_new(TRUE));
-	wrapper = g_mime_part_get_content_object (GMIME_PART (part));
-	if (wrapper && stream_filter)
-	    g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
-	if (stream_filter)
-	    g_object_unref(stream_filter);
-	if (stream_stdout)
-	    g_object_unref(stream_stdout);
+	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, NOTMUCH_SHOW_TEXT_PART_REPLY);
+	g_object_unref(stream_stdout);
     }
     else
     {
diff --git a/notmuch-show.c b/notmuch-show.c
index ff9d427..0bf5e21 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -19,6 +19,7 @@
  */
 
 #include "notmuch-client.h"
+#include "gmime-filter-reply.h"
 
 static notmuch_status_t
 format_part_text (const void *ctx, mime_node_t *node,
@@ -247,13 +248,17 @@ format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t repl
 
 /* Write a MIME text part out to the given stream.
  *
+ * If (flags & NOTMUCH_SHOW_TEXT_PART_REPLY), this prepends "> " to
+ * each output line.
+ *
  * Both line-ending conversion (CRLF->LF) and charset conversion ( ->
  * UTF-8) will be performed, so it is inappropriate to call this
  * function with a non-text part. Doing so will trigger an internal
  * error.
  */
-static void
-show_text_part_content (GMimeObject *part, GMimeStream *stream_out)
+void
+show_text_part_content (GMimeObject *part, GMimeStream *stream_out,
+			notmuch_show_text_part_flags flags)
 {
     GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
     GMimeStream *stream_filter = NULL;
@@ -286,6 +291,16 @@ show_text_part_content (GMimeObject *part, GMimeStream *stream_out)
 
     }
 
+    if (flags & NOTMUCH_SHOW_TEXT_PART_REPLY) {
+	GMimeFilter *reply_filter;
+	reply_filter = g_mime_filter_reply_new (TRUE);
+	if (reply_filter) {
+	    g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
+				      reply_filter);
+	    g_object_unref (reply_filter);
+	}
+    }
+
     wrapper = g_mime_part_get_content_object (GMIME_PART (part));
     if (wrapper && stream_filter)
 	g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
@@ -532,7 +547,7 @@ format_part_text (const void *ctx, mime_node_t *node,
 	{
 	    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);
+	    show_text_part_content (node->part, stream_stdout, 0);
 	    g_object_unref(stream_stdout);
 	} else {
 	    printf ("Non-text part: %s\n",
@@ -624,7 +639,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
 	} else if (g_mime_content_type_is_type (content_type, "text", "*")) {
 	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
 	    GByteArray *part_content;
-	    show_text_part_content (node->part, stream_memory);
+	    show_text_part_content (node->part, stream_memory, 0);
 	    part_content = g_mime_stream_mem_get_byte_array (GMIME_STREAM_MEM (stream_memory));
 
 	    printf (", \"content\": %s", json_quote_chararray (local, (char *) part_content->data, part_content->len));
-- 
1.7.9.1

[PATCH v2 2/3] reply: Convert default reply format to self-recursive style

[Austin Clements]

This re-arranges the default reply formatter code to use the
mime_node_t abstraction.  There are no semantic changes.
---
 notmuch-reply.c |  123 +++++++++++++++++++++----------------------------------
 1 files changed, 47 insertions(+), 76 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2f5ed3d..84a1220 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -24,28 +24,6 @@
 #include "gmime-filter-headers.h"
 
 static void
-reply_headers_message_part (GMimeMessage *message);
-
-static void
-reply_part_content (GMimeObject *part);
-
-static const notmuch_show_format_t format_reply = {
-    "", NULL,
-	"", NULL,
-	    "", NULL, reply_headers_message_part, ">\n",
-	    "",
-	        NULL,
-	        NULL,
-	        NULL,
-	        reply_part_content,
-	        NULL,
-	        "",
-	    "",
-	"", "",
-    ""
-};
-
-static void
 show_reply_headers (GMimeMessage *message)
 {
     GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
@@ -65,66 +43,55 @@ show_reply_headers (GMimeMessage *message)
 }
 
 static void
-reply_headers_message_part (GMimeMessage *message)
+format_part_reply (mime_node_t *node)
 {
-    InternetAddressList *recipients;
-    const char *recipients_string;
-
-    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 ("> Subject: %s\n", g_mime_message_get_subject (message));
-    printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
-}
-
-
-static void
-reply_part_content (GMimeObject *part)
-{
-    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
-    GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
+    int i;
 
-    if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
-	g_mime_content_type_is_type (content_type, "message", "rfc822"))
-    {
-	/* Output nothing, since multipart subparts will be handled individually. */
-    }
-    else if (g_mime_content_type_is_type (content_type, "application", "pgp-encrypted") ||
-	     g_mime_content_type_is_type (content_type, "application", "pgp-signature"))
-    {
-	/* Ignore PGP/MIME cruft parts */
-    }
-    else 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, NOTMUCH_SHOW_TEXT_PART_REPLY);
-	g_object_unref(stream_stdout);
-    }
-    else
-    {
-	if (disposition &&
-	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-	{
-	    const char *filename = g_mime_part_get_filename (GMIME_PART (part));
+    if (GMIME_IS_MESSAGE (node->part)) {
+	GMimeMessage *message = GMIME_MESSAGE (node->part);
+	InternetAddressList *recipients;
+	const char *recipients_string;
+
+	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 ("> Subject: %s\n", g_mime_message_get_subject (message));
+	printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
+	printf (">\n");
+    } else if (GMIME_IS_PART (node->part)) {
+	GMimeContentType *content_type = g_mime_object_get_content_type (node->part);
+	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (node->part);
+
+	if (g_mime_content_type_is_type (content_type, "application", "pgp-encrypted") ||
+	    g_mime_content_type_is_type (content_type, "application", "pgp-signature")) {
+	    /* Ignore PGP/MIME cruft parts */
+	} else 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, NOTMUCH_SHOW_TEXT_PART_REPLY);
+	    g_object_unref(stream_stdout);
+	} else if (disposition &&
+		   strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0) {
+	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
 	    printf ("Attachment: %s (%s)\n", filename,
 		    g_mime_content_type_to_string (content_type));
-	}
-	else
-	{
+	} else {
 	    printf ("Non-text part: %s\n",
 		    g_mime_content_type_to_string (content_type));
 	}
     }
+
+    for (i = 0; i < node->nchildren; i++)
+	format_part_reply (mime_node_child (node, i));
 }
 
 /* Is the given address configured as one of the user's "personal" or
@@ -550,7 +517,7 @@ notmuch_reply_format_default(void *ctx,
     GMimeMessage *reply;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    const notmuch_show_format_t *format = &format_reply;
+    mime_node_t *root;
 
     for (messages = notmuch_query_search_messages (query);
 	 notmuch_messages_valid (messages);
@@ -577,7 +544,11 @@ notmuch_reply_format_default(void *ctx,
 		notmuch_message_get_header (message, "date"),
 		notmuch_message_get_header (message, "from"));
 
-	show_message_body (message, format, params);
+	if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
+			    &root) == NOTMUCH_STATUS_SUCCESS) {
+	    format_part_reply (mime_node_child (root, 0));
+	    talloc_free (root);
+	}
 
 	notmuch_message_destroy (message);
     }
-- 
1.7.9.1

[PATCH v2 3/3] reply: Move reply citation printing to the recursive MIME walk

[Austin Clements]

This makes more logical sense, since it makes the recursive printer
responsible for the entire reply body and lets it start at the root of
the MIME tree instead of the first child.  (We could move reply header
creation in there, too, but if we ever support proper reply to
multiple messages, we'll want just one set of reply headers computed
from the entire message set and many bodies.)
---
 notmuch-reply.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 84a1220..0949d9f 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -47,7 +47,11 @@ format_part_reply (mime_node_t *node)
 {
     int i;
 
-    if (GMIME_IS_MESSAGE (node->part)) {
+    if (node->envelope_file) {
+	printf ("On %s, %s wrote:\n",
+		notmuch_message_get_header (node->envelope_file, "date"),
+		notmuch_message_get_header (node->envelope_file, "from"));
+    } else if (GMIME_IS_MESSAGE (node->part)) {
 	GMimeMessage *message = GMIME_MESSAGE (node->part);
 	InternetAddressList *recipients;
 	const char *recipients_string;
@@ -540,13 +544,9 @@ notmuch_reply_format_default(void *ctx,
 	g_object_unref (G_OBJECT (reply));
 	reply = NULL;
 
-	printf ("On %s, %s wrote:\n",
-		notmuch_message_get_header (message, "date"),
-		notmuch_message_get_header (message, "from"));
-
 	if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
 			    &root) == NOTMUCH_STATUS_SUCCESS) {
-	    format_part_reply (mime_node_child (root, 0));
+	    format_part_reply (root);
 	    talloc_free (root);
 	}
 
-- 
1.7.9.1

Re: [PATCH v2 0/3] Rewrite default reply format

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> This version fixes two minor formatting issues that Tomi pointed out
> [1].  There are no other changes.
>
> [1] id:"m2d382ia9d.fsf@guru.guru-group.fi"

pushed, 

d