From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 3DAF6431FAF for ; Fri, 30 Nov 2012 11:06:21 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pVuXvy4b7LGJ for ; Fri, 30 Nov 2012 11:06:17 -0800 (PST) Received: from mail-la0-f53.google.com (mail-la0-f53.google.com [209.85.215.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 92602431FAE for ; Fri, 30 Nov 2012 11:06:16 -0800 (PST) Received: by mail-la0-f53.google.com with SMTP id w12so671033lag.26 for ; Fri, 30 Nov 2012 11:06:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:subject:in-reply-to:references:user-agent:date:message-id :mime-version:content-type:x-gm-message-state; bh=t5FzzORnmfTlOnmQEQK+dP9922JCCwH7QY7cA6bQ2Wo=; b=XadchziVujplO5ykyu2ic38dHxYzqXorEAsMmFZ3k5CBxP00MwxgpO5+Uj7RIsJhYy 8D2HHR0TfAcOEJpMUJmULPCs6ZBc2JMvqqBPsQ0Ng/9kUwKhfSNUV3OAIDE5Sowiba3Y FzbnE1QB5DHGP1kKrFHgAgznDgR3ud4ny/KUJr7l1R7Dpe6gs5TSucKKiPrPzgEgVlvi 6gG5MUKOvDc62aSznrJmHTWxEo+W6Z5cEZDKqMUFrwuFvcPYcck/V2YAfQj9C1CjhEGv UdTtm1wjS+KY90d2D+9A1gcCgZkErSYVNGk9jc0WmJ2daWEfM0NM8BsYp5Znjv8FQudd FW6Q== Received: by 10.152.147.100 with SMTP id tj4mr2195859lab.42.1354302375001; Fri, 30 Nov 2012 11:06:15 -0800 (PST) Received: from localhost (dsl-hkibrasgw4-fe51df00-27.dhcp.inet.fi. [80.223.81.27]) by mx.google.com with ESMTPS id fb1sm2398289lbb.15.2012.11.30.11.06.12 (version=SSLv3 cipher=OTHER); Fri, 30 Nov 2012 11:06:13 -0800 (PST) From: Jani Nikula To: Peter Feigl , notmuch@notmuchmail.org Subject: Re: [PATCH 3/3] Use the S-Expression structured printer in notmuch show and notmuch reply. In-Reply-To: <1354264143-30173-3-git-send-email-craven@gmx.net> References: <1354264143-30173-1-git-send-email-craven@gmx.net> <1354264143-30173-3-git-send-email-craven@gmx.net> User-Agent: Notmuch/0.14+124~g3b17402 (http://notmuchmail.org) Emacs/23.4.1 (i686-pc-linux-gnu) Date: Fri, 30 Nov 2012 21:06:11 +0200 Message-ID: <8738zqkj18.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Gm-Message-State: ALoCoQmfBfFT4UdLmO4s7CNUCm9tKAt59OMienZ5fvN4URTQlXzxayTVvNLdQ/gTKa5ZwXCOspwr X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Nov 2012 19:06:21 -0000 On Fri, 30 Nov 2012, Peter Feigl 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