Date: Sat, 21 Jan 2012 17:04:07 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Peter Feigl <craven@gmx.net>
Subject: Re: [PATCH] rewriting notmuch-search for structured output to make
	other output formats easier
Message-ID: <20120121220407.GK16740@mit.edu>
References: <1327180568-30385-1-git-send-email-craven@gmx.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1327180568-30385-1-git-send-email-craven@gmx.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Peter Feigl on Jan 21 at 10:16 pm:
> The output routines have been rewritten so that logical structure
> (objects with key/value pairs, arrays, strings and numbers) are
> written instead of ad-hoc printfs. This allows for easier adaptation
> of other output formats, as only the routines that start/end an object
> etc. have to be rewritten. The logic is the same for all formats.
> The default text output is handled differently, special cases are
> inserted at the proper places, as it differs too much from the
> structured output.

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.

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

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.