Date: Thu, 12 Jul 2012 18:07:04 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: craven@gmx.net
Subject: Re: [PATCH v4 2/3] Add structured output formatter for JSON.
Message-ID: <20120712220703.GI7332@mit.edu>
References: <87d34hsdx8.fsf@awakening.csail.mit.edu>
	<1342079004-5300-1-git-send-email-craven@gmx.net>
	<1342079004-5300-3-git-send-email-craven@gmx.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1342079004-5300-3-git-send-email-craven@gmx.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth craven@gmx.net on Jul 12 at  9:43 am:
> Using the new structured printer support in sprinter.h, implement
> sprinter_json_new, which returns a new JSON structured output
> formatter.
> 
> The formatter prints output similar to the existing JSON, but with
> differences in whitespace (mostly newlines).
> ---
>  Makefile.local |   1 +
>  sprinter.c     | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 173 insertions(+)
>  create mode 100644 sprinter.c
> 
> diff --git a/Makefile.local b/Makefile.local
> index a890df2..8baf0c2 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -290,6 +290,7 @@ notmuch_client_srcs =		\
>  	notmuch-show.c		\
>  	notmuch-tag.c		\
>  	notmuch-time.c		\
> +	sprinter.c		\
>  	query-string.c		\
>  	mime-node.c		\
>  	crypto.c		\
> diff --git a/sprinter.c b/sprinter.c
> new file mode 100644
> index 0000000..649f79a
> --- /dev/null
> +++ b/sprinter.c
> @@ -0,0 +1,172 @@
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <talloc.h>
> +#include "sprinter.h"
> +
> +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
> +
> +struct sprinter *
> +sprinter_text = NULL;
> +
> +/*
> + * Every below here is the implementation of the JSON printer.

s/Every/Everything/

Or this can be shortened to just

/*
 * JSON printer
 */

If we wind up with separate files for the JSON and S-exp printers, I
don't think this comment is necessary at all.

> + */
> +
> +struct sprinter_json
> +{
> +    struct sprinter vtable;
> +    FILE *stream;
> +    /* Top of the state stack, or NULL if the printer is not currently
> +     * inside any aggregate types. */
> +    struct json_state *state;
> +};
> +
> +struct json_state
> +{
> +    struct json_state *parent;
> +    /* True if nothing has been printed in this aggregate yet.
> +     * Suppresses the comma before a value. */
> +    notmuch_bool_t first;
> +    /* The character that closes the current aggregate. */
> +    char close;
> +};
> +
> +/* Helper function to set up the stream to print a value.  If this
> + * value follows another value, prints a comma. */
> +static struct sprinter_json *
> +json_begin_value(struct sprinter *sp)

As Tomi pointed out, there should be spaces before the parameter
lists (sorry).

> +{
> +    struct sprinter_json *spj = (struct sprinter_json*)sp;
> +    if (spj->state) {
> +	if (!spj->state->first)
> +	    fputs (", ", spj->stream);
> +	else
> +	    spj->state->first = false;
> +    }
> +    return spj;
> +}
> +
> +/* Helper function to begin an aggregate type.  Prints the open
> + * character and pushes a new state frame. */
> +static void
> +json_begin_aggregate(struct sprinter *sp, char open, char close)
> +{
> +    struct sprinter_json *spj = json_begin_value (sp);
> +    struct json_state *state = talloc (spj, struct json_state);
> +
> +    fputc (open, spj->stream);
> +    state->parent = spj->state;
> +    state->first = true;
> +    state->close = close;
> +    spj->state = state;
> +}
> +
> +static void
> +json_begin_map(struct sprinter *sp)
> +{
> +    json_begin_aggregate (sp, '{', '}');
> +}
> +
> +static void
> +json_begin_list(struct sprinter *sp)
> +{
> +    json_begin_aggregate (sp, '[', ']');
> +}
> +
> +static void
> +json_end(struct sprinter *sp)
> +{
> +    struct sprinter_json *spj = (struct sprinter_json*)sp;
> +    struct json_state *state = spj->state;
> +
> +    fputc (spj->state->close, spj->stream);
> +    spj->state = state->parent;
> +    talloc_free (state);
> +    if(spj->state == NULL)

Ah, another missing space.

> +	fputc ('\n', spj->stream);
> +}
> +
> +static void
> +json_string(struct sprinter *sp, const char *val)
> +{
> +    static const char * const escapes[] = {
> +	['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
> +	['\f'] = "\\f",  ['\n'] = "\\n",  ['\t'] = "\\t"
> +    };
> +    struct sprinter_json *spj = json_begin_value (sp);
> +    fputc ('"', spj->stream);
> +    for (; *val; ++val) {
> +	unsigned char ch = *val;
> +	if (ch < ARRAY_SIZE(escapes) && escapes[ch])
> +	    fputs (escapes[ch], spj->stream);
> +	else if (ch >= 32)
> +	    fputc (ch, spj->stream);
> +	else
> +	    fprintf (spj->stream, "\\u%04x", ch);
> +    }
> +    fputc ('"', spj->stream);
> +}
> +
> +static void
> +json_integer(struct sprinter *sp, int val)
> +{
> +    struct sprinter_json *spj = json_begin_value (sp);
> +    fprintf (spj->stream, "%d", val);
> +}
> +
> +static void
> +json_boolean(struct sprinter *sp, notmuch_bool_t val)
> +{
> +    struct sprinter_json *spj = json_begin_value (sp);
> +    fputs (val ? "true" : "false", spj->stream);
> +}
> +
> +static void
> +json_null(struct sprinter *sp)
> +{
> +    struct sprinter_json *spj = json_begin_value (sp);
> +    fputs ("null", spj->stream);
> +}
> +
> +static void
> +json_map_key(struct sprinter *sp, const char *key)
> +{
> +    struct sprinter_json *spj = (struct sprinter_json*)sp;
> +    json_string (sp, key);
> +    fputs (": ", spj->stream);
> +    spj->state->first = true;
> +}
> +
> +static void
> +json_frame(struct sprinter *sp)
> +{
> +    struct sprinter_json *spj = (struct sprinter_json*)sp;
> +    fputc ('\n', spj->stream);
> +}
> +
> +struct sprinter *
> +sprinter_json_new(const void *ctx, FILE *stream)
> +{
> +    static const struct sprinter_json template = {
> +	.vtable = {
> +	    .begin_map = json_begin_map,
> +	    .begin_list = json_begin_list,
> +	    .end = json_end,
> +	    .string = json_string,
> +	    .integer = json_integer,
> +	    .boolean = json_boolean,
> +	    .null = json_null,
> +	    .map_key = json_map_key,
> +	    .frame = json_frame,
> +	}
> +    };
> +    struct sprinter_json *res;
> +
> +    res = talloc (ctx, struct sprinter_json);
> +    if (!res)
> +	return NULL;
> +
> +    *res = template;
> +    res->stream = stream;
> +    return &res->vtable;
> +}