[PATCH] rewriting notmuch-search for structured output to make other output formats easier

[Peter Feigl]

On Sat, 21 Jan 2012 17:04:07 -0500, Austin Clements <amdragon at MIT.EDU> wrote:
> Quoth Peter Feigl on Jan 21 at 10:16 pm:
> I think this is a great idea and I'm a fan of having an S-expression
> format, but I also think there's a much easier and more general way to
> structure this.
> 
> In particular, I don't think you should hijack search_format, since
> you'll wind up repeating most of this work for anything else that
> needs to output structured data (notmuch show, at least).  Rather, I'd
> suggest creating a general "structure printer" struct that isn't tied
> to search.  You've essentially already done this, you just called it
> search_format.  Then, leave the existing format callbacks in place,
> just use this new API instead of lots of printf calls.

I'm sorry I haven't been more clear about this, the intention was all
along to check whether this would be ok in notmuch-search, and if it got
accepted there, to factor it out into a separate file and then use it in
notmuch-show and notmuch-reply. There's nothing in the printer (except
for the name of the struct) that ties it to search.

> What about all of those annoying {tag,item}_{start,sep,end} fields?  I
> think you can simultaneously get rid of those *and* simplify the
> structure printer API.  If the structure printer is allowed to keep a
> little state, it can automatically insert separators.  With a little
> nesting state, it could even insert terminators by just saying "pop me
> to this level".  This could probably be better, but I'm imagining
> something like

I agree, however this is complicated by the fact that there are
additional restrictions on the actually printed code: newlines should be
placed at strategic locations to improve parsability, which could be
hard to decide in the printer alone without support from the logic that
drives it.

> struct sprinter *
> new_json_sprinter (const void *ctx, FILE *stream);
> struct sprinter *
> new_sexp_sprinter (const void *ctx, FILE *stream);
> 
> /* Start a map (a JSON object or a S-expression alist/plist/whatever)
>  * and return the nesting level of the map. */
> int
> sprinter_map (struct sprinter *sp);
> /* Start a list (aka array) and return the nesting level of the list. */
> int
> sprinter_list (struct sprinter *sp);
> 
> /* Close maps and lists until reaching level. */
> void
> sprinter_pop (struct sprinter *sp, int level);
> 
> void
> sprinter_map_key (struct sprinter *sp, const char *key);
> void
> sprinter_number (struct sprinter *sp, int val);
> void
> sprinter_string (struct sprinter *sp, const char *val);
> void
> sprinter_bool (struct sprinter *sp, notmuch_bool_t val);
> 
> and that's it.  This would also subsume your format_attribute_*
> helpers.
> 
> Unfortunately, it's a pain to pass things like a structure printer
> object around formatters (too bad notmuch isn't written in C++, eh?).
> I think it's better to address this than to structure around it.
> Probably the simplest thing to do is to make a struct for formatter
> state and pass that in to the callbacks.  You could also more
> completely emulate classes, but that would probably be overkill for
> this.

I believe this approach is similar to the one I've implemented (though
probably higher level, not so many details explicitly written into the
formatting code). I would suggest trying to get any sort of structured
formatters (whether more like your suggestions or like the thing I
implemented doesn't matter so much) into the main codebase, and then
refactoring the other parts to use it. I've thought about providing a
single patch to all of notmuch that accomplishes this, but I've deemed
it too large and complicated to be accepted, I thought limiting it to
notmuch-search would be a way to get it set up, so that it could be
expanded to the other parts later.

Thanks for the comments, I'll keep thinking about your design, a very
interesting idea!

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 274 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120122/7a526e63/attachment.pgp>