--- /dev/null
+Return-Path: <tomi.ollila@iki.fi>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 4B4B1429E47\r
+ for <notmuch@notmuchmail.org>; Thu, 12 Jul 2012 05:05:05 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: 0\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
+ autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id W87MHVdWr7vF for <notmuch@notmuchmail.org>;\r
+ Thu, 12 Jul 2012 05:05:03 -0700 (PDT)\r
+Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
+ by olra.theworths.org (Postfix) with ESMTP id CC832429E25\r
+ for <notmuch@notmuchmail.org>; Thu, 12 Jul 2012 05:05:02 -0700 (PDT)\r
+Received: by guru.guru-group.fi (Postfix, from userid 501)\r
+ id 376E3100386; Thu, 12 Jul 2012 15:05:14 +0300 (EEST)\r
+From: Tomi Ollila <tomi.ollila@iki.fi>\r
+To: Mark Walters <markwalters1009@gmail.com>, craven@gmx.net,\r
+ notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v4 1/3] Add support for structured output formatters.\r
+In-Reply-To: <87liipte5m.fsf@qmul.ac.uk>\r
+References: <87d34hsdx8.fsf@awakening.csail.mit.edu>\r
+ <1342079004-5300-1-git-send-email-craven@gmx.net>\r
+ <1342079004-5300-2-git-send-email-craven@gmx.net>\r
+ <87pq81tjv4.fsf@qmul.ac.uk> <87a9z5teaj.fsf@nexoid.at>\r
+ <87liipte5m.fsf@qmul.ac.uk>\r
+User-Agent: Notmuch/0.13.2+74~g65b26b0 (http://notmuchmail.org) Emacs/23.1.1\r
+ (x86_64-redhat-linux-gnu)\r
+X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
+ $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
+ !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
+Date: Thu, 12 Jul 2012 15:05:14 +0300\r
+Message-ID: <m2pq81tcat.fsf@guru.guru-group.fi>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Thu, 12 Jul 2012 12:05:05 -0000\r
+\r
+On Thu, Jul 12 2012, Mark Walters <markwalters1009@gmail.com> wrote:\r
+\r
+> On Thu, 12 Jul 2012, craven@gmx.net wrote:\r
+>>> what is the advantage of having this as one function rather than end_map\r
+>>> and end_list? Indeed, my choice (but I think most other people would\r
+>>> disagree) would be to have both functions but still keep state as this\r
+>>> currently does and then throw an error if the code closes the wrong\r
+>>> thing.\r
+>>\r
+>> There's probably no advantage, one way or the other is fine, I'd say.\r
+>> I've thought about introducing checks into the formatter functions, to\r
+>> raise errors for improper closing, map_key not inside a map and things\r
+>> like that, I just wasn't sure that would be acceptable.\r
+>\r
+> I will leave others to comment.\r
+\r
+I like the current implementation -- record what has been opened and\r
+have a common closing function which knows what it is closing. Less\r
+checking in the code (i.e. less possible branches).\r
+This makes the use of the interface a bit less self-documenting as\r
+'end' is used instead of list/hash end (defining macros for this \r
+documentation purposes would be really dumb thing to do ;)\r
+\r
+What needs to be checked is that software doesn't attempt to 'end'\r
+too many contexts (i quess it's doing it already). Any other output\r
+errors (like forgetting to 'end' some blocks should be taken care\r
+by proper test cases).\r
+\r
+>>> A second question: do you have an implementation in this style for\r
+>>> s-expressions. I find it hard to tell whether the interface is right\r
+>>> with just a single example. Even a completely hacky not ready for review\r
+>>> example would be helpful.\r
+>>\r
+>> See the attached patch :)\r
+>\r
+> This looks great. I found it much easier to review by splitting\r
+> sprinter.c into two files sprinter-json.c and sprinter-sexp.c and then\r
+> running meld on them. The similarity is then very clear. It might be\r
+> worth submitting them as two files, but I leave other people to comment.\r
+\r
+I like that -- is there (currently) need for splinter.c for common code ?\r
+(splinter.h is definitely needed).\r
+\r
+> (Doing so made some of the difference between json and s-exp clear: like\r
+> that keys in mapkeys are quoted in json but not in s-exp)\r
+>\r
+> It could be that some of the code could be merged, but I am not sure\r
+> that there is much advantage. I would imagine that these two sprinter.c\r
+> files would basically never change so there is not much risk of them\r
+> diverging.\r
+\r
+I was thinking the same when looking splinter code yesterday -- how to\r
+have even more common code for json&sexp. Maybe there could be something\r
+to do just for these 2 purposes but It requires more effort and might\r
+add more complexity for humans to perceive. ATM I'd go with this interface\r
+and see later if anyone wants to do/experiment more -- as you said the\r
+risk of diverging is minimal -- and in case there are separate source\r
+files for json & sexp diffing those will be easy.\r
+\r
+\r
+> I wonder if it would be worth using aggregate_t for both rather than\r
+> using the closing symbol for this purpose in the json output.\r
+>\r
+> In any case this patch answers my query: the new structure does\r
+> generalise very easily to s-expressions!\r
+>\r
+> Best wishes\r
+>\r
+> Mark\r
+\r
+Tomi\r
+\r
+\r
+>\r
+>\r
+>\r
+>>\r
+>> Thanks for the suggestions!\r
+>>\r
+>> Peter\r
+>> From cf2c5eeab814970736510ca2210b909643a8cf19 Mon Sep 17 00:00:00 2001\r
+>> From: <craven@gmx.net>\r
+>> Date: Thu, 12 Jul 2012 10:17:05 +0200\r
+>> Subject: [PATCH] Add an S-Expression output format.\r
+>>\r
+>> ---\r
+>> notmuch-search.c | 7 ++-\r
+>> sprinter.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\r
+>> sprinter.h | 4 ++\r
+>> 3 files changed, 180 insertions(+), 1 deletion(-)\r
+>>\r
+>> diff --git a/notmuch-search.c b/notmuch-search.c\r
+>> index b853f5f..f8bea9b 100644\r
+>> --- a/notmuch-search.c\r
+>> +++ b/notmuch-search.c\r
+>> @@ -77,6 +77,7 @@ do_search_threads (sprinter_t *format,\r
+>> \r
+>> if (format != sprinter_text) {\r
+>> format->begin_list (format);\r
+>> + format->frame (format);\r
+>> }\r
+>> \r
+>> for (i = 0;\r
+>> @@ -380,7 +381,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
+>> int exclude = EXCLUDE_TRUE;\r
+>> unsigned int i;\r
+>> \r
+>> - enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }\r
+>> + enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }\r
+>> format_sel = NOTMUCH_FORMAT_TEXT;\r
+>> \r
+>> notmuch_opt_desc_t options[] = {\r
+>> @@ -391,6 +392,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
+>> { NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',\r
+>> (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },\r
+>> { "text", NOTMUCH_FORMAT_TEXT },\r
+>> + { "sexp", NOTMUCH_FORMAT_SEXP },\r
+>> { 0, 0 } } },\r
+>> { NOTMUCH_OPT_KEYWORD, &output, "output", 'o',\r
+>> (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },\r
+>> @@ -422,6 +424,9 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
+>> case NOTMUCH_FORMAT_JSON:\r
+>> format = sprinter_json_new (ctx, stdout);\r
+>> break;\r
+>> + case NOTMUCH_FORMAT_SEXP:\r
+>> + format = sprinter_sexp_new (ctx, stdout);\r
+>> + break;\r
+>> }\r
+>> \r
+>> config = notmuch_config_open (ctx, NULL, NULL);\r
+>> diff --git a/sprinter.c b/sprinter.c\r
+>> index 649f79a..fce0f9b 100644\r
+>> --- a/sprinter.c\r
+>> +++ b/sprinter.c\r
+>> @@ -170,3 +170,173 @@ sprinter_json_new(const void *ctx, FILE *stream)\r
+>> res->stream = stream;\r
+>> return &res->vtable;\r
+>> }\r
+>> +\r
+>> +/*\r
+>> + * Every below here is the implementation of the SEXP printer.\r
+>> + */\r
+>> +\r
+>> +typedef enum { MAP, LIST } aggregate_t;\r
+>> +\r
+>> +struct sprinter_sexp\r
+>> +{\r
+>> + struct sprinter vtable;\r
+>> + FILE *stream;\r
+>> + /* Top of the state stack, or NULL if the printer is not currently\r
+>> + * inside any aggregate types. */\r
+>> + struct sexp_state *state;\r
+>> +};\r
+>> +\r
+>> +struct sexp_state\r
+>> +{\r
+>> + struct sexp_state *parent;\r
+>> + /* True if nothing has been printed in this aggregate yet.\r
+>> + * Suppresses the comma before a value. */\r
+>> + notmuch_bool_t first;\r
+>> + /* The character that closes the current aggregate. */\r
+>> + aggregate_t type;\r
+>> +};\r
+>> +\r
+>> +static struct sprinter_sexp *\r
+>> +sexp_begin_value(struct sprinter *sp)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
+>> + if (spsx->state) {\r
+>> + if (!spsx->state->first)\r
+>> + fputc (' ', spsx->stream);\r
+>> + else\r
+>> + spsx->state->first = false;\r
+>> + }\r
+>> + return spsx;\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_begin_aggregate(struct sprinter *sp, aggregate_t type)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
+>> + struct sexp_state *state = talloc (spsx, struct sexp_state);\r
+>> +\r
+>> + fputc ('(', spsx->stream);\r
+>> + state->parent = spsx->state;\r
+>> + state->first = true;\r
+>> + spsx->state = state;\r
+>> + state->type = type;\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_begin_map(struct sprinter *sp)\r
+>> +{\r
+>> + sexp_begin_aggregate (sp, MAP);\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_begin_list(struct sprinter *sp)\r
+>> +{\r
+>> + sexp_begin_aggregate (sp, LIST);\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_end(struct sprinter *sp)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
+>> + struct sexp_state *state = spsx->state;\r
+>> +\r
+>> + fputc (')', spsx->stream);\r
+>> + spsx->state = state->parent;\r
+>> + talloc_free (state);\r
+>> + if(spsx->state == NULL)\r
+>> + fputc ('\n', spsx->stream);\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_string(struct sprinter *sp, const char *val)\r
+>> +{\r
+>> + static const char * const escapes[] = {\r
+>> + ['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",\r
+>> + ['\f'] = "\\f", ['\n'] = "\\n", ['\t'] = "\\t"\r
+>> + };\r
+>> + struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
+>> + fputc ('"', spsx->stream);\r
+>> + for (; *val; ++val) {\r
+>> + unsigned char ch = *val;\r
+>> + if (ch < ARRAY_SIZE(escapes) && escapes[ch])\r
+>> + fputs (escapes[ch], spsx->stream);\r
+>> + else if (ch >= 32)\r
+>> + fputc (ch, spsx->stream);\r
+>> + else\r
+>> + fprintf (spsx->stream, "\\u%04x", ch);\r
+>> + }\r
+>> + fputc ('"', spsx->stream);\r
+>> + if (spsx->state != NULL && spsx->state->type == MAP)\r
+>> + fputc (')', spsx->stream);\r
+>> + spsx->state->first = false;\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_integer(struct sprinter *sp, int val)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
+>> + fprintf (spsx->stream, "%d", val);\r
+>> + if (spsx->state != NULL && spsx->state->type == MAP)\r
+>> + fputc (')', spsx->stream);\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_boolean(struct sprinter *sp, notmuch_bool_t val)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
+>> + fputs (val ? "#t" : "#f", spsx->stream);\r
+>> + if (spsx->state != NULL && spsx->state->type == MAP)\r
+>> + fputc (')', spsx->stream);\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_null(struct sprinter *sp)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
+>> + fputs ("'()", spsx->stream);\r
+>> + spsx->state->first = false;\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_map_key(struct sprinter *sp, const char *key)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
+>> + fputc ('(', spsx->stream);\r
+>> + fputs (key, spsx->stream);\r
+>> + fputs (" . ", spsx->stream);\r
+>> + spsx->state->first = true;\r
+>> +}\r
+>> +\r
+>> +static void\r
+>> +sexp_frame(struct sprinter *sp)\r
+>> +{\r
+>> + struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
+>> + fputc ('\n', spsx->stream);\r
+>> +}\r
+>> +\r
+>> +struct sprinter *\r
+>> +sprinter_sexp_new(const void *ctx, FILE *stream)\r
+>> +{\r
+>> + static const struct sprinter_sexp template = {\r
+>> + .vtable = {\r
+>> + .begin_map = sexp_begin_map,\r
+>> + .begin_list = sexp_begin_list,\r
+>> + .end = sexp_end,\r
+>> + .string = sexp_string,\r
+>> + .integer = sexp_integer,\r
+>> + .boolean = sexp_boolean,\r
+>> + .null = sexp_null,\r
+>> + .map_key = sexp_map_key,\r
+>> + .frame = sexp_frame,\r
+>> + }\r
+>> + };\r
+>> + struct sprinter_sexp *res;\r
+>> +\r
+>> + res = talloc (ctx, struct sprinter_sexp);\r
+>> + if (!res)\r
+>> + return NULL;\r
+>> +\r
+>> + *res = template;\r
+>> + res->stream = stream;\r
+>> + return &res->vtable;\r
+>> +}\r
+>> diff --git a/sprinter.h b/sprinter.h\r
+>> index 1dad9a0..a89eaa5 100644\r
+>> --- a/sprinter.h\r
+>> +++ b/sprinter.h\r
+>> @@ -40,6 +40,10 @@ typedef struct sprinter\r
+>> struct sprinter *\r
+>> sprinter_json_new(const void *ctx, FILE *stream);\r
+>> \r
+>> +/* Create a new structure printer that emits S-Expressions */\r
+>> +struct sprinter *\r
+>> +sprinter_sexp_new(const void *ctx, FILE *stream);\r
+>> +\r
+>> /* A dummy structure printer that signifies that standard text output is\r
+>> * to be used instead of any structured format.\r
+>> */\r
+>> -- \r
+>> 1.7.11.1\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r
projects / notmuch-archives.git / commitdiff