From: Austin Clements <aclements@csail.mit.edu>
To: Peter Feigl <craven@gmx.net>, notmuch@notmuchmail.org
Subject: Re: [PATCH v4 1/5] Adding an S-expression structured output printer.
In-Reply-To:
 <1409adf6cbd310d7313bdc3a69e7e78c25859d81.1354794428.git.craven@gmx.net>
References: <cover.1354794428.git.craven@gmx.net>
	<1409adf6cbd310d7313bdc3a69e7e78c25859d81.1354794428.git.craven@gmx.net>
User-Agent: Notmuch/0.14+159~g6895fee (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Thu, 06 Dec 2012 10:44:05 -0500
Message-ID: <87hanzp4my.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: list

On Thu, 06 Dec 2012, Peter Feigl <craven@gmx.net> wrote:
> This commit adds a structured output printer for Lisp
> S-Expressions. Later commits will use this printer in notmuch search,
> show and reply.
>
> The structure is the same as json, but:
> - arrays are written as lists: ("foo" "bar" "baaz" 1 2 3)
> - maps are written as p-lists: (:key "value" :other-key "other-value")
> - true is written as t
> - false is written as nil
> - null is written as nil
> ---
>  Makefile.local  |   1 +
>  sprinter-sexp.c | 238 ++++++++++++++++++++++++++++++++++++++++++++++++++=
++++++
>  sprinter.h      |   4 +
>  3 files changed, 243 insertions(+)
>  create mode 100644 sprinter-sexp.c
>
> diff --git a/Makefile.local b/Makefile.local
> index 2b91946..0db1713 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -270,6 +270,7 @@ notmuch_client_srcs =3D		\
>  	notmuch-tag.c		\
>  	notmuch-time.c		\
>  	sprinter-json.c		\
> +	sprinter-sexp.c		\
>  	sprinter-text.c		\
>  	query-string.c		\
>  	mime-node.c		\
> diff --git a/sprinter-sexp.c b/sprinter-sexp.c
> new file mode 100644
> index 0000000..8f84eed
> --- /dev/null
> +++ b/sprinter-sexp.c
> @@ -0,0 +1,238 @@
> +/* notmuch - Not much of an email program, (just index and search)
> + *
> + * Copyright =C2=A9 2012 Peter Feigl
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Author: Peter Feigl <peter.feigl@gmx.at>
> + */
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <talloc.h>
> +#include "sprinter.h"
> +
> +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;
> +
> +    /* A flag to signify that a separator should be inserted in the
> +     * output as soon as possible. */
> +    notmuch_bool_t insert_separator;
> +};
> +
> +struct sexp_state {
> +    struct sexp_state *parent;
> +
> +    /* True if nothing has been printed in this aggregate yet.
> +     * Suppresses the space before a value. */
> +    notmuch_bool_t first;
> +};
> +
> +/* Helper function to set up the stream to print a value.  If this
> + * value follows another value, prints a space. */
> +static struct sprinter_sexp *
> +sexp_begin_value (struct sprinter *sp)
> +{
> +    struct sprinter_sexp *sps =3D (struct sprinter_sexp *) sp;
> +
> +    if (sps->state) {
> +	if (! sps->state->first) {
> +	    if (sps->insert_separator) {
> +		fputc ('\n', sps->stream);
> +		sps->insert_separator =3D FALSE;
> +	    } else {
> +		fputc (' ', sps->stream);
> +	    }
> +	} else {
> +	    sps->state->first =3D FALSE;
> +	}
> +    }
> +    return sps;
> +}
> +
> +/* Helper function to begin an aggregate type.  Prints the open
> + * character and pushes a new state frame. */
> +static void
> +sexp_begin_aggregate (struct sprinter *sp)
> +{
> +    struct sprinter_sexp *sps =3D sexp_begin_value (sp);
> +    struct sexp_state *state =3D talloc (sps, struct sexp_state);
> +    fputc ('(', sps->stream);
> +    state->parent =3D sps->state;
> +    state->first =3D TRUE;
> +    sps->state =3D state;
> +}
> +
> +static void
> +sexp_begin_map (struct sprinter *sp)
> +{
> +    sexp_begin_aggregate (sp);
> +}
> +
> +static void
> +sexp_begin_list (struct sprinter *sp)
> +{
> +    sexp_begin_aggregate (sp);
> +}
> +
> +static void
> +sexp_end (struct sprinter *sp)
> +{
> +    struct sprinter_sexp *sps =3D (struct sprinter_sexp *) sp;
> +    struct sexp_state *state =3D sps->state;
> +
> +    fputc (')', sps->stream);
> +    sps->state =3D state->parent;
> +    talloc_free (state);
> +    if (sps->state =3D=3D NULL)
> +	fputc ('\n', sps->stream);
> +}
> +
> +static void
> +sexp_string_len (struct sprinter *sp, const char *val, size_t len)
> +{
> +    /* Some characters need escaping. " and \ work fine in all Lisps,
> +     * \n is not supported in CL, but all others work fine.
> +     * Characters below 32 are printed as \123o (three-digit=20
> +     * octals), which work fine in most Schemes and Emacs. */
> +    static const char *const escapes[] =3D {
> +	['\"'] =3D "\\\"", ['\\'] =3D "\\\\",  ['\n'] =3D "\\n"
> +    };
> +    struct sprinter_sexp *sps =3D sexp_begin_value (sp);
> +
> +    fputc ('"', sps->stream);
> +    for (; len; ++val, --len) {
> +	unsigned char ch =3D *val;
> +	if (ch < ARRAY_SIZE (escapes) && escapes[ch])
> +	    fputs (escapes[ch], sps->stream);
> +	else if (ch >=3D 32)
> +	    fputc (ch, sps->stream);
> +	else
> +	    fprintf (sps->stream, "\\%03oo", ch);

Should be "\\%03o" (one less o).

> +    }
> +    fputc ('"', sps->stream);
> +}
> +
> +static void
> +sexp_string (struct sprinter *sp, const char *val)
> +{
> +    if (val =3D=3D NULL)
> +	val =3D "";
> +    sexp_string_len (sp, val, strlen (val));
> +}
> +
> +/* Prints a symbol, i.e. the name preceded by a colon. This should work
> + * in all Lisps, at least as a symbol, if not as a proper keyword */
> +static void
> +sexp_symbol (struct sprinter *sp, const char *val)
> +{
> +    static const char illegal_characters[] =3D {
> +	' ', '\t', '\n'

There are a large number of illegal symbol characters.  I would go the
conservative route here (since we get to choose our symbols) and only
allow
  isalnum(x) || (x =3D=3D '-') || (x =3D=3D '_')
(I would prefer not to allow _ but it's already in the schema.)

> +    };
> +    unsigned int i =3D 0;
> +    struct sprinter_sexp *sps =3D (struct sprinter_sexp *) sp;
> +
> +    if (val =3D=3D NULL)
> +	INTERNAL_ERROR ("illegal symbol NULL");
> +
> +    for(i =3D 0; i < ARRAY_SIZE (illegal_characters); i++) {
> +	if(strchr(val, illegal_characters[i]) !=3D NULL) {
> +	    INTERNAL_ERROR ("illegal character in symbol %s", val);
> +	}
> +    }
> +    fputc (':', sps->stream);
> +    fputs (val, sps->stream);
> +}
> +
> +static void
> +sexp_integer (struct sprinter *sp, int val)
> +{
> +    struct sprinter_sexp *sps =3D sexp_begin_value (sp);
> +
> +    fprintf (sps->stream, "%d", val);
> +}
> +
> +static void
> +sexp_boolean (struct sprinter *sp, notmuch_bool_t val)
> +{
> +    struct sprinter_sexp *sps =3D sexp_begin_value (sp);
> +
> +    fputs (val ? "t" : "nil", sps->stream);
> +}
> +
> +static void
> +sexp_null (struct sprinter *sp)
> +{
> +    struct sprinter_sexp *sps =3D sexp_begin_value (sp);
> +
> +    fputs ("nil", sps->stream);
> +}
> +
> +static void
> +sexp_map_key (struct sprinter *sp, const char *key)
> +{
> +    struct sprinter_sexp *sps =3D (struct sprinter_sexp *) sp;
> +    if (sps->state && ! sps->state->first)
> +	fputc (' ', sps->stream);
> +
> +    sps->state->first =3D FALSE;
> +    sexp_symbol (sp, key);
> +}

How about
    struct sprinter_sexp *sps =3D sexp_begin_value (sp);

    sexp_symbol (sp, key);
?

> +
> +static void
> +sexp_set_prefix (unused (struct sprinter *sp), unused (const char *name))
> +{
> +}
> +
> +static void
> +sexp_separator (struct sprinter *sp)
> +{
> +    struct sprinter_sexp *sps =3D (struct sprinter_sexp *) sp;
> +
> +    sps->insert_separator =3D TRUE;
> +}
> +
> +struct sprinter *
> +sprinter_sexp_create (const void *ctx, FILE *stream)
> +{
> +    static const struct sprinter_sexp template =3D {
> +	.vtable =3D {
> +	    .begin_map =3D sexp_begin_map,
> +	    .begin_list =3D sexp_begin_list,
> +	    .end =3D sexp_end,
> +	    .string =3D sexp_string,
> +	    .string_len =3D sexp_string_len,
> +	    .integer =3D sexp_integer,
> +	    .boolean =3D sexp_boolean,
> +	    .null =3D sexp_null,
> +	    .map_key =3D sexp_map_key,
> +	    .separator =3D sexp_separator,
> +	    .set_prefix =3D sexp_set_prefix,
> +	    .is_text_printer =3D FALSE,
> +	}
> +    };
> +    struct sprinter_sexp *res;
> +
> +    res =3D talloc (ctx, struct sprinter_sexp);
> +    if (! res)
> +	return NULL;
> +
> +    *res =3D template;
> +    res->stream =3D stream;
> +    return &res->vtable;
> +}
> diff --git a/sprinter.h b/sprinter.h
> index 912a526..59776a9 100644
> --- a/sprinter.h
> +++ b/sprinter.h
> @@ -70,4 +70,8 @@ sprinter_text_create (const void *ctx, FILE *stream);
>  struct sprinter *
>  sprinter_json_create (const void *ctx, FILE *stream);
>=20=20
> +/* Create a new structure printer that emits S-Expressions. */
> +struct sprinter *
> +sprinter_sexp_create (const void *ctx, FILE *stream);
> +
>  #endif // NOTMUCH_SPRINTER_H
> --=20
> 1.8.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch