From: Austin Clements <aclements@csail.mit.edu>
To: Peter Feigl <craven@gmx.net>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/5] Rename the -json printer functions in
	notmuch-reply and notmuch-show to generic -sprinter functions.
In-Reply-To: <1354632382-15609-3-git-send-email-craven@gmx.net>
References: <1354632382-15609-1-git-send-email-craven@gmx.net>
	<1354632382-15609-3-git-send-email-craven@gmx.net>
User-Agent: Notmuch/0.14+100~gcb9b0b0 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Tue, 04 Dec 2012 14:18:00 -0500
Message-ID: <87fw3ly6c7.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Tue, 04 Dec 2012, Peter Feigl <craven@gmx.net> wrote:
> All the structured output functions in notmuch-reply and notmuch-show
> are renamed to a generic name (as they do not contain any json-specific
> code anyway). This patch is a preparation to actually using the new
> S-Expression sprinter in notmuch-reply and notmuch-show.
> ---
>  notmuch-client.h |  8 ++++----
>  notmuch-reply.c  | 38 +++++++++++++++++++++-----------------
>  notmuch-show.c   | 30 +++++++++++++++---------------
>  3 files changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index ae9344b..1c336dc 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -175,12 +175,12 @@ notmuch_status_t
>  show_one_part (const char *filename, int part);
>  
>  void
> -format_part_json (const void *ctx, struct sprinter *sp, mime_node_t *node,
> -		  notmuch_bool_t first, notmuch_bool_t output_body);
> +format_part_sprinter (const void *ctx, struct sprinter *sp, mime_node_t *node,
> +		      notmuch_bool_t first, notmuch_bool_t output_body);
>  
>  void
> -format_headers_json (struct sprinter *sp, GMimeMessage *message,
> -		     notmuch_bool_t reply);
> +format_headers_sprinter (struct sprinter *sp, GMimeMessage *message,
> +			 notmuch_bool_t reply);
>  
>  typedef enum {
>      NOTMUCH_SHOW_TEXT_PART_REPLY = 1 << 0,
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index e60a264..53aefa7 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -548,7 +548,8 @@ notmuch_reply_format_default(void *ctx,
>  			     notmuch_config_t *config,
>  			     notmuch_query_t *query,
>  			     notmuch_show_params_t *params,
> -			     notmuch_bool_t reply_all)
> +			     notmuch_bool_t reply_all,
> +			     unused (sprinter_t *sp))
>  {
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
> @@ -587,17 +588,17 @@ notmuch_reply_format_default(void *ctx,
>  }
>  
>  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)
> +notmuch_reply_format_sprinter(void *ctx,
> +			      notmuch_config_t *config,
> +			      notmuch_query_t *query,
> +			      notmuch_show_params_t *params,
> +			      notmuch_bool_t reply_all,
> +			      sprinter_t *sp)
>  {
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
>      mime_node_t *node;
> -    sprinter_t *sp;
>  
>      if (notmuch_query_count_messages (query) != 1) {
>  	fprintf (stderr, "Error: search term did not match precisely one message.\n");
> @@ -613,18 +614,17 @@ notmuch_reply_format_json(void *ctx,
>      if (!reply)
>  	return 1;
>  
> -    sp = sprinter_json_create (ctx, stdout);
>      sp->begin_map (sp);
>  
>      /* The headers of the reply message we've created */
>      sp->map_key (sp, "reply-headers");
> -    format_headers_json (sp, reply, TRUE);
> +    format_headers_sprinter (sp, reply, TRUE);
>      g_object_unref (G_OBJECT (reply));
>      reply = NULL;
>  
>      /* Start the original */
>      sp->map_key (sp, "original");
> -    format_part_json (ctx, sp, node, TRUE, TRUE);
> +    format_part_sprinter (ctx, sp, node, TRUE, TRUE);
>  
>      /* End */
>      sp->end (sp);
> @@ -639,7 +639,8 @@ notmuch_reply_format_headers_only(void *ctx,
>  				  notmuch_config_t *config,
>  				  notmuch_query_t *query,
>  				  unused (notmuch_show_params_t *params),
> -				  notmuch_bool_t reply_all)
> +				  notmuch_bool_t reply_all,
> +				  unused (sprinter_t *sp))
>  {
>      GMimeMessage *reply;
>      notmuch_messages_t *messages;
> @@ -707,7 +708,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>      notmuch_query_t *query;
>      char *query_string;
>      int opt_index, ret = 0;
> -    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all);
> +    int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all, struct sprinter *sp);

Yikes.  This could use some line wrapping.

This patch LGTM other than this.

>      notmuch_show_params_t params = {
>  	.part = -1,
>  	.crypto = {
> @@ -717,6 +718,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>      };
>      int format = FORMAT_DEFAULT;
>      int reply_all = TRUE;
> +    struct sprinter *sp = NULL;
>  
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
> @@ -738,12 +740,14 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>  	return 1;
>      }
>  
> -    if (format == FORMAT_HEADERS_ONLY)
> +    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
> +    } else if (format == FORMAT_JSON) {
> +	reply_format_func = notmuch_reply_format_sprinter;
> +	sp = sprinter_json_create (ctx, stdout);
> +    } else {
>  	reply_format_func = notmuch_reply_format_default;
> +    }
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
> @@ -770,7 +774,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>  	return 1;
>      }
>  
> -    if (reply_format_func (ctx, config, query, &params, reply_all) != 0)
> +    if (reply_format_func (ctx, config, query, &params, reply_all, sp) != 0)
>  	return 1;
>  
>      notmuch_crypto_cleanup (&params.crypto);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 2fa2292..38c621f 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -32,12 +32,12 @@ static const notmuch_show_format_t format_text = {
>  };
>  
>  static notmuch_status_t
> -format_part_json_entry (const void *ctx, sprinter_t *sp, mime_node_t *node,
> +format_part_sprinter_entry (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  			int indent, const notmuch_show_params_t *params);
>  
>  static const notmuch_show_format_t format_json = {
>      .new_sprinter = sprinter_json_create,
> -    .part = format_part_json_entry,
> +    .part = format_part_sprinter_entry,
>  };
>  
>  static notmuch_status_t
> @@ -108,7 +108,7 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
>  /* Emit a sequence of key/value pairs for the metadata of message.
>   * The caller should begin a map before calling this. */
>  static void
> -format_message_json (sprinter_t *sp, notmuch_message_t *message)
> +format_message_sprinter (sprinter_t *sp, notmuch_message_t *message)
>  {
>      /* Any changes to the JSON format should be reflected in the file
>       * devel/schemata. */
> @@ -208,7 +208,7 @@ _is_from_line (const char *line)
>  }
>  
>  void
> -format_headers_json (sprinter_t *sp, GMimeMessage *message,
> +format_headers_sprinter (sprinter_t *sp, GMimeMessage *message,
>  		     notmuch_bool_t reply)
>  {
>      /* Any changes to the JSON format should be reflected in the file
> @@ -363,7 +363,7 @@ signer_status_to_string (GMimeSignerStatus x)
>  
>  #ifdef GMIME_ATLEAST_26
>  static void
> -format_part_sigstatus_json (sprinter_t *sp, mime_node_t *node)
> +format_part_sigstatus_sprinter (sprinter_t *sp, mime_node_t *node)
>  {
>      /* Any changes to the JSON format should be reflected in the file
>       * devel/schemata. */
> @@ -438,7 +438,7 @@ format_part_sigstatus_json (sprinter_t *sp, mime_node_t *node)
>  }
>  #else
>  static void
> -format_part_sigstatus_json (sprinter_t *sp, mime_node_t *node)
> +format_part_sigstatus_sprinter (sprinter_t *sp, mime_node_t *node)
>  {
>      const GMimeSignatureValidity* validity = node->sig_validity;
>  
> @@ -595,7 +595,7 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  }
>  
>  void
> -format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
> +format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  		  notmuch_bool_t first, notmuch_bool_t output_body)
>  {
>      /* Any changes to the JSON format should be reflected in the file
> @@ -603,15 +603,15 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  
>      if (node->envelope_file) {
>  	sp->begin_map (sp);
> -	format_message_json (sp, node->envelope_file);
> +	format_message_sprinter (sp, node->envelope_file);
>  
>  	sp->map_key (sp, "headers");
> -	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
> +	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), FALSE);
>  
>  	if (output_body) {
>  	    sp->map_key (sp, "body");
>  	    sp->begin_list (sp);
> -	    format_part_json (ctx, sp, mime_node_child (node, 0), first, TRUE);
> +	    format_part_sprinter (ctx, sp, mime_node_child (node, 0), first, TRUE);
>  	    sp->end (sp);
>  	}
>  	sp->end (sp);
> @@ -646,7 +646,7 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  
>      if (node->verify_attempted) {
>  	sp->map_key (sp, "sigstatus");
> -	format_part_sigstatus_json (sp, node);
> +	format_part_sigstatus_sprinter (sp, node);
>      }
>  
>      sp->map_key (sp, "content-type");
> @@ -698,7 +698,7 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  	sp->begin_map (sp);
>  
>  	sp->map_key (sp, "headers");
> -	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
> +	format_headers_sprinter (sp, GMIME_MESSAGE (node->part), FALSE);
>  
>  	sp->map_key (sp, "body");
>  	sp->begin_list (sp);
> @@ -706,7 +706,7 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>      }
>  
>      for (i = 0; i < node->nchildren; i++)
> -	format_part_json (ctx, sp, mime_node_child (node, i), i == 0, TRUE);
> +	format_part_sprinter (ctx, sp, mime_node_child (node, i), i == 0, TRUE);
>  
>      /* Close content structures */
>      for (i = 0; i < nclose; i++)
> @@ -716,11 +716,11 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>  }
>  
>  static notmuch_status_t
> -format_part_json_entry (const void *ctx, sprinter_t *sp,
> +format_part_sprinter_entry (const void *ctx, sprinter_t *sp,
>  			mime_node_t *node, unused (int indent),
>  			const notmuch_show_params_t *params)
>  {
> -    format_part_json (ctx, sp, node, TRUE, params->output_body);
> +    format_part_sprinter (ctx, sp, node, TRUE, params->output_body);
>  
>      return NOTMUCH_STATUS_SUCCESS;
>  }
> -- 
> 1.8.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch