50/61a4cf0f676bc3b8d930f0de08359c2367547c

   1 Return-Path: <m.walters@qmul.ac.uk>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 16126429E34\r
   6         for <notmuch@notmuchmail.org>; Thu, 12 Jul 2012 04:25:17 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -1.098\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
  12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
  13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id 8cEXHoa72d83 for <notmuch@notmuchmail.org>;\r
  17         Thu, 12 Jul 2012 04:25:15 -0700 (PDT)\r
  18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id 45246431FAE\r
  22         for <notmuch@notmuchmail.org>; Thu, 12 Jul 2012 04:25:15 -0700 (PDT)\r
  23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
  24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
  25         (envelope-from <m.walters@qmul.ac.uk>)\r
  26         id 1SpHWC-0003YE-Ou; Thu, 12 Jul 2012 12:25:13 +0100\r
  27 Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223]\r
  28         helo=localhost)\r
  29         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
  30         (envelope-from <m.walters@qmul.ac.uk>)\r
  31         id 1SpHWC-0004vz-3a; Thu, 12 Jul 2012 12:25:12 +0100\r
  32 From: Mark Walters <markwalters1009@gmail.com>\r
  33 To: craven@gmx.net, notmuch@notmuchmail.org\r
  34 Subject: Re: [PATCH v4 1/3] Add support for structured output formatters.\r
  35 In-Reply-To: <87a9z5teaj.fsf@nexoid.at>\r
  36 References: <87d34hsdx8.fsf@awakening.csail.mit.edu>\r
  37         <1342079004-5300-1-git-send-email-craven@gmx.net>\r
  38         <1342079004-5300-2-git-send-email-craven@gmx.net>\r
  39         <87pq81tjv4.fsf@qmul.ac.uk> <87a9z5teaj.fsf@nexoid.at>\r
  40 User-Agent: Notmuch/0.13.2+61~gf708609 (http://notmuchmail.org) Emacs/23.4.1\r
  41         (x86_64-pc-linux-gnu)\r
  42 Date: Thu, 12 Jul 2012 12:25:09 +0100\r
  43 Message-ID: <87liipte5m.fsf@qmul.ac.uk>\r
  44 MIME-Version: 1.0\r
  45 Content-Type: text/plain; charset=us-ascii\r
  46 X-Sender-Host-Address: 94.192.233.223\r
  47 X-QM-SPAM-Info: Sender has good ham record.  :)\r
  48 X-QM-Body-MD5: cffec11c2a64bc7d6e3fe01dff04a7c9 (of first 20000 bytes)\r
  49 X-SpamAssassin-Score: -1.8\r
  50 X-SpamAssassin-SpamBar: -\r
  51 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
  52         determine if it is\r
  53         spam. We require at least 5.0 points to mark a message as spam.\r
  54         This message scored -1.8 points.\r
  55         Summary of the scoring: \r
  56         * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
  57         *      medium trust\r
  58         *      [138.37.6.40 listed in list.dnswl.org]\r
  59         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
  60         provider *      (markwalters1009[at]gmail.com)\r
  61         * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay\r
  62         *      domain\r
  63         *  0.5 AWL AWL: From: address is in the auto white-list\r
  64 X-QM-Scan-Virus: ClamAV says the message is clean\r
  65 X-BeenThere: notmuch@notmuchmail.org\r
  66 X-Mailman-Version: 2.1.13\r
  67 Precedence: list\r
  68 List-Id: "Use and development of the notmuch mail system."\r
  69         <notmuch.notmuchmail.org>\r
  70 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  71         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  72 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  73 List-Post: <mailto:notmuch@notmuchmail.org>\r
  74 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  75 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  76         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  77 X-List-Received-Date: Thu, 12 Jul 2012 11:25:17 -0000\r
  78 \r
  79 On Thu, 12 Jul 2012, craven@gmx.net wrote:\r
  80 >> what is the advantage of having this as one function rather than end_map\r
  81 >> and end_list? Indeed, my choice (but I think most other people would\r
  82 >> disagree) would be to have both functions but still keep state as this\r
  83 >> currently does and then throw an error if the code closes the wrong\r
  84 >> thing.\r
  85 >\r
  86 > There's probably no advantage, one way or the other is fine, I'd say.\r
  87 > I've thought about introducing checks into the formatter functions, to\r
  88 > raise errors for improper closing, map_key not inside a map and things\r
  89 > like that, I just wasn't sure that would be acceptable.\r
  90 \r
  91 I will leave others to comment.\r
  92 \r
  93 >> A second question: do you have an implementation in this style for\r
  94 >> s-expressions. I find it hard to tell whether the interface is right\r
  95 >> with just a single example. Even a completely hacky not ready for review\r
  96 >> example would be helpful.\r
  97 >\r
  98 > See the attached patch :)\r
  99 \r
 100 This looks great. I found it much easier to review by splitting\r
 101 sprinter.c into two files sprinter-json.c and sprinter-sexp.c and then\r
 102 running meld on them. The similarity is then very clear. It might be\r
 103 worth submitting them as two files, but I leave other people to comment.\r
 104 \r
 105 (Doing so made some of the difference between json and s-exp clear: like\r
 106 that keys in mapkeys are quoted in json but not in s-exp)\r
 107 \r
 108 It could be that some of the code could be merged, but I am not sure\r
 109 that there is much advantage. I would imagine that these two sprinter.c\r
 110 files would basically never change so there is not much risk of them\r
 111 diverging.\r
 112 \r
 113 I wonder if it would be worth using aggregate_t for both rather than\r
 114 using the closing symbol for this purpose in the json output.\r
 115 \r
 116 In any case this patch answers my query: the new structure does\r
 117 generalise very easily to s-expressions!\r
 118 \r
 119 Best wishes\r
 120 \r
 121 Mark\r
 122 \r
 123 \r
 124 \r
 125 >\r
 126 > Thanks for the suggestions!\r
 127 >\r
 128 > Peter\r
 129 > From cf2c5eeab814970736510ca2210b909643a8cf19 Mon Sep 17 00:00:00 2001\r
 130 > From: <craven@gmx.net>\r
 131 > Date: Thu, 12 Jul 2012 10:17:05 +0200\r
 132 > Subject: [PATCH] Add an S-Expression output format.\r
 133 >\r
 134 > ---\r
 135 >  notmuch-search.c |   7 ++-\r
 136 >  sprinter.c       | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\r
 137 >  sprinter.h       |   4 ++\r
 138 >  3 files changed, 180 insertions(+), 1 deletion(-)\r
 139 >\r
 140 > diff --git a/notmuch-search.c b/notmuch-search.c\r
 141 > index b853f5f..f8bea9b 100644\r
 142 > --- a/notmuch-search.c\r
 143 > +++ b/notmuch-search.c\r
 144 > @@ -77,6 +77,7 @@ do_search_threads (sprinter_t *format,\r
 145 >  \r
 146 >      if (format != sprinter_text) {\r
 147 >       format->begin_list (format);\r
 148 > +     format->frame (format);\r
 149 >      }\r
 150 >  \r
 151 >      for (i = 0;\r
 152 > @@ -380,7 +381,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
 153 >      int exclude = EXCLUDE_TRUE;\r
 154 >      unsigned int i;\r
 155 >  \r
 156 > -    enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }\r
 157 > +    enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP }\r
 158 >       format_sel = NOTMUCH_FORMAT_TEXT;\r
 159 >  \r
 160 >      notmuch_opt_desc_t options[] = {\r
 161 > @@ -391,6 +392,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
 162 >       { NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f',\r
 163 >         (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },\r
 164 >                                 { "text", NOTMUCH_FORMAT_TEXT },\r
 165 > +                               { "sexp", NOTMUCH_FORMAT_SEXP },\r
 166 >                                 { 0, 0 } } },\r
 167 >       { NOTMUCH_OPT_KEYWORD, &output, "output", 'o',\r
 168 >         (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },\r
 169 > @@ -422,6 +424,9 @@ notmuch_search_command (void *ctx, int argc, char *argv[])\r
 170 >      case NOTMUCH_FORMAT_JSON:\r
 171 >       format = sprinter_json_new (ctx, stdout);\r
 172 >       break;\r
 173 > +    case NOTMUCH_FORMAT_SEXP:\r
 174 > +     format = sprinter_sexp_new (ctx, stdout);\r
 175 > +        break;\r
 176 >      }\r
 177 >  \r
 178 >      config = notmuch_config_open (ctx, NULL, NULL);\r
 179 > diff --git a/sprinter.c b/sprinter.c\r
 180 > index 649f79a..fce0f9b 100644\r
 181 > --- a/sprinter.c\r
 182 > +++ b/sprinter.c\r
 183 > @@ -170,3 +170,173 @@ sprinter_json_new(const void *ctx, FILE *stream)\r
 184 >      res->stream = stream;\r
 185 >      return &res->vtable;\r
 186 >  }\r
 187 > +\r
 188 > +/*\r
 189 > + * Every below here is the implementation of the SEXP printer.\r
 190 > + */\r
 191 > +\r
 192 > +typedef enum { MAP, LIST } aggregate_t;\r
 193 > +\r
 194 > +struct sprinter_sexp\r
 195 > +{\r
 196 > +    struct sprinter vtable;\r
 197 > +    FILE *stream;\r
 198 > +    /* Top of the state stack, or NULL if the printer is not currently\r
 199 > +     * inside any aggregate types. */\r
 200 > +    struct sexp_state *state;\r
 201 > +};\r
 202 > +\r
 203 > +struct sexp_state\r
 204 > +{\r
 205 > +    struct sexp_state *parent;\r
 206 > +    /* True if nothing has been printed in this aggregate yet.\r
 207 > +     * Suppresses the comma before a value. */\r
 208 > +    notmuch_bool_t first;\r
 209 > +    /* The character that closes the current aggregate. */\r
 210 > +    aggregate_t type;\r
 211 > +};\r
 212 > +\r
 213 > +static struct sprinter_sexp *\r
 214 > +sexp_begin_value(struct sprinter *sp)\r
 215 > +{\r
 216 > +    struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
 217 > +    if (spsx->state) {\r
 218 > +     if (!spsx->state->first)\r
 219 > +         fputc (' ', spsx->stream);\r
 220 > +     else\r
 221 > +         spsx->state->first = false;\r
 222 > +    }\r
 223 > +    return spsx;\r
 224 > +}\r
 225 > +\r
 226 > +static void\r
 227 > +sexp_begin_aggregate(struct sprinter *sp, aggregate_t type)\r
 228 > +{\r
 229 > +    struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
 230 > +    struct sexp_state *state = talloc (spsx, struct sexp_state);\r
 231 > +\r
 232 > +    fputc ('(', spsx->stream);\r
 233 > +    state->parent = spsx->state;\r
 234 > +    state->first = true;\r
 235 > +    spsx->state = state;\r
 236 > +    state->type = type;\r
 237 > +}\r
 238 > +\r
 239 > +static void\r
 240 > +sexp_begin_map(struct sprinter *sp)\r
 241 > +{\r
 242 > +    sexp_begin_aggregate (sp, MAP);\r
 243 > +}\r
 244 > +\r
 245 > +static void\r
 246 > +sexp_begin_list(struct sprinter *sp)\r
 247 > +{\r
 248 > +    sexp_begin_aggregate (sp, LIST);\r
 249 > +}\r
 250 > +\r
 251 > +static void\r
 252 > +sexp_end(struct sprinter *sp)\r
 253 > +{\r
 254 > +    struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
 255 > +    struct sexp_state *state = spsx->state;\r
 256 > +\r
 257 > +    fputc (')', spsx->stream);\r
 258 > +    spsx->state = state->parent;\r
 259 > +    talloc_free (state);\r
 260 > +    if(spsx->state == NULL)\r
 261 > +     fputc ('\n', spsx->stream);\r
 262 > +}\r
 263 > +\r
 264 > +static void\r
 265 > +sexp_string(struct sprinter *sp, const char *val)\r
 266 > +{\r
 267 > +    static const char * const escapes[] = {\r
 268 > +     ['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",\r
 269 > +     ['\f'] = "\\f",  ['\n'] = "\\n",  ['\t'] = "\\t"\r
 270 > +    };\r
 271 > +    struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
 272 > +    fputc ('"', spsx->stream);\r
 273 > +    for (; *val; ++val) {\r
 274 > +     unsigned char ch = *val;\r
 275 > +     if (ch < ARRAY_SIZE(escapes) && escapes[ch])\r
 276 > +         fputs (escapes[ch], spsx->stream);\r
 277 > +     else if (ch >= 32)\r
 278 > +         fputc (ch, spsx->stream);\r
 279 > +     else\r
 280 > +         fprintf (spsx->stream, "\\u%04x", ch);\r
 281 > +    }\r
 282 > +    fputc ('"', spsx->stream);\r
 283 > +    if (spsx->state != NULL &&  spsx->state->type == MAP)\r
 284 > +     fputc (')', spsx->stream);\r
 285 > +    spsx->state->first = false;\r
 286 > +}\r
 287 > +\r
 288 > +static void\r
 289 > +sexp_integer(struct sprinter *sp, int val)\r
 290 > +{\r
 291 > +    struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
 292 > +    fprintf (spsx->stream, "%d", val);\r
 293 > +    if (spsx->state != NULL &&  spsx->state->type == MAP)\r
 294 > +     fputc (')', spsx->stream);\r
 295 > +}\r
 296 > +\r
 297 > +static void\r
 298 > +sexp_boolean(struct sprinter *sp, notmuch_bool_t val)\r
 299 > +{\r
 300 > +    struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
 301 > +    fputs (val ? "#t" : "#f", spsx->stream);\r
 302 > +    if (spsx->state != NULL &&  spsx->state->type == MAP)\r
 303 > +     fputc (')', spsx->stream);\r
 304 > +}\r
 305 > +\r
 306 > +static void\r
 307 > +sexp_null(struct sprinter *sp)\r
 308 > +{\r
 309 > +    struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
 310 > +    fputs ("'()", spsx->stream);\r
 311 > +    spsx->state->first = false;\r
 312 > +}\r
 313 > +\r
 314 > +static void\r
 315 > +sexp_map_key(struct sprinter *sp, const char *key)\r
 316 > +{\r
 317 > +    struct sprinter_sexp *spsx = sexp_begin_value(sp);\r
 318 > +    fputc ('(', spsx->stream);\r
 319 > +    fputs (key, spsx->stream);\r
 320 > +    fputs (" . ", spsx->stream);\r
 321 > +    spsx->state->first = true;\r
 322 > +}\r
 323 > +\r
 324 > +static void\r
 325 > +sexp_frame(struct sprinter *sp)\r
 326 > +{\r
 327 > +    struct sprinter_sexp *spsx = (struct sprinter_sexp*)sp;\r
 328 > +    fputc ('\n', spsx->stream);\r
 329 > +}\r
 330 > +\r
 331 > +struct sprinter *\r
 332 > +sprinter_sexp_new(const void *ctx, FILE *stream)\r
 333 > +{\r
 334 > +    static const struct sprinter_sexp template = {\r
 335 > +     .vtable = {\r
 336 > +         .begin_map = sexp_begin_map,\r
 337 > +         .begin_list = sexp_begin_list,\r
 338 > +         .end = sexp_end,\r
 339 > +         .string = sexp_string,\r
 340 > +         .integer = sexp_integer,\r
 341 > +         .boolean = sexp_boolean,\r
 342 > +         .null = sexp_null,\r
 343 > +         .map_key = sexp_map_key,\r
 344 > +         .frame = sexp_frame,\r
 345 > +     }\r
 346 > +    };\r
 347 > +    struct sprinter_sexp *res;\r
 348 > +\r
 349 > +    res = talloc (ctx, struct sprinter_sexp);\r
 350 > +    if (!res)\r
 351 > +     return NULL;\r
 352 > +\r
 353 > +    *res = template;\r
 354 > +    res->stream = stream;\r
 355 > +    return &res->vtable;\r
 356 > +}\r
 357 > diff --git a/sprinter.h b/sprinter.h\r
 358 > index 1dad9a0..a89eaa5 100644\r
 359 > --- a/sprinter.h\r
 360 > +++ b/sprinter.h\r
 361 > @@ -40,6 +40,10 @@ typedef struct sprinter\r
 362 >  struct sprinter *\r
 363 >  sprinter_json_new(const void *ctx, FILE *stream);\r
 364 >  \r
 365 > +/* Create a new structure printer that emits S-Expressions */\r
 366 > +struct sprinter *\r
 367 > +sprinter_sexp_new(const void *ctx, FILE *stream);\r
 368 > +\r
 369 >  /* A dummy structure printer that signifies that standard text output is\r
 370 >   * to be used instead of any structured format.\r
 371 >   */\r
 372 > -- \r
 373 > 1.7.11.1\r