From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Adam Wolfe Gordon <awg+notmuch@xvx.ca>, notmuch@notmuchmail.org
Subject: Re: [PATCH v4 2/4] reply: Add a JSON reply format.
In-Reply-To: <1328746916-25447-3-git-send-email-awg+notmuch@xvx.ca>
References: <1326995217-27423-1-git-send-email-awg+notmuch@xvx.ca>
	<1328746916-25447-1-git-send-email-awg+notmuch@xvx.ca>
	<1328746916-25447-3-git-send-email-awg+notmuch@xvx.ca>
User-Agent: Notmuch/0.11.1+167~g6e72434 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Thu, 09 Feb 2012 11:22:32 +0400
Message-ID: <87mx8sa2tj.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

Hi Adam.

On Wed,  8 Feb 2012 17:21:54 -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 using w3m.
> 
> Reply now enforces that only one message is returned, as the semantics
> of replying to multiple messages are not wel-defined.
> 

s/wel/well/

> Show is modified such that --format=json no longer implies
> --entire-thread, as MUAs will use --format=json when constructing
> replies. The man page is updated to reflect this change.

I did not look into details.  But I am surprised that user needs to call
notmuch show --format=json to make reply.  I would expect notmuch reply
to provide all required info (except for bodies).

Anyway, I think you should put this change in a separate patch.

Also, we clearly need a NEWS entry for it and user-customizable Emacs
variable changes.  Though it can be done after this series is pushed, I
guess.

Thank you for this work, it is much appreciated.

Regards,
  Dmitry

> ---
>  emacs/notmuch-query.el  |    2 +-
>  man/man1/notmuch-show.1 |    6 +--
>  notmuch-reply.c         |  167 +++++++++++++++++++++++++++++++++++------------
>  notmuch-show.c          |    1 -
>  4 files changed, 126 insertions(+), 50 deletions(-)
> 
> diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
> index d66baea..cdf2d6b 100644
> --- a/emacs/notmuch-query.el
> +++ b/emacs/notmuch-query.el
> @@ -29,7 +29,7 @@ A thread is a forest or list of trees. A tree is a two element
>  list where the first element is a message, and the second element
>  is a possibly empty forest of replies.
>  "
> -  (let  ((args '("show" "--format=json"))
> +  (let  ((args '("show" "--format=json" "--entire-thread"))
>  	 (json-object-type 'plist)
>  	 (json-array-type 'list)
>  	 (json-false 'nil))
> diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
> index b2301d8..c86b9db 100644
> --- a/man/man1/notmuch-show.1
> +++ b/man/man1/notmuch-show.1
> @@ -55,11 +55,7 @@ be nested.
>  The output is formatted with Javascript Object Notation (JSON). This
>  format is more robust than the text format for automated
>  processing. The nested structure of multipart MIME messages is
> -reflected in nested JSON output. JSON output always includes all
> -messages in a matching thread; in effect
> -.B \-\-format=json
> -implies
> -.B \-\-entire\-thread
> +reflected in nested JSON output.
>  
>  .RE
>  .RS 4
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index f55b1d2..bfbc307 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);
> -
> -	if (from_addr == NULL)
> -	    from_addr = guess_from_received_header (config, message);
> +	reply = create_reply_message (ctx, config, message, reply_all);
>  
> -	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);
>  
> @@ -584,6 +599,68 @@ notmuch_reply_format_default(void *ctx,
>      return 0;
>  }
>  
> +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 char *reply_headers[] = {"from", "to", "subject", "in-reply-to", "references"};
> +    const char *orig_headers[] = {"from", "to", "cc", "subject", "date", "in-reply-to", "references", "message-id"};
> +    unsigned int hidx;
> +
> +    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);
> +
> +    reply = create_reply_message (ctx, config, message, reply_all);
> +    if (!reply)
> +	return 1;
> +
> +    /* Start a reply object */
> +    printf ("{ \"reply\": { ");
> +
> +    for (hidx = 0; hidx < ARRAY_SIZE (reply_headers); hidx++) {
> +	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;
> +
> +    /* Done the headers for the reply, which has no body */
> +    printf (" }");
> +
> +    /* Start the original */
> +    printf (", \"original\": { ");
> +
> +    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 */
> +    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,
> @@ -646,6 +723,7 @@ notmuch_reply_format_headers_only(void *ctx,
>  
>  enum {
>      FORMAT_DEFAULT,
> +    FORMAT_JSON,
>      FORMAT_HEADERS_ONLY,
>  };
>  
> @@ -666,6 +744,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 +763,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 dec799c..344d08c 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -1082,7 +1082,6 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  		format = &format_text;
>  	    } else if (strcmp (opt, "json") == 0) {
>  		format = &format_json;
> -		params.entire_thread = 1;
>  	    } else if (strcmp (opt, "mbox") == 0) {
>  		format = &format_mbox;
>  		mbox = 1;
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch