On Fri, 30 Nov 2012, Peter Feigl <craven@gmx.net> wrote:
> This commit changes the json printer (which doesn't do anything
> json-specific anyway but just uses the structured printers) to generic
> sprinters, and adds support for the printer defined in sprinter-sexp.c.
I think this patch should be split in two or three: First a
non-functional preparatory patch that does all the renaming and moves
sprinter creation up and passes it as parameter. Second the functional
patch(es) that add sexp support to reply and show.
Please find some other comments below.
BR,
Jani.
> ---
> notmuch-client.h | 8 ++++----
> notmuch-reply.c | 40 ++++++++++++++++++++++++----------------
> notmuch-show.c | 48 +++++++++++++++++++++++++++++-------------------
> 3 files changed, 57 insertions(+), 39 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..a7469a2 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;
> @@ -696,6 +697,7 @@ notmuch_reply_format_headers_only(void *ctx,
> enum {
> FORMAT_DEFAULT,
> FORMAT_JSON,
> + FORMAT_SEXP,
> FORMAT_HEADERS_ONLY,
> };
>
> @@ -707,7 +709,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);
> notmuch_show_params_t params = {
> .part = -1,
> .crypto = {
> @@ -717,11 +719,13 @@ 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',
> (notmuch_keyword_t []){ { "default", FORMAT_DEFAULT },
> { "json", FORMAT_JSON },
> + { "sexp", FORMAT_SEXP },
> { "headers-only", FORMAT_HEADERS_ONLY },
> { 0, 0 } } },
> { NOTMUCH_OPT_KEYWORD, &reply_all, "reply-to", 'r',
> @@ -740,9 +744,13 @@ 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
> + else if (format == FORMAT_JSON) {
> + reply_format_func = notmuch_reply_format_sprinter;
> + sp = sprinter_json_create(ctx, stdout);
Space before (.
> + } else if (format == FORMAT_SEXP) {
> + reply_format_func = notmuch_reply_format_sprinter;
> + sp = sprinter_sexp_create(ctx, stdout);
Ditto.
> + } else
> reply_format_func = notmuch_reply_format_default;
Please add braces to all branches if you add them to some.
>
> config = notmuch_config_open (ctx, NULL, NULL);
> @@ -770,7 +778,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> return 1;
> }
>
> - if (reply_format_func (ctx, config, query, ¶ms, reply_all) != 0)
> + if (reply_format_func (ctx, config, query, ¶ms, reply_all, sp) != 0)
> return 1;
>
> notmuch_crypto_cleanup (¶ms.crypto);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 2fa2292..e6da2ff 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -32,12 +32,17 @@ 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 const notmuch_show_format_t format_sexp = {
> + .new_sprinter = sprinter_sexp_create,
> + .part = format_part_sprinter_entry,
> };
>
> static notmuch_status_t
> @@ -108,9 +113,9 @@ _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
> + /* Any changes to the JSON or S-Expression format should be reflected in the file
> * devel/schemata. */
You should probably add sexp format description to devel/schemata.
>
> void *local = talloc_new (NULL);
> @@ -208,7 +213,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 +368,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 +443,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 +600,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 +608,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 +651,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 +703,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 +711,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 +721,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;
> }
> @@ -1012,6 +1017,7 @@ do_show (void *ctx,
> enum {
> NOTMUCH_FORMAT_NOT_SPECIFIED,
> NOTMUCH_FORMAT_JSON,
> + NOTMUCH_FORMAT_SEXP,
> NOTMUCH_FORMAT_TEXT,
> NOTMUCH_FORMAT_MBOX,
> NOTMUCH_FORMAT_RAW
> @@ -1056,6 +1062,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> { NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
> (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
> { "text", NOTMUCH_FORMAT_TEXT },
> + { "sexp", NOTMUCH_FORMAT_SEXP },
> { "mbox", NOTMUCH_FORMAT_MBOX },
> { "raw", NOTMUCH_FORMAT_RAW },
> { 0, 0 } } },
> @@ -1100,6 +1107,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> case NOTMUCH_FORMAT_TEXT:
> format = &format_text;
> break;
> + case NOTMUCH_FORMAT_SEXP:
> + format = &format_sexp;
> + break;
> case NOTMUCH_FORMAT_MBOX:
> if (params.part > 0) {
> fprintf (stderr, "Error: specifying parts is incompatible with mbox output format.\n");
> @@ -1120,7 +1130,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>
> /* Default is entire-thread = FALSE except for format=json. */
> if (entire_thread == ENTIRE_THREAD_DEFAULT) {
> - if (format == &format_json)
> + if (format == &format_json || format == &format_sexp)
> entire_thread = ENTIRE_THREAD_TRUE;
> else
> entire_thread = ENTIRE_THREAD_FALSE;
> @@ -1131,8 +1141,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
> fprintf (stderr, "Warning: --body=false is incompatible with --part > 0. Disabling.\n");
> params.output_body = TRUE;
> } else {
> - if (format != &format_json)
> - fprintf (stderr, "Warning: --body=false only implemented for format=json\n");
> + if (format != &format_json && format != &format_sexp)
> + fprintf (stderr, "Warning: --body=false only implemented for format=json or format=sexp\n");
s/or/and/ ?
> }
> }
>
> --
> 1.8.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch