--- /dev/null
+Return-Path: <m.walters@qmul.ac.uk>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 9DA8C431FAF\r
+ for <notmuch@notmuchmail.org>; Sun, 5 Feb 2012 03:49:11 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -1.098\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
+ tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
+ NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id U6gucuK9F16g for <notmuch@notmuchmail.org>;\r
+ Sun, 5 Feb 2012 03:49:10 -0800 (PST)\r
+Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
+ (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id E4651431FAE\r
+ for <notmuch@notmuchmail.org>; Sun, 5 Feb 2012 03:49:09 -0800 (PST)\r
+Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
+ by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
+ (envelope-from <m.walters@qmul.ac.uk>)\r
+ id 1Ru0ad-0007P8-Nh; Sun, 05 Feb 2012 11:49:06 +0000\r
+Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223]\r
+ helo=localhost)\r
+ by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
+ (envelope-from <m.walters@qmul.ac.uk>)\r
+ id 1Ru0ac-0001Gu-RZ; Sun, 05 Feb 2012 11:49:03 +0000\r
+From: Mark Walters <markwalters1009@gmail.com>\r
+To: Adam Wolfe Gordon <awg+notmuch@xvx.ca>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v3 2/5] reply: Add a JSON reply format.\r
+In-Reply-To: <1326995217-27423-3-git-send-email-awg+notmuch@xvx.ca>\r
+References: <1326995217-27423-1-git-send-email-awg+notmuch@xvx.ca>\r
+ <1326995217-27423-3-git-send-email-awg+notmuch@xvx.ca>\r
+User-Agent: Notmuch/0.11+154~ged6d37e (http://notmuchmail.org) Emacs/23.2.1\r
+ (i486-pc-linux-gnu)\r
+Date: Sun, 05 Feb 2012 11:50:12 +0000\r
+Message-ID: <87mx8xpki3.fsf@qmul.ac.uk>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-Sender-Host-Address: 94.192.233.223\r
+X-QM-SPAM-Info: Sender has good ham record. :)\r
+X-QM-Body-MD5: 894b95a91825f7f31952e25cbefcec99 (of first 20000 bytes)\r
+X-SpamAssassin-Score: -1.8\r
+X-SpamAssassin-SpamBar: -\r
+X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
+ determine if it is\r
+ spam. We require at least 5.0 points to mark a message as spam.\r
+ This message scored -1.8 points.\r
+ Summary of the scoring: \r
+ * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
+ * medium trust\r
+ * [138.37.6.40 listed in list.dnswl.org]\r
+ * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
+ provider * (markwalters1009[at]gmail.com)\r
+ * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay\r
+ * domain\r
+ * 0.5 AWL AWL: From: address is in the auto white-list\r
+X-QM-Scan-Virus: ClamAV says the message is clean\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Sun, 05 Feb 2012 11:49:11 -0000\r
+\r
+On Thu, 19 Jan 2012 10:46:54 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:\r
+> This new JSON format for replies includes headers generated for a reply\r
+> message as well as the headers and all text parts of the original message.\r
+> Using this data, a client can intelligently create a reply. For example,\r
+> the emacs client will be able to create replies with quoted HTML parts by\r
+> parsing the HTML parts using w3m.\r
+\r
+Hi this is only a preliminary look so far as I read the code. Note this\r
+is the first time I have tried reviewing a substantial chunk of code so\r
+sorry for any stupidities on my part!\r
+\r
+Best wishes\r
+\r
+Mark\r
+\r
+> notmuch-reply.c | 271 +++++++++++++++++++++++++++++++++++++++++++++++--------\r
+> 1 files changed, 231 insertions(+), 40 deletions(-)\r
+> \r
+> diff --git a/notmuch-reply.c b/notmuch-reply.c\r
+> index 0f682db..b4c2426 100644\r
+> --- a/notmuch-reply.c\r
+> +++ b/notmuch-reply.c\r
+> @@ -30,6 +30,15 @@ reply_headers_message_part (GMimeMessage *message);\r
+> static void\r
+> reply_part_content (GMimeObject *part);\r
+> \r
+> +static void\r
+> +reply_part_start_json (GMimeObject *part, int *part_count);\r
+> +\r
+> +static void\r
+> +reply_part_content_json (GMimeObject *part);\r
+> +\r
+> +static void\r
+> +reply_part_end_json (GMimeObject *part);\r
+> +\r
+> static const notmuch_show_format_t format_reply = {\r
+> "",\r
+> "", NULL,\r
+> @@ -46,6 +55,22 @@ static const notmuch_show_format_t format_reply = {\r
+> ""\r
+> };\r
+> \r
+> +static const notmuch_show_format_t format_json = {\r
+> + "",\r
+> + "", NULL,\r
+> + "", NULL, NULL, "",\r
+> + "",\r
+> + reply_part_start_json,\r
+> + NULL,\r
+> + NULL,\r
+> + reply_part_content_json,\r
+> + reply_part_end_json,\r
+> + "",\r
+> + "",\r
+> + "", "",\r
+> + ""\r
+> +};\r
+> +\r
+> static void\r
+> show_reply_headers (GMimeMessage *message)\r
+> {\r
+> @@ -86,6 +111,17 @@ reply_headers_message_part (GMimeMessage *message)\r
+> printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));\r
+> }\r
+> \r
+> +static notmuch_bool_t\r
+> +reply_check_part_type (GMimeObject *part, const char *type, const char *subtype,\r
+> + const char *disposition)\r
+> +{\r
+> + GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));\r
+> + GMimeContentDisposition *part_disposition = g_mime_object_get_content_disposition (part);\r
+> +\r
+> + return (g_mime_content_type_is_type (content_type, type, subtype) &&\r
+> + (!part_disposition ||\r
+> + strcmp (part_disposition->disposition, disposition) == 0));\r
+> +}\r
+> \r
+> static void\r
+> reply_part_content (GMimeObject *part)\r
+> @@ -147,6 +183,63 @@ reply_part_content (GMimeObject *part)\r
+> }\r
+> }\r
+> \r
+> +static void\r
+> +reply_part_start_json (GMimeObject *part, unused (int *part_count))\r
+> +{\r
+> + if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))\r
+> + printf ("{ ");\r
+> +}\r
+> +\r
+> +static void\r
+> +reply_part_end_json (GMimeObject *part)\r
+> +{\r
+> + if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))\r
+> + printf ("}, ");\r
+> +}\r
+> +\r
+> +static void\r
+> +reply_part_content_json (GMimeObject *part)\r
+> +{\r
+> + GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part));\r
+> + void *ctx = talloc_new (NULL);\r
+> +\r
+> + /* We only care about inline text parts for reply purposes */\r
+> + if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE)) {\r
+\r
+This seems to be different from the logic in the text output: I think\r
+that inlines all text/* regardless of disposition. I think the JSON\r
+output should include at least as much as the text output as it is easy\r
+for the caller to discard parts.\r
+\r
+> + GMimeDataWrapper *wrapper;\r
+> + GByteArray *part_content;\r
+> +\r
+> + printf ("\"content-type\": %s, \"content\": ",\r
+> + json_quote_str (ctx, g_mime_content_type_to_string (content_type)));\r
+> +\r
+> + wrapper = g_mime_part_get_content_object (GMIME_PART (part));\r
+> + if (wrapper) {\r
+> + const char *charset = g_mime_object_get_content_type_parameter (part, "charset");\r
+> + GMimeStream *stream_memory = g_mime_stream_mem_new ();\r
+> + if (stream_memory) {\r
+> + GMimeStream *stream_filter = NULL;\r
+> + stream_filter = g_mime_stream_filter_new (stream_memory);\r
+\r
+> + if (charset) {\r
+> + g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),\r
+> + g_mime_filter_charset_new (charset, "UTF-8"));\r
+> + }\r
+> +\r
+> + if (stream_filter) {\r
+\r
+should the if (charset) block be inside the if (stream_filter) block?\r
+\r
+> + g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);\r
+> + part_content = g_mime_stream_mem_get_byte_array (GMIME_STREAM_MEM (stream_memory));\r
+> +\r
+> + printf ("%s", json_quote_chararray (ctx, (char *) part_content->data, part_content->len));\r
+> + g_object_unref (stream_filter);\r
+> + }\r
+> + }\r
+> +\r
+> + if (stream_memory)\r
+> + g_object_unref (stream_memory);\r
+> + }\r
+> + }\r
+> +\r
+> + talloc_free (ctx);\r
+\r
+Does wrapper need to a free/unref somewhere?\r
+\r
+> +}\r
+> +\r
+> /* Is the given address configured as one of the user's "personal" or\r
+> * "other" addresses. */\r
+> static int\r
+> @@ -505,6 +598,61 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message\r
+> return NULL;\r
+> }\r
+> \r
+> +static GMimeMessage *\r
+> +create_reply_message(void *ctx,\r
+> + notmuch_config_t *config,\r
+> + notmuch_message_t *message,\r
+> + notmuch_bool_t reply_all)\r
+> +{\r
+> + const char *subject, *from_addr = NULL;\r
+> + const char *in_reply_to, *orig_references, *references;\r
+> +\r
+> + /* The 1 means we want headers in a "pretty" order. */\r
+> + GMimeMessage *reply = g_mime_message_new (1);\r
+> + if (reply == NULL) {\r
+> + fprintf (stderr, "Out of memory\n");\r
+> + return NULL;\r
+> + }\r
+> +\r
+> + subject = notmuch_message_get_header (message, "subject");\r
+> + if (subject) {\r
+> + if (strncasecmp (subject, "Re:", 3))\r
+> + subject = talloc_asprintf (ctx, "Re: %s", subject);\r
+> + g_mime_message_set_subject (reply, subject);\r
+> + }\r
+> +\r
+> + from_addr = add_recipients_from_message (reply, config,\r
+> + message, reply_all);\r
+> +\r
+> + if (from_addr == NULL)\r
+> + from_addr = guess_from_received_header (config, message);\r
+> +\r
+> + if (from_addr == NULL)\r
+> + from_addr = notmuch_config_get_user_primary_email (config);\r
+> +\r
+> + from_addr = talloc_asprintf (ctx, "%s <%s>",\r
+> + notmuch_config_get_user_name (config),\r
+> + from_addr);\r
+> + g_mime_object_set_header (GMIME_OBJECT (reply),\r
+> + "From", from_addr);\r
+> +\r
+> + in_reply_to = talloc_asprintf (ctx, "<%s>",\r
+> + notmuch_message_get_message_id (message));\r
+> +\r
+> + g_mime_object_set_header (GMIME_OBJECT (reply),\r
+> + "In-Reply-To", in_reply_to);\r
+> +\r
+> + orig_references = notmuch_message_get_header (message, "references");\r
+> + references = talloc_asprintf (ctx, "%s%s%s",\r
+> + orig_references ? orig_references : "",\r
+> + orig_references ? " " : "",\r
+> + in_reply_to);\r
+> + g_mime_object_set_header (GMIME_OBJECT (reply),\r
+> + "References", references);\r
+> +\r
+> + return reply;\r
+> +}\r
+> +\r
+> static int\r
+> notmuch_reply_format_default(void *ctx,\r
+> notmuch_config_t *config,\r
+> @@ -515,8 +663,6 @@ notmuch_reply_format_default(void *ctx,\r
+> GMimeMessage *reply;\r
+> notmuch_messages_t *messages;\r
+> notmuch_message_t *message;\r
+> - const char *subject, *from_addr = NULL;\r
+> - const char *in_reply_to, *orig_references, *references;\r
+> const notmuch_show_format_t *format = &format_reply;\r
+> \r
+> for (messages = notmuch_query_search_messages (query);\r
+> @@ -525,62 +671,103 @@ notmuch_reply_format_default(void *ctx,\r
+> {\r
+> message = notmuch_messages_get (messages);\r
+> \r
+> - /* The 1 means we want headers in a "pretty" order. */\r
+> - reply = g_mime_message_new (1);\r
+> - if (reply == NULL) {\r
+> - fprintf (stderr, "Out of memory\n");\r
+> - return 1;\r
+> - }\r
+> + reply = create_reply_message (ctx, config, message, reply_all);\r
+> \r
+> - subject = notmuch_message_get_header (message, "subject");\r
+> - if (subject) {\r
+> - if (strncasecmp (subject, "Re:", 3))\r
+> - subject = talloc_asprintf (ctx, "Re: %s", subject);\r
+> - g_mime_message_set_subject (reply, subject);\r
+> - }\r
+> + if (!reply)\r
+> + continue;\r
+> \r
+> - from_addr = add_recipients_from_message (reply, config, message,\r
+> - reply_all);\r
+> + show_reply_headers (reply);\r
+> \r
+> - if (from_addr == NULL)\r
+> - from_addr = guess_from_received_header (config, message);\r
+> + g_object_unref (G_OBJECT (reply));\r
+> + reply = NULL;\r
+> \r
+> - if (from_addr == NULL)\r
+> - from_addr = notmuch_config_get_user_primary_email (config);\r
+> + printf ("On %s, %s wrote:\n",\r
+> + notmuch_message_get_header (message, "date"),\r
+> + notmuch_message_get_header (message, "from"));\r
+> \r
+> - from_addr = talloc_asprintf (ctx, "%s <%s>",\r
+> - notmuch_config_get_user_name (config),\r
+> - from_addr);\r
+> - g_mime_object_set_header (GMIME_OBJECT (reply),\r
+> - "From", from_addr);\r
+> + show_message_body (message, format, params);\r
+> \r
+> - in_reply_to = talloc_asprintf (ctx, "<%s>",\r
+> - notmuch_message_get_message_id (message));\r
+> + notmuch_message_destroy (message);\r
+> + }\r
+> + return 0;\r
+> +}\r
+> \r
+> - g_mime_object_set_header (GMIME_OBJECT (reply),\r
+> - "In-Reply-To", in_reply_to);\r
+> +static int\r
+> +notmuch_reply_format_json(void *ctx,\r
+> + notmuch_config_t *config,\r
+> + notmuch_query_t *query,\r
+> + unused (notmuch_show_params_t *params),\r
+> + notmuch_bool_t reply_all)\r
+> +{\r
+> + GMimeMessage *reply;\r
+> + notmuch_messages_t *messages;\r
+> + notmuch_message_t *message;\r
+> + const notmuch_show_format_t *format = &format_json;\r
+> \r
+> - orig_references = notmuch_message_get_header (message, "references");\r
+> - references = talloc_asprintf (ctx, "%s%s%s",\r
+> - orig_references ? orig_references : "",\r
+> - orig_references ? " " : "",\r
+> - in_reply_to);\r
+> - g_mime_object_set_header (GMIME_OBJECT (reply),\r
+> - "References", references);\r
+> + const char *reply_headers[] = {"from", "to", "subject", "in-reply-to", "references"};\r
+> + const char *orig_headers[] = {"from", "to", "cc", "subject", "date", "in-reply-to", "references"};\r
+> + unsigned int hidx;\r
+> \r
+> - show_reply_headers (reply);\r
+> + /* Start array of reply objects */\r
+> + printf ("[");\r
+> +\r
+> + for (messages = notmuch_query_search_messages (query);\r
+> + notmuch_messages_valid (messages);\r
+> + notmuch_messages_move_to_next (messages))\r
+> + {\r
+> + message = notmuch_messages_get (messages);\r
+> + reply = create_reply_message (ctx, config, message, reply_all);\r
+> + if (!reply)\r
+> + continue;\r
+> +\r
+> + /* Start a reply object */\r
+> + printf ("{ \"reply\": { \"headers\": { ");\r
+> +\r
+> + for (hidx = 0; hidx < ARRAY_SIZE (reply_headers); hidx++)\r
+> + {\r
+\r
+Nit: I think the preferred style is brace on the same line as the for loop.\r
+\r
+> + if (hidx)\r
+> + printf (", ");\r
+> +\r
+> + printf ("%s: %s", json_quote_str (ctx, reply_headers[hidx]),\r
+> + json_quote_str (ctx, g_mime_object_get_header (GMIME_OBJECT (reply), reply_headers[hidx])));\r
+> + }\r
+> \r
+> g_object_unref (G_OBJECT (reply));\r
+> reply = NULL;\r
+> \r
+> - printf ("On %s, %s wrote:\n",\r
+> - notmuch_message_get_header (message, "date"),\r
+> - notmuch_message_get_header (message, "from"));\r
+> + /* Done the headers for the reply, which has no body parts */\r
+> + printf ("} }");\r
+\r
+If replying to multiple messages (such as a whole thread) you get\r
+multiple sets of "new headers". I think that probably is not what is\r
+wanted but its still better than the weird things the text version\r
+does. Might be worth putting a comment. [What I think should happen is\r
+that a union of all the headers from all these is taken throwing away\r
+duplicate addresses but that is obviously not part of this patch set]\r
+\r
+> + /* Start the original */\r
+> + printf (", \"original\": { \"headers\": { ");\r
+> +\r
+> + for (hidx = 0; hidx < ARRAY_SIZE (orig_headers); hidx++)\r
+> + {\r
+> + if (hidx)\r
+> + printf (", ");\r
+> +\r
+> + printf ("%s: %s", json_quote_str (ctx, orig_headers[hidx]),\r
+> + json_quote_str (ctx, notmuch_message_get_header (message, orig_headers[hidx])));\r
+> + }\r
+> +\r
+> + /* End headers */\r
+> + printf (" }, \"body\": [ ");\r
+> \r
+> + /* Show body parts */\r
+> show_message_body (message, format, params);\r
+> \r
+> notmuch_message_destroy (message);\r
+> +\r
+> + /* Done the original */\r
+> + printf ("{} ] }");\r
+> +\r
+> + /* End the reply object. */\r
+> + printf (" }, ");\r
+> }\r
+> +\r
+> + /* End array of reply objects */\r
+> + printf ("{} ]\n");\r
+> +\r
+> return 0;\r
+> }\r
+> \r
+> @@ -646,6 +833,7 @@ notmuch_reply_format_headers_only(void *ctx,\r
+> \r
+> enum {\r
+> FORMAT_DEFAULT,\r
+> + FORMAT_JSON,\r
+> FORMAT_HEADERS_ONLY,\r
+> };\r
+> \r
+> @@ -666,6 +854,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])\r
+> notmuch_opt_desc_t options[] = {\r
+> { NOTMUCH_OPT_KEYWORD, &format, "format", 'f',\r
+> (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },\r
+> + { "json", FORMAT_JSON },\r
+> { "headers-only", FORMAT_HEADERS_ONLY },\r
+> { 0, 0 } } },\r
+> { NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',\r
+> @@ -684,6 +873,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])\r
+> \r
+> if (format == FORMAT_HEADERS_ONLY)\r
+> reply_format_func = notmuch_reply_format_headers_only;\r
+> + else if (format == FORMAT_JSON)\r
+> + reply_format_func = notmuch_reply_format_json;\r
+> else\r
+> reply_format_func = notmuch_reply_format_default;\r
+> \r
+> -- \r
+> 1.7.5.4\r
+> \r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r