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

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

Date: Sun, 22 Jan 2012 00:21:37 +0100

To: Jameson Graef Rollins

Cc: notmuch@notmuchmail.org

From: Peter Feigl


On Sat, 21 Jan 2012 15:12:57 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sat, 21 Jan 2012 22:16:08 +0100, Peter Feigl <craven@gmx.net> wrote:
> > 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.
> 
> Hi, Peter.  Thanks for the contribution.
> 
> There are a lot of changes in this patch so I think you need to think
> about how you can break this up into multiple smaller and more atomic
> patches.  In particular, the addition of the sexp output format needs to
> definitely be in a separate patch from the restructuring of the output
> formatting.  You also don't mention anywhere in the commit log that
> you've added this new output format.  You'll also need to include
> documentation and test suite updates.

I'm sorry I forgot to mention that, it was mainly meant as a way to show
that this is easily possible (i.e. that the formatting is decoupled from
the logic, so that additional and different formats can be added without
influencing the printing logic). It'd be easy to split this up. What
kind of documentation should I include? 
The test suite should work fine, *if* it compares EXPECTED and OUTPUT
not character-by-character, but rather by pretty-printing both the
expected and the actual outputs by some JSON pretty-printer (like python
-mjson.tool). I can of course provide additional test-cases for
--format=sexp.

How should I proceed on this? Re-submit the patch with the sexp-support
removed and only JSON updated?

Thanks!

Peter

Thread: