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