[PATCH v3 0/5] Quoting HTML emails in reply

[PATCH v3 0/5] Quoting HTML emails in reply

[Adam Wolfe Gordon]

Hi everyone,

Thanks to all who reviewed bits of my previous series [1]. This version
contains:

* The latest version of the emacs patch [2], rebased against the current
  master.
* An updated version of the patch to notmuch-reply.c to address Jani's
  review comments from yesterday.
* The additional emacs patch I sent previously [3] to use the existing
  message-citation-line-format customization variable, as suggested by
  Gregor Zattler [4].

I'd be happy to hear more reviews, and would especially like if someone
can test this out with a variety of emails. My mail is almost exclusively
HTML-only, with a bit of plaintext-only, so I've tested very little with
multipart messages.

[1] id:"1326737603-21166-1-git-send-email-awg+notmuch@xvx.ca"
[2] id:"1326904354-22419-1-git-send-email-awg+notmuch@xvx.ca"
[3] id:"1326840818-6821-2-git-send-email-awg+notmuch@xvx.ca"
[4] id:"20120117071749.GB2110@shi.workgroup"

Adam Wolfe Gordon (5):
  test: Add broken test for the new JSON reply format.
  reply: Add a JSON reply format.
  man: Update notmuch-reply man page for JSON format.
  emacs: Use the new JSON reply format.
  emacs: Use message-citation-line-format in reply

 emacs/notmuch-lib.el     |    8 ++
 emacs/notmuch-mua.el     |  108 +++++++++++++------
 man/man1/notmuch-reply.1 |    5 +
 notmuch-reply.c          |  271 +++++++++++++++++++++++++++++++++++++++-------
 test/emacs               |    2 +-
 test/multipart           |    7 ++
 6 files changed, 328 insertions(+), 73 deletions(-)

-- 
1.7.5.4

[PATCH v3 1/5] test: Add broken test for the new JSON reply format.

[Adam Wolfe Gordon]

---
 test/multipart |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/test/multipart b/test/multipart
index f83526b..f5ebf04 100755
--- a/test/multipart
+++ b/test/multipart
@@ -589,6 +589,13 @@ Non-text part: text/html
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "'notmuch reply' to a multipart message with json format"
+notmuch reply --format=json 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+[{ "reply": { "headers": { "from": "Notmuch Test Suite <test_suite@notmuchmail.org>", "to": "Carl Worth <cworth@cworth.org>, cworth@cworth.org", "subject": "Re: Multipart message", "in-reply-to": "<87liy5ap00.fsf@yoom.home.cworth.org>", "references": " <87liy5ap00.fsf@yoom.home.cworth.org>"} }, "original": { "headers": { "from": "Carl Worth <cworth@cworth.org>", "to": "cworth@cworth.org", "cc": "", "subject": "Multipart message", "date": "Fri, 05 Jan 2001 15:43:57 +0000", "in-reply-to": "", "references": "" }, "body": [ { "content-type": "text/html", "content": "<p>This is an embedded message, with a multipart/alternative part.</p>\n"}, { "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}, { "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}, {} ] } }, {} ]
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "'notmuch show --part' does not corrupt a part with CRLF pair"
 notmuch show --format=raw --part=3 id:base64-part-with-crlf > crlf.out
 echo -n -e "\xEF\x0D\x0A" > crlf.expected
-- 
1.7.5.4

[PATCH v3 2/5] reply: Add a JSON reply format.

[Adam Wolfe Gordon]

This new JSON format for replies includes headers generated for a reply
message as well as the headers and all text parts of the original message.
Using this data, a client can intelligently create a reply. For example,
the emacs client will be able to create replies with quoted HTML parts by
parsing the HTML parts using w3m.
---
 notmuch-reply.c |  271 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 231 insertions(+), 40 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 0f682db..b4c2426 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -30,6 +30,15 @@ reply_headers_message_part (GMimeMessage *message);
 static void
 reply_part_content (GMimeObject *part);
 
+static void
+reply_part_start_json (GMimeObject *part, int *part_count);
+
+static void
+reply_part_content_json (GMimeObject *part);
+
+static void
+reply_part_end_json (GMimeObject *part);
+
 static const notmuch_show_format_t format_reply = {
     "",
 	"", NULL,
@@ -46,6 +55,22 @@ static const notmuch_show_format_t format_reply = {
     ""
 };
 
+static const notmuch_show_format_t format_json = {
+    "",
+	"", NULL,
+	    "", NULL, NULL, "",
+	    "",
+	        reply_part_start_json,
+	        NULL,
+	        NULL,
+	        reply_part_content_json,
+	        reply_part_end_json,
+	        "",
+	    "",
+	"", "",
+    ""
+};
+
 static void
 show_reply_headers (GMimeMessage *message)
 {
@@ -86,6 +111,17 @@ reply_headers_message_part (GMimeMessage *message)
     printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
 }
 
+static notmuch_bool_t
+reply_check_part_type (GMimeObject *part, const char *type, const char *subtype,
+		       const char *disposition)
+{
+    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
+    GMimeContentDisposition *part_disposition = g_mime_object_get_content_disposition (part);
+
+    return (g_mime_content_type_is_type (content_type, type, subtype) &&
+	    (!part_disposition ||
+	     strcmp (part_disposition->disposition, disposition) == 0));
+}
 
 static void
 reply_part_content (GMimeObject *part)
@@ -147,6 +183,63 @@ reply_part_content (GMimeObject *part)
     }
 }
 
+static void
+reply_part_start_json (GMimeObject *part, unused (int *part_count))
+{
+    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))
+	printf ("{ ");
+}
+
+static void
+reply_part_end_json (GMimeObject *part)
+{
+    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))
+	printf ("}, ");
+}
+
+static void
+reply_part_content_json (GMimeObject *part)
+{
+    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
+    void *ctx = talloc_new (NULL);
+
+    /* We only care about inline text parts for reply purposes */
+    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE)) {
+	GMimeDataWrapper *wrapper;
+	GByteArray *part_content;
+
+	printf ("\"content-type\": %s, \"content\": ",
+	       json_quote_str (ctx, g_mime_content_type_to_string (content_type)));
+
+	wrapper = g_mime_part_get_content_object (GMIME_PART (part));
+	if (wrapper) {
+	    const char *charset = g_mime_object_get_content_type_parameter (part, "charset");
+	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
+	    if (stream_memory) {
+		GMimeStream *stream_filter = NULL;
+		stream_filter = g_mime_stream_filter_new (stream_memory);
+		if (charset) {
+		    g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
+					      g_mime_filter_charset_new (charset, "UTF-8"));
+		}
+
+		if (stream_filter) {
+		    g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
+		    part_content = g_mime_stream_mem_get_byte_array (GMIME_STREAM_MEM (stream_memory));
+
+		    printf ("%s", json_quote_chararray (ctx, (char *) part_content->data, part_content->len));
+		    g_object_unref (stream_filter);
+		}
+	    }
+
+	    if (stream_memory)
+		g_object_unref (stream_memory);
+	}
+    }
+
+    talloc_free (ctx);
+}
+
 /* Is the given address configured as one of the user's "personal" or
  * "other" addresses. */
 static int
@@ -505,6 +598,61 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
     return NULL;
 }
 
+static GMimeMessage *
+create_reply_message(void *ctx,
+		     notmuch_config_t *config,
+		     notmuch_message_t *message,
+		     notmuch_bool_t reply_all)
+{
+    const char *subject, *from_addr = NULL;
+    const char *in_reply_to, *orig_references, *references;
+
+    /* The 1 means we want headers in a "pretty" order. */
+    GMimeMessage *reply = g_mime_message_new (1);
+    if (reply == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return NULL;
+    }
+
+    subject = notmuch_message_get_header (message, "subject");
+    if (subject) {
+	if (strncasecmp (subject, "Re:", 3))
+	    subject = talloc_asprintf (ctx, "Re: %s", subject);
+	g_mime_message_set_subject (reply, subject);
+    }
+
+    from_addr = add_recipients_from_message (reply, config,
+					     message, reply_all);
+
+    if (from_addr == NULL)
+	from_addr = guess_from_received_header (config, message);
+
+    if (from_addr == NULL)
+	from_addr = notmuch_config_get_user_primary_email (config);
+
+    from_addr = talloc_asprintf (ctx, "%s <%s>",
+				 notmuch_config_get_user_name (config),
+				 from_addr);
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "From", from_addr);
+
+    in_reply_to = talloc_asprintf (ctx, "<%s>",
+				   notmuch_message_get_message_id (message));
+
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "In-Reply-To", in_reply_to);
+
+    orig_references = notmuch_message_get_header (message, "references");
+    references = talloc_asprintf (ctx, "%s%s%s",
+				  orig_references ? orig_references : "",
+				  orig_references ? " " : "",
+				  in_reply_to);
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "References", references);
+
+    return reply;
+}
+
 static int
 notmuch_reply_format_default(void *ctx,
 			     notmuch_config_t *config,
@@ -515,8 +663,6 @@ notmuch_reply_format_default(void *ctx,
     GMimeMessage *reply;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    const char *subject, *from_addr = NULL;
-    const char *in_reply_to, *orig_references, *references;
     const notmuch_show_format_t *format = &format_reply;
 
     for (messages = notmuch_query_search_messages (query);
@@ -525,62 +671,103 @@ notmuch_reply_format_default(void *ctx,
     {
 	message = notmuch_messages_get (messages);
 
-	/* The 1 means we want headers in a "pretty" order. */
-	reply = g_mime_message_new (1);
-	if (reply == NULL) {
-	    fprintf (stderr, "Out of memory\n");
-	    return 1;
-	}
+	reply = create_reply_message (ctx, config, message, reply_all);
 
-	subject = notmuch_message_get_header (message, "subject");
-	if (subject) {
-	    if (strncasecmp (subject, "Re:", 3))
-		subject = talloc_asprintf (ctx, "Re: %s", subject);
-	    g_mime_message_set_subject (reply, subject);
-	}
+	if (!reply)
+	    continue;
 
-	from_addr = add_recipients_from_message (reply, config, message,
-						 reply_all);
+	show_reply_headers (reply);
 
-	if (from_addr == NULL)
-	    from_addr = guess_from_received_header (config, message);
+	g_object_unref (G_OBJECT (reply));
+	reply = NULL;
 
-	if (from_addr == NULL)
-	    from_addr = notmuch_config_get_user_primary_email (config);
+	printf ("On %s, %s wrote:\n",
+		notmuch_message_get_header (message, "date"),
+		notmuch_message_get_header (message, "from"));
 
-	from_addr = talloc_asprintf (ctx, "%s <%s>",
-				     notmuch_config_get_user_name (config),
-				     from_addr);
-	g_mime_object_set_header (GMIME_OBJECT (reply),
-				  "From", from_addr);
+	show_message_body (message, format, params);
 
-	in_reply_to = talloc_asprintf (ctx, "<%s>",
-			     notmuch_message_get_message_id (message));
+	notmuch_message_destroy (message);
+    }
+    return 0;
+}
 
-	g_mime_object_set_header (GMIME_OBJECT (reply),
-				  "In-Reply-To", in_reply_to);
+static int
+notmuch_reply_format_json(void *ctx,
+			  notmuch_config_t *config,
+			  notmuch_query_t *query,
+			  unused (notmuch_show_params_t *params),
+			  notmuch_bool_t reply_all)
+{
+    GMimeMessage *reply;
+    notmuch_messages_t *messages;
+    notmuch_message_t *message;
+    const notmuch_show_format_t *format = &format_json;
 
-	orig_references = notmuch_message_get_header (message, "references");
-	references = talloc_asprintf (ctx, "%s%s%s",
-				      orig_references ? orig_references : "",
-				      orig_references ? " " : "",
-				      in_reply_to);
-	g_mime_object_set_header (GMIME_OBJECT (reply),
-				  "References", references);
+    const char *reply_headers[] = {"from", "to", "subject", "in-reply-to", "references"};
+    const char *orig_headers[] = {"from", "to", "cc", "subject", "date", "in-reply-to", "references"};
+    unsigned int hidx;
 
-	show_reply_headers (reply);
+    /* Start array of reply objects */
+    printf ("[");
+
+    for (messages = notmuch_query_search_messages (query);
+	 notmuch_messages_valid (messages);
+	 notmuch_messages_move_to_next (messages))
+    {
+	message = notmuch_messages_get (messages);
+	reply = create_reply_message (ctx, config, message, reply_all);
+	if (!reply)
+	    continue;
+
+	/* Start a reply object */
+	printf ("{ \"reply\": { \"headers\": { ");
+
+	for (hidx = 0; hidx < ARRAY_SIZE (reply_headers); hidx++)
+	{
+	    if (hidx)
+		printf (", ");
+
+	    printf ("%s: %s", json_quote_str (ctx, reply_headers[hidx]),
+		    json_quote_str (ctx, g_mime_object_get_header (GMIME_OBJECT (reply), reply_headers[hidx])));
+	}
 
 	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"));
+	/* Done the headers for the reply, which has no body parts */
+	printf ("} }");
+
+	/* Start the original */
+	printf (", \"original\": { \"headers\": { ");
+
+	for (hidx = 0; hidx < ARRAY_SIZE (orig_headers); hidx++)
+	{
+	    if (hidx)
+		printf (", ");
+
+	    printf ("%s: %s", json_quote_str (ctx, orig_headers[hidx]),
+		    json_quote_str (ctx, notmuch_message_get_header (message, orig_headers[hidx])));
+	}
+
+	/* End headers */
+	printf (" }, \"body\": [ ");
 
+	/* Show body parts */
 	show_message_body (message, format, params);
 
 	notmuch_message_destroy (message);
+
+	/* Done the original */
+	printf ("{} ] }");
+
+	/* End the reply object. */
+	printf (" }, ");
     }
+
+    /* End array of reply objects */
+    printf ("{} ]\n");
+
     return 0;
 }
 
@@ -646,6 +833,7 @@ notmuch_reply_format_headers_only(void *ctx,
 
 enum {
     FORMAT_DEFAULT,
+    FORMAT_JSON,
     FORMAT_HEADERS_ONLY,
 };
 
@@ -666,6 +854,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
 	  (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },
+				  { "json", FORMAT_JSON },
 				  { "headers-only", FORMAT_HEADERS_ONLY },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
@@ -684,6 +873,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 
     if (format == FORMAT_HEADERS_ONLY)
 	reply_format_func = notmuch_reply_format_headers_only;
+    else if (format == FORMAT_JSON)
+	reply_format_func = notmuch_reply_format_json;
     else
 	reply_format_func = notmuch_reply_format_default;
 
-- 
1.7.5.4

Re: [PATCH v3 2/5] reply: Add a JSON reply format.

[Mark Walters]

On Thu, 19 Jan 2012 10:46:54 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> This new JSON format for replies includes headers generated for a reply
> message as well as the headers and all text parts of the original message.
> Using this data, a client can intelligently create a reply. For example,
> the emacs client will be able to create replies with quoted HTML parts by
> parsing the HTML parts using w3m.

Hi this is only a preliminary look so far as I read the code. Note this
is the first time I have tried reviewing a substantial chunk of code so
sorry for any stupidities on my part!

Best wishes

Mark

>  notmuch-reply.c |  271 +++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 231 insertions(+), 40 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 0f682db..b4c2426 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -30,6 +30,15 @@ reply_headers_message_part (GMimeMessage *message);
>  static void
>  reply_part_content (GMimeObject *part);
>  
> +static void
> +reply_part_start_json (GMimeObject *part, int *part_count);
> +
> +static void
> +reply_part_content_json (GMimeObject *part);
> +
> +static void
> +reply_part_end_json (GMimeObject *part);
> +
>  static const notmuch_show_format_t format_reply = {
>      "",
>  	"", NULL,
> @@ -46,6 +55,22 @@ static const notmuch_show_format_t format_reply = {
>      ""
>  };
>  
> +static const notmuch_show_format_t format_json = {
> +    "",
> +	"", NULL,
> +	    "", NULL, NULL, "",
> +	    "",
> +	        reply_part_start_json,
> +	        NULL,
> +	        NULL,
> +	        reply_part_content_json,
> +	        reply_part_end_json,
> +	        "",
> +	    "",
> +	"", "",
> +    ""
> +};
> +
>  static void
>  show_reply_headers (GMimeMessage *message)
>  {
> @@ -86,6 +111,17 @@ reply_headers_message_part (GMimeMessage *message)
>      printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
>  }
>  
> +static notmuch_bool_t
> +reply_check_part_type (GMimeObject *part, const char *type, const char *subtype,
> +		       const char *disposition)
> +{
> +    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
> +    GMimeContentDisposition *part_disposition = g_mime_object_get_content_disposition (part);
> +
> +    return (g_mime_content_type_is_type (content_type, type, subtype) &&
> +	    (!part_disposition ||
> +	     strcmp (part_disposition->disposition, disposition) == 0));
> +}
>  
>  static void
>  reply_part_content (GMimeObject *part)
> @@ -147,6 +183,63 @@ reply_part_content (GMimeObject *part)
>      }
>  }
>  
> +static void
> +reply_part_start_json (GMimeObject *part, unused (int *part_count))
> +{
> +    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))
> +	printf ("{ ");
> +}
> +
> +static void
> +reply_part_end_json (GMimeObject *part)
> +{
> +    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))
> +	printf ("}, ");
> +}
> +
> +static void
> +reply_part_content_json (GMimeObject *part)
> +{
> +    GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));
> +    void *ctx = talloc_new (NULL);
> +
> +    /* We only care about inline text parts for reply purposes */
> +    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE)) {

This seems to be different from the logic in the text output: I think
that inlines all text/* regardless of disposition. I think the JSON
output should include at least as much as the text output as it is easy
for the caller to discard parts.

> +	GMimeDataWrapper *wrapper;
> +	GByteArray *part_content;
> +
> +	printf ("\"content-type\": %s, \"content\": ",
> +	       json_quote_str (ctx, g_mime_content_type_to_string (content_type)));
> +
> +	wrapper = g_mime_part_get_content_object (GMIME_PART (part));
> +	if (wrapper) {
> +	    const char *charset = g_mime_object_get_content_type_parameter (part, "charset");
> +	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
> +	    if (stream_memory) {
> +		GMimeStream *stream_filter = NULL;
> +		stream_filter = g_mime_stream_filter_new (stream_memory);

> +		if (charset) {
> +		    g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
> +					      g_mime_filter_charset_new (charset, "UTF-8"));
> +		}
> +
> +		if (stream_filter) {

should the if (charset) block be inside the if (stream_filter) block?

> +		    g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
> +		    part_content = g_mime_stream_mem_get_byte_array (GMIME_STREAM_MEM (stream_memory));
> +
> +		    printf ("%s", json_quote_chararray (ctx, (char *) part_content->data, part_content->len));
> +		    g_object_unref (stream_filter);
> +		}
> +	    }
> +
> +	    if (stream_memory)
> +		g_object_unref (stream_memory);
> +	}
> +    }
> +
> +    talloc_free (ctx);

Does wrapper need to a free/unref somewhere?

> +}
> +
>  /* Is the given address configured as one of the user's "personal" or
>   * "other" addresses. */
>  static int
> @@ -505,6 +598,61 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
>      return NULL;
>  }
>  
> +static GMimeMessage *
> +create_reply_message(void *ctx,
> +		     notmuch_config_t *config,
> +		     notmuch_message_t *message,
> +		     notmuch_bool_t reply_all)
> +{
> +    const char *subject, *from_addr = NULL;
> +    const char *in_reply_to, *orig_references, *references;
> +
> +    /* The 1 means we want headers in a "pretty" order. */
> +    GMimeMessage *reply = g_mime_message_new (1);
> +    if (reply == NULL) {
> +	fprintf (stderr, "Out of memory\n");
> +	return NULL;
> +    }
> +
> +    subject = notmuch_message_get_header (message, "subject");
> +    if (subject) {
> +	if (strncasecmp (subject, "Re:", 3))
> +	    subject = talloc_asprintf (ctx, "Re: %s", subject);
> +	g_mime_message_set_subject (reply, subject);
> +    }
> +
> +    from_addr = add_recipients_from_message (reply, config,
> +					     message, reply_all);
> +
> +    if (from_addr == NULL)
> +	from_addr = guess_from_received_header (config, message);
> +
> +    if (from_addr == NULL)
> +	from_addr = notmuch_config_get_user_primary_email (config);
> +
> +    from_addr = talloc_asprintf (ctx, "%s <%s>",
> +				 notmuch_config_get_user_name (config),
> +				 from_addr);
> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "From", from_addr);
> +
> +    in_reply_to = talloc_asprintf (ctx, "<%s>",
> +				   notmuch_message_get_message_id (message));
> +
> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "In-Reply-To", in_reply_to);
> +
> +    orig_references = notmuch_message_get_header (message, "references");
> +    references = talloc_asprintf (ctx, "%s%s%s",
> +				  orig_references ? orig_references : "",
> +				  orig_references ? " " : "",
> +				  in_reply_to);
> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "References", references);
> +
> +    return reply;
> +}
> +
>  static int
>  notmuch_reply_format_default(void *ctx,
>  			     notmuch_config_t *config,
> @@ -515,8 +663,6 @@ notmuch_reply_format_default(void *ctx,
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
> -    const char *subject, *from_addr = NULL;
> -    const char *in_reply_to, *orig_references, *references;
>      const notmuch_show_format_t *format = &format_reply;
>  
>      for (messages = notmuch_query_search_messages (query);
> @@ -525,62 +671,103 @@ notmuch_reply_format_default(void *ctx,
>      {
>  	message = notmuch_messages_get (messages);
>  
> -	/* The 1 means we want headers in a "pretty" order. */
> -	reply = g_mime_message_new (1);
> -	if (reply == NULL) {
> -	    fprintf (stderr, "Out of memory\n");
> -	    return 1;
> -	}
> +	reply = create_reply_message (ctx, config, message, reply_all);
>  
> -	subject = notmuch_message_get_header (message, "subject");
> -	if (subject) {
> -	    if (strncasecmp (subject, "Re:", 3))
> -		subject = talloc_asprintf (ctx, "Re: %s", subject);
> -	    g_mime_message_set_subject (reply, subject);
> -	}
> +	if (!reply)
> +	    continue;
>  
> -	from_addr = add_recipients_from_message (reply, config, message,
> -						 reply_all);
> +	show_reply_headers (reply);
>  
> -	if (from_addr == NULL)
> -	    from_addr = guess_from_received_header (config, message);
> +	g_object_unref (G_OBJECT (reply));
> +	reply = NULL;
>  
> -	if (from_addr == NULL)
> -	    from_addr = notmuch_config_get_user_primary_email (config);
> +	printf ("On %s, %s wrote:\n",
> +		notmuch_message_get_header (message, "date"),
> +		notmuch_message_get_header (message, "from"));
>  
> -	from_addr = talloc_asprintf (ctx, "%s <%s>",
> -				     notmuch_config_get_user_name (config),
> -				     from_addr);
> -	g_mime_object_set_header (GMIME_OBJECT (reply),
> -				  "From", from_addr);
> +	show_message_body (message, format, params);
>  
> -	in_reply_to = talloc_asprintf (ctx, "<%s>",
> -			     notmuch_message_get_message_id (message));
> +	notmuch_message_destroy (message);
> +    }
> +    return 0;
> +}
>  
> -	g_mime_object_set_header (GMIME_OBJECT (reply),
> -				  "In-Reply-To", in_reply_to);
> +static int
> +notmuch_reply_format_json(void *ctx,
> +			  notmuch_config_t *config,
> +			  notmuch_query_t *query,
> +			  unused (notmuch_show_params_t *params),
> +			  notmuch_bool_t reply_all)
> +{
> +    GMimeMessage *reply;
> +    notmuch_messages_t *messages;
> +    notmuch_message_t *message;
> +    const notmuch_show_format_t *format = &format_json;
>  
> -	orig_references = notmuch_message_get_header (message, "references");
> -	references = talloc_asprintf (ctx, "%s%s%s",
> -				      orig_references ? orig_references : "",
> -				      orig_references ? " " : "",
> -				      in_reply_to);
> -	g_mime_object_set_header (GMIME_OBJECT (reply),
> -				  "References", references);
> +    const char *reply_headers[] = {"from", "to", "subject", "in-reply-to", "references"};
> +    const char *orig_headers[] = {"from", "to", "cc", "subject", "date", "in-reply-to", "references"};
> +    unsigned int hidx;
>  
> -	show_reply_headers (reply);
> +    /* Start array of reply objects */
> +    printf ("[");
> +
> +    for (messages = notmuch_query_search_messages (query);
> +	 notmuch_messages_valid (messages);
> +	 notmuch_messages_move_to_next (messages))
> +    {
> +	message = notmuch_messages_get (messages);
> +	reply = create_reply_message (ctx, config, message, reply_all);
> +	if (!reply)
> +	    continue;
> +
> +	/* Start a reply object */
> +	printf ("{ \"reply\": { \"headers\": { ");
> +
> +	for (hidx = 0; hidx < ARRAY_SIZE (reply_headers); hidx++)
> +	{

Nit: I think the preferred style is brace on the same line as the for loop.

> +	    if (hidx)
> +		printf (", ");
> +
> +	    printf ("%s: %s", json_quote_str (ctx, reply_headers[hidx]),
> +		    json_quote_str (ctx, g_mime_object_get_header (GMIME_OBJECT (reply), reply_headers[hidx])));
> +	}
>  
>  	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"));
> +	/* Done the headers for the reply, which has no body parts */
> +	printf ("} }");

If replying to multiple messages (such as a whole thread) you get
multiple sets of "new headers". I think that probably is not what is
wanted but its still better than the weird things the text version
does. Might be worth putting a comment. [What I think should happen is
that a union of all the headers from all these is taken throwing away
duplicate addresses but that is obviously not part of this patch set]

> +	/* Start the original */
> +	printf (", \"original\": { \"headers\": { ");
> +
> +	for (hidx = 0; hidx < ARRAY_SIZE (orig_headers); hidx++)
> +	{
> +	    if (hidx)
> +		printf (", ");
> +
> +	    printf ("%s: %s", json_quote_str (ctx, orig_headers[hidx]),
> +		    json_quote_str (ctx, notmuch_message_get_header (message, orig_headers[hidx])));
> +	}
> +
> +	/* End headers */
> +	printf (" }, \"body\": [ ");
>  
> +	/* Show body parts */
>  	show_message_body (message, format, params);
>  
>  	notmuch_message_destroy (message);
> +
> +	/* Done the original */
> +	printf ("{} ] }");
> +
> +	/* End the reply object. */
> +	printf (" }, ");
>      }
> +
> +    /* End array of reply objects */
> +    printf ("{} ]\n");
> +
>      return 0;
>  }
>  
> @@ -646,6 +833,7 @@ notmuch_reply_format_headers_only(void *ctx,
>  
>  enum {
>      FORMAT_DEFAULT,
> +    FORMAT_JSON,
>      FORMAT_HEADERS_ONLY,
>  };
>  
> @@ -666,6 +854,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
>  	  (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },
> +				  { "json", FORMAT_JSON },
>  				  { "headers-only", FORMAT_HEADERS_ONLY },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
> @@ -684,6 +873,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>  
>      if (format == FORMAT_HEADERS_ONLY)
>  	reply_format_func = notmuch_reply_format_headers_only;
> +    else if (format == FORMAT_JSON)
> +	reply_format_func = notmuch_reply_format_json;
>      else
>  	reply_format_func = notmuch_reply_format_default;
>  
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 2/5] reply: Add a JSON reply format.

[Mark Walters]

On Sun, 05 Feb 2012 11:50:12 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> On Thu, 19 Jan 2012 10:46:54 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> > This new JSON format for replies includes headers generated for a reply
> > message as well as the headers and all text parts of the original message.
> > Using this data, a client can intelligently create a reply. For example,
> > the emacs client will be able to create replies with quoted HTML parts by
> > parsing the HTML parts using w3m.
> 
> Hi this is only a preliminary look so far as I read the code. Note this
> is the first time I have tried reviewing a substantial chunk of code so
> sorry for any stupidities on my part!

After Austin's show modifications (commit 7430a42) I needed the
following patch which is probably trivial but I was only guessing based
on the other change to notmuch-reply Austin made at the time.

Best wishes

Mark

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9aefce8..1c62b54 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -56,7 +56,7 @@ static const notmuch_show_format_t format_reply = {
 };
 
 static const notmuch_show_format_t format_json = {
-    "",
+    "", NULL,
 	"", NULL,
 	    "", NULL, NULL, "",
 	    "",

Re: [PATCH v3 2/5] reply: Add a JSON reply format.

[Adam Wolfe Gordon]

Thanks for the review. The style nits are things I missed in my
previous cleanup, so thanks for pointing them out. I should probably
run uncrustify and see if it complains about anything else.

The other points are definitely up for discussion, and some are areas
where I was unsure to start with. Discussion inline:

On Sun, Feb 5, 2012 at 04:50, Mark Walters <markwalters1009@gmail.com> wrote:
>> +    /* We only care about inline text parts for reply purposes */
>> +    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE)) {
>
> This seems to be different from the logic in the text output: I think
> that inlines all text/* regardless of disposition. I think the JSON
> output should include at least as much as the text output as it is easy
> for the caller to discard parts.

Indeed, the text output includes all text/* parts except for
text/html, regardless of disposition. My thought was that it doesn't
really make sense to quote an attachment, or at least it's not the
behavior I would expect. But, perhaps it makes more sense to include
all the text parts, with their dispositions, and let the MUA decide
what it wants to quote. If anyone has thoughts on this I'm happy to
hear them.

> Does wrapper need to a free/unref somewhere?

The text format doesn't free or unref wrapper, so I followed its
example. But, I'm not a gmime expert, and I agree intuitively that it
should be freed somehow. Can anyone enlighten me?

> If replying to multiple messages (such as a whole thread) you get
> multiple sets of "new headers". I think that probably is not what is
> wanted but its still better than the weird things the text version
> does. Might be worth putting a comment. [What I think should happen is
> that a union of all the headers from all these is taken throwing away
> duplicate addresses but that is obviously not part of this patch set]

I've never been sure about what the intended behavior is when replying
to multiple messages in the CLI. My thought was that it should create
a reply to each message, so an MUA could iterate over them allowing
you to compose replies to multiple messages. But, I've never wanted or
used such a feature, so I'm agnostic on whether it's right. The emacs
MUA (at least with my patch) ignores all but the first reply object in
the array, my assumption being that reply only operates on multiple
messages by accident.

Does anyone use reply with multiple messages? If so, what semantics do
you expect?

Re: [PATCH v3 2/5] reply: Add a JSON reply format.

[Dmitry Kurochkin]

On Sun, 5 Feb 2012 12:42:12 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Thanks for the review. The style nits are things I missed in my
> previous cleanup, so thanks for pointing them out. I should probably
> run uncrustify and see if it complains about anything else.
> 
> The other points are definitely up for discussion, and some are areas
> where I was unsure to start with. Discussion inline:
> 
> On Sun, Feb 5, 2012 at 04:50, Mark Walters <markwalters1009@gmail.com> wrote:
> >> +    /* We only care about inline text parts for reply purposes */
> >> +    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE)) {
> >
> > This seems to be different from the logic in the text output: I think
> > that inlines all text/* regardless of disposition. I think the JSON
> > output should include at least as much as the text output as it is easy
> > for the caller to discard parts.
> 
> Indeed, the text output includes all text/* parts except for
> text/html, regardless of disposition. My thought was that it doesn't
> really make sense to quote an attachment, or at least it's not the
> behavior I would expect. But, perhaps it makes more sense to include
> all the text parts, with their dispositions, and let the MUA decide
> what it wants to quote. If anyone has thoughts on this I'm happy to
> hear them.
> 
> > Does wrapper need to a free/unref somewhere?
> 
> The text format doesn't free or unref wrapper, so I followed its
> example. But, I'm not a gmime expert, and I agree intuitively that it
> should be freed somehow. Can anyone enlighten me?
> 
> > If replying to multiple messages (such as a whole thread) you get
> > multiple sets of "new headers". I think that probably is not what is
> > wanted but its still better than the weird things the text version
> > does. Might be worth putting a comment. [What I think should happen is
> > that a union of all the headers from all these is taken throwing away
> > duplicate addresses but that is obviously not part of this patch set]
> 
> I've never been sure about what the intended behavior is when replying
> to multiple messages in the CLI. My thought was that it should create
> a reply to each message, so an MUA could iterate over them allowing
> you to compose replies to multiple messages. But, I've never wanted or
> used such a feature, so I'm agnostic on whether it's right. The emacs
> MUA (at least with my patch) ignores all but the first reply object in
> the array, my assumption being that reply only operates on multiple
> messages by accident.
> 

In some cases, notmuch CLI insists that the search query matches exactly
one message (e.g. notmuch show for parts).  IMO the same behavior makes
sense for notmuch reply.

Regards,
  Dmitry

> Does anyone use reply with multiple messages? If so, what semantics do
> you expect?
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v3 2/5] reply: Add a JSON reply format.

[Austin Clements]

Quoth Adam Wolfe Gordon on Jan 19 at 10:46 am:
> This new JSON format for replies includes headers generated for a reply
> message as well as the headers and all text parts of the original message.
> Using this data, a client can intelligently create a reply. For example,
> the emacs client will be able to create replies with quoted HTML parts by
> parsing the HTML parts using w3m.

Sorry for coming late to the party.  I really like this idea, but it
seems like your implementation is duplicating a lot of the work of
notmuch show.  This makes me wonder if it would be better to return
reply header information in the JSON (which is definitely the way to
go) but to fetch the part body from the UI via show (and maybe reuse
some of the show-mode logic, if it makes sense to do so).  If this has
already been discussed, just point me at the thread and I'll catch
myself up.

Re: [PATCH v3 2/5] reply: Add a JSON reply format.

[Adam Wolfe Gordon]

On Sun, Feb 5, 2012 at 20:44, Austin Clements <amdragon@mit.edu> wrote:
> Sorry for coming late to the party.  I really like this idea, but it
> seems like your implementation is duplicating a lot of the work of
> notmuch show.  This makes me wonder if it would be better to return
> reply header information in the JSON (which is definitely the way to
> go) but to fetch the part body from the UI via show (and maybe reuse
> some of the show-mode logic, if it makes sense to do so).  If this has
> already been discussed, just point me at the thread and I'll catch
> myself up.

Thanks for taking a look. Dmitry noted on IRC that inlining the HTML
in JSON could cause issues with non-UTF8 character sets. Right now I'm
working on essentially what you've suggested - having the CLI produce
only headers, and then using show to get the quotable body.

Something else that was mentioned on IRC is using some of the notmuch
show logic to produce the show JSON format as part of reply. I looked
into this, but it would take some serious refactoring (to make the
show JSON stuff accessible to reply), and since emacs will need to end
up calling show anyway, I'm not sure it's worth it. I do like the idea
of different CLI commands being able to produce standardized formats
through some shared interface, I'm just not sure it's necessary here,
and not sure what the interface should look like.

[PATCH v3 3/5] man: Update notmuch-reply man page for JSON format.

[Adam Wolfe Gordon]

---
 man/man1/notmuch-reply.1 |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index 5160ece..ea7f87b 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -43,6 +43,11 @@ include
 .BR default
 Includes subject and quoted message body.
 .TP
+.BR json
+Produces JSON output containing headers for a reply message and the
+headers and text parts of the original message. This output can be used
+by a client to create a reply message intelligently.
+.TP
 .BR headers\-only
 Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
 .RE
-- 
1.7.5.4

[PATCH v3 4/5] emacs: Use the new JSON reply format.

[Adam Wolfe Gordon]

Using the new JSON reply format allows emacs to quote HTML
parts nicely by using mm-display-part to turn them into displayable
text, then quoting them. This is very useful for users who
regularly receive HTML-only email.

The behavior for messages that contain plain text parts should be
unchanged.
---
 emacs/notmuch-lib.el |    8 ++++
 emacs/notmuch-mua.el |   95 +++++++++++++++++++++++++++++++++-----------------
 2 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 9242537..9863d69 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -21,6 +21,8 @@
 
 ;; This is an part of an emacs-based interface to the notmuch mail system.
 
+(eval-when-compile (require 'cl))
+
 (defvar notmuch-command "notmuch"
   "Command to run the notmuch binary.")
 
@@ -160,6 +162,12 @@ the user hasn't set this variable with the old or new value."
   (list 'when (< emacs-major-version 23)
 	form))
 
+(defun notmuch-parts-filter-by-type (parts type)
+  "Return a list of message parts with the given type"
+  (loop for part across parts
+	if (string= (cdr (assq 'content-type part)) type)
+	collect (cdr (assq 'content part))))
+
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
 ;; Both functions here were copied from emacs 23 with the following copyright:
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 023645e..5ae0ccf 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -19,6 +19,7 @@
 ;;
 ;; Authors: David Edmondson <dme@dme.org>
 
+(require 'json)
 (require 'message)
 
 (require 'notmuch-lib)
@@ -72,49 +73,79 @@ list."
 	    (push header message-hidden-headers)))
 	notmuch-mua-hidden-headers))
 
+(defun notmuch-mua-insert-part-quoted (part)
+  (let ((start (point))
+	limit)
+    (insert part)
+    (setq limit (point-marker))
+    (goto-char start)
+    (while (re-search-forward "\\(^\\)[^$]" (marker-position limit) 0)
+      (replace-match "> " nil nil nil 1))
+    (set-buffer-modified-p nil)))
+
+(defun notmuch-mua-parse-html-part (part)
+  (with-temp-buffer
+    (insert part)
+    (let ((handle (mm-make-handle (current-buffer) (list "text/html")))
+	  (end-of-orig (point-max)))
+      (mm-display-part handle)
+      (delete-region (point-min) end-of-orig)
+      (fill-region (point-min) (point-max))
+      (buffer-substring (point-min) (point-max)))))
+
 (defun notmuch-mua-reply (query-string &optional sender reply-all)
-  (let (headers
-	body
-	(args '("reply")))
+  (let ((args '("reply" "--format=json"))
+	reply
+	body)
     (if notmuch-show-process-crypto
 	(setq args (append args '("--decrypt"))))
     (if reply-all
 	(setq args (append args '("--reply-to=all")))
       (setq args (append args '("--reply-to=sender"))))
     (setq args (append args (list query-string)))
-    ;; This make assumptions about the output of `notmuch reply', but
-    ;; really only that the headers come first followed by a blank
-    ;; line and then the body.
+    ;; Get the reply object as JSON, and parse it into an elisp object.
     (with-temp-buffer
       (apply 'call-process (append (list notmuch-command nil (list t t) nil) args))
       (goto-char (point-min))
-      (if (re-search-forward "^$" nil t)
-	  (save-excursion
-	    (save-restriction
-	      (narrow-to-region (point-min) (point))
-	      (goto-char (point-min))
-	      (setq headers (mail-header-extract)))))
-      (forward-line 1)
-      (setq body (buffer-substring (point) (point-max))))
-    ;; If sender is non-nil, set the From: header to its value.
-    (when sender
-      (mail-header-set 'from sender headers))
-    (let
-	;; Overlay the composition window on that being used to read
-	;; the original message.
-	((same-window-regexps '("\\*mail .*")))
-      (notmuch-mua-mail (mail-header 'to headers)
-			(mail-header 'subject headers)
-			(message-headers-to-generate headers t '(to subject))))
-    ;; insert the message body - but put it in front of the signature
-    ;; if one is present
-    (goto-char (point-max))
-    (if (re-search-backward message-signature-separator nil t)
+      (setq reply (aref (json-read) 0)))
+
+    ;; Start with the prelude, based on the headers of the original message.
+    (let* ((original (cdr (assq 'original reply)))
+	   (headers (cdr (assq 'headers (assq 'reply reply))))
+	   (original-headers (cdr (assq 'headers original)))
+	   (body-parts (cdr (assq 'body original)))
+	   (plain-parts (notmuch-parts-filter-by-type body-parts "text/plain"))
+	   (html-parts (notmuch-parts-filter-by-type body-parts "text/html")))
+
+      ;; If sender is non-nil, set the From: header to its value.
+      (when sender
+	(mail-header-set 'from sender headers))
+      (let
+	  ;; Overlay the composition window on that being used to read
+	  ;; the original message.
+	  ((same-window-regexps '("\\*mail .*")))
+	(notmuch-mua-mail (mail-header 'to headers)
+			  (mail-header 'subject headers)
+			  (message-headers-to-generate headers t '(to subject))))
+      ;; insert the message body - but put it in front of the signature
+      ;; if one is present
+      (goto-char (point-max))
+      (if (re-search-backward message-signature-separator nil t)
 	  (forward-line -1)
-      (goto-char (point-max)))
-    (insert body)
-    (push-mark))
-  (set-buffer-modified-p nil)
+	(goto-char (point-max)))
+
+      (insert (format "On %s, %s wrote:\n"
+		      (cdr (assq 'date original-headers))
+		      (cdr (assq 'from original-headers))))
+
+      (if plain-parts
+	  (mapc (lambda (part) (notmuch-mua-insert-part-quoted part)) plain-parts)
+	(mapc (lambda (part)
+		(notmuch-mua-insert-part-quoted (notmuch-mua-parse-html-part part)))
+	      html-parts))
+
+      (push-mark))
+    (set-buffer-modified-p nil))
 
   (message-goto-body))
 
-- 
1.7.5.4

Re: [PATCH v3 4/5] emacs: Use the new JSON reply format.

[Mark Walters]

On Thu, 19 Jan 2012 10:46:56 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Using the new JSON reply format allows emacs to quote HTML
> parts nicely by using mm-display-part to turn them into displayable
> text, then quoting them. This is very useful for users who
> regularly receive HTML-only email.
> 
> The behavior for messages that contain plain text parts should be
> unchanged.

Hi I have tried using this patch and it did give an identical emacs
buffer when replying to text/plain or multipart/alternative messages
except for two things:

the old one put a line in the reply for each attachment not being
included in the reply (eg "Attachment: file.pdf (application/pdf)" ) and
the new one does not.

and your version does not include text/plain attachments with
disposition attachment (obviously as the cli part does not).

[Note I am not saying whether either of these correct or not, just that
they are changes.]

It also worked nicely on the html only messages I tried it on.

I have not reviewed the lisp yet; I will try and have a look at it but I
am a lisp beginner.

Best wishes

Mark

PS Sorry I sent this from my unsubscribed address first: since I am
using your patch-set my recent reply From: modification wasn't there!

[PATCH v3 5/5] emacs: Use message-citation-line-format in reply

[Adam Wolfe Gordon]

Instead of using a static citation line for the first line of the
reply message, use the customizable one defined by message-mode.
This makes it easy for users to customize the reply style, and
retains consistency for users with existing message-mode
customizations.
---
 emacs/notmuch-mua.el |   19 ++++++++++++++++---
 test/emacs           |    2 +-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 5ae0ccf..e485d93 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -134,9 +134,22 @@ list."
 	  (forward-line -1)
 	(goto-char (point-max)))
 
-      (insert (format "On %s, %s wrote:\n"
-		      (cdr (assq 'date original-headers))
-		      (cdr (assq 'from original-headers))))
+      (let* ((quoth message-citation-line-format)
+	     (case-fold-search nil)
+	     (full-from (cdr (assq 'from original-headers)))
+	     (from-addr (car (mail-header-parse-address full-from)))
+	     (from-name (cdr (mail-header-parse-address full-from)))
+	     (first-name (car (split-string from-name)))
+	     (last-name (append (cdr (split-string from-name))))
+	     (time (date-to-time (cdr (assq 'date original-headers)))))
+
+	(setq quoth (replace-regexp-in-string "%f" full-from quoth t t))
+	(setq quoth (replace-regexp-in-string "%n" from-addr quoth t t))
+	(setq quoth (replace-regexp-in-string "%N" from-name quoth t t))
+	(setq quoth (replace-regexp-in-string "%F" first-name quoth t t))
+	(setq quoth (replace-regexp-in-string "%L" last-name quoth t t))
+	(setq quoth (format-time-string quoth time))
+	(insert quoth))
 
       (if plain-parts
 	  (mapc (lambda (part) (notmuch-mua-insert-part-quoted part)) plain-parts)
diff --git a/test/emacs b/test/emacs
index ac47b16..3f59b23 100755
--- a/test/emacs
+++ b/test/emacs
@@ -268,7 +268,7 @@ Subject: Re: Testing message sent via SMTP
 In-Reply-To: <XXX>
 Fcc: $(pwd)/mail/sent
 --text follows this line--
-On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
+On Sat, Jan 01 2000, Notmuch Test Suite wrote:
 > This is a test that messages are sent via SMTP
 EOF
 test_expect_equal_file OUTPUT EXPECTED
-- 
1.7.5.4

Re: [PATCH v3 5/5] emacs: Use message-citation-line-format in reply

[Aaron Ecay]

On Thu, 19 Jan 2012 10:46:57 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Instead of using a static citation line for the first line of the
> reply message, use the customizable one defined by message-mode.
> This makes it easy for users to customize the reply style, and
> retains consistency for users with existing message-mode
> customizations.
> ---
>  emacs/notmuch-mua.el |   19 ++++++++++++++++---
>  test/emacs           |    2 +-
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 5ae0ccf..e485d93 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -134,9 +134,22 @@ list."
>  	  (forward-line -1)
>  	(goto-char (point-max)))
>  
> -      (insert (format "On %s, %s wrote:\n"
> -		      (cdr (assq 'date original-headers))
> -		      (cdr (assq 'from original-headers))))
> +      (let* ((quoth message-citation-line-format)
> +	     (case-fold-search nil)
> +	     (full-from (cdr (assq 'from original-headers)))
> +	     (from-addr (car (mail-header-parse-address full-from)))
> +	     (from-name (cdr (mail-header-parse-address full-from)))
> +	     (first-name (car (split-string from-name)))
> +	     (last-name (append (cdr (split-string from-name))))
> +	     (time (date-to-time (cdr (assq 'date original-headers)))))
> +
> +	(setq quoth (replace-regexp-in-string "%f" full-from quoth t t))
> +	(setq quoth (replace-regexp-in-string "%n" from-addr quoth t t))
> +	(setq quoth (replace-regexp-in-string "%N" from-name quoth t t))
> +	(setq quoth (replace-regexp-in-string "%F" first-name quoth t t))
> +	(setq quoth (replace-regexp-in-string "%L" last-name quoth t t))
> +	(setq quoth (format-time-string quoth time))
> +	(insert quoth))

Shouldn’t this just use message-insert-formatted-citation-line?

Another approach you might take with this patch series is to look at
the message-cite-original function (which I just discovered as I was
plumbing around in message.el looking for the function to format the
citation line).  I think that what one does to use this fn is to put
the original message text into the reply buffer (unquoted), set point
and mark to encompass it, then call the fn.  It automatically handles
inserting the quotes, and has some customization options (stripping
signatures from replies, customizable quote character instead of “>”,
...).

The message-cite-original function also adds escape characters to the
cookies that message-mode uses to indicate sign/encrypt/attach
directives.  I think notmuch exposes files on the user’s computer to
others, if a user can be tricked into replying to a message with an
attachment cookie and not stripping the cookie from the reply text.  So
to mitigate this, whatever reply mechanism winds up being used should
call mml-quote-region on the reply text (as message-cite-original does).

I just sent a patch to the list to do this in the current version of
notmuch, which should show up in
id:"1326998589-37187-1-git-send-email-aaronecay@gmail.com" .

-- 
Aaron Ecay

Re: [PATCH v3 5/5] emacs: Use message-citation-line-format in reply

[Adam Wolfe Gordon]

On Thu, Jan 19, 2012 at 11:45, Aaron Ecay <ecay@sas.upenn.edu> wrote:
> Shouldn’t this just use message-insert-formatted-citation-line?

Yes, good idea.  I just tried this and it almost works.  The only
issue is that the default message-mode-citation-line-format has a
newline at the end, and this function inserts an *additional* newline,
so we end up with a blank line before the beginning of the quoted
text.  This is fixable by the user, of course, but it means the
default out-of-the-box setup will create funny-looking replies, which
is probably bad.  Thoughts?

> Another approach you might take with this patch series is to look at
> the message-cite-original function (which I just discovered as I was
> plumbing around in message.el looking for the function to format the
> citation line).  I think that what one does to use this fn is to put
> the original message text into the reply buffer (unquoted), set point
> and mark to encompass it, then call the fn.  It automatically handles
> inserting the quotes, and has some customization options (stripping
> signatures from replies, customizable quote character instead of “>”,
> ...).
>
> The message-cite-original function also adds escape characters to the
> cookies that message-mode uses to indicate sign/encrypt/attach
> directives.  I think notmuch exposes files on the user’s computer to
> others, if a user can be tricked into replying to a message with an
> attachment cookie and not stripping the cookie from the reply text.  So
> to mitigate this, whatever reply mechanism winds up being used should
> call mml-quote-region on the reply text (as message-cite-original does).

I've also tried using message-cite-original to create the reply body,
and it also almost works.  The issue, again, is one of defaults.  In
addition to the blank line I mentioned above, the default
message-citation-line-function inserts the "plain" citation line "Foo
<foo@bar.baz> writes:" instead of the formatted one.  This is a big
change from the current notmuch default.

If everyone's OK with this and willing to customize it themselves,
then I'm happy to go with this solution, but I'm pretty reluctant to
change the default in such a significant way.

Re: [PATCH v3 5/5] emacs: Use message-citation-line-format in reply

[Aaron Ecay]

On Thu, 19 Jan 2012 21:46:46 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> On Thu, Jan 19, 2012 at 11:45, Aaron Ecay <ecay@sas.upenn.edu> wrote:
> > Shouldn’t this just use message-insert-formatted-citation-line?
> 
> Yes, good idea.  I just tried this and it almost works.  The only
> issue is that the default message-mode-citation-line-format has a
> newline at the end, and this function inserts an *additional* newline,
> so we end up with a blank line before the beginning of the quoted
> text.  This is fixable by the user, of course, but it means the
> default out-of-the-box setup will create funny-looking replies, which
> is probably bad.  Thoughts?

(let ((message-citation-line-format
       (remove ?\n message-citation-line-format)))
  ...)

(Or, if you think someone might have a newline other than at the end of
the string, you could do something a little more complicated to remove
only a newline at the end.)

Or you could introduce a new defcustom
‘notmuch-message-citation-line-format’ (with newline-less default), and
let-bind m-l-c-f to that value.  But that is pretty ugly – we don’t want
to have to “wrap” every variable whose default we don’t like.

> I've also tried using message-cite-original to create the reply body,
> and it also almost works.  The issue, again, is one of defaults.  In
> addition to the blank line I mentioned above, the default
> message-citation-line-function inserts the "plain" citation line "Foo
> <foo@bar.baz> writes:" instead of the formatted one.  This is a big
> change from the current notmuch default.
> 
> If everyone's OK with this and willing to customize it themselves,
> then I'm happy to go with this solution, but I'm pretty reluctant to
> change the default in such a significant way.

I’m personally of the opinion that notmuch should just say “the mail
composition facility is provided by message mode (here is the
documentation on customizing it)”.  Message mode is 8,000 lines of
elisp.  We have the choice to:
- write our own message composition mode
- wrap (as explained above) the default message-mode variables we don’t
  like in notmuch-prefixed variants, with suitable let-bindings.
- use only the parts of message-mode that we like
- pass composition off to message mode

The first option just doesn’t make sense.  The second is also a little
nuts.  The third is what we are trying now, but it’s painful – the
message mode code has its own structure and its own defaults, which
don’t always agree with notmuch’s.  I am in favor of the fourth –
notmuch’s elisp code should pass data to message mode in the most low
level form it can, and let message mode do any extra work in the way
it already does.  And users should customize message composition via
message mode, not notmuch.  There’s a tradeoff to this approach – by
controlling more within notmuch, we can have nicer defaults at the
expense of more brittle code and/or fewer user-visible customizations.

This is not in any way a criticism of your work (which is great) –
you’re right that you need “permission” to uproot the defaults, and I’m
advocating for it to be given.

One possible step that might ease the transition pain could be for
notmuch’s emacs interface to have a configuration file (similar to
Wanderlust’s ~/.wl; I believe Gnus also uses a ~/.gnus).  The idea is
that this file contains elisp code, and is loaded by notmuch the first
time any notmuch-related commands are invoked by the user.  If the file
does not exist, notmuch could create it with default content that sets
message-citation-function, message-citation-line-format,
message-yank-prefix (to get rid of the ugly default whereby message-mode
indents the original message by four spaces instead of inserting “> ”),
etc.  If there is interest in this approach, I’d be happy to work on a
patch for it.

I’ve sort of stumped for this idea before
(id:"m239bgcd1d.fsf@gmail.com") and it didn’t exactly get rave reviews.
So I’ll shut up if it’s really not something people want to see.

I’ll close with an example of a nice feature that message mode has
(which I’ve been really wanting since the reply keybindings changed)
that notmuch would get for free if it hooked into message mode better:
the function message-widen-reply takes a reply-to-sender message and
makes it reply-to-all.

-- 
Aaron Ecay

Re: [PATCH v3 5/5] emacs: Use message-citation-line-format in reply

[David Edmondson]

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

On Fri, 20 Jan 2012 00:53:39 -0500, Aaron Ecay <ecay@sas.upenn.edu> wrote:
> I’m personally of the opinion that notmuch should just say “the mail
> composition facility is provided by message mode (here is the
> documentation on customizing it)”.

In general, +1.

> One possible step that might ease the transition pain could be for
> notmuch’s emacs interface to have a configuration file (similar to
> Wanderlust’s ~/.wl; I believe Gnus also uses a ~/.gnus).  The idea is
> that this file contains elisp code, and is loaded by notmuch the first
> time any notmuch-related commands are invoked by the user.

This is how my own configuration is stored (in ~/.notmuch.el).

> I’ll close with an example of a nice feature that message mode has
> (which I’ve been really wanting since the reply keybindings changed)
> that notmuch would get for free if it hooked into message mode better:
> the function message-widen-reply takes a reply-to-sender message and
> makes it reply-to-all.

That would require a bunch of work on our side to prepare the data that
message-mode uses, but would indeed be nice.

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

Re: [PATCH v3 5/5] emacs: Use message-citation-line-format in reply

[Adam Wolfe Gordon]

Erk, forgot to reply-all.  Aaron might get this twice.

On Thu, Jan 19, 2012 at 22:53, Aaron Ecay <ecay@sas.upenn.edu> wrote:
> (let ((message-citation-line-format
>       (remove ?\n message-citation-line-format)))
>  ...)
>
> (Or, if you think someone might have a newline other than at the end of
> the string, you could do something a little more complicated to remove
> only a newline at the end.)

I did something like this initially, to make the test pass, but my
thought is that some people might intend for that newline to be there
and we shouldn't be removing it.  Perhaps I'm being overly sensitive
to people with odd setups ;-).

> Or you could introduce a new defcustom
> ‘notmuch-message-citation-line-format’ (with newline-less default), and
> let-bind m-l-c-f to that value.  But that is pretty ugly – we don’t want
> to have to “wrap” every variable whose default we don’t like.

Agreed, not a good solution.

> I’m personally of the opinion that notmuch should just say “the mail
> composition facility is provided by message mode (here is the
> documentation on customizing it)”.  Message mode is 8,000 lines of
> elisp.  We have the choice to:
> - write our own message composition mode
> - wrap (as explained above) the default message-mode variables we don’t
>  like in notmuch-prefixed variants, with suitable let-bindings.
> - use only the parts of message-mode that we like
> - pass composition off to message mode
>
> The first option just doesn’t make sense.  The second is also a little
> nuts.  The third is what we are trying now, but it’s painful – the
> message mode code has its own structure and its own defaults, which
> don’t always agree with notmuch’s.  I am in favor of the fourth –
> notmuch’s elisp code should pass data to message mode in the most low
> level form it can, and let message mode do any extra work in the way
> it already does.  And users should customize message composition via
> message mode, not notmuch.  There’s a tradeoff to this approach – by
> controlling more within notmuch, we can have nicer defaults at the
> expense of more brittle code and/or fewer user-visible customizations.
>
> This is not in any way a criticism of your work (which is great) –
> you’re right that you need “permission” to uproot the defaults, and I’m
> advocating for it to be given.

I think we're on the same page here - I definitely prefer to have
notmuch-mua use existing emacs functionality wherever it makes sense.
The only question in my mind is how ugly we're willing to let the
defaults be in order to leverage existing stuff.

> One possible step that might ease the transition pain could be for
> notmuch’s emacs interface to have a configuration file (similar to
> Wanderlust’s ~/.wl; I believe Gnus also uses a ~/.gnus).  The idea is
> that this file contains elisp code, and is loaded by notmuch the first
> time any notmuch-related commands are invoked by the user.  If the file
> does not exist, notmuch could create it with default content that sets
> message-citation-function, message-citation-line-format,
> message-yank-prefix (to get rid of the ugly default whereby message-mode
> indents the original message by four spaces instead of inserting “> ”),
> etc.  If there is interest in this approach, I’d be happy to work on a
> patch for it.

This would be a good way to get around the ugly defaults problem, and
it would also be an easy way for people to share their notmuch/emacs
setups.  I already have my setup in a file separate from my normal
emacs config, and I run `emacs -q -l ~/.emacs-notmuch -f notmuch` so
it doesn't load my normal, programming-oriented setup.

One additional issue I noticed this morning with using
message-cite-original: it doesn't wrap/fill the quoted message.  I'm
guessing there's a way to make message-cite-original do this, but I
haven't figured it out.

Re: [PATCH v3 5/5] emacs: Use message-citation-line-format in reply

[Tomi Ollila]

On Fri, 20 Jan 2012 10:22:09 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Erk, forgot to reply-all.  Aaron might get this twice.

Pick this:  notmuch@notmuchmail.org  (and add to to/cc) next time you forgot
to press 'R' (that's what I do :)
> 
> On Thu, Jan 19, 2012 at 22:53, Aaron Ecay <ecay@sas.upenn.edu> wrote:
> 
> > One possible step that might ease the transition pain could be for
> > notmuch’s emacs interface to have a configuration file (similar to
> > Wanderlust’s ~/.wl; I believe Gnus also uses a ~/.gnus).  The idea is
> > that this file contains elisp code, and is loaded by notmuch the first
> > time any notmuch-related commands are invoked by the user.  If the file
> > does not exist, notmuch could create it with default content that sets
> > message-citation-function, message-citation-line-format,
> > message-yank-prefix (to get rid of the ugly default whereby message-mode
> > indents the original message by four spaces instead of inserting “> ”),
> > etc.  If there is interest in this approach, I’d be happy to work on a
> > patch for it.

Yes, that would be good -- then there is default file everyone can be
directed to...

> This would be a good way to get around the ugly defaults problem, and
> it would also be an easy way for people to share their notmuch/emacs
> setups.  I already have my setup in a file separate from my normal
> emacs config, and I run `emacs -q -l ~/.emacs-notmuch -f notmuch` so
> it doesn't load my normal, programming-oriented setup.

I used to do just that -- but then I cannot enter M-x notmuch in my
emacs for sending email from that particular emacs. Now I have something 
like:

(autoload 'notmuch "~/local/my-notmuch" "Notmuchmail" t)

Where first lines add load path for notmuch, then (require 'notmuch)
and then stars loading configuration...

The question, in case that configuration file is added, where is it 
located: ~/.notmuch (to add yet another file there), or into
.notmuch/ directory or XDG -like .config/notmuch/ (config.el ?)

See: id:"E1RdKCw-0007o7-J7@thinkbox.jade-hamburg.de"

Tomi

[PATCH v3 5/5] emacs: Use message-cite-original in reply

[Adam Wolfe Gordon]

Use message-mode's message-cite-original function to create the
quoted body for reply messages. In order to make this act like the
existing notmuch defaults, you will need to set the following in
your emacs configuration:

message-citation-line-format "On %a, %d %b %Y, %f wrote:"
message-citation-line-function 'message-insert-formatted-citation-line

The test has been updated to reflect the (ugly) emacs default.
---

Here is an alternate version of the patch, which uses message-cite-original.

I suggest people try out this version and see if the behavior is
acceptable with some configuration tweaks. If it is, then we can
work on implementing the notmuch-emacs config file idea, and go
with this version. As I mentioned, the one thing I haven't figured
out how to do with configuration is make message-cite-original fill
the quoted message. This would probably be a dealbreaker for me, but
I suspect it can be done somehow with the right combination of hooks.

 emacs/notmuch-mua.el |   32 +++++++++++++++++++-------------
 test/emacs           |    3 ++-
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 5ae0ccf..45c314d 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -21,6 +21,7 @@
 
 (require 'json)
 (require 'message)
+(require 'format-spec)
 
 (require 'notmuch-lib)
 (require 'notmuch-address)
@@ -134,19 +135,24 @@ list."
 	  (forward-line -1)
 	(goto-char (point-max)))
 
-      (insert (format "On %s, %s wrote:\n"
-		      (cdr (assq 'date original-headers))
-		      (cdr (assq 'from original-headers))))
-
-      (if plain-parts
-	  (mapc (lambda (part) (notmuch-mua-insert-part-quoted part)) plain-parts)
-	(mapc (lambda (part)
-		(notmuch-mua-insert-part-quoted (notmuch-mua-parse-html-part part)))
-	      html-parts))
-
-      (push-mark))
-    (set-buffer-modified-p nil))
-
+      (let ((from (cdr (assq 'from original-headers)))
+	    (date (cdr (assq 'date original-headers)))
+	    (start (point)))
+
+	(insert "From: " from "\n")
+	(insert "Date: " date "\n\n")
+      
+	(if plain-parts
+	    (mapc 'insert plain-parts)
+	  (mapc (lambda (part)
+		  (insert (notmuch-mua-parse-html-part part)))
+		html-parts))
+	(push-mark)
+	(goto-char start)
+	(message-cite-original))))
+
+  (push-mark)
+  (set-buffer-modified-p nil)
   (message-goto-body))
 
 (defun notmuch-mua-forward-message ()
diff --git a/test/emacs b/test/emacs
index ac47b16..aecbf41 100755
--- a/test/emacs
+++ b/test/emacs
@@ -268,7 +268,8 @@ Subject: Re: Testing message sent via SMTP
 In-Reply-To: <XXX>
 Fcc: $(pwd)/mail/sent
 --text follows this line--
-On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
+Notmuch Test Suite <test_suite@notmuchmail.org> writes:
+
 > This is a test that messages are sent via SMTP
 EOF
 test_expect_equal_file OUTPUT EXPECTED
-- 
1.7.5.4

[PATCH v4 0/4] Quoting HTML parts in reply (and other reply enhancements)

[Adam Wolfe Gordon]

Hi everyone,

Here is a new and much-improved version of my series [1] adding support for
quoting HTML parts in replies using a JSON reply format. This version is, as
the diffs indicate, much more ambitious than previous versions, especially
on the emacs front:

* The JSON reply format now only includes headers (for both the original and 
  reply messages), as Dmitry pointed out that non-UTF8 characters sets cannot
  be properly inlined in JSON.

* Thus, emacs now uses notmuch show to fetch the body of the original message
  before creating the quoted body of the reply.

* In order to simplify this use of show, the JSON format no longer implies
  --entire-thread. The other consumer of --format=json (notmuch-query.el) has
  been updated to use --entire-thread when that's what it means, and the man
  page has been updated.

* Emacs now pays attention to multipart structures when deciding which parts
  to quote in a reply. This uses essentially the same logic as in show, some
  of which has been factored out into notmuch-lib.el. In particular, emacs will
  include all the text parts of a message, except the non-preferred ones in a
  multipart/alternative part.

* There are two new emacs test cases to test and demonstrate the new reply
  functionality. They show how it works a multipart/mixed and multipart/alternative
  messages.

I think that covers everything... Please let me know if there is something I
haven't explained well. And, of course, please send reviews on this extensive
change.

[1] id:"1326995217-27423-1-git-send-email-awg+notmuch@xvx.ca"

Adam Wolfe Gordon (4):
  test: Add broken test for the new JSON reply format.
  reply: Add a JSON reply format.
  man: Update notmuch-reply man page for JSON format.
  emacs: Use the new JSON reply format and message-cite-original

 emacs/notmuch-lib.el     |   39 +++++++++++
 emacs/notmuch-mua.el     |  136 +++++++++++++++++++++++++++-----------
 emacs/notmuch-query.el   |    2 +-
 emacs/notmuch-show.el    |   24 +------
 man/man1/notmuch-reply.1 |    5 ++
 man/man1/notmuch-show.1  |    6 +--
 notmuch-reply.c          |  167 ++++++++++++++++++++++++++++++++++------------
 notmuch-show.c           |    1 -
 test/emacs               |  101 +++++++++++++++++++++++++++-
 test/multipart           |    7 ++
 10 files changed, 375 insertions(+), 113 deletions(-)

-- 
1.7.5.4

[PATCH v4 1/4] test: Add broken test for the new JSON reply format.

[Adam Wolfe Gordon]

---
 test/multipart |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/test/multipart b/test/multipart
index 2dd73f5..7cff74a 100755
--- a/test/multipart
+++ b/test/multipart
@@ -589,6 +589,13 @@ Non-text part: text/html
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "'notmuch reply' to a multipart message with json format"
+notmuch reply --format=json 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
+cat <<EOF >EXPECTED
+{ "reply": { "from": "Notmuch Test Suite <test_suite@notmuchmail.org>", "to": "Carl Worth <cworth@cworth.org>, cworth@cworth.org", "subject": "Re: Multipart message", "in-reply-to": "<87liy5ap00.fsf@yoom.home.cworth.org>", "references": " <87liy5ap00.fsf@yoom.home.cworth.org>" }, "original": { "from": "Carl Worth <cworth@cworth.org>", "to": "cworth@cworth.org", "cc": "", "subject": "Multipart message", "date": "Fri, 05 Jan 2001 15:43:57 +0000", "in-reply-to": "", "references": "", "message-id": "87liy5ap00.fsf@yoom.home.cworth.org" } }
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "'notmuch show --part' does not corrupt a part with CRLF pair"
 notmuch show --format=raw --part=3 id:base64-part-with-crlf > crlf.out
 echo -n -e "\xEF\x0D\x0A" > crlf.expected
-- 
1.7.5.4

[PATCH v4 2/4] reply: Add a JSON reply format.

[Adam Wolfe Gordon]

This new JSON format for replies includes headers generated for a
reply message as well as the headers of the original message.  Using
this data, a client can intelligently create a reply. For example, the
emacs client will be able to create replies with quoted HTML parts by
parsing the HTML parts using w3m.

Reply now enforces that only one message is returned, as the semantics
of replying to multiple messages are not wel-defined.

Show is modified such that --format=json no longer implies
--entire-thread, as MUAs will use --format=json when constructing
replies. The man page is updated to reflect this change.
---
 emacs/notmuch-query.el  |    2 +-
 man/man1/notmuch-show.1 |    6 +--
 notmuch-reply.c         |  167 +++++++++++++++++++++++++++++++++++------------
 notmuch-show.c          |    1 -
 4 files changed, 126 insertions(+), 50 deletions(-)

diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
index d66baea..cdf2d6b 100644
--- a/emacs/notmuch-query.el
+++ b/emacs/notmuch-query.el
@@ -29,7 +29,7 @@ A thread is a forest or list of trees. A tree is a two element
 list where the first element is a message, and the second element
 is a possibly empty forest of replies.
 "
-  (let  ((args '("show" "--format=json"))
+  (let  ((args '("show" "--format=json" "--entire-thread"))
 	 (json-object-type 'plist)
 	 (json-array-type 'list)
 	 (json-false 'nil))
diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index b2301d8..c86b9db 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -55,11 +55,7 @@ be nested.
 The output is formatted with Javascript Object Notation (JSON). This
 format is more robust than the text format for automated
 processing. The nested structure of multipart MIME messages is
-reflected in nested JSON output. JSON output always includes all
-messages in a matching thread; in effect
-.B \-\-format=json
-implies
-.B \-\-entire\-thread
+reflected in nested JSON output.
 
 .RE
 .RS 4
diff --git a/notmuch-reply.c b/notmuch-reply.c
index f55b1d2..bfbc307 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -505,6 +505,61 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
     return NULL;
 }
 
+static GMimeMessage *
+create_reply_message(void *ctx,
+		     notmuch_config_t *config,
+		     notmuch_message_t *message,
+		     notmuch_bool_t reply_all)
+{
+    const char *subject, *from_addr = NULL;
+    const char *in_reply_to, *orig_references, *references;
+
+    /* The 1 means we want headers in a "pretty" order. */
+    GMimeMessage *reply = g_mime_message_new (1);
+    if (reply == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return NULL;
+    }
+
+    subject = notmuch_message_get_header (message, "subject");
+    if (subject) {
+	if (strncasecmp (subject, "Re:", 3))
+	    subject = talloc_asprintf (ctx, "Re: %s", subject);
+	g_mime_message_set_subject (reply, subject);
+    }
+
+    from_addr = add_recipients_from_message (reply, config,
+					     message, reply_all);
+
+    if (from_addr == NULL)
+	from_addr = guess_from_received_header (config, message);
+
+    if (from_addr == NULL)
+	from_addr = notmuch_config_get_user_primary_email (config);
+
+    from_addr = talloc_asprintf (ctx, "%s <%s>",
+				 notmuch_config_get_user_name (config),
+				 from_addr);
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "From", from_addr);
+
+    in_reply_to = talloc_asprintf (ctx, "<%s>",
+				   notmuch_message_get_message_id (message));
+
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "In-Reply-To", in_reply_to);
+
+    orig_references = notmuch_message_get_header (message, "references");
+    references = talloc_asprintf (ctx, "%s%s%s",
+				  orig_references ? orig_references : "",
+				  orig_references ? " " : "",
+				  in_reply_to);
+    g_mime_object_set_header (GMIME_OBJECT (reply),
+			      "References", references);
+
+    return reply;
+}
+
 static int
 notmuch_reply_format_default(void *ctx,
 			     notmuch_config_t *config,
@@ -515,8 +570,6 @@ notmuch_reply_format_default(void *ctx,
     GMimeMessage *reply;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    const char *subject, *from_addr = NULL;
-    const char *in_reply_to, *orig_references, *references;
     const notmuch_show_format_t *format = &format_reply;
 
     for (messages = notmuch_query_search_messages (query);
@@ -525,48 +578,10 @@ notmuch_reply_format_default(void *ctx,
     {
 	message = notmuch_messages_get (messages);
 
-	/* The 1 means we want headers in a "pretty" order. */
-	reply = g_mime_message_new (1);
-	if (reply == NULL) {
-	    fprintf (stderr, "Out of memory\n");
-	    return 1;
-	}
-
-	subject = notmuch_message_get_header (message, "subject");
-	if (subject) {
-	    if (strncasecmp (subject, "Re:", 3))
-		subject = talloc_asprintf (ctx, "Re: %s", subject);
-	    g_mime_message_set_subject (reply, subject);
-	}
-
-	from_addr = add_recipients_from_message (reply, config, message,
-						 reply_all);
-
-	if (from_addr == NULL)
-	    from_addr = guess_from_received_header (config, message);
+	reply = create_reply_message (ctx, config, message, reply_all);
 
-	if (from_addr == NULL)
-	    from_addr = notmuch_config_get_user_primary_email (config);
-
-	from_addr = talloc_asprintf (ctx, "%s <%s>",
-				     notmuch_config_get_user_name (config),
-				     from_addr);
-	g_mime_object_set_header (GMIME_OBJECT (reply),
-				  "From", from_addr);
-
-	in_reply_to = talloc_asprintf (ctx, "<%s>",
-			     notmuch_message_get_message_id (message));
-
-	g_mime_object_set_header (GMIME_OBJECT (reply),
-				  "In-Reply-To", in_reply_to);
-
-	orig_references = notmuch_message_get_header (message, "references");
-	references = talloc_asprintf (ctx, "%s%s%s",
-				      orig_references ? orig_references : "",
-				      orig_references ? " " : "",
-				      in_reply_to);
-	g_mime_object_set_header (GMIME_OBJECT (reply),
-				  "References", references);
+	if (!reply)
+	    continue;
 
 	show_reply_headers (reply);
 
@@ -584,6 +599,68 @@ notmuch_reply_format_default(void *ctx,
     return 0;
 }
 
+static int
+notmuch_reply_format_json(void *ctx,
+			  notmuch_config_t *config,
+			  notmuch_query_t *query,
+			  unused (notmuch_show_params_t *params),
+			  notmuch_bool_t reply_all)
+{
+    GMimeMessage *reply;
+    notmuch_messages_t *messages;
+    notmuch_message_t *message;
+
+    const char *reply_headers[] = {"from", "to", "subject", "in-reply-to", "references"};
+    const char *orig_headers[] = {"from", "to", "cc", "subject", "date", "in-reply-to", "references", "message-id"};
+    unsigned int hidx;
+
+    if (notmuch_query_count_messages (query) != 1) {
+	fprintf (stderr, "Error: search term did not match precisely one message.\n");
+	return 1;
+    }
+
+    messages = notmuch_query_search_messages (query);
+    message = notmuch_messages_get (messages);
+
+    reply = create_reply_message (ctx, config, message, reply_all);
+    if (!reply)
+	return 1;
+
+    /* Start a reply object */
+    printf ("{ \"reply\": { ");
+
+    for (hidx = 0; hidx < ARRAY_SIZE (reply_headers); hidx++) {
+	if (hidx)
+	    printf (", ");
+
+	printf ("%s: %s", json_quote_str (ctx, reply_headers[hidx]),
+		json_quote_str (ctx, g_mime_object_get_header (GMIME_OBJECT (reply), reply_headers[hidx])));
+    }
+
+    g_object_unref (G_OBJECT (reply));
+    reply = NULL;
+
+    /* Done the headers for the reply, which has no body */
+    printf (" }");
+
+    /* Start the original */
+    printf (", \"original\": { ");
+
+    for (hidx = 0; hidx < ARRAY_SIZE (orig_headers); hidx++) {
+	if (hidx)
+	    printf (", ");
+
+	printf ("%s: %s", json_quote_str (ctx, orig_headers[hidx]),
+		json_quote_str (ctx, notmuch_message_get_header (message, orig_headers[hidx])));
+    }
+
+    /* End */
+    printf (" } }\n");
+    notmuch_message_destroy (message);
+
+    return 0;
+}
+
 /* This format is currently tuned for a git send-email --notmuch hook */
 static int
 notmuch_reply_format_headers_only(void *ctx,
@@ -646,6 +723,7 @@ notmuch_reply_format_headers_only(void *ctx,
 
 enum {
     FORMAT_DEFAULT,
+    FORMAT_JSON,
     FORMAT_HEADERS_ONLY,
 };
 
@@ -666,6 +744,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
 	  (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },
+				  { "json", FORMAT_JSON },
 				  { "headers-only", FORMAT_HEADERS_ONLY },
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
@@ -684,6 +763,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 
     if (format == FORMAT_HEADERS_ONLY)
 	reply_format_func = notmuch_reply_format_headers_only;
+    else if (format == FORMAT_JSON)
+	reply_format_func = notmuch_reply_format_json;
     else
 	reply_format_func = notmuch_reply_format_default;
 
diff --git a/notmuch-show.c b/notmuch-show.c
index dec799c..344d08c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1082,7 +1082,6 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 		format = &format_text;
 	    } else if (strcmp (opt, "json") == 0) {
 		format = &format_json;
-		params.entire_thread = 1;
 	    } else if (strcmp (opt, "mbox") == 0) {
 		format = &format_mbox;
 		mbox = 1;
-- 
1.7.5.4

Re: [PATCH v4 2/4] reply: Add a JSON reply format.

[Dmitry Kurochkin]

Hi Adam.

On Wed,  8 Feb 2012 17:21:54 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> This new JSON format for replies includes headers generated for a
> reply message as well as the headers of the original message.  Using
> this data, a client can intelligently create a reply. For example, the
> emacs client will be able to create replies with quoted HTML parts by
> parsing the HTML parts using w3m.
> 
> Reply now enforces that only one message is returned, as the semantics
> of replying to multiple messages are not wel-defined.
> 

s/wel/well/

> Show is modified such that --format=json no longer implies
> --entire-thread, as MUAs will use --format=json when constructing
> replies. The man page is updated to reflect this change.

I did not look into details.  But I am surprised that user needs to call
notmuch show --format=json to make reply.  I would expect notmuch reply
to provide all required info (except for bodies).

Anyway, I think you should put this change in a separate patch.

Also, we clearly need a NEWS entry for it and user-customizable Emacs
variable changes.  Though it can be done after this series is pushed, I
guess.

Thank you for this work, it is much appreciated.

Regards,
  Dmitry

> ---
>  emacs/notmuch-query.el  |    2 +-
>  man/man1/notmuch-show.1 |    6 +--
>  notmuch-reply.c         |  167 +++++++++++++++++++++++++++++++++++------------
>  notmuch-show.c          |    1 -
>  4 files changed, 126 insertions(+), 50 deletions(-)
> 
> diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
> index d66baea..cdf2d6b 100644
> --- a/emacs/notmuch-query.el
> +++ b/emacs/notmuch-query.el
> @@ -29,7 +29,7 @@ A thread is a forest or list of trees. A tree is a two element
>  list where the first element is a message, and the second element
>  is a possibly empty forest of replies.
>  "
> -  (let  ((args '("show" "--format=json"))
> +  (let  ((args '("show" "--format=json" "--entire-thread"))
>  	 (json-object-type 'plist)
>  	 (json-array-type 'list)
>  	 (json-false 'nil))
> diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
> index b2301d8..c86b9db 100644
> --- a/man/man1/notmuch-show.1
> +++ b/man/man1/notmuch-show.1
> @@ -55,11 +55,7 @@ be nested.
>  The output is formatted with Javascript Object Notation (JSON). This
>  format is more robust than the text format for automated
>  processing. The nested structure of multipart MIME messages is
> -reflected in nested JSON output. JSON output always includes all
> -messages in a matching thread; in effect
> -.B \-\-format=json
> -implies
> -.B \-\-entire\-thread
> +reflected in nested JSON output.
>  
>  .RE
>  .RS 4
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index f55b1d2..bfbc307 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -505,6 +505,61 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
>      return NULL;
>  }
>  
> +static GMimeMessage *
> +create_reply_message(void *ctx,
> +		     notmuch_config_t *config,
> +		     notmuch_message_t *message,
> +		     notmuch_bool_t reply_all)
> +{
> +    const char *subject, *from_addr = NULL;
> +    const char *in_reply_to, *orig_references, *references;
> +
> +    /* The 1 means we want headers in a "pretty" order. */
> +    GMimeMessage *reply = g_mime_message_new (1);
> +    if (reply == NULL) {
> +	fprintf (stderr, "Out of memory\n");
> +	return NULL;
> +    }
> +
> +    subject = notmuch_message_get_header (message, "subject");
> +    if (subject) {
> +	if (strncasecmp (subject, "Re:", 3))
> +	    subject = talloc_asprintf (ctx, "Re: %s", subject);
> +	g_mime_message_set_subject (reply, subject);
> +    }
> +
> +    from_addr = add_recipients_from_message (reply, config,
> +					     message, reply_all);
> +
> +    if (from_addr == NULL)
> +	from_addr = guess_from_received_header (config, message);
> +
> +    if (from_addr == NULL)
> +	from_addr = notmuch_config_get_user_primary_email (config);
> +
> +    from_addr = talloc_asprintf (ctx, "%s <%s>",
> +				 notmuch_config_get_user_name (config),
> +				 from_addr);
> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "From", from_addr);
> +
> +    in_reply_to = talloc_asprintf (ctx, "<%s>",
> +				   notmuch_message_get_message_id (message));
> +
> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "In-Reply-To", in_reply_to);
> +
> +    orig_references = notmuch_message_get_header (message, "references");
> +    references = talloc_asprintf (ctx, "%s%s%s",
> +				  orig_references ? orig_references : "",
> +				  orig_references ? " " : "",
> +				  in_reply_to);
> +    g_mime_object_set_header (GMIME_OBJECT (reply),
> +			      "References", references);
> +
> +    return reply;
> +}
> +
>  static int
>  notmuch_reply_format_default(void *ctx,
>  			     notmuch_config_t *config,
> @@ -515,8 +570,6 @@ notmuch_reply_format_default(void *ctx,
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
> -    const char *subject, *from_addr = NULL;
> -    const char *in_reply_to, *orig_references, *references;
>      const notmuch_show_format_t *format = &format_reply;
>  
>      for (messages = notmuch_query_search_messages (query);
> @@ -525,48 +578,10 @@ notmuch_reply_format_default(void *ctx,
>      {
>  	message = notmuch_messages_get (messages);
>  
> -	/* The 1 means we want headers in a "pretty" order. */
> -	reply = g_mime_message_new (1);
> -	if (reply == NULL) {
> -	    fprintf (stderr, "Out of memory\n");
> -	    return 1;
> -	}
> -
> -	subject = notmuch_message_get_header (message, "subject");
> -	if (subject) {
> -	    if (strncasecmp (subject, "Re:", 3))
> -		subject = talloc_asprintf (ctx, "Re: %s", subject);
> -	    g_mime_message_set_subject (reply, subject);
> -	}
> -
> -	from_addr = add_recipients_from_message (reply, config, message,
> -						 reply_all);
> -
> -	if (from_addr == NULL)
> -	    from_addr = guess_from_received_header (config, message);
> +	reply = create_reply_message (ctx, config, message, reply_all);
>  
> -	if (from_addr == NULL)
> -	    from_addr = notmuch_config_get_user_primary_email (config);
> -
> -	from_addr = talloc_asprintf (ctx, "%s <%s>",
> -				     notmuch_config_get_user_name (config),
> -				     from_addr);
> -	g_mime_object_set_header (GMIME_OBJECT (reply),
> -				  "From", from_addr);
> -
> -	in_reply_to = talloc_asprintf (ctx, "<%s>",
> -			     notmuch_message_get_message_id (message));
> -
> -	g_mime_object_set_header (GMIME_OBJECT (reply),
> -				  "In-Reply-To", in_reply_to);
> -
> -	orig_references = notmuch_message_get_header (message, "references");
> -	references = talloc_asprintf (ctx, "%s%s%s",
> -				      orig_references ? orig_references : "",
> -				      orig_references ? " " : "",
> -				      in_reply_to);
> -	g_mime_object_set_header (GMIME_OBJECT (reply),
> -				  "References", references);
> +	if (!reply)
> +	    continue;
>  
>  	show_reply_headers (reply);
>  
> @@ -584,6 +599,68 @@ notmuch_reply_format_default(void *ctx,
>      return 0;
>  }
>  
> +static int
> +notmuch_reply_format_json(void *ctx,
> +			  notmuch_config_t *config,
> +			  notmuch_query_t *query,
> +			  unused (notmuch_show_params_t *params),
> +			  notmuch_bool_t reply_all)
> +{
> +    GMimeMessage *reply;
> +    notmuch_messages_t *messages;
> +    notmuch_message_t *message;
> +
> +    const char *reply_headers[] = {"from", "to", "subject", "in-reply-to", "references"};
> +    const char *orig_headers[] = {"from", "to", "cc", "subject", "date", "in-reply-to", "references", "message-id"};
> +    unsigned int hidx;
> +
> +    if (notmuch_query_count_messages (query) != 1) {
> +	fprintf (stderr, "Error: search term did not match precisely one message.\n");
> +	return 1;
> +    }
> +
> +    messages = notmuch_query_search_messages (query);
> +    message = notmuch_messages_get (messages);
> +
> +    reply = create_reply_message (ctx, config, message, reply_all);
> +    if (!reply)
> +	return 1;
> +
> +    /* Start a reply object */
> +    printf ("{ \"reply\": { ");
> +
> +    for (hidx = 0; hidx < ARRAY_SIZE (reply_headers); hidx++) {
> +	if (hidx)
> +	    printf (", ");
> +
> +	printf ("%s: %s", json_quote_str (ctx, reply_headers[hidx]),
> +		json_quote_str (ctx, g_mime_object_get_header (GMIME_OBJECT (reply), reply_headers[hidx])));
> +    }
> +
> +    g_object_unref (G_OBJECT (reply));
> +    reply = NULL;
> +
> +    /* Done the headers for the reply, which has no body */
> +    printf (" }");
> +
> +    /* Start the original */
> +    printf (", \"original\": { ");
> +
> +    for (hidx = 0; hidx < ARRAY_SIZE (orig_headers); hidx++) {
> +	if (hidx)
> +	    printf (", ");
> +
> +	printf ("%s: %s", json_quote_str (ctx, orig_headers[hidx]),
> +		json_quote_str (ctx, notmuch_message_get_header (message, orig_headers[hidx])));
> +    }
> +
> +    /* End */
> +    printf (" } }\n");
> +    notmuch_message_destroy (message);
> +
> +    return 0;
> +}
> +
>  /* This format is currently tuned for a git send-email --notmuch hook */
>  static int
>  notmuch_reply_format_headers_only(void *ctx,
> @@ -646,6 +723,7 @@ notmuch_reply_format_headers_only(void *ctx,
>  
>  enum {
>      FORMAT_DEFAULT,
> +    FORMAT_JSON,
>      FORMAT_HEADERS_ONLY,
>  };
>  
> @@ -666,6 +744,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
>  	  (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },
> +				  { "json", FORMAT_JSON },
>  				  { "headers-only", FORMAT_HEADERS_ONLY },
>  				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
> @@ -684,6 +763,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>  
>      if (format == FORMAT_HEADERS_ONLY)
>  	reply_format_func = notmuch_reply_format_headers_only;
> +    else if (format == FORMAT_JSON)
> +	reply_format_func = notmuch_reply_format_json;
>      else
>  	reply_format_func = notmuch_reply_format_default;
>  
> diff --git a/notmuch-show.c b/notmuch-show.c
> index dec799c..344d08c 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -1082,7 +1082,6 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  		format = &format_text;
>  	    } else if (strcmp (opt, "json") == 0) {
>  		format = &format_json;
> -		params.entire_thread = 1;
>  	    } else if (strcmp (opt, "mbox") == 0) {
>  		format = &format_mbox;
>  		mbox = 1;
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v4 2/4] reply: Add a JSON reply format.

[Adam Wolfe Gordon]

Hi Dmitry,

On Thu, Feb 9, 2012 at 00:22, Dmitry Kurochkin
<dmitry.kurochkin@gmail.com> wrote:
>> Reply now enforces that only one message is returned, as the semantics
>> of replying to multiple messages are not wel-defined.
>
> s/wel/well/

Oops! git filter-branch to the rescue :-).

>> Show is modified such that --format=json no longer implies
>> --entire-thread, as MUAs will use --format=json when constructing
>> replies. The man page is updated to reflect this change.
>
> I did not look into details.  But I am surprised that user needs to call
> notmuch show --format=json to make reply.  I would expect notmuch reply
> to provide all required info (except for bodies).

I agree, it would be ideal to include the data from show --format=json
in the reply JSON. I started down the path of implementing this, but
realized it requires either copying quite a bit of code from show or
factoring it out, and both options felt kind of dirty. I'd like show
and reply to share a function that produces the JSON-formatted body of
a message, but it doesn't feel right to expose the entire JSON format
and all the functions that go with it from notmuch-show.c and put the
structure and all the prototypes in notmuch-client.h.

Will Austin's show rewrite make this easier/cleaner? Or am I being too
squeamish about moving code?

> Anyway, I think you should put this change in a separate patch.

Yeah, if I leave it as is it changes a default behavior, so a separate
patch would probably be a good idea.

> Also, we clearly need a NEWS entry for it and user-customizable Emacs
> variable changes.  Though it can be done after this series is pushed, I
> guess.

I don't think there are any emacs customization changes here, unless
we want to implement the notmuch mode config file that was discussed
before to give message-citation-line-format and other things nice
defaults. Or are you suggesting that there should be some new
customization options? (One I can think of would be the list of
preferred types for multipart/alternative display, which right now is
hardcoded in notmuch-show.el.)

> Thank you for this work, it is much appreciated.

Thanks for taking the time to review these patches! I think as a
result of everyone's reviews I've pushed the series toward what I
envisioned/wanted in the first place, rather than the kinda kludgey
thing I did initially.

Amusingly, I've started a new job since I wrote the original patch and
no longer receive much HTML-only email, but I like this patch series
enough to see it through anyway :-).

Re: [PATCH v4 2/4] reply: Add a JSON reply format.

[Dmitry Kurochkin]

Hi Adam.

On Thu, 9 Feb 2012 21:27:29 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Hi Dmitry,
> 
> On Thu, Feb 9, 2012 at 00:22, Dmitry Kurochkin
> <dmitry.kurochkin@gmail.com> wrote:
> >> Reply now enforces that only one message is returned, as the semantics
> >> of replying to multiple messages are not wel-defined.
> >
> > s/wel/well/
> 
> Oops! git filter-branch to the rescue :-).
> 
> >> Show is modified such that --format=json no longer implies
> >> --entire-thread, as MUAs will use --format=json when constructing
> >> replies. The man page is updated to reflect this change.
> >
> > I did not look into details.  But I am surprised that user needs to call
> > notmuch show --format=json to make reply.  I would expect notmuch reply
> > to provide all required info (except for bodies).
> 
> I agree, it would be ideal to include the data from show --format=json
> in the reply JSON. I started down the path of implementing this, but
> realized it requires either copying quite a bit of code from show or
> factoring it out, and both options felt kind of dirty. I'd like show
> and reply to share a function that produces the JSON-formatted body of
> a message, but it doesn't feel right to expose the entire JSON format
> and all the functions that go with it from notmuch-show.c and put the
> structure and all the prototypes in notmuch-client.h.
> 

I see.

> Will Austin's show rewrite make this easier/cleaner? Or am I being too
> squeamish about moving code?
> 

It probably will.

> > Anyway, I think you should put this change in a separate patch.
> 
> Yeah, if I leave it as is it changes a default behavior, so a separate
> patch would probably be a good idea.
> 
> > Also, we clearly need a NEWS entry for it and user-customizable Emacs
> > variable changes.  Though it can be done after this series is pushed, I
> > guess.
> 
> I don't think there are any emacs customization changes here, unless
> we want to implement the notmuch mode config file that was discussed
> before to give message-citation-line-format and other things nice
> defaults. Or are you suggesting that there should be some new
> customization options? (One I can think of would be the list of
> preferred types for multipart/alternative display, which right now is
> hardcoded in notmuch-show.el.)
> 

No.  I thought there were some user-customizable variables that were
moved to notmuch-lib and renamed.  If that is correct, it would not hurt
to document these changes.

> > Thank you for this work, it is much appreciated.
> 
> Thanks for taking the time to review these patches! I think as a
> result of everyone's reviews I've pushed the series toward what I
> envisioned/wanted in the first place, rather than the kinda kludgey
> thing I did initially.
> 

No need to thank me for review, I did not do it :)

> Amusingly, I've started a new job since I wrote the original patch and
> no longer receive much HTML-only email, but I like this patch series
> enough to see it through anyway :-).

Congratulations with the new job :)

Regards,
  Dmitry

[PATCH v4 3/4] man: Update notmuch-reply man page for JSON format.

[Adam Wolfe Gordon]

---
 man/man1/notmuch-reply.1 |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index 5160ece..9cde96d 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -43,6 +43,11 @@ include
 .BR default
 Includes subject and quoted message body.
 .TP
+.BR json
+Produces JSON output containing headers for a reply message and the
+headers of the original message. This output can be used by a client
+to create a reply message intelligently.
+.TP
 .BR headers\-only
 Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
 .RE
-- 
1.7.5.4

[PATCH v4 4/4] emacs: Use the new JSON reply format and message-cite-original

[Adam Wolfe Gordon]

Using the new JSON reply format allows emacs to quote HTML parts
nicely by using mm-display-part to turn them into displayable text,
then quoting them with message-cite-original. This is very useful for
users who regularly receive HTML-only email.

Use message-mode's message-cite-original function to create the
quoted body for reply messages. In order to make this act like the
existing notmuch defaults, you will need to set the following in
your emacs configuration:

message-citation-line-format "On %a, %d %b %Y, %f wrote:"
message-citation-line-function 'message-insert-formatted-citation-line

The test has been updated to reflect the (ugly) emacs default.
---
 emacs/notmuch-lib.el  |   39 ++++++++++++++
 emacs/notmuch-mua.el  |  136 +++++++++++++++++++++++++++++++++++--------------
 emacs/notmuch-show.el |   24 +--------
 test/emacs            |  101 +++++++++++++++++++++++++++++++++++-
 4 files changed, 237 insertions(+), 63 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index d315f76..3fc7aff 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -21,6 +21,8 @@
 
 ;; This is an part of an emacs-based interface to the notmuch mail system.
 
+(eval-when-compile (require 'cl))
+
 (defvar notmuch-command "notmuch"
   "Command to run the notmuch binary.")
 
@@ -173,6 +175,43 @@ the user hasn't set this variable with the old or new value."
   (list 'when (< emacs-major-version 23)
 	form))
 
+(defun notmuch-split-content-type (content-type)
+  "Split content/type into 'content' and 'type'"
+  (split-string content-type "/"))
+
+(defun notmuch-match-content-type (t1 t2)
+  "Return t if t1 and t2 are matching content types, taking wildcards into account"
+  (let ((st1 (notmuch-split-content-type t1))
+	(st2 (notmuch-split-content-type t2)))
+    (if (or (string= (cadr st1) "*")
+	    (string= (cadr st2) "*"))
+	(string= (car st1) (car st2))
+      (string= t1 t2))))
+
+(defvar notmuch-multipart/alternative-discouraged
+  '(
+    ;; Avoid HTML parts.
+    "text/html"
+    ;; multipart/related usually contain a text/html part and some associated graphics.
+    "multipart/related"
+    ))
+
+(defun notmuch-multipart/alternative-choose (types)
+  "Return a list of preferred types from the given list of types"
+  ;; Based on `mm-preferred-alternative-precedence'.
+  (let ((seq types))
+    (dolist (pref (reverse notmuch-multipart/alternative-discouraged))
+      (dolist (elem (copy-sequence seq))
+	(when (string-match pref elem)
+	  (setq seq (nconc (delete elem seq) (list elem))))))
+    seq))
+
+(defun notmuch-parts-filter-by-type (parts type)
+  "Given a vector of message parts, return a vector containing the ones matching the given type."
+  (loop for part across parts
+	if (notmuch-match-content-type (cdr (assq 'content-type part)) type)
+	vconcat (list part)))
+
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
 ;; Both functions here were copied from emacs 23 with the following copyright:
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 4be7c13..da0d54c 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -19,11 +19,15 @@
 ;;
 ;; Authors: David Edmondson <dme@dme.org>
 
+(require 'json)
 (require 'message)
+(require 'format-spec)
 
 (require 'notmuch-lib)
 (require 'notmuch-address)
 
+(eval-when-compile (require 'cl))
+
 ;;
 
 (defcustom notmuch-mua-send-hook '(notmuch-mua-message-send-hook)
@@ -72,56 +76,110 @@ list."
 	    (push header message-hidden-headers)))
 	notmuch-mua-hidden-headers))
 
+(defun notmuch-mua-get-displayed-part (part query-string)
+  (with-temp-buffer
+    (if (assq 'content part)
+	(insert (cdr (assq 'content part)))
+      (call-process notmuch-command nil t nil "show" "--format=raw"
+		    (format "--part=%s" (cdr (assq 'id part)))
+		    query-string))
+
+    (let ((handle (mm-make-handle (current-buffer) (list (cdr (assq 'content-type part)))))
+	  (end-of-orig (point-max)))
+      (mm-display-part handle)
+      (delete-region (point-min) end-of-orig)
+      (buffer-substring (point-min) (point-max)))))
+
+(defun notmuch-mua-multipart/*-to-list (parts)
+  (loop for part across parts
+	collect (cdr (assq 'content-type part))))
+
+(defun notmuch-mua-get-quotable-parts (parts)
+  (loop for part across parts
+	if (notmuch-match-content-type (cdr (assq 'content-type part)) "multipart/alternative")
+	  append (let* ((subparts (cdr (assq 'content part)))
+			(types (notmuch-mua-multipart/*-to-list subparts))
+			(chosen-type (car (notmuch-multipart/alternative-choose types))))
+		   (notmuch-mua-get-quotable-parts (notmuch-parts-filter-by-type subparts chosen-type)))
+	else if (notmuch-match-content-type (cdr (assq 'content-type part)) "multipart/*")
+	  append (notmuch-mua-get-quotable-parts (cdr (assq 'content part)))
+	else if (notmuch-match-content-type (cdr (assq 'content-type part)) "text/*")
+	  collect part))
+
 (defun notmuch-mua-reply (query-string &optional sender reply-all)
-  (let (headers
-	body
-	(args '("reply")))
-    (if notmuch-show-process-crypto
-	(setq args (append args '("--decrypt"))))
+  (let ((reply-args '("reply" "--format=json"))
+	(show-args '("show" "--format=json" "--part=0"))
+	reply
+	original
+	body)
+    (when notmuch-show-process-crypto
+      (setq reply-args (append reply-args '("--decrypt")))
+      (setq show-args (append show-args '("--decrypt"))))
+
     (if reply-all
-	(setq args (append args '("--reply-to=all")))
-      (setq args (append args '("--reply-to=sender"))))
-    (setq args (append args (list query-string)))
-    ;; This make assumptions about the output of `notmuch reply', but
-    ;; really only that the headers come first followed by a blank
-    ;; line and then the body.
+	(setq reply-args (append reply-args '("--reply-to=all")))
+      (setq reply-args (append reply-args '("--reply-to=sender"))))
+
+    (setq reply-args (append reply-args (list query-string)))
+    (setq show-args (append show-args (list query-string)))
+
+    ;; Get the reply object as JSON, and parse it into an elisp object.
     (with-temp-buffer
-      (apply 'call-process (append (list notmuch-command nil (list t t) nil) args))
+      (apply 'call-process (append (list notmuch-command nil (list t t) nil) reply-args))
       (goto-char (point-min))
-      (if (re-search-forward "^$" nil t)
-	  (save-excursion
-	    (save-restriction
-	      (narrow-to-region (point-min) (point))
-	      (goto-char (point-min))
-	      (setq headers (mail-header-extract)))))
-      (forward-line 1)
-      (setq body (buffer-substring (point) (point-max))))
-    ;; If sender is non-nil, set the From: header to its value.
-    (when sender
-      (mail-header-set 'from sender headers))
-    (let
-	;; Overlay the composition window on that being used to read
-	;; the original message.
-	((same-window-regexps '("\\*mail .*")))
-      (notmuch-mua-mail (mail-header 'to headers)
-			(mail-header 'subject headers)
-			(message-headers-to-generate headers t '(to subject))))
-    ;; insert the message body - but put it in front of the signature
-    ;; if one is present
-    (goto-char (point-max))
-    (if (re-search-backward message-signature-separator nil t)
+      (setq reply (json-read)))
+
+    ;; Get the original message object as JSON, and parse it into an elisp object.
+    (with-temp-buffer
+      (apply 'call-process (append (list notmuch-command nil (list t t) nil) show-args))
+      (goto-char (point-min))
+      (setq original (json-read)))
+
+    ;; Start with the prelude, based on the headers of the original message.
+    (let* ((original-headers (cdr (assq 'headers original)))
+	   (reply-headers (cdr (assq 'reply reply))))
+
+      ;; If sender is non-nil, set the From: header to its value.
+      (when sender
+	(mail-header-set 'from sender reply-headers))
+      (let
+	  ;; Overlay the composition window on that being used to read
+	  ;; the original message.
+	  ((same-window-regexps '("\\*mail .*")))
+	(notmuch-mua-mail (mail-header 'to reply-headers)
+			  (mail-header 'subject reply-headers)
+			  (message-headers-to-generate reply-headers t '(to subject))))
+      ;; insert the message body - but put it in front of the signature
+      ;; if one is present
+      (goto-char (point-max))
+      (if (re-search-backward message-signature-separator nil t)
 	  (forward-line -1)
-      (goto-char (point-max)))
-    (insert body)
-    (push-mark))
-  (set-buffer-modified-p nil)
+	(goto-char (point-max)))
+
+      (let ((from (cdr (assq 'From original-headers)))
+	    (date (cdr (assq 'Date original-headers)))
+	    (start (point)))
+
+	(insert "From: " from "\n")
+	(insert "Date: " date "\n\n")
+
+	(let ((quotable-parts (notmuch-mua-get-quotable-parts (cdr (assq 'body original)))))
+	  (mapc (lambda (part)
+		  (insert (notmuch-mua-get-displayed-part part query-string)))
+		quotable-parts))
+
+	(push-mark)
+	(goto-char start)
+	(message-cite-original))))
 
+  (push-mark)
   (message-goto-body)
   ;; Original message may contain (malicious) MML tags.  We must
   ;; properly quote them in the reply.  Note that using `point-max'
   ;; instead of `mark' here is wrong.  The buffer may include user's
   ;; signature which should not be MML-quoted.
-  (mml-quote-region (point) (mark)))
+  (mml-quote-region (point) (mark))
+  (set-buffer-modified-p nil))
 
 (defun notmuch-mua-forward-message ()
   (message-forward)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 24fde05..fc5d704 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -482,30 +482,13 @@ current buffer, if possible."
 	    (mm-display-part handle)
 	    t))))))
 
-(defvar notmuch-show-multipart/alternative-discouraged
-  '(
-    ;; Avoid HTML parts.
-    "text/html"
-    ;; multipart/related usually contain a text/html part and some associated graphics.
-    "multipart/related"
-    ))
-
 (defun notmuch-show-multipart/*-to-list (part)
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
 	  (plist-get part :content)))
 
-(defun notmuch-show-multipart/alternative-choose (types)
-  ;; Based on `mm-preferred-alternative-precedence'.
-  (let ((seq types))
-    (dolist (pref (reverse notmuch-show-multipart/alternative-discouraged))
-      (dolist (elem (copy-sequence seq))
-	(when (string-match pref elem)
-	  (setq seq (nconc (delete elem seq) (list elem))))))
-    seq))
-
 (defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type)
   (notmuch-show-insert-part-header nth declared-type content-type nil)
-  (let ((chosen-type (car (notmuch-show-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))
+  (let ((chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))
 	(inner-parts (plist-get part :content))
 	(start (point)))
     ;; This inserts all parts of the chosen type rather than just one,
@@ -744,9 +727,6 @@ current buffer, if possible."
 
 ;; Functions for determining how to handle MIME parts.
 
-(defun notmuch-show-split-content-type (content-type)
-  (split-string content-type "/"))
-
 (defun notmuch-show-handlers-for (content-type)
   "Return a list of content handlers for a part of type CONTENT-TYPE."
   (let (result)
@@ -757,7 +737,7 @@ current buffer, if possible."
 	  (list (intern (concat "notmuch-show-insert-part-*/*"))
 		(intern (concat
 			 "notmuch-show-insert-part-"
-			 (car (notmuch-show-split-content-type content-type))
+			 (car (notmuch-split-content-type content-type))
 			 "/*"))
 		(intern (concat "notmuch-show-insert-part-" content-type))))
     result))
diff --git a/test/emacs b/test/emacs
index b74cfa9..e6c718f 100755
--- a/test/emacs
+++ b/test/emacs
@@ -268,11 +268,107 @@ Subject: Re: Testing message sent via SMTP
 In-Reply-To: <XXX>
 Fcc: $(pwd)/mail/sent
 --text follows this line--
-On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
+Notmuch Test Suite <test_suite@notmuchmail.org> writes:
+
 > This is a test that messages are sent via SMTP
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Reply within emacs to a multipart/mixed message"
+test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
+		(notmuch-show-reply)
+		(test-output)'
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: Adrian Perez de Castro <aperez@igalia.com>, notmuch@notmuchmail.org
+Subject: Re: [notmuch] Introducing myself
+In-Reply-To: <20091118002059.067214ed@hikari>
+Fcc: ${MAIL_DIR}/sent
+--text follows this line--
+Adrian Perez de Castro <aperez@igalia.com> writes:
+
+> Hello to all,
+>
+> I have just heard about Not Much today in some random Linux-related news
+> site (LWN?), my name is Adrian Perez and I work as systems administrator
+> (although I can do some code as well :P). I have always thought that the
+> ideas behind Sup were great, but after some time using it, I got tired of
+> the oddities that it has. I also do not like doing things like having to
+> install Ruby just for reading and sorting mails. Some time ago I thought
+> about doing something like Not Much and in fact I played a bit with the
+> Python+Xapian and the Python+Whoosh combinations, because I find relaxing
+> to code things in Python when I am not working and also it is installed
+> by default on most distribution. I got to have some mailboxes indexed and
+> basic searching working a couple of months ago. Lately I have been very
+> busy and had no time for coding, and them... boom! Not Much appears -- and
+> it is almost exactly what I was trying to do, but faster. I have been
+> playing a bit with Not Much today, and I think it has potential.
+>
+> Also, I would like to share one idea I had in mind, that you might find
+> interesting: One thing I have found very annoying is having to re-tag my
+> mail when the indexes get b0rked (it happened a couple of times to me while
+> using Sup), so I was planning to mails as read/unread and adding the tags
+> not just to the index, but to the mail text itself, e.g. by adding a
+> "X-Tags" header field or by reusing the "Keywords" one. This way, the index
+> could be totally recreated by re-reading the mail directories, and this
+> would also allow to a tools like OfflineIMAP [1] to get the mails into a
+> local maildir, tagging and indexing the mails with the e-mail reader and
+> then syncing back the messages with the "X-Tags" header to the IMAP server.
+> This would allow to use the mail reader from a different computer and still
+> have everything tagged finely.
+>
+> Best regards,
+>
+>
+> ---
+> [1] http://software.complete.org/software/projects/show/offlineimap
+>
+> -- 
+> Adrian Perez de Castro <aperez@igalia.com>
+> Igalia - Free Software Engineering
+> _______________________________________________
+> notmuch mailing list
+> notmuch@notmuchmail.org
+> http://notmuchmail.org/mailman/listinfo/notmuch
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Reply within emacs to a multipart/alternative message"
+test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
+		(notmuch-show-reply)
+		(test-output)'
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: Alex Botero-Lowry <alex.boterolowry@gmail.com>, notmuch@notmuchmail.org
+Subject: Re: [notmuch] preliminary FreeBSD support
+In-Reply-To: <cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com>
+Fcc: ${MAIL_DIR}/sent
+--text follows this line--
+Alex Botero-Lowry <alex.boterolowry@gmail.com> writes:
+
+> I saw the announcement this morning, and was very excited, as I had been
+> hoping sup would be turned into a library,
+> since I like the concept more than the UI (I'd rather an emacs interface).
+>
+> I did a preliminary compile which worked out fine, but
+> sysconf(_SC_SC_GETPW_R_SIZE_MAX) returns -1 on
+> FreeBSD, so notmuch_config_open segfaulted.
+>
+> Attached is a patch that supplies a default buffer size of 64 in cases where
+> -1 is returned.
+>
+> http://www.opengroup.org/austin/docs/austin_328.txt - seems to indicate this
+> is acceptable behavior,
+> and http://mail-index.netbsd.org/pkgsrc-bugs/2006/06/07/msg016808.htmlspecifically
+> uses 64 as the
+> buffer size.
+> _______________________________________________
+> notmuch mailing list
+> notmuch@notmuchmail.org
+> http://notmuchmail.org/mailman/listinfo/notmuch
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Quote MML tags in reply"
 message_id='test-emacs-mml-quoting@message.id'
 add_message [id]="$message_id" \
@@ -288,7 +384,8 @@ Subject: Re: Quote MML tags in reply
 In-Reply-To: <test-emacs-mml-quoting@message.id>
 Fcc: ${MAIL_DIR}/sent
 --text follows this line--
-On Fri, 05 Jan 2001 15:43:57 +0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
+Notmuch Test Suite <test_suite@notmuchmail.org> writes:
+
 > <#!part disposition=inline>
 EOF
 test_expect_equal_file OUTPUT EXPECTED
-- 
1.7.5.4