From: Mark Walters <markwalters1009@gmail.com>
To: Tomi Ollila <tomi.ollila@iki.fi>, Peter Feigl <craven@gmx.net>,
	notmuch@notmuchmail.org
Subject: Re: [PATCH 1/3] Adding an S-expression structured output printer.
In-Reply-To: <m2txs6vwdy.fsf@guru.guru-group.fi>
References: <1354264143-30173-1-git-send-email-craven@gmx.net>
	<871ufa9jpm.fsf@qmul.ac.uk> <m2txs6vwdy.fsf@guru.guru-group.fi>
User-Agent: Notmuch/0.14+81~g9730584 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sat, 01 Dec 2012 12:24:41 +0000
Message-ID: <87d2yu559y.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


On Sat, 01 Dec 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sat, Dec 01 2012, Mark Walters wrote:
>
>> Hi
>>
>> Overall I like the series: I think I agree with all of Jani's
>> comments. 
>>
>> My one extra comment is that I think we should decide on whether we also
>> want a sexp plist version. I think we might want one for the emacs
>> front-end as that currently uses plists for everything.
>>
>> If we do we might want to change the names a little, both for functions
>> and options (eg sexp_a and sexp_p or something). Probably a lot of
>> sprinter-sexp would be common to both versions.
>
> This is an important question that needs to be addressed fast: options
> are:
>
> 1) have options to spit both alist & plist formats
> 2) when converting emacs to use s-expressions, convert it to use alists
> 3) start using plists instead of alists in Peter's android client

Ok I have looked at this and the changes needed to output plist (or
both) are pretty small: the only functions from sprinter-sexp.c that
need to be changed are sexp_end and sexp_map_key. The total diff from
alist to plist is about 10 lines. I have a version which allows both
(the same sprinter file creates both possibilities) and have hooked it
into emacs/notmuch-show.el and it all seems to work. 

(Search is more difficult as that uses the async parser; indeed even for
show I used sexp-at-point as suggested by Tomi which seems rather
underdocumented but does seem to work)

Given the ease with which we can allow both I think that would be my
preference: the biggest problem is that slightly more cluttered option
list (i.e., we have to allow both --format=sexpa and --format=sexpp or
similar).

(I can post the patch doing the above but almost all of it is modifying the
commands to choose alist or plist rather than modifying the
sprinter-sexp itself)

Best wishes

Mark





>
>
> In case (1) is chosen then we just need to support one more format.
>
> How much work would it involve to convert emacs to receive content in
> alists (and how feasible would that be)?
>
> How much work would it require in Peter's client to use plists (and how
> feasible would that be)?
>
>>
>> Best wishes
>>
>> Mark
>
> Tomi
>
>
>>
>>
>> On Fri, 30 Nov 2012, Peter Feigl <craven@gmx.net> wrote:
>>> This commit adds an sprinter for Lisp S-Expressions. Later commits will
>>> use this printer.
>>>
>>> The structure is the same as json, but:
>>> - arrays are written as lists: ("foo" "bar" "baaz" 1 2 3)
>>> - maps are written as a-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 | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 236 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 =		\
>>>  	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..8401c52
>>> --- /dev/null
>>> +++ b/sprinter-sexp.c
>>> @@ -0,0 +1,235 @@
>>> +#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;
>>> +
>>> +    /* True if the state is a map state.
>>> +       Used to add a space between key/value pairs. */
>>> +    notmuch_bool_t in_map;
>>> +
>>> +    /* 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 space. */
>>> +static struct sprinter_sexp *
>>> +sexp_begin_value (struct sprinter *sp)
>>> +{
>>> +    struct sprinter_sexp *sps = (struct sprinter_sexp *) sp;
>>> +
>>> +    if (sps->state) {
>>> +        if (! sps->state->first) {
>>> +            if (sps->insert_separator) {
>>> +                fputc ('\n', sps->stream);
>>> +                sps->insert_separator = FALSE;
>>> +            } else {
>>> +                if( ! sps->state->in_map)
>>> +                    fputc (' ', sps->stream);
>>> +            }
>>> +        } else {
>>> +            sps->state->first = 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, char open, char close)
>>> +{
>>> +    struct sprinter_sexp *sps = sexp_begin_value (sp);
>>> +    struct sexp_state *state = talloc (sps, struct sexp_state);
>>> +    fputc (open, sps->stream);
>>> +    state->parent = sps->state;
>>> +    state->first = TRUE;
>>> +    state->in_map = FALSE;
>>> +    state->close = close;
>>> +    sps->state = state;
>>> +}
>>> +
>>> +static void
>>> +sexp_begin_map (struct sprinter *sp)
>>> +{
>>> +    struct sprinter_sexp *sps = (struct sprinter_sexp *) sp;
>>> +    sexp_begin_aggregate (sp, '(', ')');
>>> +    sps->state->in_map = TRUE;
>>> +}
>>> +
>>> +static void
>>> +sexp_begin_list (struct sprinter *sp)
>>> +{
>>> +    sexp_begin_aggregate (sp, '(', ')');
>>> +}
>>> +
>>> +static void
>>> +sexp_end (struct sprinter *sp)
>>> +{
>>> +    struct sprinter_sexp *sps = (struct sprinter_sexp *) sp;
>>> +    struct sexp_state *state = sps->state;
>>> +
>>> +    if (sps->state->in_map)
>>> +        fputc (')', sps->stream);
>>> +    fputc (sps->state->close, sps->stream);
>>> +    sps->state = state->parent;
>>> +    talloc_free (state);
>>> +    if (sps->state == NULL)
>>> +        fputc ('\n', sps->stream);
>>> +}
>>> +
>>> +/* This implementation supports embedded NULs as allowed by the JSON
>>> + * specification and Unicode.  Support for *parsing* embedded NULs
>>> + * varies, but is generally not a problem outside of C-based parsers
>>> + * (Python's json module and Emacs' json.el take embedded NULs in
>>> + * stride). */
>>> +static void
>>> +sexp_string_len_internal (struct sprinter *sp, const char *val, size_t len, notmuch_bool_t quote)
>>> +{
>>> +    static const char *const escapes[] = {
>>> +        ['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
>>> +        ['\f'] = "\\f",  ['\n'] = "\\n",  ['\t'] = "\\t"
>>> +    };
>>> +    struct sprinter_sexp *sps = sexp_begin_value (sp);
>>> +
>>> +    if(quote)
>>> +        fputc ('"', sps->stream);
>>> +    for (; len; ++val, --len) {
>>> +        unsigned char ch = *val;
>>> +        if (ch < ARRAY_SIZE (escapes) && escapes[ch])
>>> +            fputs (escapes[ch], sps->stream);
>>> +        else if (ch >= 32)
>>> +            fputc (ch, sps->stream);
>>> +        else
>>> +            fprintf (sps->stream, "\\u%04x", ch);
>>> +    }
>>> +    if(quote)
>>> +        fputc ('"', sps->stream);
>>> +}
>>> +
>>> +static void
>>> +sexp_string_len (struct sprinter *sp, const char *val, size_t len)
>>> +{
>>> +    sexp_string_len_internal (sp, val, len, TRUE); /* print quoted */
>>> +}
>>> +
>>> +static void
>>> +sexp_symbol_len (struct sprinter *sp, const char *val, size_t len)
>>> +{
>>> +    sexp_string_len_internal (sp, val, len, FALSE); /* print unquoted */
>>> +}
>>> +
>>> +static void
>>> +sexp_string (struct sprinter *sp, const char *val)
>>> +{
>>> +    if (val == NULL)
>>> +        val = "";
>>> +    sexp_string_len (sp, val, strlen (val));
>>> +}
>>> +
>>> +static void
>>> +sexp_symbol (struct sprinter *sp, const char *val)
>>> +{
>>> +    if (val == NULL)
>>> +        val = "";
>>> +    sexp_symbol_len (sp, val, strlen (val));
>>> +}
>>> +
>>> +static void
>>> +sexp_integer (struct sprinter *sp, int val)
>>> +{
>>> +    struct sprinter_sexp *sps = sexp_begin_value (sp);
>>> +
>>> +    fprintf (sps->stream, "%d", val);
>>> +}
>>> +
>>> +static void
>>> +sexp_boolean (struct sprinter *sp, notmuch_bool_t val)
>>> +{
>>> +    struct sprinter_sexp *sps = sexp_begin_value (sp);
>>> +
>>> +    fputs (val ? "t" : "nil", sps->stream);
>>> +}
>>> +
>>> +static void
>>> +sexp_null (struct sprinter *sp)
>>> +{
>>> +    struct sprinter_sexp *sps = sexp_begin_value (sp);
>>> +
>>> +    fputs ("nil", sps->stream);
>>> +}
>>> +
>>> +static void
>>> +sexp_map_key (struct sprinter *sp, const char *key)
>>> +{
>>> +    struct sprinter_sexp *sps = (struct sprinter_sexp *) sp;
>>> +
>>> +    if( sps->state->in_map && ! sps->state->first)
>>> +        fputs (") ", sps->stream);
>>> +    fputc ('(', sps->stream);
>>> +    sexp_symbol (sp, key);
>>> +    fputc (' ', sps->stream);
>>> +}
>>> +
>>> +static void
>>> +sexp_set_prefix (unused (struct sprinter *sp), unused (const char *name))
>>> +{
>>> +}
>>> +
>>> +static void
>>> +sexp_separator (struct sprinter *sp)
>>> +{
>>> +    struct sprinter_sexp *sps = (struct sprinter_sexp *) sp;
>>> +
>>> +    sps->insert_separator = TRUE;
>>> +}
>>> +
>>> +struct sprinter *
>>> +sprinter_sexp_create (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,
>>> +            .string_len = sexp_string_len,
>>> +            .integer = sexp_integer,
>>> +            .boolean = sexp_boolean,
>>> +            .null = sexp_null,
>>> +            .map_key = sexp_map_key,
>>> +            .separator = sexp_separator,
>>> +            .set_prefix = sexp_set_prefix,
>>> +            .is_text_printer = FALSE,
>>> +        }
>>> +    };
>>> +    struct sprinter_sexp *res;
>>> +
>>> +    res = talloc (ctx, struct sprinter_sexp);
>>> +    if (! res)
>>> +        return NULL;
>>> +
>>> +    *res = template;
>>> +    res->stream = stream;
>>> +    return &res->vtable;
>>> +}
>>> -- 
>>> 1.8.0
>>>
>>> _______________________________________________
>>> notmuch mailing list
>>> notmuch@notmuchmail.org
>>> http://notmuchmail.org/mailman/listinfo/notmuch
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch