[PATCH 2/4] reply: Add a JSON reply format.

2012-01-14 Thread Jani Nikula
On Sun,  8 Jan 2012 00:52:40 -0700, Adam Wolfe Gordon  
wrote:
> From: Adam Wolfe Gordon 
> 
> 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 Adam, this is a drive-by-review on some things I spotted, but can't
say I would've thought the whole thing through. I'm pretty ignorant
about MIME parts etc. Please find comments inline.

The reply-to-sender set was just merged. You'll have conflicts both here
and in emacs code, but they should be trivial to sort out.


BR,
Jani.


> ---
>  notmuch-reply.c |  269 
> +++
>  1 files changed, 230 insertions(+), 39 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index f8d5f64..82df396 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);
> +

I know there are existing forward declarations like this, but would it
be much trouble to arrange the code so that they were not needed at all?

>  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)
>  {
> @@ -147,6 +172,78 @@ reply_part_content (GMimeObject *part)
>  }
>  }
>  
> +static void
> +reply_part_start_json (GMimeObject *part, unused(int *part_count))
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +GMimeContentDisposition *disposition = 
> g_mime_object_get_content_disposition (part);
> +
> +if (g_mime_content_type_is_type (content_type, "text", "*") &&
> + (!disposition ||
> +  strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))
> +{
> + printf("{ ");
> +}
> +}
> +
> +static void
> +reply_part_end_json (GMimeObject *part)
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +GMimeContentDisposition *disposition = 
> g_mime_object_get_content_disposition (part);
> +
> +if (g_mime_content_type_is_type (content_type, "text", "*") &&
> + (!disposition ||
> +  strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))
> + printf ("}, ");
> +}

The two functions above, while small, are almost identical. Please move
the common stuff to a common helper, and you can have something like
this:

if (the_common_function (part))
printf ("}, ");

> +
> +static void
> +reply_part_content_json (GMimeObject *part)
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +GMimeContentDisposition *disposition = 
> g_mime_object_get_content_disposition (part);
> +
> +void *ctx = talloc_new (NULL);
> +
> +/* We only care about inline text parts for reply purposes */
> +if (g_mime_content_type_is_type (content_type, "text", "*") &&
> + (!disposition ||
> +  strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))

Oh, you can use the common helper here too.

> +{
> + GMimeStream *stream_memory = NULL, *stream_filter = NULL;

No need to initialize stream_memory here.

> + GMimeDataWrapper *wrapper;
> + GByteArray *part_content;
> + const char *charset;
> +
> + printf("\"content-type\": %s, \"content\": ",
> +json_quote_str(ctx, 
> g_mime_content_type_to_string(content_type)));

The style in notmuch is to have a space before the opening brace in
function calls. Check elsewhere also. I always forget that too. :)

> +
> + charset = g_mime_object_get_content_type_parameter (part, "charset");

AFAICT charset declaration and the above call could be moved inside "if
(stream_memory)" below.

> + stream_memory = g_mime_stream_mem_new ();
> + if (stream_memory) {
> + stream_filter = g_mime_stream_filter_new(stream_memory);
> + if (charset) {
> + g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
> +   

Re: [PATCH 2/4] reply: Add a JSON reply format.

2012-01-14 Thread Jani Nikula
On Sun,  8 Jan 2012 00:52:40 -0700, Adam Wolfe Gordon awg+notm...@xvx.ca 
wrote:
 From: Adam Wolfe Gordon a...@xvx.ca
 
 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 Adam, this is a drive-by-review on some things I spotted, but can't
say I would've thought the whole thing through. I'm pretty ignorant
about MIME parts etc. Please find comments inline.

The reply-to-sender set was just merged. You'll have conflicts both here
and in emacs code, but they should be trivial to sort out.


BR,
Jani.


 ---
  notmuch-reply.c |  269 
 +++
  1 files changed, 230 insertions(+), 39 deletions(-)
 
 diff --git a/notmuch-reply.c b/notmuch-reply.c
 index f8d5f64..82df396 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);
 +

I know there are existing forward declarations like this, but would it
be much trouble to arrange the code so that they were not needed at all?

  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)
  {
 @@ -147,6 +172,78 @@ reply_part_content (GMimeObject *part)
  }
  }
  
 +static void
 +reply_part_start_json (GMimeObject *part, unused(int *part_count))
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +GMimeContentDisposition *disposition = 
 g_mime_object_get_content_disposition (part);
 +
 +if (g_mime_content_type_is_type (content_type, text, *) 
 + (!disposition ||
 +  strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))
 +{
 + printf({ );
 +}
 +}
 +
 +static void
 +reply_part_end_json (GMimeObject *part)
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +GMimeContentDisposition *disposition = 
 g_mime_object_get_content_disposition (part);
 +
 +if (g_mime_content_type_is_type (content_type, text, *) 
 + (!disposition ||
 +  strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))
 + printf (}, );
 +}

The two functions above, while small, are almost identical. Please move
the common stuff to a common helper, and you can have something like
this:

if (the_common_function (part))
printf (}, );

 +
 +static void
 +reply_part_content_json (GMimeObject *part)
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +GMimeContentDisposition *disposition = 
 g_mime_object_get_content_disposition (part);
 +
 +void *ctx = talloc_new (NULL);
 +
 +/* We only care about inline text parts for reply purposes */
 +if (g_mime_content_type_is_type (content_type, text, *) 
 + (!disposition ||
 +  strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))

Oh, you can use the common helper here too.

 +{
 + GMimeStream *stream_memory = NULL, *stream_filter = NULL;

No need to initialize stream_memory here.

 + GMimeDataWrapper *wrapper;
 + GByteArray *part_content;
 + const char *charset;
 +
 + printf(\content-type\: %s, \content\: ,
 +json_quote_str(ctx, 
 g_mime_content_type_to_string(content_type)));

The style in notmuch is to have a space before the opening brace in
function calls. Check elsewhere also. I always forget that too. :)

 +
 + charset = g_mime_object_get_content_type_parameter (part, charset);

AFAICT charset declaration and the above call could be moved inside if
(stream_memory) below.

 + stream_memory = g_mime_stream_mem_new ();
 + if (stream_memory) {
 + 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));
 + }
 + }
 + wrapper = g_mime_part_get_content_object (GMIME_PART (part));
 + if (wrapper  stream_filter)
 

[PATCH 2/4] reply: Add a JSON reply format.

2012-01-08 Thread Adam Wolfe Gordon
From: Adam Wolfe Gordon 

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.
---
 notmuch-reply.c |  269 +++
 1 files changed, 230 insertions(+), 39 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index f8d5f64..82df396 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)
 {
@@ -147,6 +172,78 @@ reply_part_content (GMimeObject *part)
 }
 }

+static void
+reply_part_start_json (GMimeObject *part, unused(int *part_count))
+{
+GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
+GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
+
+if (g_mime_content_type_is_type (content_type, "text", "*") &&
+   (!disposition ||
+strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))
+{
+   printf("{ ");
+}
+}
+
+static void
+reply_part_end_json (GMimeObject *part)
+{
+GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
+GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
+
+if (g_mime_content_type_is_type (content_type, "text", "*") &&
+   (!disposition ||
+strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))
+   printf ("}, ");
+}
+
+static void
+reply_part_content_json (GMimeObject *part)
+{
+GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
+GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
+
+void *ctx = talloc_new (NULL);
+
+/* We only care about inline text parts for reply purposes */
+if (g_mime_content_type_is_type (content_type, "text", "*") &&
+   (!disposition ||
+strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))
+{
+   GMimeStream *stream_memory = NULL, *stream_filter = NULL;
+   GMimeDataWrapper *wrapper;
+   GByteArray *part_content;
+   const char *charset;
+
+   printf("\"content-type\": %s, \"content\": ",
+  json_quote_str(ctx, 
g_mime_content_type_to_string(content_type)));
+
+   charset = g_mime_object_get_content_type_parameter (part, "charset");
+   stream_memory = g_mime_stream_mem_new ();
+   if (stream_memory) {
+   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"));
+   }
+   }
+   wrapper = g_mime_part_get_content_object (GMIME_PART (part));
+   if (wrapper && stream_filter)
+   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));
+
+   if (stream_filter)
+   g_object_unref(stream_filter);
+   if (stream_memory)
+   g_object_unref(stream_memory);
+}
+
+talloc_free (ctx);
+}
+
 /* Is the given address configured as one of the user's "personal" or
  * "other" addresses. */
 static int
@@ -476,6 +573,59 @@ 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)
+{
+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 

[PATCH 2/4] reply: Add a JSON reply format.

2012-01-07 Thread Adam Wolfe Gordon
From: Adam Wolfe Gordon a...@xvx.ca

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.
---
 notmuch-reply.c |  269 +++
 1 files changed, 230 insertions(+), 39 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index f8d5f64..82df396 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)
 {
@@ -147,6 +172,78 @@ reply_part_content (GMimeObject *part)
 }
 }
 
+static void
+reply_part_start_json (GMimeObject *part, unused(int *part_count))
+{
+GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
+GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
+
+if (g_mime_content_type_is_type (content_type, text, *) 
+   (!disposition ||
+strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))
+{
+   printf({ );
+}
+}
+
+static void
+reply_part_end_json (GMimeObject *part)
+{
+GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
+GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
+
+if (g_mime_content_type_is_type (content_type, text, *) 
+   (!disposition ||
+strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))
+   printf (}, );
+}
+
+static void
+reply_part_content_json (GMimeObject *part)
+{
+GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
+GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
+
+void *ctx = talloc_new (NULL);
+
+/* We only care about inline text parts for reply purposes */
+if (g_mime_content_type_is_type (content_type, text, *) 
+   (!disposition ||
+strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))
+{
+   GMimeStream *stream_memory = NULL, *stream_filter = NULL;
+   GMimeDataWrapper *wrapper;
+   GByteArray *part_content;
+   const char *charset;
+
+   printf(\content-type\: %s, \content\: ,
+  json_quote_str(ctx, 
g_mime_content_type_to_string(content_type)));
+
+   charset = g_mime_object_get_content_type_parameter (part, charset);
+   stream_memory = g_mime_stream_mem_new ();
+   if (stream_memory) {
+   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));
+   }
+   }
+   wrapper = g_mime_part_get_content_object (GMIME_PART (part));
+   if (wrapper  stream_filter)
+   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));
+
+   if (stream_filter)
+   g_object_unref(stream_filter);
+   if (stream_memory)
+   g_object_unref(stream_memory);
+}
+
+talloc_free (ctx);
+}
+
 /* Is the given address configured as one of the user's personal or
  * other addresses. */
 static int
@@ -476,6 +573,59 @@ 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)
+{
+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 =