[PATCH v6 00/10] Reply improvements

[PATCH v6 00/10] Reply improvements

[Adam Wolfe Gordon]

Hi all,

This is an updated version of my previous series [1], primarily addressing
the concerns from Austin's review. The big changes are:

* Documentation: add a TODO regarding reply to multiple messages and the
  JSON reply format to schemata.

* Emacs: using plists instead of alists in JSON parsing, and the resulting
  adjustments to the code that consumes the parsed data.

* Emacs: slightly nicer handling of multipart/alternative parts. In
  particular, quote only the last part of desirable type instead of all the
  parts of desirable type.

* Split up the reply patch to make things a bit more clear.

I noticed that the factoring out reply creation patch has been promoted to
"maybe ready" in nmbug. I think this version is unchanged, but since the
old one hasn't been pushed this one can probably replace it there.

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

Adam Wolfe Gordon (10):
  test: Add broken test for the new JSON reply format.
  reply: Factor out reply creation
  reply: Require that only one message is returned
  TODO: Add replying to multiple messages
  reply: Add a JSON reply format.
  schemata: Add documentation for JSON reply format.
  man: Update notmuch-reply man page for JSON format.
  emacs: Factor out useful functions into notmuch-lib
  test: Add broken tests for new emacs reply functionality
  emacs: Use the new JSON reply format and message-cite-original

 devel/TODO               |    5 ++
 devel/schemata           |   27 +++++++-
 emacs/notmuch-lib.el     |   44 ++++++++++++
 emacs/notmuch-mua.el     |  132 +++++++++++++++++++++++++-----------
 emacs/notmuch-show.el    |   24 +------
 man/man1/notmuch-reply.1 |    5 ++
 notmuch-client.h         |   10 ++-
 notmuch-reply.c          |  166 ++++++++++++++++++++++++++++++++--------------
 notmuch-show.c           |   30 ++++++--
 test/emacs               |  101 +++++++++++++++++++++++++++-
 test/multipart           |   51 ++++++++++++++
 11 files changed, 468 insertions(+), 127 deletions(-)

-- 
1.7.5.4

[PATCH v6 01/10] test: Add broken test for the new JSON reply format.

[Adam Wolfe Gordon]

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

diff --git a/test/multipart b/test/multipart
index a3036b4..f102294 100755
--- a/test/multipart
+++ b/test/multipart
@@ -589,6 +589,58 @@ Non-text part: text/html
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "'notmuch reply' to a multipart message with json format"
+test_subtest_known_broken
+notmuch reply --format=json 'id:87liy5ap00.fsf@yoom.home.cworth.org' | notmuch_json_show_sanitize >OUTPUT
+cat <<EOF >EXPECTED
+{"reply-headers": {"Subject": "Re: Multipart message",
+ "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+ "To": "Carl Worth <cworth@cworth.org>,
+ cworth@cworth.org",
+ "In-reply-to": "<87liy5ap00.fsf@yoom.home.cworth.org>",
+ "References": " <87liy5ap00.fsf@yoom.home.cworth.org>"},
+ "original": {"id": "XXXXX",
+ "match": false,
+ "filename": "YYYYY",
+ "timestamp": 978709437,
+ "date_relative": "2001-01-05",
+ "tags": ["attachment","inbox","signed","unread"],
+ "headers": {"Subject": "Multipart message",
+ "From": "Carl Worth <cworth@cworth.org>",
+ "To": "cworth@cworth.org",
+ "Date": "Fri,
+ 05 Jan 2001 15:43:57 +0000"},
+ "body": [{"id": 1,
+ "content-type": "multipart/signed",
+ "content": [{"id": 2,
+ "content-type": "multipart/mixed",
+ "content": [{"id": 3,
+ "content-type": "message/rfc822",
+ "content": [{"headers": {"Subject": "html message",
+ "From": "Carl Worth <cworth@cworth.org>",
+ "To": "cworth@cworth.org",
+ "Date": "Fri,
+ 05 Jan 2001 15:42:57 +0000"},
+ "body": [{"id": 4,
+ "content-type": "multipart/alternative",
+ "content": [{"id": 5,
+ "content-type": "text/html"},
+ {"id": 6,
+ "content-type": "text/plain",
+ "content": "This is an embedded message,
+ with a multipart/alternative part.\n"}]}]}]},
+ {"id": 7,
+ "content-type": "text/plain",
+ "filename": "YYYYY",
+ "content": "This is a text attachment.\n"},
+ {"id": 8,
+ "content-type": "text/plain",
+ "content": "And this message is signed.\n\n-Carl\n"}]},
+ {"id": 9,
+ "content-type": "application/pgp-signature"}]}]}}
+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 v6 02/10] reply: Factor out reply creation

[Adam Wolfe Gordon]

Factor out the creation of a reply message based on an original
message so it can be shared by different reply formats.
---
 notmuch-reply.c |  101 +++++++++++++++++++++++++++++++-----------------------
 1 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 6b244e6..8e56245 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);
+	reply = create_reply_message (ctx, 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);
+	if (!reply)
+	    continue;
 
 	show_reply_headers (reply);
 
-- 
1.7.5.4

[PATCH v6 03/10] reply: Require that only one message is returned

[Adam Wolfe Gordon]

As the semantics of replying to multiple messages have not yet been
defined well, make notmuch reply require that the search given returns
only a single message.
---
 notmuch-reply.c |   36 +++++++++++++++++++-----------------
 1 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 8e56245..177e6ca 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -572,30 +572,32 @@ notmuch_reply_format_default(void *ctx,
     notmuch_message_t *message;
     const notmuch_show_format_t *format = &format_reply;
 
-    for (messages = notmuch_query_search_messages (query);
-	 notmuch_messages_valid (messages);
-	 notmuch_messages_move_to_next (messages))
-    {
-	message = notmuch_messages_get (messages);
+    if (notmuch_query_count_messages (query) != 1) {
+	fprintf (stderr, "Error: search term did not match precisely one message.\n");
+	return 1;
+    }
 
-	reply = create_reply_message (ctx, config, message, reply_all);
+    messages = notmuch_query_search_messages (query);
+    message = notmuch_messages_get (messages);
 
-	if (!reply)
-	    continue;
+    reply = create_reply_message (ctx, config, message, reply_all);
 
-	show_reply_headers (reply);
+    if (!reply)
+	return 1;
 
-	g_object_unref (G_OBJECT (reply));
-	reply = NULL;
+    show_reply_headers (reply);
 
-	printf ("On %s, %s wrote:\n",
-		notmuch_message_get_header (message, "date"),
-		notmuch_message_get_header (message, "from"));
+    g_object_unref (G_OBJECT (reply));
+    reply = NULL;
 
-	show_message_body (message, format, params);
+    printf ("On %s, %s wrote:\n",
+	    notmuch_message_get_header (message, "date"),
+	    notmuch_message_get_header (message, "from"));
+
+    show_message_body (message, format, params);
+
+    notmuch_message_destroy (message);
 
-	notmuch_message_destroy (message);
-    }
     return 0;
 }
 
-- 
1.7.5.4

[PATCH v6 04/10] TODO: Add replying to multiple messages

[Adam Wolfe Gordon]

---
 devel/TODO |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/devel/TODO b/devel/TODO
index 4dda6f4..4ad1ee5 100644
--- a/devel/TODO
+++ b/devel/TODO
@@ -141,6 +141,11 @@ Simplify notmuch-reply to simply print the headers (we have the
 original values) rather than calling GMime (which encodes) and adding
 the confusing gmime-filter-headers.c code (which decodes).
 
+Properly handle replying to multiple messages. Currently, calling
+notmuch reply on a search that returns multiple messages will return
+an error. This will include defining a semantics for replying to
+multiple messages that makes sense and makes the feature useful.
+
 notmuch library
 ---------------
 Add support for custom flag<->tag mappings. In the notmuch
-- 
1.7.5.4

[PATCH v6 05/10] 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.
---
 notmuch-client.h |   10 ++++++++--
 notmuch-reply.c  |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-show.c   |   30 ++++++++++++++++++++++--------
 test/multipart   |    1 -
 4 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index f4a62cc..ef4eaba 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -62,13 +62,13 @@
 #define STRINGIFY(s) STRINGIFY_(s)
 #define STRINGIFY_(s) #s
 
-struct mime_node;
+typedef struct mime_node mime_node_t;
 struct notmuch_show_params;
 
 typedef struct notmuch_show_format {
     const char *message_set_start;
     void (*part) (const void *ctx,
-		  struct mime_node *node, int indent,
+		  mime_node_t *node, int indent,
 		  const struct notmuch_show_params *params);
     const char *message_start;
     void (*message) (const void *ctx,
@@ -191,6 +191,12 @@ show_message_body (notmuch_message_t *message,
 notmuch_status_t
 show_one_part (const char *filename, int part);
 
+void
+format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first);
+
+void
+format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
+
 char *
 json_quote_chararray (const void *ctx, const char *str, const size_t len);
 
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 177e6ca..017c6ae 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -601,6 +601,51 @@ notmuch_reply_format_default(void *ctx,
     return 0;
 }
 
+static int
+notmuch_reply_format_json(void *ctx,
+			  notmuch_config_t *config,
+			  notmuch_query_t *query,
+			  notmuch_show_params_t *params,
+			  notmuch_bool_t reply_all)
+{
+    GMimeMessage *reply;
+    notmuch_messages_t *messages;
+    notmuch_message_t *message;
+    mime_node_t *node;
+
+    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);
+    if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
+			&node) != NOTMUCH_STATUS_SUCCESS)
+	return 1;
+
+    reply = create_reply_message (ctx, config, message, reply_all);
+    if (!reply)
+	return 1;
+
+    /* The headers of the reply message we've created */
+    printf ("{\"reply-headers\": ");
+    format_headers_json (ctx, reply, TRUE);
+    g_object_unref (G_OBJECT (reply));
+    reply = NULL;
+
+    /* Start the original */
+    printf (", \"original\": ");
+
+    format_part_json (ctx, node, TRUE);
+
+    /* 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,
@@ -663,6 +708,7 @@ notmuch_reply_format_headers_only(void *ctx,
 
 enum {
     FORMAT_DEFAULT,
+    FORMAT_JSON,
     FORMAT_HEADERS_ONLY,
 };
 
@@ -682,6 +728,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',
@@ -700,6 +747,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 6a171a4..2126d78 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -290,8 +290,8 @@ format_headers_message_part_text (GMimeMessage *message)
     printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
 }
 
-static void
-format_headers_json (const void *ctx, GMimeMessage *message)
+void
+format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply)
 {
     void *local = talloc_new (ctx);
     InternetAddressList *recipients;
@@ -315,9 +315,23 @@ format_headers_json (const void *ctx, GMimeMessage *message)
 	printf (", %s: %s",
 		json_quote_str (local, "Cc"),
 		json_quote_str (local, recipients_string));
-    printf (", %s: %s}",
-	    json_quote_str (local, "Date"),
-	    json_quote_str (local, g_mime_message_get_date_as_string (message)));
+
+    if (!reply)
+	printf (", %s: %s",
+		json_quote_str (local, "Date"),
+		json_quote_str (local, g_mime_message_get_date_as_string (message)));
+
+    if (reply) {
+	printf (", %s: %s",
+		json_quote_str (local, "In-reply-to"),
+		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "In-reply-to")));
+
+	printf (", %s: %s",
+		json_quote_str (local, "References"),
+		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "References")));
+    }
+
+    printf ("}");
 
     talloc_free (local);
 }
@@ -652,7 +666,7 @@ format_part_text (const void *ctx, mime_node_t *node,
     printf ("\f%s}\n", part_type);
 }
 
-static void
+void
 format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
 {
     /* Any changes to the JSON format should be reflected in the file
@@ -663,7 +677,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
 	format_message_json (ctx, node->envelope_file);
 
 	printf ("\"headers\": ");
-	format_headers_json (ctx, GMIME_MESSAGE (node->part));
+	format_headers_json (ctx, GMIME_MESSAGE (node->part), FALSE);
 
 	printf (", \"body\": [");
 	format_part_json (ctx, mime_node_child (node, 0), first);
@@ -737,7 +751,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
     } else if (GMIME_IS_MESSAGE (node->part)) {
 	printf (", \"content\": [{");
 	printf ("\"headers\": ");
-	format_headers_json (local, GMIME_MESSAGE (node->part));
+	format_headers_json (local, GMIME_MESSAGE (node->part), FALSE);
 
 	printf (", \"body\": [");
 	terminator = "]}]";
diff --git a/test/multipart b/test/multipart
index f102294..e7abcc2 100755
--- a/test/multipart
+++ b/test/multipart
@@ -590,7 +590,6 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "'notmuch reply' to a multipart message with json format"
-test_subtest_known_broken
 notmuch reply --format=json 'id:87liy5ap00.fsf@yoom.home.cworth.org' | notmuch_json_show_sanitize >OUTPUT
 cat <<EOF >EXPECTED
 {"reply-headers": {"Subject": "Re: Multipart message",
-- 
1.7.5.4

[PATCH v6 06/10] schemata: Add documentation for JSON reply format.

[Adam Wolfe Gordon]

---
 devel/schemata |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index d90d4c6..ee8cfc0 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -74,8 +74,9 @@ part = {
     content?:       string    # pre-fetched body content
 }
 
-# The headers of a message (format_headers_json with raw headers) or
-# a part (format_headers_message_part_json with pretty-printed headers)
+# The headers of a message (format_headers_json with raw headers
+# and reply = FALSE) or a part (format_headers_message_part_json
+# with pretty-printed headers)
 headers = {
     Subject:        string,
     From:           string,
@@ -133,3 +134,25 @@ thread = {
                               # matched and unmatched
     subject:        string
 }
+
+notmuch reply schema
+--------------------
+
+reply = {
+    # The headers of the constructed reply (format_headers_json with
+    # raw headers and reply = TRUE)
+    reply-headers: reply_headers,
+
+    # As in the show format (format_message_json)
+    original: message
+}
+
+reply_headers = {
+    Subject:        string,
+    From:           string,
+    To?:            string,
+    Cc?:            string,
+    Bcc?:           string,
+    In-reply-to:    string,
+    References:     string
+}
-- 
1.7.5.4

[PATCH v6 07/10] 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..307abee 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
+contents 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 v6 08/10] emacs: Factor out useful functions into notmuch-lib

[Adam Wolfe Gordon]

Move a few functions related to handling multipart/alternative parts
into notmuch-lib.el, so they can be used by future reply code.
---
 emacs/notmuch-lib.el  |   33 +++++++++++++++++++++++++++++++++
 emacs/notmuch-show.el |   24 ++----------------------
 2 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index d315f76..7e3f110 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,37 @@ 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))
+
 ;; 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-show.el b/emacs/notmuch-show.el
index aa9ccee..3618f88 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -513,30 +513,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,
@@ -775,9 +758,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)
@@ -788,7 +768,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))
-- 
1.7.5.4

[PATCH v6 09/10] test: Add broken tests for new emacs reply functionality

[Adam Wolfe Gordon]

Add tests for creating nice replies to multipart messages, including
those with HTML parts. These tests are expected to fail for now.
---
 test/emacs |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 97 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index d4a8d30..c3a75e9 100755
--- a/test/emacs
+++ b/test/emacs
@@ -273,6 +273,103 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Reply within emacs to a multipart/mixed message"
+test_subtest_known_broken
+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_subtest_known_broken
+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" \
-- 
1.7.5.4

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

[Adam Wolfe Gordon]

Use the new JSON reply format to create replies in emacs. 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 tests have been updated to reflect the (ugly) emacs default.
---
 emacs/notmuch-lib.el |   11 ++++
 emacs/notmuch-mua.el |  136 ++++++++++++++++++++++++++++++++++---------------
 test/emacs           |    8 ++--
 3 files changed, 109 insertions(+), 46 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 7e3f110..8bac596 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -206,6 +206,17 @@ the user hasn't set this variable with the old or new value."
 	  (setq seq (nconc (delete elem seq) (list elem))))))
     seq))
 
+(defun notmuch-parts-filter-by-type (parts type)
+  "Given a list of message parts, return a list containing the ones matching
+the given type."
+  (remove-if-not
+   (lambda (part) (notmuch-match-content-type (plist-get part :content-type) type))
+   parts))
+
+(defun notmuch-plist-to-alist (plist)
+  (loop for (key value . rest) on plist by #'cddr
+	collect (cons (substring (symbol-name key) 1) value)))
+
 ;; 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..5adf4d8 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,104 @@ list."
 	    (push header message-hidden-headers)))
 	notmuch-mua-hidden-headers))
 
+(defun notmuch-mua-get-displayed-part (part query-string)
+  (with-temp-buffer
+    (if (plist-get part :content)
+	(insert (plist-get part :content))
+      (call-process notmuch-command nil t nil "show" "--format=raw"
+		    (format "--part=%s" (plist-get part :id))
+		    query-string))
+
+    (let ((handle (mm-make-handle (current-buffer) (list (plist-get part :content-type))))
+	  (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-get-quotable-parts (parts)
+  (loop for part in parts
+	if (notmuch-match-content-type (plist-get part :content-type) "multipart/alternative")
+	  collect (let* ((subparts (plist-get part :content))
+			(types (mapcar (lambda (part) (plist-get part :content-type)) subparts))
+			(chosen-type (car (notmuch-multipart/alternative-choose types))))
+		   (loop for part in (reverse subparts)
+			 if (notmuch-match-content-type (plist-get part :content-type) chosen-type)
+			 return part))
+	else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
+	  append (notmuch-mua-get-quotable-parts (plist-get part :content))
+	else if (notmuch-match-content-type (plist-get part :content-type) "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 ((args '("reply" "--format=json"))
+	(json-object-type 'plist)
+	(json-array-type 'list)
+	(json-false 'nil)
+	reply
+	original)
+    (when 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 (json-read)))
+
+    ;; Extract the original message to simplify the following code.
+    (setq original (plist-get reply :original))
+
+    ;; Extract the headers of both the reply and the original message.
+    (let* ((original-headers (plist-get original :headers))
+	   (reply-headers (plist-get reply :reply-headers)))
+
+      ;; If sender is non-nil, set the From: header to its value.
+      (when sender
+	(plist-put reply-headers :From sender))
+      (let
+	  ;; Overlay the composition window on that being used to read
+	  ;; the original message.
+	  ((same-window-regexps '("\\*mail .*")))
+	(notmuch-mua-mail (plist-get reply-headers :To)
+			  (plist-get reply-headers :Subject)
+			  (notmuch-plist-to-alist reply-headers)))
+      ;; 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 (plist-get original-headers :From))
+	    (date (plist-get original-headers :Date))
+	    (start (point)))
+
+	;; message-cite-original constructs a citation line based on the From and Date
+	;; headers of the original message, which are assumed to be in the buffer.
+	(insert "From: " from "\n")
+	(insert "Date: " date "\n\n")
+
+	;; Get the parts of the original message that should be quoted; this includes
+	;; all the text parts, except the non-preferred ones in a multipart/alternative.
+	(let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body))))
+	  (mapc (lambda (part)
+		  (insert (notmuch-mua-get-displayed-part part query-string)))
+		quotable-parts))
+
+	(set-mark (point))
+	(goto-char start)
+	;; Quote the original message according to the user's configured style.
+	(message-cite-original)
+	(goto-char (point-max)))))
+
+  (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)))
+  (set-buffer-modified-p nil))
 
 (defun notmuch-mua-forward-message ()
   (message-forward)
@@ -147,7 +199,7 @@ OTHER-ARGS are passed through to `message-mail'."
       (when (not (string= "" user-agent))
 	(push (cons "User-Agent" user-agent) other-headers))))
 
-  (unless (mail-header 'from other-headers)
+  (unless (mail-header 'From other-headers)
     (push (cons "From" (concat
 			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
@@ -210,7 +262,7 @@ the From: address first."
   (interactive "P")
   (let ((other-headers
 	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
-	   (list (cons 'from (notmuch-mua-prompt-for-sender))))))
+	   (list (cons 'From (notmuch-mua-prompt-for-sender))))))
     (notmuch-mua-mail nil nil other-headers)))
 
 (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
diff --git a/test/emacs b/test/emacs
index c3a75e9..a6786d4 100755
--- a/test/emacs
+++ b/test/emacs
@@ -268,13 +268,13 @@ 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_subtest_known_broken
 test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
 		(notmuch-show-reply)
 		(test-output)'
@@ -334,7 +334,6 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Reply within emacs to a multipart/alternative message"
-test_subtest_known_broken
 test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
 		(notmuch-show-reply)
 		(test-output)'
@@ -385,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

Re: [PATCH v6 00/10] Reply improvements

[David Bremner]

On Tue, 21 Feb 2012 23:46:29 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> 
> I noticed that the factoring out reply creation patch has been promoted to
> "maybe ready" in nmbug. I think this version is unchanged, but since the
> old one hasn't been pushed this one can probably replace it there.
> 

By coincidence I "unpromoted it", perhaps because I was missing
context. It seemed strange to push one patch from the middle of the
series.

d

Re: [PATCH v6 00/10] Reply improvements

[Adam Wolfe Gordon]

On Sat, Feb 25, 2012 at 14:29, David Bremner <david@tethera.net> wrote:
> By coincidence I "unpromoted it", perhaps because I was missing
> context. It seemed strange to push one patch from the middle of the
> series.

I think that part was promoted by Austin because one of his
otherwise-ready patches relies on it, and it doesn't change any
functionality.

Austin, can you confirm this?

Re: [PATCH v6 00/10] Reply improvements

[Austin Clements]

Quoth Adam Wolfe Gordon on Feb 25 at  3:25 pm:
> On Sat, Feb 25, 2012 at 14:29, David Bremner <david@tethera.net> wrote:
> > By coincidence I "unpromoted it", perhaps because I was missing
> > context. It seemed strange to push one patch from the middle of the
> > series.
> 
> I think that part was promoted by Austin because one of his
> otherwise-ready patches relies on it, and it doesn't change any
> functionality.
> 
> Austin, can you confirm this?

I was thinking that the reply formatter conversion depended on it,
though I later realized that it doesn't really.  I think this patch
series and the reply formatter conversion won't commute anyway, but I
think it'll be a simple fix either way.

Re: [PATCH v6 02/10] reply: Factor out reply creation

[Jani Nikula]

On Tue, 21 Feb 2012 23:46:31 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Factor out the creation of a reply message based on an original
> message so it can be shared by different reply formats.
> ---
>  notmuch-reply.c |  101 +++++++++++++++++++++++++++++++-----------------------
>  1 files changed, 58 insertions(+), 43 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 6b244e6..8e56245 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);
> +	reply = create_reply_message (ctx, 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);
> +	if (!reply)
> +	    continue;

This changes the existing behaviour, which was to abort on errors. Also,
"continue" here skips notmuch_message_destroy (message). With out of
memory being the only possible error here, perhaps it's just easiest to
abort instead of trying to go on?


BR,
Jani.



>  
>  	show_reply_headers (reply);
>  
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v6 03/10] reply: Require that only one message is returned

[Jani Nikula]

On Tue, 21 Feb 2012 23:46:32 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> As the semantics of replying to multiple messages have not yet been
> defined well, make notmuch reply require that the search given returns
> only a single message.

Is there any real reason, apart from consistency between
--format=default and --format=json, to disable the current multiple
message reply? Also "notmuch show" has format specific features and
limitations.

I agree the semantics should be clarified, and eventually multiple
message reply should be uniformly supported by all formats, including
--format=json, but IMHO this patch should be dropped (and the TODO patch
amended accordingly).

BR,
Jani.


> ---
>  notmuch-reply.c |   36 +++++++++++++++++++-----------------
>  1 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 8e56245..177e6ca 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -572,30 +572,32 @@ notmuch_reply_format_default(void *ctx,
>      notmuch_message_t *message;
>      const notmuch_show_format_t *format = &format_reply;
>  
> -    for (messages = notmuch_query_search_messages (query);
> -	 notmuch_messages_valid (messages);
> -	 notmuch_messages_move_to_next (messages))
> -    {
> -	message = notmuch_messages_get (messages);
> +    if (notmuch_query_count_messages (query) != 1) {
> +	fprintf (stderr, "Error: search term did not match precisely one message.\n");
> +	return 1;
> +    }
>  
> -	reply = create_reply_message (ctx, config, message, reply_all);
> +    messages = notmuch_query_search_messages (query);
> +    message = notmuch_messages_get (messages);
>  
> -	if (!reply)
> -	    continue;
> +    reply = create_reply_message (ctx, config, message, reply_all);
>  
> -	show_reply_headers (reply);
> +    if (!reply)
> +	return 1;
>  
> -	g_object_unref (G_OBJECT (reply));
> -	reply = NULL;
> +    show_reply_headers (reply);
>  
> -	printf ("On %s, %s wrote:\n",
> -		notmuch_message_get_header (message, "date"),
> -		notmuch_message_get_header (message, "from"));
> +    g_object_unref (G_OBJECT (reply));
> +    reply = NULL;
>  
> -	show_message_body (message, format, params);
> +    printf ("On %s, %s wrote:\n",
> +	    notmuch_message_get_header (message, "date"),
> +	    notmuch_message_get_header (message, "from"));
> +
> +    show_message_body (message, format, params);
> +
> +    notmuch_message_destroy (message);
>  
> -	notmuch_message_destroy (message);
> -    }
>      return 0;
>  }
>  
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v6 05/10] reply: Add a JSON reply format.

[Jani Nikula]

On Tue, 21 Feb 2012 23:46:34 -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.
> ---
>  notmuch-client.h |   10 ++++++++--
>  notmuch-reply.c  |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  notmuch-show.c   |   30 ++++++++++++++++++++++--------
>  test/multipart   |    1 -
>  4 files changed, 79 insertions(+), 11 deletions(-)
> 
> diff --git a/notmuch-client.h b/notmuch-client.h
> index f4a62cc..ef4eaba 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -62,13 +62,13 @@
>  #define STRINGIFY(s) STRINGIFY_(s)
>  #define STRINGIFY_(s) #s
>  
> -struct mime_node;
> +typedef struct mime_node mime_node_t;
>  struct notmuch_show_params;
>  
>  typedef struct notmuch_show_format {
>      const char *message_set_start;
>      void (*part) (const void *ctx,
> -		  struct mime_node *node, int indent,
> +		  mime_node_t *node, int indent,
>  		  const struct notmuch_show_params *params);
>      const char *message_start;
>      void (*message) (const void *ctx,
> @@ -191,6 +191,12 @@ show_message_body (notmuch_message_t *message,
>  notmuch_status_t
>  show_one_part (const char *filename, int part);
>  
> +void
> +format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first);
> +
> +void
> +format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
> +
>  char *
>  json_quote_chararray (const void *ctx, const char *str, const size_t len);
>  
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 177e6ca..017c6ae 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -601,6 +601,51 @@ notmuch_reply_format_default(void *ctx,
>      return 0;
>  }
>  
> +static int
> +notmuch_reply_format_json(void *ctx,
> +			  notmuch_config_t *config,
> +			  notmuch_query_t *query,
> +			  notmuch_show_params_t *params,
> +			  notmuch_bool_t reply_all)
> +{
> +    GMimeMessage *reply;
> +    notmuch_messages_t *messages;
> +    notmuch_message_t *message;
> +    mime_node_t *node;
> +
> +    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);
> +    if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
> +			&node) != NOTMUCH_STATUS_SUCCESS)
> +	return 1;
> +
> +    reply = create_reply_message (ctx, config, message, reply_all);
> +    if (!reply)
> +	return 1;
> +
> +    /* The headers of the reply message we've created */
> +    printf ("{\"reply-headers\": ");
> +    format_headers_json (ctx, reply, TRUE);
> +    g_object_unref (G_OBJECT (reply));
> +    reply = NULL;
> +
> +    /* Start the original */
> +    printf (", \"original\": ");
> +
> +    format_part_json (ctx, node, TRUE);
> +
> +    /* 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,
> @@ -663,6 +708,7 @@ notmuch_reply_format_headers_only(void *ctx,
>  
>  enum {
>      FORMAT_DEFAULT,
> +    FORMAT_JSON,
>      FORMAT_HEADERS_ONLY,
>  };
>  
> @@ -682,6 +728,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',
> @@ -700,6 +747,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 6a171a4..2126d78 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -290,8 +290,8 @@ format_headers_message_part_text (GMimeMessage *message)
>      printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
>  }
>  
> -static void
> -format_headers_json (const void *ctx, GMimeMessage *message)
> +void
> +format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply)
>  {
>      void *local = talloc_new (ctx);
>      InternetAddressList *recipients;
> @@ -315,9 +315,23 @@ format_headers_json (const void *ctx, GMimeMessage *message)
>  	printf (", %s: %s",
>  		json_quote_str (local, "Cc"),
>  		json_quote_str (local, recipients_string));
> -    printf (", %s: %s}",
> -	    json_quote_str (local, "Date"),
> -	    json_quote_str (local, g_mime_message_get_date_as_string (message)));
> +
> +    if (!reply)
> +	printf (", %s: %s",
> +		json_quote_str (local, "Date"),
> +		json_quote_str (local, g_mime_message_get_date_as_string (message)));
> +
> +    if (reply) {

Isn't that an "else"?

Otherwise looks good.

BR,
Jani.


> +	printf (", %s: %s",
> +		json_quote_str (local, "In-reply-to"),
> +		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "In-reply-to")));
> +
> +	printf (", %s: %s",
> +		json_quote_str (local, "References"),
> +		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "References")));
> +    }
> +
> +    printf ("}");
>  
>      talloc_free (local);
>  }
> @@ -652,7 +666,7 @@ format_part_text (const void *ctx, mime_node_t *node,
>      printf ("\f%s}\n", part_type);
>  }
>  
> -static void
> +void
>  format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
>  {
>      /* Any changes to the JSON format should be reflected in the file
> @@ -663,7 +677,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
>  	format_message_json (ctx, node->envelope_file);
>  
>  	printf ("\"headers\": ");
> -	format_headers_json (ctx, GMIME_MESSAGE (node->part));
> +	format_headers_json (ctx, GMIME_MESSAGE (node->part), FALSE);
>  
>  	printf (", \"body\": [");
>  	format_part_json (ctx, mime_node_child (node, 0), first);
> @@ -737,7 +751,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first)
>      } else if (GMIME_IS_MESSAGE (node->part)) {
>  	printf (", \"content\": [{");
>  	printf ("\"headers\": ");
> -	format_headers_json (local, GMIME_MESSAGE (node->part));
> +	format_headers_json (local, GMIME_MESSAGE (node->part), FALSE);
>  
>  	printf (", \"body\": [");
>  	terminator = "]}]";
> diff --git a/test/multipart b/test/multipart
> index f102294..e7abcc2 100755
> --- a/test/multipart
> +++ b/test/multipart
> @@ -590,7 +590,6 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
>  test_begin_subtest "'notmuch reply' to a multipart message with json format"
> -test_subtest_known_broken
>  notmuch reply --format=json 'id:87liy5ap00.fsf@yoom.home.cworth.org' | notmuch_json_show_sanitize >OUTPUT
>  cat <<EOF >EXPECTED
>  {"reply-headers": {"Subject": "Re: Multipart message",
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v6] emacs: Use the new JSON reply format and message-cite-original

[Jani Nikula]

On Tue, 21 Feb 2012 23:46:39 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Use the new JSON reply format to create replies in emacs. 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 tests have been updated to reflect the (ugly) emacs default.
> ---
>  emacs/notmuch-lib.el |   11 ++++
>  emacs/notmuch-mua.el |  136 ++++++++++++++++++++++++++++++++++---------------
>  test/emacs           |    8 ++--
>  3 files changed, 109 insertions(+), 46 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 7e3f110..8bac596 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -206,6 +206,17 @@ the user hasn't set this variable with the old or new value."
>  	  (setq seq (nconc (delete elem seq) (list elem))))))
>      seq))
>  
> +(defun notmuch-parts-filter-by-type (parts type)
> +  "Given a list of message parts, return a list containing the ones matching
> +the given type."
> +  (remove-if-not
> +   (lambda (part) (notmuch-match-content-type (plist-get part :content-type) type))
> +   parts))
> +
> +(defun notmuch-plist-to-alist (plist)
> +  (loop for (key value . rest) on plist by #'cddr
> +	collect (cons (substring (symbol-name key) 1) value)))
> +
>  ;; 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..5adf4d8 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,104 @@ list."
>  	    (push header message-hidden-headers)))
>  	notmuch-mua-hidden-headers))
>  
> +(defun notmuch-mua-get-displayed-part (part query-string)
> +  (with-temp-buffer
> +    (if (plist-get part :content)
> +	(insert (plist-get part :content))
> +      (call-process notmuch-command nil t nil "show" "--format=raw"
> +		    (format "--part=%s" (plist-get part :id))
> +		    query-string))
> +
> +    (let ((handle (mm-make-handle (current-buffer) (list (plist-get part :content-type))))
> +	  (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-get-quotable-parts (parts)
> +  (loop for part in parts
> +	if (notmuch-match-content-type (plist-get part :content-type) "multipart/alternative")
> +	  collect (let* ((subparts (plist-get part :content))
> +			(types (mapcar (lambda (part) (plist-get part :content-type)) subparts))
> +			(chosen-type (car (notmuch-multipart/alternative-choose types))))
> +		   (loop for part in (reverse subparts)
> +			 if (notmuch-match-content-type (plist-get part :content-type) chosen-type)
> +			 return part))
> +	else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
> +	  append (notmuch-mua-get-quotable-parts (plist-get part :content))
> +	else if (notmuch-match-content-type (plist-get part :content-type) "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 ((args '("reply" "--format=json"))
> +	(json-object-type 'plist)
> +	(json-array-type 'list)
> +	(json-false 'nil)
> +	reply
> +	original)
> +    (when 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 (json-read)))
> +
> +    ;; Extract the original message to simplify the following code.
> +    (setq original (plist-get reply :original))
> +
> +    ;; Extract the headers of both the reply and the original message.
> +    (let* ((original-headers (plist-get original :headers))
> +	   (reply-headers (plist-get reply :reply-headers)))
> +
> +      ;; If sender is non-nil, set the From: header to its value.
> +      (when sender
> +	(plist-put reply-headers :From sender))
> +      (let
> +	  ;; Overlay the composition window on that being used to read
> +	  ;; the original message.
> +	  ((same-window-regexps '("\\*mail .*")))
> +	(notmuch-mua-mail (plist-get reply-headers :To)
> +			  (plist-get reply-headers :Subject)
> +			  (notmuch-plist-to-alist reply-headers)))
> +      ;; 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 (plist-get original-headers :From))
> +	    (date (plist-get original-headers :Date))
> +	    (start (point)))
> +
> +	;; message-cite-original constructs a citation line based on the From and Date
> +	;; headers of the original message, which are assumed to be in the buffer.
> +	(insert "From: " from "\n")
> +	(insert "Date: " date "\n\n")
> +
> +	;; Get the parts of the original message that should be quoted; this includes
> +	;; all the text parts, except the non-preferred ones in a multipart/alternative.
> +	(let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body))))
> +	  (mapc (lambda (part)
> +		  (insert (notmuch-mua-get-displayed-part part query-string)))
> +		quotable-parts))
> +
> +	(set-mark (point))
> +	(goto-char start)
> +	;; Quote the original message according to the user's configured style.
> +	(message-cite-original)
> +	(goto-char (point-max)))))
> +
> +  (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)))

Is it okay to drop mml quoting? Why?

BR,
Jani.


> +  (set-buffer-modified-p nil))
>  
>  (defun notmuch-mua-forward-message ()
>    (message-forward)
> @@ -147,7 +199,7 @@ OTHER-ARGS are passed through to `message-mail'."
>        (when (not (string= "" user-agent))
>  	(push (cons "User-Agent" user-agent) other-headers))))
>  
> -  (unless (mail-header 'from other-headers)
> +  (unless (mail-header 'From other-headers)
>      (push (cons "From" (concat
>  			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
>  
> @@ -210,7 +262,7 @@ the From: address first."
>    (interactive "P")
>    (let ((other-headers
>  	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
> -	   (list (cons 'from (notmuch-mua-prompt-for-sender))))))
> +	   (list (cons 'From (notmuch-mua-prompt-for-sender))))))
>      (notmuch-mua-mail nil nil other-headers)))
>  
>  (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
> diff --git a/test/emacs b/test/emacs
> index c3a75e9..a6786d4 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -268,13 +268,13 @@ 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_subtest_known_broken
>  test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
>  		(notmuch-show-reply)
>  		(test-output)'
> @@ -334,7 +334,6 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
>  test_begin_subtest "Reply within emacs to a multipart/alternative message"
> -test_subtest_known_broken
>  test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
>  		(notmuch-show-reply)
>  		(test-output)'
> @@ -385,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
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v6] emacs: Use the new JSON reply format and message-cite-original

[Adam Wolfe Gordon]

On Fri, Mar 9, 2012 at 16:13, Jani Nikula <jani@nikula.org> wrote:
>> +     (set-mark (point))
>> +     (goto-char start)
>> +     ;; Quote the original message according to the user's configured style.
>> +     (message-cite-original)
>> +     (goto-char (point-max)))))
>> +
>> +  (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)))
>
> Is it okay to drop mml quoting? Why?

As Austin pointed out on an earlier version, message-cite-original
already does the quoting, so doing it ourselves will result in
double-quoting MML tags.

Re: [PATCH v6 05/10] reply: Add a JSON reply format.

[Adam Wolfe Gordon]

On Fri, Mar 9, 2012 at 16:08, Jani Nikula <jani@nikula.org> wrote:
>> +    if (!reply)
>> +     printf (", %s: %s",
>> +             json_quote_str (local, "Date"),
>> +             json_quote_str (local, g_mime_message_get_date_as_string (message)));
>> +
>> +    if (reply) {
>
> Isn't that an "else"?

Indeed. Fixed.

Re: [PATCH v6 03/10] reply: Require that only one message is returned

[Adam Wolfe Gordon]

On Fri, Mar 9, 2012 at 16:00, Jani Nikula <jani@nikula.org> wrote:
> On Tue, 21 Feb 2012 23:46:32 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
>> As the semantics of replying to multiple messages have not yet been
>> defined well, make notmuch reply require that the search given returns
>> only a single message.
>
> Is there any real reason, apart from consistency between
> --format=default and --format=json, to disable the current multiple
> message reply? Also "notmuch show" has format specific features and
> limitations.
>
> I agree the semantics should be clarified, and eventually multiple
> message reply should be uniformly supported by all formats, including
> --format=json, but IMHO this patch should be dropped (and the TODO patch
> amended accordingly).

It's just for consistency. I like the idea of dropping support for
multi-message replies everywhere now, then re-adding it with new
semantics later, but I don't feel very strongly about it. If people
who use the CLI would prefer to have the default format left as-is for
now that's OK with me.

Re: [PATCH v6 03/10] reply: Require that only one message is returned

[Austin Clements]

Quoth Jani Nikula on Mar 10 at  1:00 am:
> On Tue, 21 Feb 2012 23:46:32 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> > As the semantics of replying to multiple messages have not yet been
> > defined well, make notmuch reply require that the search given returns
> > only a single message.
> 
> Is there any real reason, apart from consistency between
> --format=default and --format=json, to disable the current multiple
> message reply? Also "notmuch show" has format specific features and
> limitations.
> 
> I agree the semantics should be clarified, and eventually multiple
> message reply should be uniformly supported by all formats, including
> --format=json, but IMHO this patch should be dropped (and the TODO patch
> amended accordingly).

I agree with Jani.  We might as well leave the existing format as it
is and say in the TODO that we should both fix multiple reply and
support it in the JSON format.

> BR,
> Jani.

Re: [PATCH v6 06/10] schemata: Add documentation for JSON reply format.

[Austin Clements]

Oops.  Looks like I left references to various old JSON functions in
the schemata file.  I'll submit a patch to fix those up, but you might
as well use the correct function names in the new documentation.

Quoth Adam Wolfe Gordon on Feb 21 at 11:46 pm:
> ---
>  devel/schemata |   27 +++++++++++++++++++++++++--
>  1 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/devel/schemata b/devel/schemata
> index d90d4c6..ee8cfc0 100644
> --- a/devel/schemata
> +++ b/devel/schemata
> @@ -74,8 +74,9 @@ part = {
>      content?:       string    # pre-fetched body content
>  }
>  
> -# The headers of a message (format_headers_json with raw headers) or
> -# a part (format_headers_message_part_json with pretty-printed headers)
> +# The headers of a message (format_headers_json with raw headers
> +# and reply = FALSE) or a part (format_headers_message_part_json
> +# with pretty-printed headers)

There's only format_headers_json now and it outputs pretty-printed
headers.

>  headers = {
>      Subject:        string,
>      From:           string,
> @@ -133,3 +134,25 @@ thread = {
>                                # matched and unmatched
>      subject:        string
>  }
> +
> +notmuch reply schema
> +--------------------
> +
> +reply = {
> +    # The headers of the constructed reply (format_headers_json with
> +    # raw headers and reply = TRUE)
> +    reply-headers: reply_headers,
> +
> +    # As in the show format (format_message_json)

format_part_json

> +    original: message
> +}
> +
> +reply_headers = {
> +    Subject:        string,
> +    From:           string,
> +    To?:            string,
> +    Cc?:            string,
> +    Bcc?:           string,
> +    In-reply-to:    string,
> +    References:     string
> +}

Re: [PATCH v6] emacs: Use the new JSON reply format and message-cite-original

[Austin Clements]

Quoth Adam Wolfe Gordon on Feb 21 at 11:46 pm:
> Use the new JSON reply format to create replies in emacs. 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 tests have been updated to reflect the (ugly) emacs default.
> ---
>  emacs/notmuch-lib.el |   11 ++++
>  emacs/notmuch-mua.el |  136 ++++++++++++++++++++++++++++++++++---------------
>  test/emacs           |    8 ++--
>  3 files changed, 109 insertions(+), 46 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 7e3f110..8bac596 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -206,6 +206,17 @@ the user hasn't set this variable with the old or new value."
>  	  (setq seq (nconc (delete elem seq) (list elem))))))
>      seq))
>  
> +(defun notmuch-parts-filter-by-type (parts type)
> +  "Given a list of message parts, return a list containing the ones matching
> +the given type."
> +  (remove-if-not
> +   (lambda (part) (notmuch-match-content-type (plist-get part :content-type) type))
> +   parts))
> +
> +(defun notmuch-plist-to-alist (plist)
> +  (loop for (key value . rest) on plist by #'cddr
> +	collect (cons (substring (symbol-name key) 1) value)))
> +
>  ;; 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..5adf4d8 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,104 @@ list."
>  	    (push header message-hidden-headers)))
>  	notmuch-mua-hidden-headers))
>  
> +(defun notmuch-mua-get-displayed-part (part query-string)
> +  (with-temp-buffer
> +    (if (plist-get part :content)
> +	(insert (plist-get part :content))
> +      (call-process notmuch-command nil t nil "show" "--format=raw"
> +		    (format "--part=%s" (plist-get part :id))
> +		    query-string))
> +
> +    (let ((handle (mm-make-handle (current-buffer) (list (plist-get part :content-type))))
> +	  (end-of-orig (point-max)))
> +      (mm-display-part handle)
> +      (delete-region (point-min) end-of-orig)
> +      (buffer-substring (point-min) (point-max)))))

Even if it's not possible to completely reuse the show mechanisms
here, it would be nice to reuse the easy ones.  In particular,
notmuch-show-get-bodypart-content looks like it could easily be lifted
to the lib with the addition of a process-crypto argument.  It would
be slightly less efficient, but even now there's some important logic
in notmuch-show-get-bodypart-content that's missing here regarding
encoding handling.

> +
> +(defun notmuch-mua-get-quotable-parts (parts)
> +  (loop for part in parts
> +	if (notmuch-match-content-type (plist-get part :content-type) "multipart/alternative")
> +	  collect (let* ((subparts (plist-get part :content))
> +			(types (mapcar (lambda (part) (plist-get part :content-type)) subparts))
> +			(chosen-type (car (notmuch-multipart/alternative-choose types))))
> +		   (loop for part in (reverse subparts)
> +			 if (notmuch-match-content-type (plist-get part :content-type) chosen-type)
> +			 return part))
> +	else if (notmuch-match-content-type (plist-get part :content-type) "multipart/*")
> +	  append (notmuch-mua-get-quotable-parts (plist-get part :content))
> +	else if (notmuch-match-content-type (plist-get part :content-type) "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 ((args '("reply" "--format=json"))
> +	(json-object-type 'plist)
> +	(json-array-type 'list)
> +	(json-false 'nil)

These should be bound just around the setq reply below since they're
global controls (I highly doubt anything else this function calls
would invoke the JSON parser, but we shouldn't tempt dynamic scoping).

> +	reply
> +	original)
> +    (when 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 (json-read)))
> +
> +    ;; Extract the original message to simplify the following code.
> +    (setq original (plist-get reply :original))
> +
> +    ;; Extract the headers of both the reply and the original message.
> +    (let* ((original-headers (plist-get original :headers))
> +	   (reply-headers (plist-get reply :reply-headers)))
> +
> +      ;; If sender is non-nil, set the From: header to its value.
> +      (when sender
> +	(plist-put reply-headers :From sender))
> +      (let
> +	  ;; Overlay the composition window on that being used to read
> +	  ;; the original message.
> +	  ((same-window-regexps '("\\*mail .*")))
> +	(notmuch-mua-mail (plist-get reply-headers :To)
> +			  (plist-get reply-headers :Subject)
> +			  (notmuch-plist-to-alist reply-headers)))
> +      ;; 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 (plist-get original-headers :From))
> +	    (date (plist-get original-headers :Date))
> +	    (start (point)))
> +
> +	;; message-cite-original constructs a citation line based on the From and Date
> +	;; headers of the original message, which are assumed to be in the buffer.
> +	(insert "From: " from "\n")
> +	(insert "Date: " date "\n\n")
> +
> +	;; Get the parts of the original message that should be quoted; this includes
> +	;; all the text parts, except the non-preferred ones in a multipart/alternative.
> +	(let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body))))
> +	  (mapc (lambda (part)
> +		  (insert (notmuch-mua-get-displayed-part part query-string)))
> +		quotable-parts))
> +
> +	(set-mark (point))
> +	(goto-char start)
> +	;; Quote the original message according to the user's configured style.
> +	(message-cite-original)
> +	(goto-char (point-max)))))

Since the goto-char is really about setting up point for the push-mark
below, it probably makes sense to lift it out of the let and put it
immediately before the push-mark.  I spent a while trying to figure
out what it had to do with the message-cite-original before realizing
that it really had to do with what followed the let.

> +
> +  (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)))
> +  (set-buffer-modified-p nil))
>  
>  (defun notmuch-mua-forward-message ()
>    (message-forward)
> @@ -147,7 +199,7 @@ OTHER-ARGS are passed through to `message-mail'."
>        (when (not (string= "" user-agent))
>  	(push (cons "User-Agent" user-agent) other-headers))))
>  
> -  (unless (mail-header 'from other-headers)
> +  (unless (mail-header 'From other-headers)
>      (push (cons "From" (concat
>  			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
>  
> @@ -210,7 +262,7 @@ the From: address first."
>    (interactive "P")
>    (let ((other-headers
>  	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
> -	   (list (cons 'from (notmuch-mua-prompt-for-sender))))))
> +	   (list (cons 'From (notmuch-mua-prompt-for-sender))))))
>      (notmuch-mua-mail nil nil other-headers)))
>  
>  (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
> diff --git a/test/emacs b/test/emacs
> index c3a75e9..a6786d4 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -268,13 +268,13 @@ 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_subtest_known_broken
>  test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
>  		(notmuch-show-reply)
>  		(test-output)'
> @@ -334,7 +334,6 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
>  test_begin_subtest "Reply within emacs to a multipart/alternative message"
> -test_subtest_known_broken
>  test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
>  		(notmuch-show-reply)
>  		(test-output)'
> @@ -385,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

Re: [PATCH v6 06/10] schemata: Add documentation for JSON reply format.

[Adam Wolfe Gordon]

On Sun, Mar 11, 2012 at 18:36, Austin Clements <amdragon@mit.edu> wrote:
> Oops.  Looks like I left references to various old JSON functions in
> the schemata file.  I'll submit a patch to fix those up, but you might
> as well use the correct function names in the new documentation.

Erk, I missed this review as I was making up the latest version of the
series. I guess I'll have to send another, but I'll wait for reviews
on the other patches first.

Re: [PATCH v6 06/10] schemata: Add documentation for JSON reply format.

[Austin Clements]

Quoth Adam Wolfe Gordon on Mar 11 at 10:09 pm:
> On Sun, Mar 11, 2012 at 18:36, Austin Clements <amdragon@mit.edu> wrote:
> > Oops.  Looks like I left references to various old JSON functions in
> > the schemata file.  I'll submit a patch to fix those up, but you might
> > as well use the correct function names in the new documentation.
> 
> Erk, I missed this review as I was making up the latest version of the
> series. I guess I'll have to send another, but I'll wait for reviews
> on the other patches first.

No worries.  I'm happy to fold these in to fixing up other stale
references in that file.  This shouldn't hold up your series.