unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v2 0/4] Quoting HTML Emails in Reply
@ 2012-01-16 18:13 Adam Wolfe Gordon
  2012-01-16 18:13 ` [PATCH v2 1/4] test: Add broken test for the new JSON reply format Adam Wolfe Gordon
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-16 18:13 UTC (permalink / raw)
  To: notmuch

Hi everyone,

This is the second version of my patch series [1] implementing a new JSON format
for notmuch reply and modifying the emacs UI to use this format, allowing for
nice quoting of HTML-only emails.

This version of the patches corrects some flaws mentioned on the mailing list
and on IRC. The biggest change is that instead of directly using w3 to parse
HTML emails in emacs, notmuch-mua uses mm-display-part, which should allow HTML
emails to be quoted even if the user does not have w3m.el (or even the w3m
binary) installed. Thanks to Dmitry Kurochkin for this suggestion.

Still outstanding is adding customization variables to the emacs interface to
control which parts are preferred for quoting, and possibly to allow
customization of the "On %s, %s wrote" line. I don't think this necessarily
needs to be added before these patches are merged, since they retain the current
functionality for emails with plaintext parts, but I'm happy to hear opinions on
this. Either way, I'll add customization as a separate patch.

Of course, I'm happy to hear any comments on the updated code, especially if I
missed anything from the last set of reviews.

[1] id:1326009162-19524-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.

 emacs/notmuch-lib.el     |    8 ++
 emacs/notmuch-mua.el     |   95 +++++++++-----
 man/man1/notmuch-reply.1 |    5 +
 notmuch-reply.c          |  313 +++++++++++++++++++++++++++++++++++++---------
 test/emacs               |    1 +
 test/multipart           |    7 +
 6 files changed, 336 insertions(+), 93 deletions(-)

-- 
1.7.5.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/4] test: Add broken test for the new JSON reply format.
  2012-01-16 18:13 [PATCH v2 0/4] Quoting HTML Emails in Reply Adam Wolfe Gordon
@ 2012-01-16 18:13 ` Adam Wolfe Gordon
  2012-01-16 18:13 ` [PATCH v2 2/4] reply: Add a " Adam Wolfe Gordon
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-16 18:13 UTC (permalink / raw)
  To: notmuch

---
 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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/4] reply: Add a JSON reply format.
  2012-01-16 18:13 [PATCH v2 0/4] Quoting HTML Emails in Reply Adam Wolfe Gordon
  2012-01-16 18:13 ` [PATCH v2 1/4] test: Add broken test for the new JSON reply format Adam Wolfe Gordon
@ 2012-01-16 18:13 ` Adam Wolfe Gordon
  2012-01-18 23:07   ` Jani Nikula
  2012-01-16 18:13 ` [PATCH v2 3/4] man: Update notmuch-reply man page for JSON format Adam Wolfe Gordon
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-16 18:13 UTC (permalink / raw)
  To: notmuch

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 |  313 ++++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 253 insertions(+), 60 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index da3acce..f5a5dcf 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)
 {
@@ -54,14 +79,14 @@ show_reply_headers (GMimeMessage *message)
     stream_stdout = g_mime_stream_file_new (stdout);
     if (stream_stdout) {
 	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	stream_filter = g_mime_stream_filter_new(stream_stdout);
+	stream_filter = g_mime_stream_filter_new (stream_stdout);
 	if (stream_filter) {
-		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-					 g_mime_filter_headers_new());
-		g_mime_object_write_to_stream(GMIME_OBJECT(message), stream_filter);
-		g_object_unref(stream_filter);
+		g_mime_stream_filter_add (GMIME_STREAM_FILTER(stream_filter),
+					  g_mime_filter_headers_new());
+		g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_filter);
+		g_object_unref (stream_filter);
 	}
-	g_object_unref(stream_stdout);
+	g_object_unref (stream_stdout);
     }
 }
 
@@ -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)
@@ -108,32 +144,29 @@ reply_part_content (GMimeObject *part)
     {
 	GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
 	GMimeDataWrapper *wrapper;
-	const char *charset;
-
-	charset = g_mime_object_get_content_type_parameter (part, "charset");
 	stream_stdout = g_mime_stream_file_new (stdout);
 	if (stream_stdout) {
 	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    stream_filter = g_mime_stream_filter_new(stream_stdout);
-	    if (charset) {
-		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-					 g_mime_filter_charset_new(charset, "UTF-8"));
-	    }
+	    stream_filter = g_mime_stream_filter_new (stream_stdout);
+
+	    const char *charset = g_mime_object_get_content_type_parameter (part, "charset");
+	    if (charset)
+		g_mime_stream_filter_add(GMIME_STREAM_FILTER (stream_filter),
+					 g_mime_filter_charset_new (charset, "UTF-8"));
 	}
-	g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-				 g_mime_filter_reply_new(TRUE));
+	g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
+				  g_mime_filter_reply_new (TRUE));
 	wrapper = g_mime_part_get_content_object (GMIME_PART (part));
 	if (wrapper && stream_filter)
 	    g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
 	if (stream_filter)
-	    g_object_unref(stream_filter);
+	    g_object_unref (stream_filter);
 	if (stream_stdout)
-	    g_object_unref(stream_stdout);
+	    g_object_unref (stream_stdout);
     }
     else
     {
-	if (disposition &&
-	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+	if (disposition && strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
 	{
 	    const char *filename = g_mime_part_get_filename (GMIME_PART (part));
 	    printf ("Attachment: %s (%s)\n", filename,
@@ -147,6 +180,67 @@ 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));
+		}
+		if (stream_filter)
+		    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 +599,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 +664,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 +672,104 @@ 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))
+    {
+	/* Start a reply object */
+	printf ("{ \"reply\": { \"headers\": { ");
+
+	message = notmuch_messages_get (messages);
+
+	reply = create_reply_message (ctx, config, message, reply_all);
+	if (!reply)
+	    continue;
+
+	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 +835,7 @@ notmuch_reply_format_headers_only(void *ctx,
 
 enum {
     FORMAT_DEFAULT,
+    FORMAT_JSON,
     FORMAT_HEADERS_ONLY,
 };
 
@@ -666,6 +856,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 +875,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/4] man: Update notmuch-reply man page for JSON format.
  2012-01-16 18:13 [PATCH v2 0/4] Quoting HTML Emails in Reply Adam Wolfe Gordon
  2012-01-16 18:13 ` [PATCH v2 1/4] test: Add broken test for the new JSON reply format Adam Wolfe Gordon
  2012-01-16 18:13 ` [PATCH v2 2/4] reply: Add a " Adam Wolfe Gordon
@ 2012-01-16 18:13 ` Adam Wolfe Gordon
  2012-01-16 18:13 ` [PATCH v2 4/4] emacs: Use the new JSON reply format Adam Wolfe Gordon
  2012-01-17  1:18 ` [PATCH v2 5/4] emacs: Add customization for the first line of quotes Adam Wolfe Gordon
  4 siblings, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-16 18:13 UTC (permalink / raw)
  To: notmuch

---
 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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 4/4] emacs: Use the new JSON reply format.
  2012-01-16 18:13 [PATCH v2 0/4] Quoting HTML Emails in Reply Adam Wolfe Gordon
                   ` (2 preceding siblings ...)
  2012-01-16 18:13 ` [PATCH v2 3/4] man: Update notmuch-reply man page for JSON format Adam Wolfe Gordon
@ 2012-01-16 18:13 ` Adam Wolfe Gordon
  2012-01-17  9:04   ` David Edmondson
  2012-01-17  1:18 ` [PATCH v2 5/4] emacs: Add customization for the first line of quotes Adam Wolfe Gordon
  4 siblings, 1 reply; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-16 18:13 UTC (permalink / raw)
  To: notmuch

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, except that an additional quoted line is added to the end
of the reply message.  The test has been updated to reflect this.
---
 emacs/notmuch-lib.el |    8 ++++
 emacs/notmuch-mua.el |   95 ++++++++++++++++++++++++++++++++-----------------
 test/emacs           |    1 +
 3 files changed, 71 insertions(+), 33 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 0f856bf..d4dd011 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -127,6 +127,14 @@ the user hasn't set this variable with the old or new value."
   (list 'when (< emacs-major-version 23)
 	form))
 
+(defun find-parts (parts type)
+  "Return a list of message parts with the given type"
+  (delq nil (mapcar (lambda (part)
+		      (if (string= (cdr (assq 'content-type part)) type)
+			  (cdr (assq 'content part))))
+		    parts)))
+
+
 ;; 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 d8ab822..b03c62c 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)
@@ -71,50 +72,78 @@ list."
 	    (push header message-hidden-headers)))
 	notmuch-mua-hidden-headers))
 
+(defun notmuch-mua-insert-part-quoted (part)
+  (save-restriction
+    (narrow-to-region (point) (point))
+    (insert part)
+    (goto-char (point-min))
+    (perform-replace "^" "> " nil t nil)
+    (insert "\n")
+    (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)
+      (kill-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 (find-parts body-parts "text/plain"))
+	   (html-parts (find-parts 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 (null plain-parts)
+	  (mapc (lambda (part) (notmuch-mua-insert-part-quoted (notmuch-mua-parse-html-part part))) html-parts)
+	(mapc (lambda (part) (notmuch-mua-insert-part-quoted part)) plain-parts))
+      
+      (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..4219917 100755
--- a/test/emacs
+++ b/test/emacs
@@ -270,6 +270,7 @@ Fcc: $(pwd)/mail/sent
 --text follows this line--
 On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
 > This is a test that messages are sent via SMTP
+> 
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 5/4] emacs: Add customization for the first line of quotes.
  2012-01-16 18:13 [PATCH v2 0/4] Quoting HTML Emails in Reply Adam Wolfe Gordon
                   ` (3 preceding siblings ...)
  2012-01-16 18:13 ` [PATCH v2 4/4] emacs: Use the new JSON reply format Adam Wolfe Gordon
@ 2012-01-17  1:18 ` Adam Wolfe Gordon
  2012-01-17  7:17   ` how about message-citation-line-format (was: Re: [PATCH v2 5/4] emacs: Add customization for the first line of quotes.) Gregor Zattler
  4 siblings, 1 reply; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-17  1:18 UTC (permalink / raw)
  To: notmuch

Add a customization option, notmuch-mua-reply-quoth, which controls
the first line of the reply body (typically, "On %date%, %from% wrote:").
This allows users who like other styles or correspond in other languages
to set an appropriate line using any of the quoted message's headers.
---

Due to some subsequent discussions in the thread about my original patch series,
I decided I may as well implement this sooner rather than later.

 emacs/notmuch-mua.el |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index b03c62c..c7fd9f8 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -91,6 +91,14 @@ list."
       (fill-region (point-min) (point-max))
       (buffer-substring (point-min) (point-max)))))
 
+(defcustom notmuch-mua-reply-quoth "On %date%, %from% wrote:"
+  "The first line of a reply body, typically 'On %date%, %from% wrote:'.
+
+Any header fields from the message being replied to can be inserted by
+enclosing them in percents."
+  :group 'notmuch
+  :type 'string)
+
 (defun notmuch-mua-reply (query-string &optional sender reply-all)
   (let ((args '("reply" "--format=json"))
 	reply
@@ -132,10 +140,10 @@ 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 (replace-regexp-in-string "%.+?%" 
+					     (lambda (hdr) (cdr (assq (intern (substring hdr 1 (- (length hdr) 1))) original-headers)))
+					     notmuch-mua-reply-quoth)))
+	(insert quoth "\n"))
 
       (if (null plain-parts)
 	  (mapc (lambda (part) (notmuch-mua-insert-part-quoted (notmuch-mua-parse-html-part part))) html-parts)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* how about message-citation-line-format (was: Re: [PATCH v2 5/4] emacs: Add customization for the first line of quotes.)
  2012-01-17  1:18 ` [PATCH v2 5/4] emacs: Add customization for the first line of quotes Adam Wolfe Gordon
@ 2012-01-17  7:17   ` Gregor Zattler
  2012-01-17  9:05     ` David Edmondson
  2012-01-17 16:20     ` Adam Wolfe Gordon
  0 siblings, 2 replies; 20+ messages in thread
From: Gregor Zattler @ 2012-01-17  7:17 UTC (permalink / raw)
  To: notmuch

Hi Adam, notmuch developers,
* Adam Wolfe Gordon <awg+notmuch@xvx.ca> [16. Jan. 2012]:
> Add a customization option, notmuch-mua-reply-quoth, which controls
> the first line of the reply body (typically, "On %date%, %from% wrote:").
> This allows users who like other styles or correspond in other languages
> to set an appropriate line using any of the quoted message's headers.

I really missed this feature in notmuch.  There is already
message-citation-line-format which is part of message mode or
gnus and is not used in notmuch.  Wouldn't it be more consistent
to reuse this?


Ciao; Gregor

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/4] emacs: Use the new JSON reply format.
  2012-01-16 18:13 ` [PATCH v2 4/4] emacs: Use the new JSON reply format Adam Wolfe Gordon
@ 2012-01-17  9:04   ` David Edmondson
  2012-01-17 16:18     ` Adam Wolfe Gordon
  2012-01-17 22:53     ` [PATCH v2 4/5] " Adam Wolfe Gordon
  0 siblings, 2 replies; 20+ messages in thread
From: David Edmondson @ 2012-01-17  9:04 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

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

Much nicer now that it uses the mm stuff.

On Mon, 16 Jan 2012 11:13:23 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> +(defun find-parts (parts type)

Sorry for being a nuisance - this needs a name that indicates that it
relates to notmuch. How about `notmuch-parts-filter-by-type'?

> +  "Return a list of message parts with the given type"
> +  (delq nil (mapcar (lambda (part)
> +		      (if (string= (cdr (assq 'content-type part)) type)
> +			  (cdr (assq 'content part))))
> +		    parts)))

'(delq nil ...)' can more readably be implemented using something like:

       (loop for part in parts
             if (string= (cdr (assq 'content-type part)) type)
             collect (cdr (assq 'content part)))

(untested).

> +(defun notmuch-mua-insert-part-quoted (part)
> +  (save-restriction
> +    (narrow-to-region (point) (point))
> +    (insert part)
> +    (goto-char (point-min))
> +    (perform-replace "^" "> " nil t nil)

Narrowing to '(point) (point)' seems a bit weird and using
`perform-replace' is discouraged in programs. It would be more normal to
use a loop of `re-search-forward' and `replace-match' with a limit after
the insertion. Something like:

  (let ((start (point))
	limit)
    (insert part)
    (setq limit (point))
    (goto-char start)
    (while (re-search-forward "^" limit t)
      (replace-match "> "))

    ...

(untested).

> +    (insert "\n")
> +    (set-buffer-modified-p nil)))
      
Is this newline always required? Is it the cause of the spurious blank
line down below?

> +(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)
> +      (kill-region (point-min) end-of-orig)
> +      (fill-region (point-min) (point-max))
> +      (buffer-substring (point-min) (point-max)))))

`kill-region' will save content in the kill ring. Was that intended?
(Maybe `delete-region' instead?)

>  (defun notmuch-mua-reply (query-string &optional sender reply-all)
> ...
> +      (insert (format "On %s, %s wrote:\n"
> +		      (cdr (assq 'date original-headers))
> +		      (cdr (assq 'from original-headers))))

I wonder whether emacs should be regenerating this or not. I'm okay with
it, but previous discussion was that it should remain the responsibility
of the CLI.

> +      (if (null plain-parts)
> +	  (mapc (lambda (part) (notmuch-mua-insert-part-quoted (notmuch-mua-parse-html-part part))) html-parts)
> +	(mapc (lambda (part) (notmuch-mua-insert-part-quoted part)) plain-parts))

Flip the 'then' and 'else' clauses to get rid of the `null'?

> --- a/test/emacs
> +++ b/test/emacs
> @@ -270,6 +270,7 @@ Fcc: $(pwd)/mail/sent
>  --text follows this line--
>  On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
>  > This is a test that messages are sent via SMTP
> +> 

It would be good if you could get rid of this trailing blank line.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: how about message-citation-line-format (was: Re: [PATCH v2 5/4] emacs: Add customization for the first line of quotes.)
  2012-01-17  7:17   ` how about message-citation-line-format (was: Re: [PATCH v2 5/4] emacs: Add customization for the first line of quotes.) Gregor Zattler
@ 2012-01-17  9:05     ` David Edmondson
  2012-01-17 16:20     ` Adam Wolfe Gordon
  1 sibling, 0 replies; 20+ messages in thread
From: David Edmondson @ 2012-01-17  9:05 UTC (permalink / raw)
  To: Gregor Zattler, notmuch

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

On Tue, 17 Jan 2012 08:17:49 +0100, Gregor Zattler <telegraph@gmx.net> wrote:
> Hi Adam, notmuch developers,
> * Adam Wolfe Gordon <awg+notmuch@xvx.ca> [16. Jan. 2012]:
> > Add a customization option, notmuch-mua-reply-quoth, which controls
> > the first line of the reply body (typically, "On %date%, %from% wrote:").
> > This allows users who like other styles or correspond in other languages
> > to set an appropriate line using any of the quoted message's headers.
> 
> I really missed this feature in notmuch.  There is already
> message-citation-line-format which is part of message mode or
> gnus and is not used in notmuch.  Wouldn't it be more consistent
> to reuse this?

If we're going to do it, this would be the right approach.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/4] emacs: Use the new JSON reply format.
  2012-01-17  9:04   ` David Edmondson
@ 2012-01-17 16:18     ` Adam Wolfe Gordon
  2012-01-17 22:53     ` [PATCH v2 4/5] " Adam Wolfe Gordon
  1 sibling, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-17 16:18 UTC (permalink / raw)
  To: David Edmondson; +Cc: notmuch

Hi David,

Thanks for the review. A couple of comments inline:

On Tue, Jan 17, 2012 at 02:04, David Edmondson <dme@dme.org> wrote:
>> +    (insert "\n")
>> +    (set-buffer-modified-p nil)))
>
> Is this newline always required? Is it the cause of the spurious blank
> line down below?

This is the cause of the spurious blank line, but without it the tests
complain about missing a blank line at the end of the file.  There may
be a better way to deal with this - I'll experiment.

>>  (defun notmuch-mua-reply (query-string &optional sender reply-all)
>> ...
>> +      (insert (format "On %s, %s wrote:\n"
>> +                   (cdr (assq 'date original-headers))
>> +                   (cdr (assq 'from original-headers))))
>
> I wonder whether emacs should be regenerating this or not. I'm okay with
> it, but previous discussion was that it should remain the responsibility
> of the CLI.

I like this being generated in the MUA because then it can be
customized easily (e.g. in my later patch).  Of course, it would also
be possible to add this as a config option for the CLI and generate it
there, but it feels to me like if there's a line between notmuch and
the MUA, this belongs on the MUA side, especially in MUAs that are
formatting the reply themselves anyway.

I'm happy to hear more discussion on this, and will implement whatever
seems best.  I don't actually use the customization myself, it just
strikes me as a useful feature.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: how about message-citation-line-format (was: Re: [PATCH v2 5/4] emacs: Add customization for the first line of quotes.)
  2012-01-17  7:17   ` how about message-citation-line-format (was: Re: [PATCH v2 5/4] emacs: Add customization for the first line of quotes.) Gregor Zattler
  2012-01-17  9:05     ` David Edmondson
@ 2012-01-17 16:20     ` Adam Wolfe Gordon
  1 sibling, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-17 16:20 UTC (permalink / raw)
  To: notmuch

On Tue, Jan 17, 2012 at 00:17, Gregor Zattler <telegraph@gmx.net> wrote:
> I really missed this feature in notmuch.  There is already
> message-citation-line-format which is part of message mode or
> gnus and is not used in notmuch.  Wouldn't it be more consistent
> to reuse this?

Glad to hear someone would use the feature.  I wasn't aware of the
existing variable, but I agree it's a better solution.  I'll look into
it.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 4/5] emacs: Use the new JSON reply format.
  2012-01-17  9:04   ` David Edmondson
  2012-01-17 16:18     ` Adam Wolfe Gordon
@ 2012-01-17 22:53     ` Adam Wolfe Gordon
  2012-01-17 22:53       ` [PATCH v2 5/5] emacs: Use message-citation-line-format in reply Adam Wolfe Gordon
  2012-01-18  6:54       ` [PATCH v2 4/5] emacs: Use the new JSON reply format David Edmondson
  1 sibling, 2 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-17 22:53 UTC (permalink / raw)
  To: notmuch

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.
---

Here is an updated patch that addresses David's concerns. A separate
patch implementing use of the message-citation-line-format variable
will follow.

 emacs/notmuch-lib.el |    8 ++++
 emacs/notmuch-mua.el |   97 +++++++++++++++++++++++++++++++++----------------
 2 files changed, 73 insertions(+), 32 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 0f856bf..2681634 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -127,6 +127,14 @@ 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"
+  (let (result)
+    (dolist (part (append parts nil) result)
+      (if (string= (cdr (assq 'content-type part)) type)
+	  (setq result (append result (list (cdr (assq 'content part)))))))
+    result))
+
 ;; 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 d8ab822..64e160a 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)
@@ -71,49 +72,81 @@ 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))
+    (goto-char start)
+    (while (re-search-forward "\\(^\\)[^$]" limit 0)
+      (replace-match "> " nil nil nil 1)
+      ;; We have added two characters to the quotable region
+      (setq limit (+ limit 2)))
+    (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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 5/5] emacs: Use message-citation-line-format in reply
  2012-01-17 22:53     ` [PATCH v2 4/5] " Adam Wolfe Gordon
@ 2012-01-17 22:53       ` Adam Wolfe Gordon
  2012-01-18  6:54       ` [PATCH v2 4/5] emacs: Use the new JSON reply format David Edmondson
  1 sibling, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-17 22:53 UTC (permalink / raw)
  To: notmuch

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.

The test has been updated to reflect how the message will be
formatted with the default value of message-citation-line-format.

---
 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 64e160a..b58f919 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -135,9 +135,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] emacs: Use the new JSON reply format.
  2012-01-17 22:53     ` [PATCH v2 4/5] " Adam Wolfe Gordon
  2012-01-17 22:53       ` [PATCH v2 5/5] emacs: Use message-citation-line-format in reply Adam Wolfe Gordon
@ 2012-01-18  6:54       ` David Edmondson
  2012-01-18 16:29         ` Adam Wolfe Gordon
  2012-01-18 16:32         ` Adam Wolfe Gordon
  1 sibling, 2 replies; 20+ messages in thread
From: David Edmondson @ 2012-01-18  6:54 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

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

On Tue, 17 Jan 2012 15:53:37 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> +(defun notmuch-parts-filter-by-type (parts type)
> +  "Return a list of message parts with the given type"
> +  (let (result)
> +    (dolist (part (append parts nil) result)
> +      (if (string= (cdr (assq 'content-type part)) type)
> +	  (setq result (append result (list (cdr (assq 'content part)))))))
> +    result))

I still think that `loop' is easier to read :-) But no objection to this
code.

> +(defun notmuch-mua-insert-part-quoted (part)
> +  (let ((start (point))
> +	limit)
> +    (insert part)
> +    (setq limit (point))
> +    (goto-char start)
> +    (while (re-search-forward "\\(^\\)[^$]" limit 0)
> +      (replace-match "> " nil nil nil 1)
> +      ;; We have added two characters to the quotable region
> +      (setq limit (+ limit 2)))
> +    (set-buffer-modified-p nil)))

You could use a marker for the limit, as it would then move along
automagically (sorry for not noticing this last time).

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] emacs: Use the new JSON reply format.
  2012-01-18  6:54       ` [PATCH v2 4/5] emacs: Use the new JSON reply format David Edmondson
@ 2012-01-18 16:29         ` Adam Wolfe Gordon
  2012-01-18 16:32         ` Adam Wolfe Gordon
  1 sibling, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-18 16:29 UTC (permalink / raw)
  To: David Edmondson; +Cc: notmuch

On Tue, Jan 17, 2012 at 23:54, David Edmondson <dme@dme.org> wrote:
> On Tue, 17 Jan 2012 15:53:37 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
>> +(defun notmuch-parts-filter-by-type (parts type)
>> +  "Return a list of message parts with the given type"
>> +  (let (result)
>> +    (dolist (part (append parts nil) result)
>> +      (if (string= (cdr (assq 'content-type part)) type)
>> +       (setq result (append result (list (cdr (assq 'content part)))))))
>> +    result))
>
> I still think that `loop' is easier to read :-) But no objection to this
> code.

I couldn't find the loop construct initially, but after the discussion
of common lisp on IRC yesterday I found the cl package and figured out
how your example worked.  With loop and collect figured out, it is
easier to read.

>> +(defun notmuch-mua-insert-part-quoted (part)
>> +  (let ((start (point))
>> +     limit)
>> +    (insert part)
>> +    (setq limit (point))
>> +    (goto-char start)
>> +    (while (re-search-forward "\\(^\\)[^$]" limit 0)
>> +      (replace-match "> " nil nil nil 1)
>> +      ;; We have added two characters to the quotable region
>> +      (setq limit (+ limit 2)))
>> +    (set-buffer-modified-p nil)))
>
> You could use a marker for the limit, as it would then move along
> automagically (sorry for not noticing this last time).

Aha!  Another trick I didn't know about.

New patch forthcoming.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 4/5] emacs: Use the new JSON reply format.
  2012-01-18  6:54       ` [PATCH v2 4/5] emacs: Use the new JSON reply format David Edmondson
  2012-01-18 16:29         ` Adam Wolfe Gordon
@ 2012-01-18 16:32         ` Adam Wolfe Gordon
  2012-01-18 16:41           ` David Edmondson
  1 sibling, 1 reply; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-18 16:32 UTC (permalink / raw)
  To: notmuch

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 0f856bf..c5c6f91 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.")
 
@@ -127,6 +129,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 d8ab822..b5c5493 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)
@@ -71,49 +72,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] emacs: Use the new JSON reply format.
  2012-01-18 16:32         ` Adam Wolfe Gordon
@ 2012-01-18 16:41           ` David Edmondson
  2012-01-18 17:08             ` Adam Wolfe Gordon
  0 siblings, 1 reply; 20+ messages in thread
From: David Edmondson @ 2012-01-18 16:41 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

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

On Wed, 18 Jan 2012 09:32:34 -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.

I think that this looks pretty good now. We should get some experience
of using it.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] emacs: Use the new JSON reply format.
  2012-01-18 16:41           ` David Edmondson
@ 2012-01-18 17:08             ` Adam Wolfe Gordon
  0 siblings, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-18 17:08 UTC (permalink / raw)
  To: David Edmondson; +Cc: notmuch

On Wed, Jan 18, 2012 at 09:41, David Edmondson <dme@dme.org> wrote:
> I think that this looks pretty good now. We should get some experience
> of using it.

Thanks for the reviews and guidance - I think the patch is in much
better shape than when it started.

FWIW, I've been using the original w3-based version of the patch for a
while now, and the mm-based version since I sent it.  It works well
with all the emails I've tested it with, but almost all the email I
get is HTML-only.  I'd like to hear some experiences from people
trying it on multipart/alternative messages and others.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/4] reply: Add a JSON reply format.
  2012-01-16 18:13 ` [PATCH v2 2/4] reply: Add a " Adam Wolfe Gordon
@ 2012-01-18 23:07   ` Jani Nikula
  2012-01-18 23:29     ` Adam Wolfe Gordon
  0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2012-01-18 23:07 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

On Mon, 16 Jan 2012 11:13:21 -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, admittedly not a very thorough review, but please find a couple of
comments below.

BR,
Jani.


> ---
>  notmuch-reply.c |  313 ++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 253 insertions(+), 60 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index da3acce..f5a5dcf 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)
>  {
> @@ -54,14 +79,14 @@ show_reply_headers (GMimeMessage *message)
>      stream_stdout = g_mime_stream_file_new (stdout);
>      if (stream_stdout) {
>  	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> -	stream_filter = g_mime_stream_filter_new(stream_stdout);
> +	stream_filter = g_mime_stream_filter_new (stream_stdout);
>  	if (stream_filter) {
> -		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
> -					 g_mime_filter_headers_new());
> -		g_mime_object_write_to_stream(GMIME_OBJECT(message), stream_filter);
> -		g_object_unref(stream_filter);
> +		g_mime_stream_filter_add (GMIME_STREAM_FILTER(stream_filter),
> +					  g_mime_filter_headers_new());
> +		g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_filter);
> +		g_object_unref (stream_filter);
>  	}
> -	g_object_unref(stream_stdout);
> +	g_object_unref (stream_stdout);

I know I asked you to adhere to notmuch coding style like above, but I
meant in the context of your patch, not elsewhere. Cleanups like this
should really be separate patches. Sorry if I was ambiguous.

>      }
>  }
>  
> @@ -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)
> @@ -108,32 +144,29 @@ reply_part_content (GMimeObject *part)
>      {
>  	GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
>  	GMimeDataWrapper *wrapper;
> -	const char *charset;
> -
> -	charset = g_mime_object_get_content_type_parameter (part, "charset");
>  	stream_stdout = g_mime_stream_file_new (stdout);
>  	if (stream_stdout) {
>  	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> -	    stream_filter = g_mime_stream_filter_new(stream_stdout);
> -	    if (charset) {
> -		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
> -					 g_mime_filter_charset_new(charset, "UTF-8"));
> -	    }
> +	    stream_filter = g_mime_stream_filter_new (stream_stdout);
> +
> +	    const char *charset = g_mime_object_get_content_type_parameter (part, "charset");
> +	    if (charset)
> +		g_mime_stream_filter_add(GMIME_STREAM_FILTER (stream_filter),
> +					 g_mime_filter_charset_new (charset, "UTF-8"));
>  	}
> -	g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
> -				 g_mime_filter_reply_new(TRUE));
> +	g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
> +				  g_mime_filter_reply_new (TRUE));
>  	wrapper = g_mime_part_get_content_object (GMIME_PART (part));
>  	if (wrapper && stream_filter)
>  	    g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
>  	if (stream_filter)
> -	    g_object_unref(stream_filter);
> +	    g_object_unref (stream_filter);
>  	if (stream_stdout)
> -	    g_object_unref(stream_stdout);
> +	    g_object_unref (stream_stdout);
>      }
>      else
>      {
> -	if (disposition &&
> -	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> +	if (disposition && strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)

This is also a change not related to your patch.

>  	{
>  	    const char *filename = g_mime_part_get_filename (GMIME_PART (part));
>  	    printf ("Attachment: %s (%s)\n", filename,
> @@ -147,6 +180,67 @@ 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))
> +    {

The style in notmuch is to put the opening brace in the end of the if
line. Same applies below.

> +	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));
> +		}
> +		if (stream_filter)
> +		    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 +599,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 +664,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 +672,104 @@ 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))
> +    {
> +	/* Start a reply object */
> +	printf ("{ \"reply\": { \"headers\": { ");
> +
> +	message = notmuch_messages_get (messages);
> +
> +	reply = create_reply_message (ctx, config, message, reply_all);
> +	if (!reply)
> +	    continue;

If this occurs, it will create broken JSON.

> +
> +	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 +835,7 @@ notmuch_reply_format_headers_only(void *ctx,
>  
>  enum {
>      FORMAT_DEFAULT,
> +    FORMAT_JSON,
>      FORMAT_HEADERS_ONLY,
>  };
>  
> @@ -666,6 +856,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 +875,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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/4] reply: Add a JSON reply format.
  2012-01-18 23:07   ` Jani Nikula
@ 2012-01-18 23:29     ` Adam Wolfe Gordon
  0 siblings, 0 replies; 20+ messages in thread
From: Adam Wolfe Gordon @ 2012-01-18 23:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: notmuch

On Wed, Jan 18, 2012 at 16:07, Jani Nikula <jani@nikula.org> wrote:
> I know I asked you to adhere to notmuch coding style like above, but I
> meant in the context of your patch, not elsewhere. Cleanups like this
> should really be separate patches. Sorry if I was ambiguous.

Oops - my bad.  I assumed I had introduced all the spacing problems in
the file and blindly fixed them.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2012-01-18 23:29 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-16 18:13 [PATCH v2 0/4] Quoting HTML Emails in Reply Adam Wolfe Gordon
2012-01-16 18:13 ` [PATCH v2 1/4] test: Add broken test for the new JSON reply format Adam Wolfe Gordon
2012-01-16 18:13 ` [PATCH v2 2/4] reply: Add a " Adam Wolfe Gordon
2012-01-18 23:07   ` Jani Nikula
2012-01-18 23:29     ` Adam Wolfe Gordon
2012-01-16 18:13 ` [PATCH v2 3/4] man: Update notmuch-reply man page for JSON format Adam Wolfe Gordon
2012-01-16 18:13 ` [PATCH v2 4/4] emacs: Use the new JSON reply format Adam Wolfe Gordon
2012-01-17  9:04   ` David Edmondson
2012-01-17 16:18     ` Adam Wolfe Gordon
2012-01-17 22:53     ` [PATCH v2 4/5] " Adam Wolfe Gordon
2012-01-17 22:53       ` [PATCH v2 5/5] emacs: Use message-citation-line-format in reply Adam Wolfe Gordon
2012-01-18  6:54       ` [PATCH v2 4/5] emacs: Use the new JSON reply format David Edmondson
2012-01-18 16:29         ` Adam Wolfe Gordon
2012-01-18 16:32         ` Adam Wolfe Gordon
2012-01-18 16:41           ` David Edmondson
2012-01-18 17:08             ` Adam Wolfe Gordon
2012-01-17  1:18 ` [PATCH v2 5/4] emacs: Add customization for the first line of quotes Adam Wolfe Gordon
2012-01-17  7:17   ` how about message-citation-line-format (was: Re: [PATCH v2 5/4] emacs: Add customization for the first line of quotes.) Gregor Zattler
2012-01-17  9:05     ` David Edmondson
2012-01-17 16:20     ` Adam Wolfe Gordon

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).