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