[PATCH 1/3] Adding an S-expression structured output printer.
Mark Walters
markwalters1009 at gmail.com
Sat Dec 1 06:26:46 PST 2012
On Sat, 01 Dec 2012, Mark Walters <markwalters1009 at gmail.com> wrote:
> On Sat, 01 Dec 2012, Tomi Ollila <tomi.ollila at iki.fi> wrote:
>> On Sat, Dec 01 2012, Mark Walters <markwalters1009 at gmail.com> wrote:
>>
>>> On Sat, 01 Dec 2012, Tomi Ollila <tomi.ollila at 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)
>>
>> As the diff is so small I agree that supporting 2 formats is good
>> option.
>>
>> In case this is done I suggest that we proceed the following way:
>>
>> 1) Agree how we call these formats (sexpa & sexpp or something else)
>> 2) Peter does his updates, including to call the format as will be decided
>> 3) Mark posts his patches after Peter's work is pushed
>
> I think this seems a good approach. I should note that while the diff
> between alist and plist is only about 10 lines I do need to duplicate
> some of the surrounding code to allow both: its not huge but definitely
> more than 10 lines.
>
> I will clean it up a little and then post (as a diff on top of Peter's
> patches) just so people can judge if it looks basically acceptable.
Here is my current diff: note this is just the sprinter diff: it does
not contain the plumbing into the cli functions. The largest part is the
duplicated create function: I couldn't persuade the compiler to let me
unify this (as the static const thing needs to be constant)
The patch is -U6 as this makes it easier to see what is going on.
Best wishes
Mark
---
sprinter-sexp.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++-----
sprinter.h | 6 +++-
2 files changed, 73 insertions(+), 8 deletions(-)
diff --git a/sprinter-sexp.c b/sprinter-sexp.c
index 8401c52..51bab68 100644
--- a/sprinter-sexp.c
+++ b/sprinter-sexp.c
@@ -1,11 +1,16 @@
#include <stdbool.h>
#include <stdio.h>
#include <talloc.h>
#include "sprinter.h"
+typedef enum {
+ SEXP_ALIST,
+ SEXP_PLIST
+} sexp_list_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;
@@ -81,26 +86,39 @@ static void
sexp_begin_list (struct sprinter *sp)
{
sexp_begin_aggregate (sp, '(', ')');
}
static void
-sexp_end (struct sprinter *sp)
+sexp_end (struct sprinter *sp, sexp_list_t list_type)
{
struct sprinter_sexp *sps = (struct sprinter_sexp *) sp;
struct sexp_state *state = sps->state;
- if (sps->state->in_map)
- fputc (')', sps->stream);
+ if (list_type == SEXP_ALIST)
+ 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);
}
+static void
+sexp_end_alist (struct sprinter *sp)
+{
+ sexp_end(sp, SEXP_ALIST);
+}
+
+static void
+sexp_end_plist (struct sprinter *sp)
+{
+ sexp_end(sp, SEXP_PLIST);
+}
+
/* 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
@@ -177,24 +195,36 @@ 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)
+sexp_map_key_alist (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_map_key_plist (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)
@@ -202,25 +232,56 @@ 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)
+sprinter_sexp_alist_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_alist,
+ .string = sexp_string,
+ .string_len = sexp_string_len,
+ .integer = sexp_integer,
+ .boolean = sexp_boolean,
+ .null = sexp_null,
+ .map_key = sexp_map_key_alist,
+ .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;
+}
+
+
+struct sprinter *
+sprinter_sexp_plist_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,
+ .end = sexp_end_plist,
.string = sexp_string,
.string_len = sexp_string_len,
.integer = sexp_integer,
.boolean = sexp_boolean,
.null = sexp_null,
- .map_key = sexp_map_key,
+ .map_key = sexp_map_key_plist,
.separator = sexp_separator,
.set_prefix = sexp_set_prefix,
.is_text_printer = FALSE,
}
};
struct sprinter_sexp *res;
diff --git a/sprinter.h b/sprinter.h
index 59776a9..9a496cd 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -69,9 +69,13 @@ sprinter_text_create (const void *ctx, FILE *stream);
/* Create a new structure printer that emits JSON. */
struct sprinter *
sprinter_json_create (const void *ctx, FILE *stream);
/* Create a new structure printer that emits S-Expressions. */
struct sprinter *
-sprinter_sexp_create (const void *ctx, FILE *stream);
+sprinter_sexp_alist_create (const void *ctx, FILE *stream);
+
+/* Create a new structure printer that emits S-Expressions. */
+struct sprinter *
+sprinter_sexp_plist_create (const void *ctx, FILE *stream);
#endif // NOTMUCH_SPRINTER_H
--
1.7.9.1
More information about the notmuch
mailing list