[PATCH v4 1/3] Add support for structured output formatters.
Mark Walters
markwalters1009 at gmail.com
Thu Jul 12 04:25:09 PDT 2012
On Thu, 12 Jul 2012, craven at gmx.net wrote:
>> what is the advantage of having this as one function rather than end_map
>> and end_list? Indeed, my choice (but I think most other people would
>> disagree) would be to have both functions but still keep state as this
>> currently does and then throw an error if the code closes the wrong
>> thing.
>
> There's probably no advantage, one way or the other is fine, I'd say.
> I've thought about introducing checks into the formatter functions, to
> raise errors for improper closing, map_key not inside a map and things
> like that, I just wasn't sure that would be acceptable.
I will leave others to comment.
>> A second question: do you have an implementation in this style for
>> s-expressions. I find it hard to tell whether the interface is right
>> with just a single example. Even a completely hacky not ready for review
>> example would be helpful.
>
> See the attached patch :)
This looks great. I found it much easier to review by splitting
sprinter.c into two files sprinter-json.c and sprinter-sexp.c and then
running meld on them. The similarity is then very clear. It might be
worth submitting them as two files, but I leave other people to comment.
(Doing so made some of the difference between json and s-exp clear: like
that keys in mapkeys are quoted in json but not in s-exp)
It could be that some of the code could be merged, but I am not sure
that there is much advantage. I would imagine that these two sprinter.c
files would basically never change so there is not much risk of them
diverging.
I wonder if it would be worth using aggregate_t for both rather than
using the closing symbol for this purpose in the json output.
In any case this patch answers my query: the new structure does
generalise very easily to s-expressions!
Best wishes
Mark
>
> Thanks for the suggestions!
>
> Peter
> From cf2c5eeab814970736510ca2210b909643a8cf19 Mon Sep 17 00:00:00 2001
> From: <craven at gmx.net>
> Date: Thu, 12 Jul 2012 10:17:05 +0200
> Subject: [PATCH] Add an S-Expression output format.
>
> ---
> notmuch-search.c | 7 ++-
> sprinter.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> sprinter.h | 4 ++
> 3 files changed, 180 insertions(+), 1 deletion(-)
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index b853f5f..f8bea9b 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -77,6 +77,7 @@ do_search_threads (sprinter_t *format,
>
> if (format != sprinter_text) {
> format->begin_list (format);
> + format->frame (format);
> }
>
> for (i = 0;
> @@ -380,7 +381,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
> int exclude = EXCLUDE_TRUE;
> unsigned int i;
>
> - enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
> + enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }
> format_sel = NOTMUCH_FORMAT_TEXT;
>
> notmuch_opt_desc_t options[] = {
> @@ -391,6 +392,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
> { NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',
> (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
> { "text", NOTMUCH_FORMAT_TEXT },
> + { "sexp", NOTMUCH_FORMAT_SEXP },
> { 0, 0 } } },
> { NOTMUCH_OPT_KEYWORD, &output, "output", 'o',
> (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
> @@ -422,6 +424,9 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
> case NOTMUCH_FORMAT_JSON:
> format = sprinter_json_new (ctx, stdout);
> break;
> + case NOTMUCH_FORMAT_SEXP:
> + format = sprinter_sexp_new (ctx, stdout);
> + break;
> }
>
> config = notmuch_config_open (ctx, NULL, NULL);
> diff --git a/sprinter.c b/sprinter.c
> index 649f79a..fce0f9b 100644
> --- a/sprinter.c
> +++ b/sprinter.c
> @@ -170,3 +170,173 @@ sprinter_json_new(const void *ctx, FILE *stream)
> res->stream = stream;
> return &res->vtable;
> }
> +
> +/*
> + * Every below here is the implementation of the SEXP printer.
> + */
> +
> +typedef enum { MAP, LIST } aggregate_t;
> +
> +struct sprinter_sexp
> +{
> + struct sprinter vtable;
> + FILE *stream;
> + /* Top of the state stack, or NULL if the printer is not currently
> + * inside any aggregate types. */
> + struct sexp_state *state;
> +};
> +
> +struct sexp_state
> +{
> + struct sexp_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. */
> + aggregate_t type;
> +};
> +
> +static struct sprinter_sexp *
> +sexp_begin_value(struct sprinter *sp)
> +{
> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
> + if (spsx->state) {
> + if (!spsx->state->first)
> + fputc (' ', spsx->stream);
> + else
> + spsx->state->first = false;
> + }
> + return spsx;
> +}
> +
> +static void
> +sexp_begin_aggregate(struct sprinter *sp, aggregate_t type)
> +{
> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
> + struct sexp_state *state = talloc (spsx, struct sexp_state);
> +
> + fputc ('(', spsx->stream);
> + state->parent = spsx->state;
> + state->first = true;
> + spsx->state = state;
> + state->type = type;
> +}
> +
> +static void
> +sexp_begin_map(struct sprinter *sp)
> +{
> + sexp_begin_aggregate (sp, MAP);
> +}
> +
> +static void
> +sexp_begin_list(struct sprinter *sp)
> +{
> + sexp_begin_aggregate (sp, LIST);
> +}
> +
> +static void
> +sexp_end(struct sprinter *sp)
> +{
> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
> + struct sexp_state *state = spsx->state;
> +
> + fputc (')', spsx->stream);
> + spsx->state = state->parent;
> + talloc_free (state);
> + if(spsx->state == NULL)
> + fputc ('\n', spsx->stream);
> +}
> +
> +static void
> +sexp_string(struct sprinter *sp, const char *val)
> +{
> + static const char * const escapes[] = {
> + ['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
> + ['\f'] = "\\f", ['\n'] = "\\n", ['\t'] = "\\t"
> + };
> + struct sprinter_sexp *spsx = sexp_begin_value(sp);
> + fputc ('"', spsx->stream);
> + for (; *val; ++val) {
> + unsigned char ch = *val;
> + if (ch < ARRAY_SIZE(escapes) && escapes[ch])
> + fputs (escapes[ch], spsx->stream);
> + else if (ch >= 32)
> + fputc (ch, spsx->stream);
> + else
> + fprintf (spsx->stream, "\\u%04x", ch);
> + }
> + fputc ('"', spsx->stream);
> + if (spsx->state != NULL && spsx->state->type == MAP)
> + fputc (')', spsx->stream);
> + spsx->state->first = false;
> +}
> +
> +static void
> +sexp_integer(struct sprinter *sp, int val)
> +{
> + struct sprinter_sexp *spsx = sexp_begin_value(sp);
> + fprintf (spsx->stream, "%d", val);
> + if (spsx->state != NULL && spsx->state->type == MAP)
> + fputc (')', spsx->stream);
> +}
> +
> +static void
> +sexp_boolean(struct sprinter *sp, notmuch_bool_t val)
> +{
> + struct sprinter_sexp *spsx = sexp_begin_value(sp);
> + fputs (val ? "#t" : "#f", spsx->stream);
> + if (spsx->state != NULL && spsx->state->type == MAP)
> + fputc (')', spsx->stream);
> +}
> +
> +static void
> +sexp_null(struct sprinter *sp)
> +{
> + struct sprinter_sexp *spsx = sexp_begin_value(sp);
> + fputs ("'()", spsx->stream);
> + spsx->state->first = false;
> +}
> +
> +static void
> +sexp_map_key(struct sprinter *sp, const char *key)
> +{
> + struct sprinter_sexp *spsx = sexp_begin_value(sp);
> + fputc ('(', spsx->stream);
> + fputs (key, spsx->stream);
> + fputs (" . ", spsx->stream);
> + spsx->state->first = true;
> +}
> +
> +static void
> +sexp_frame(struct sprinter *sp)
> +{
> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;
> + fputc ('\n', spsx->stream);
> +}
> +
> +struct sprinter *
> +sprinter_sexp_new(const void *ctx, FILE *stream)
> +{
> + static const struct sprinter_sexp template = {
> + .vtable = {
> + .begin_map = sexp_begin_map,
> + .begin_list = sexp_begin_list,
> + .end = sexp_end,
> + .string = sexp_string,
> + .integer = sexp_integer,
> + .boolean = sexp_boolean,
> + .null = sexp_null,
> + .map_key = sexp_map_key,
> + .frame = sexp_frame,
> + }
> + };
> + struct sprinter_sexp *res;
> +
> + res = talloc (ctx, struct sprinter_sexp);
> + if (!res)
> + return NULL;
> +
> + *res = template;
> + res->stream = stream;
> + return &res->vtable;
> +}
> diff --git a/sprinter.h b/sprinter.h
> index 1dad9a0..a89eaa5 100644
> --- a/sprinter.h
> +++ b/sprinter.h
> @@ -40,6 +40,10 @@ typedef struct sprinter
> struct sprinter *
> sprinter_json_new(const void *ctx, FILE *stream);
>
> +/* Create a new structure printer that emits S-Expressions */
> +struct sprinter *
> +sprinter_sexp_new(const void *ctx, FILE *stream);
> +
> /* A dummy structure printer that signifies that standard text output is
> * to be used instead of any structured format.
> */
> --
> 1.7.11.1
More information about the notmuch
mailing list