From: Austin Clements Date: Fri, 17 Feb 2012 17:04:57 +0000 (+1900) Subject: Re: [PATCH v5.2 3/7] reply: Add a JSON reply format. X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=c27faffbb2a44678217cc4bcfe68270b3fd226da;p=notmuch-archives.git Re: [PATCH v5.2 3/7] reply: Add a JSON reply format. --- diff --git a/0f/6a7ce9f2bc00e01f7eec90b27711ab7b2c3471 b/0f/6a7ce9f2bc00e01f7eec90b27711ab7b2c3471 new file mode 100644 index 000000000..eca27eb7e --- /dev/null +++ b/0f/6a7ce9f2bc00e01f7eec90b27711ab7b2c3471 @@ -0,0 +1,308 @@ +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 23EC1429E42 + for ; Fri, 17 Feb 2012 09:06:48 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 R+5ECyW2woE9 for ; + Fri, 17 Feb 2012 09:06:47 -0800 (PST) +Received: from dmz-mailsec-scanner-1.mit.edu (DMZ-MAILSEC-SCANNER-1.MIT.EDU + [18.9.25.12]) + by olra.theworths.org (Postfix) with ESMTP id 290E6431FB6 + for ; Fri, 17 Feb 2012 09:06:47 -0800 (PST) +X-AuditID: 1209190c-b7fad6d000000920-07-4f3e8924764b +Received: from mailhub-auth-3.mit.edu ( [18.9.21.43]) + by dmz-mailsec-scanner-1.mit.edu (Symantec Messaging Gateway) with SMTP + id 08.F0.02336.4298E3F4; Fri, 17 Feb 2012 12:06:44 -0500 (EST) +Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) + by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id q1HH6hJ4005424; + Fri, 17 Feb 2012 12:06:44 -0500 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q1HH6fmh016474 + (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); + Fri, 17 Feb 2012 12:06:43 -0500 (EST) +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) + (envelope-from ) + id 1RyREv-0003Ys-5N; Fri, 17 Feb 2012 12:04:57 -0500 +Date: Fri, 17 Feb 2012 12:04:57 -0500 +From: Austin Clements +To: Adam Wolfe Gordon +Subject: Re: [PATCH v5.2 3/7] reply: Add a JSON reply format. +Message-ID: <20120217170457.GE5991@mit.edu> +References: <1329361957-28493-1-git-send-email-awg+notmuch@xvx.ca> + <1329361957-28493-4-git-send-email-awg+notmuch@xvx.ca> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: <1329361957-28493-4-git-send-email-awg+notmuch@xvx.ca> +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFupnleLIzCtJLcpLzFFi42IR4hTV1lXptPM3eHqV2+LInlnsFtdvzmR2 + YPJ4tuoWs0fTj8WsAUxRXDYpqTmZZalF+nYJXBl7P/UxF7y1r3g79w5bA+Nfwy5GTg4JAROJ + yxN2sUPYYhIX7q1n62Lk4hAS2Mco0dF3gw0kISSwgVHi5R9uiMRJJonTu44xQThLGCWOPpsD + VsUioCqxu3MtmM0moCGxbf9yRhBbREBL4sf6r6wgNrOAtMS3381MILawgK3Es0+XmEFsXgFt + id1PjrBAbKuWOD53KRNEXFDi5MwnLBC9WhI3/r0EinOAzVn+jwMkzCngLPFyQSfYGFEBFYkp + J7exTWAUmoWkexaS7lkI3QsYmVcxyqbkVunmJmbmFKcm6xYnJ+blpRbpGurlZpbopaaUbmIE + h7Ukzw7GNweVDjEKcDAq8fAmGdr6C7EmlhVX5h5ilORgUhLl3dpu5y/El5SfUpmRWJwRX1Sa + k1p8iFGCg1lJhPdbLlCONyWxsiq1KB8mJc3BoiTOq6L1zk9IID2xJDU7NbUgtQgmK8PBoSTB + W9MB1ChYlJqeWpGWmVOCkGbi4AQZzgM0vAqkhre4IDG3ODMdIn+KUZfj44EnFxiFWPLy81Kl + xHmTQYoEQIoySvPg5sDS0StGcaC3hHmzQKp4gKkMbtIroCVMQEt4hcCWlCQipKQaGHlZeX+t + 4Zq03/Kl8AqH2wUH/vqYHpdYeD60XjnDeE/B1Gqr6VNNZni6Jwi2PP8d8OzT27z5dsZGqyoP + fJ/97jnfotrEo5edThkHvOG4t7DqjGqFzpfIjkOCKa88QwOSup4uWR6W4xTnu4lTyjrp4i6W + 6XsVhN5GJH+a4djr+eqbvu9tyZc5r5RYijMSDbWYi4oTAQZdxeQiAwAA +Cc: notmuch@notmuchmail.org +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: Fri, 17 Feb 2012 17:06:48 -0000 + +The first two patches LGTM. A few nits in this one. + +Quoth Adam Wolfe Gordon on Feb 15 at 8:12 pm: +> 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. +> +> Reply now enforces that only one message is returned, as the semantics +> of replying to multiple messages are not well-defined. +> --- +> notmuch-client.h | 3 + +> notmuch-reply.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++------- +> notmuch-show.c | 2 +- +> 3 files changed, 109 insertions(+), 17 deletions(-) +> +> diff --git a/notmuch-client.h b/notmuch-client.h +> index 60828aa..d28ea07 100644 +> --- a/notmuch-client.h +> +++ b/notmuch-client.h +> @@ -344,6 +344,9 @@ typedef struct mime_node { +> int next_part_num; +> } mime_node_t; +> +> +void +> +format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first); +> + + +This is the wrong place for this declaration, since it is not part of +the MIME node abstraction. It should go somewhere above the /* +notmuch-config.c */ comment. Above that, it's a bit of a jumble. I'd +probably put it right after notmuch_show_command. + +> /* Construct a new MIME node pointing to the root message part of +> * message. If cryptoctx is non-NULL, it will be used to verify +> * signatures on any child parts. If decrypt is true, then cryptoctx +> diff --git a/notmuch-reply.c b/notmuch-reply.c +> index 8e56245..6670288 100644 +> --- a/notmuch-reply.c +> +++ b/notmuch-reply.c +> @@ -572,30 +572,115 @@ 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; +> + } + +Technically count_messages does not have to be accurate, but since +this is the same thing notmuch-show does, it's probably fine for now. + +Perhaps we should add proper handling of multi-message replies to +devel/TODO? + +> +> - 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")); +> +> - notmuch_message_destroy (message); +> + show_message_body (message, format, params); +> + +> + notmuch_message_destroy (message); +> + +> + return 0; +> +} + +The above change could be separated out in to a separate patch, since +it has nothing to do with JSON. + +> + +> +static void +> +format_reply_headers_json (const void *ctx, GMimeMessage *message) +> +{ +> + void *local = talloc_new (ctx); +> + InternetAddressList *recipients; +> + const char *recipients_string; +> + +> + printf ("{%s: %s", +> + json_quote_str (local, "Subject"), +> + json_quote_str (local, g_mime_message_get_subject (message))); +> + printf (", %s: %s", +> + json_quote_str (local, "From"), +> + json_quote_str (local, g_mime_message_get_sender (message))); +> + recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO); +> + recipients_string = internet_address_list_to_string (recipients, 0); +> + if (recipients_string) +> + printf (", %s: %s", +> + json_quote_str (local, "To"), +> + json_quote_str (local, recipients_string)); +> + recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC); +> + recipients_string = internet_address_list_to_string (recipients, 0); +> + if (recipients_string) +> + printf (", %s: %s", +> + json_quote_str (local, "Cc"), +> + json_quote_str (local, recipients_string)); +> + +> + 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", + +There should be a close brace in this string, rather than opening the +object in this function and closing it in the caller. + +> + json_quote_str (local, "References"), +> + json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "References"))); +> + +> + talloc_free (local); +> +} + +I would much rather see format_headers_json in notmuch-show.c gain a +parameter that tells it whether or not to include in-reply-to and +references. + +> + +> +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_reply_headers_json (ctx, reply); +> + printf ("}"); +> + 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; +> } +> +> @@ -661,6 +746,7 @@ notmuch_reply_format_headers_only(void *ctx, +> +> enum { +> FORMAT_DEFAULT, +> + FORMAT_JSON, +> FORMAT_HEADERS_ONLY, +> }; +> +> @@ -680,6 +766,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', +> @@ -698,6 +785,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..c570a16 100644 +> --- a/notmuch-show.c +> +++ b/notmuch-show.c +> @@ -652,7 +652,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