From: Mark Walters <markwalters1009@gmail.com>
To: craven@gmx.net, notmuch@notmuchmail.org
Subject: Re: [PATCH] v2: Added better support for multiple structured output
	formats.
In-Reply-To: <87a9z7vj3p.fsf@nexoid.at>
References: <87liisue70.fsf@nexoid.at> <87fw8zvqmm.fsf@nexoid.at>
	<87liir944f.fsf@qmul.ac.uk> <87a9z7vj3p.fsf@nexoid.at>
User-Agent: Notmuch/0.13.2+61~gf708609 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Tue, 10 Jul 2012 18:34:51 +0100
Message-ID: <87d3438qpw.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Tue, 10 Jul 2012, craven@gmx.net wrote:
> As discussed in <id:20120121220407.GK16740@mit.edu>, this patch adds
> support for new structured output formats (like s-expressions) by using
> stateful structure_printers. An implementation of the JSON structure
> printer that passes all tests is included. The output for JSON (and
> text) is identical to the current output. S-Expressions will be added in
> a later patch.
>

Hi I have some more comments inline below. I agree with Jamie that it
would be nice to split it into smaller chunks but I am not sure that is
practical to do.

> A large part of this patch just implements the differentiation between
> structured and non-structured output (all the code within 
> "if(format == unstructured_text_printer)").

I also agree with Jamie that the text output is annoying. However, at
least until Austin's async json parser goes in even the emacs interface
is using it so I think we have to live with it for a while (at the very
least we would need to give clients a reasonable time to migrate).

Anyway, I think this patch has to support it.

> In a second patch, the structured output code should be isolated in a
> separate file, and also used in all other parts of notmuch.
>
> The interface is a structure structure_printer, which contains the following methods:
>
> - initial_state: is called to create a state object, that is passed to all invocations. This should be used to keep track of the output file and everything else necessary to correctly format output.
> - map: is called when a new map (associative array, dictionary) is started. map_key and the primitives (string, number, bool) are used alternatingly to add key/value pairs. pop is used to close the map (see there). This function must return a nesting level identifier that can be used to close all nested structures (maps and lists), backing out to the returned nesting level.
> - list: is called when a new list (array, vector) is started. the primitives (string, number, bool) are used consecutively to add values to the list. pop is used to close the list. This function must return a nesting level identifier that can be used to close all nested structures (maps and lists), backing out to the returned nesting level.
> - pop: is called to return to a given nesting level. All lists and maps with a deeper nesting level must be closed.
> - number, string, bool: output one element of the specific type.
>
> All functions should use the state object to insert delimiters etc. automatically when appropriate.
>
> Example:
> int top, one;
> top = map(state);
> map_key(state, "foo");
> one = list(state);
> number(state, 1);
> number(state, 2);
> number(state, 3);
> pop(state, i);
> map_key(state, "bar");
> map(state);
> map_key(state, "baaz");
> string(state, "hello world");
> pop(state, top);
>
> would output JSON as follows:
>
> {"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
> ---
>  notmuch-search.c | 491 ++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 361 insertions(+), 130 deletions(-)
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 3be296d..4127777 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -28,6 +28,210 @@ typedef enum {
>      OUTPUT_TAGS
>  } output_t;
>  
> +/* structured formatting, useful for JSON, S-Expressions, ...
> +
> +- initial_state: is called to create a state object, that is passed to all invocations. This should be used to keep track of the output file and everything else necessary to correctly format output.
> +- map: is called when a new map (associative array, dictionary) is started. map_key and the primitives (string, number, bool) are used alternatingly to add key/value pairs. pop is used to close the map (see there). This function must return a nesting level identifier that can be used to close all nested structures (maps and lists), backing out to the returned nesting level.
> +- list: is called when a new list (array, vector) is started. the primitives (string, number, bool) are used consecutively to add values to the list. pop is used to close the list. This function must return a nesting level identifier that can be used to close all nested structures (maps and lists), backing out to the returned nesting level.
> +- pop: is called to return to a given nesting level. All lists and maps with a deeper nesting level must be closed.
> +- number, string, bool: output one element of the specific type.
> +
> +All functions should use state to insert delimiters etc. automatically when appropriate.
> +
> +Example:
> +int top, one;
> +top = map(state);
> +map_key(state, "foo");
> +one = list(state);
> +number(state, 1);
> +number(state, 2);
> +number(state, 3);
> +pop(state, i);

I think `i` should be `one` or vice versa (and the same in the commit message)? 

> +map_key(state, "bar");
> +map(state);
> +map_key(state, "baaz");
> +string(state, "hello world");
> +pop(state, top);
> +
> +would output JSON as follows:
> +
> +{"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
> +
> + */
> +typedef struct structure_printer {
> +    int (*map)(void *state);
> +    int (*list)(void *state);
> +    void (*pop)(void *state, int level);
> +    void (*map_key)(void *state, const char *key);
> +    void (*number)(void *state, int val);
> +    void (*string)(void *state, const char *val);
> +    void (*bool)(void *state, notmuch_bool_t val);
> +    void *(*initial_state)(const struct structure_printer *sp, FILE *output);
> +} structure_printer_t;
> +
> +/* JSON structure printer */
> +
> +/* single linked list implementation for keeping track of the array/map nesting state */
> +typedef struct json_list {
> +    int type;
> +    int first_already_seen;
> +    struct json_list *rest;
> +} json_list_t;
> +
> +#define TYPE_JSON_MAP 1
> +#define TYPE_JSON_ARRAY 2
> +
> +typedef struct json_state {
> +    FILE *output;
> +    json_list_t *stack;
> +    int level;
> +} json_state_t;
> +
> +int json_map(void *state);
> +int json_list(void *state);
> +void json_pop(void *state, int level);
> +void json_map_key(void *state, const char *key);
> +void json_number(void *state, int val);
> +void json_string(void *state, const char *val);
> +void json_bool(void *state, notmuch_bool_t val);
> +void *json_initial_state(const struct structure_printer *sp, FILE *output);
> +
> +structure_printer_t json_structure_printer = {
> +    &json_map,
> +    &json_list,
> +    &json_pop,
> +    &json_map_key,
> +    &json_number,
> +    &json_string,
> +    &json_bool,
> +    &json_initial_state
> +};
> +
> +int json_map(void *st) {

I think these should be of the form (see devel/STYLE)

int
json_map(void *st) 
{

> +    json_state_t *state = (json_state_t*)st;
> +    FILE *output = state->output;
> +    if(state->stack != NULL && state->stack->type == TYPE_JSON_ARRAY && state->stack->first_already_seen) {
> +	fputs(",", output);
> +	if(state->level == 1)
> +	    fputs("\n", output);
> +	else
> +	    fputs(" ", output);
> +    }
> +    if(state->stack != NULL) {
> +	state->stack->first_already_seen = TRUE;
> +    }
> +    fputs("{", output);
> +    void *ctx_json_map = talloc_new (0);
> +    json_list_t *el = talloc(ctx_json_map, json_list_t);
> +    el->type = TYPE_JSON_MAP;
> +    el->first_already_seen = FALSE;
> +    el->rest = state->stack;
> +    state->stack = el;
> +    return state->level++;
> +}
> +
> +int json_list(void *st) {
> +    json_state_t *state = (json_state_t*)st;
> +    FILE *output = state->output;
> +    if(state->stack != NULL && state->stack->type == TYPE_JSON_ARRAY && state->stack->first_already_seen) {
> +	fputs(",", output);
> +	if(state->level == 1)
> +	    fputs("\n", output);
> +	else
> +	    fputs(" ", output);
> +    }
> +    if(state->stack != NULL) {
> +	state->stack->first_already_seen = TRUE;
> +    }
> +    fputs("[", output);
> +    void *ctx_json_map = talloc_new (0);
> +    json_list_t *el = talloc(ctx_json_map, json_list_t);
> +    el->type = TYPE_JSON_ARRAY;
> +    el->first_already_seen = FALSE;
> +    el->rest = state->stack;
> +    state->stack = el;
> +    return state->level++;
> +}
> +
> +void json_pop(void *st, int level) {
> +    int i;
> +    json_state_t *state = (json_state_t*)st;
> +    FILE *output = state->output;
> +    for(i = state->level; i > level; i--) {
> +	json_list_t *tos = state->stack;
> +	if(tos->type == TYPE_JSON_MAP) {
> +	    fputs("}", output);
> +	}
> +	if(tos->type == TYPE_JSON_ARRAY) {
> +	    fputs("]", output);
> +	}
> +	state->stack = tos->rest;
> +	state->level--;
> +	talloc_free(tos);
> +    }
> +    if(state->level == 0)
> +	fputs("\n", output);
> +}
> +
> +void json_map_key(void *st, const char *key) {
> +    json_state_t *state = (json_state_t*)st;
> +    FILE *output = state->output;
> +    if(state->stack != NULL && state->stack->first_already_seen) {
> +	fputs(",\n", output);
> +    }
> +    fputs("\"", output);
> +    fputs(key, output);
> +    fputs("\": ", output);
> +}
> +
> +void json_number(void *st, int val) {
> +    json_state_t *state = (json_state_t*)st;
> +    FILE *output = state->output;
> +    if(state->stack != NULL && state->stack->type == TYPE_JSON_ARRAY && state->stack->first_already_seen) {
> +	fputs(", ", output);
> +    }
> +    state->stack->first_already_seen = TRUE;
> +    fprintf(output, "%i", val);
> +}
> +
> +void json_string(void *st, const char *val) {
> +    json_state_t *state = (json_state_t*)st;
> +    FILE *output = state->output;
> +    void *ctx = talloc_new(0);
> +    if(state->stack != NULL && state->stack->type == TYPE_JSON_ARRAY && state->stack->first_already_seen) {
> +	fputs(",", output);
> +	if(state->level == 1)
> +	    fputs("\n", output);
> +	else
> +	    fputs(" ", output);
> +    }
> +
> +    state->stack->first_already_seen = TRUE;
> +    fprintf(output, "%s", json_quote_str(ctx, val));
> +    talloc_free(ctx);
> +}
> +
> +void json_bool(void *st, notmuch_bool_t val) {
> +    json_state_t *state = (json_state_t*)st;
> +    FILE *output = state->output;
> +    if(val)
> +	fputs("true", output);
> +    else
> +	fputs("false", output);
> +}

These three functions are all different but I feel they should all be
the same. We may not have arrays of booleans but that looks like it
would be legal JSON. This might suggest that it is worth extracting a
small helper function to do the

if(state->stack != NULL && state->stack->type == TYPE_JSON_ARRAY && state->stack->first_already_seen) {
	fputs(",", output);
	if(state->level == 1)
	    fputs("\n", output);
	else
	    fputs(" ", output);
    }

bit.


> +
> +void *json_initial_state(const struct structure_printer *sp, FILE *output) {
> +    (void)sp;
> +    json_state_t *st = talloc(0, json_state_t);
> +    st->level = 0;
> +    st->stack = NULL;
> +    st->output = output;
> +    return st;
> +}
> +
> +structure_printer_t *unstructured_text_printer = NULL;
> +
> +/* legacy, only needed for non-structured text output */
>  typedef struct search_format {
>      const char *results_start;
>      const char *item_start;
> @@ -51,6 +255,7 @@ typedef struct search_format {
>      const char *results_null;
>  } search_format_t;
>  
> +
>  static void
>  format_item_id_text (const void *ctx,
>  		     const char *item_type,
> @@ -64,6 +269,7 @@ format_thread_text (const void *ctx,
>  		    const int total,
>  		    const char *authors,
>  		    const char *subject);
> +
>  static const search_format_t format_text = {
>      "",
>  	"",
> @@ -78,35 +284,6 @@ static const search_format_t format_text = {
>  };
>  
>  static void
> -format_item_id_json (const void *ctx,
> -		     const char *item_type,
> -		     const char *item_id);
> -
> -static void
> -format_thread_json (const void *ctx,
> -		    const char *thread_id,
> -		    const time_t date,
> -		    const int matched,
> -		    const int total,
> -		    const char *authors,
> -		    const char *subject);
> -
> -/* Any changes to the JSON format should be reflected in the file
> - * devel/schemata. */
> -static const search_format_t format_json = {
> -    "[",
> -	"{",
> -	    format_item_id_json,
> -	    format_thread_json,
> -	    "\"tags\": [",
> -		"\"%s\"", ", ",
> -	    "]", ",\n",
> -	"}",
> -    "]\n",
> -    "]\n",
> -};
> -
> -static void
>  format_item_id_text (unused (const void *ctx),
>  		     const char *item_type,
>  		     const char *item_id)
> @@ -153,50 +330,9 @@ format_thread_text (const void *ctx,
>      talloc_free (ctx_quote);
>  }
>  
> -static void
> -format_item_id_json (const void *ctx,
> -		     unused (const char *item_type),
> -		     const char *item_id)
> -{
> -    void *ctx_quote = talloc_new (ctx);
> -
> -    printf ("%s", json_quote_str (ctx_quote, item_id));
> -
> -    talloc_free (ctx_quote);
> -    
> -}
> -
> -static void
> -format_thread_json (const void *ctx,
> -		    const char *thread_id,
> -		    const time_t date,
> -		    const int matched,
> -		    const int total,
> -		    const char *authors,
> -		    const char *subject)
> -{
> -    void *ctx_quote = talloc_new (ctx);
> -
> -    printf ("\"thread\": %s,\n"
> -	    "\"timestamp\": %ld,\n"
> -	    "\"date_relative\": \"%s\",\n"
> -	    "\"matched\": %d,\n"
> -	    "\"total\": %d,\n"
> -	    "\"authors\": %s,\n"
> -	    "\"subject\": %s,\n",
> -	    json_quote_str (ctx_quote, thread_id),
> -	    date,
> -	    notmuch_time_relative_date (ctx, date),
> -	    matched,
> -	    total,
> -	    json_quote_str (ctx_quote, authors),
> -	    json_quote_str (ctx_quote, subject));
> -
> -    talloc_free (ctx_quote);
> -}
> -
>  static int
> -do_search_threads (const search_format_t *format,
> +do_search_threads (const structure_printer_t *format,
> +		   void *state,
>  		   notmuch_query_t *query,
>  		   notmuch_sort_t sort,
>  		   output_t output,
> @@ -210,6 +346,8 @@ do_search_threads (const search_format_t *format,
>      int first_thread = 1;
>      int i;
>  
> +    int outermost_level = 0;
> +    int items_level = 0;
>      if (offset < 0) {
>  	offset += notmuch_query_count_threads (query);
>  	if (offset < 0)
> @@ -220,7 +358,11 @@ do_search_threads (const search_format_t *format,
>      if (threads == NULL)
>  	return 1;
>  
> -    fputs (format->results_start, stdout);
> +    if(format == unstructured_text_printer) {
> +	fputs(format_text.results_start, stdout);
> +    } else { /* structured output */
> +	outermost_level = format->list(state);
> +    }
>  
>      for (i = 0;
>  	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
> @@ -235,43 +377,92 @@ do_search_threads (const search_format_t *format,
>  	    continue;
>  	}
>  
> -	if (! first_thread)
> -	    fputs (format->item_sep, stdout);
> +	if (format == unstructured_text_printer && ! first_thread)
> +	    fputs (format_text.item_sep, stdout);
>  
>  	if (output == OUTPUT_THREADS) {
> -	    format->item_id (thread, "thread:",
> -			     notmuch_thread_get_thread_id (thread));
> +	    if(format == unstructured_text_printer) {
> +		format_text.item_id (thread, "thread:",
> +				     notmuch_thread_get_thread_id (thread));
> +	    } else { /* structured output */
> +		char buffer[128];
> +		snprintf(buffer, 128, "thread:%s", notmuch_thread_get_thread_id (thread));
> +		format->string(state, buffer);

As mentioned on irc I think you don't need the thread: bit for the
current JSON output in this case.

Best wishes

Mark


>  	} else { /* output == OUTPUT_SUMMARY */
> -	    fputs (format->item_start, stdout);
> +	    int tags_level = 0;
> +	    void *ctx = talloc_new (0);
> +
> +	    if(format == unstructured_text_printer) {
> +		fputs (format_text.item_start, stdout);
> +	    } else { /* structured output */
> +		items_level = format->map(state);
> +	    }
>  
>  	    if (sort == NOTMUCH_SORT_OLDEST_FIRST)
>  		date = notmuch_thread_get_oldest_date (thread);
>  	    else
>  		date = notmuch_thread_get_newest_date (thread);
>  
> -	    format->thread_summary (thread,
> -				    notmuch_thread_get_thread_id (thread),
> -				    date,
> -				    notmuch_thread_get_matched_messages (thread),
> -				    notmuch_thread_get_total_messages (thread),
> -				    notmuch_thread_get_authors (thread),
> -				    notmuch_thread_get_subject (thread));
> +	    if(format == unstructured_text_printer) {
> +		format_text.thread_summary (thread,
> +					    notmuch_thread_get_thread_id (thread),
> +					    date,
> +					    notmuch_thread_get_matched_messages (thread),
> +					    notmuch_thread_get_total_messages (thread),
> +					    notmuch_thread_get_authors (thread),
> +					    notmuch_thread_get_subject (thread));
> +	    } else { /* structured output */
> +		format->map_key(state, "thread");
> +		format->string(state, notmuch_thread_get_thread_id (thread));
> +		format->map_key(state, "timestamp");
> +		format->number(state, date);
> +		format->map_key(state, "date_relative");
> +		format->string(state, notmuch_time_relative_date(ctx, date));
> +		format->map_key(state, "matched");
> +		format->number(state, notmuch_thread_get_matched_messages(thread));
> +		format->map_key(state, "total");
> +		format->number(state, notmuch_thread_get_total_messages(thread));
> +		format->map_key(state, "authors");
> +		format->string(state, notmuch_thread_get_authors(thread));
> +		format->map_key(state, "subject");
> +		format->string(state, notmuch_thread_get_subject(thread));
> +	    }
> +
> +	    if(format == unstructured_text_printer) {
> +		fputs (format_text.tag_start, stdout);
> +	    } else { /* structured output */
> +		format->map_key(state, "tags");
> +		tags_level = format->list(state);
> +	    }
>  
> -	    fputs (format->tag_start, stdout);
>  
>  	    for (tags = notmuch_thread_get_tags (thread);
>  		 notmuch_tags_valid (tags);
>  		 notmuch_tags_move_to_next (tags))
>  	    {
> -		if (! first_tag)
> -		    fputs (format->tag_sep, stdout);
> -		printf (format->tag, notmuch_tags_get (tags));
> +		if (format == unstructured_text_printer && ! first_tag) {
> +		    fputs (format_text.tag_sep, stdout);
> +		}
> +		if(format == unstructured_text_printer) {
> +		    printf (format_text.tag, notmuch_tags_get (tags));
> +		} else { /* structured output */
> +		    format->string(state, notmuch_tags_get(tags));
> +		}
>  		first_tag = 0;
>  	    }
>  
> -	    fputs (format->tag_end, stdout);
> +	    if(format == unstructured_text_printer) {
> +		fputs (format_text.tag_end, stdout);
> +	    } else { /* structured output */
> +		format->pop(state, tags_level);
> +	    }
>  
> -	    fputs (format->item_end, stdout);
> +	    if(format == unstructured_text_printer) {
> +		fputs (format_text.item_end, stdout);
> +	    } else { /* structured output */
> +		format->pop(state, items_level);
> +	    }
>  	}
>  
>  	first_thread = 0;
> @@ -279,16 +470,21 @@ do_search_threads (const search_format_t *format,
>  	notmuch_thread_destroy (thread);
>      }
>  
> -    if (first_thread)
> -	fputs (format->results_null, stdout);
> -    else
> -	fputs (format->results_end, stdout);
> +    if(format == unstructured_text_printer) {
> +	if (first_thread)
> +	    fputs (format_text.results_null, stdout);
> +	else
> +	    fputs (format_text.results_end, stdout);
> +    } else { /* structured output */
> +	format->pop(state, outermost_level);
> +    }
>  
>      return 0;
>  }
>  
>  static int
> -do_search_messages (const search_format_t *format,
> +do_search_messages (const structure_printer_t *format,
> +		    void *state,
>  		    notmuch_query_t *query,
>  		    output_t output,
>  		    int offset,
> @@ -299,6 +495,7 @@ do_search_messages (const search_format_t *format,
>      notmuch_filenames_t *filenames;
>      int first_message = 1;
>      int i;
> +    int outermost_level = 0;
>  
>      if (offset < 0) {
>  	offset += notmuch_query_count_messages (query);
> @@ -310,7 +507,11 @@ do_search_messages (const search_format_t *format,
>      if (messages == NULL)
>  	return 1;
>  
> -    fputs (format->results_start, stdout);
> +    if(format == unstructured_text_printer) {
> +	fputs (format_text.results_start, stdout);
> +    } else { /* structured output */
> +	outermost_level = format->list(state);
> +    }
>  
>      for (i = 0;
>  	 notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit);
> @@ -328,23 +529,32 @@ do_search_messages (const search_format_t *format,
>  		 notmuch_filenames_valid (filenames);
>  		 notmuch_filenames_move_to_next (filenames))
>  	    {
> -		if (! first_message)
> -		    fputs (format->item_sep, stdout);
> +		if(format == unstructured_text_printer) {
> +		    if (! first_message)
> +			fputs (format_text.item_sep, stdout);
>  
> -		format->item_id (message, "",
> -				 notmuch_filenames_get (filenames));
> +		    format_text.item_id (message, "",
> +					 notmuch_filenames_get (filenames));
> +		} else { /* structured output */
> +		format->string(state, notmuch_filenames_get (filenames));
> +		}
>  
>  		first_message = 0;
>  	    }
> -	    
> +
>  	    notmuch_filenames_destroy( filenames );
>  
>  	} else { /* output == OUTPUT_MESSAGES */
> -	    if (! first_message)
> -		fputs (format->item_sep, stdout);
> +	    if(format == unstructured_text_printer) {
> +		if (! first_message)
> +		    fputs (format_text.item_sep, stdout);
> +
> +		format_text.item_id (message, "id:",
> +				     notmuch_message_get_message_id (message));
> +	    } else { /* structured output */
> +		format->string(state, notmuch_message_get_message_id (message));
> +	    }
>  
> -	    format->item_id (message, "id:",
> -			     notmuch_message_get_message_id (message));
>  	    first_message = 0;
>  	}
>  
> @@ -353,23 +563,29 @@ do_search_messages (const search_format_t *format,
>  
>      notmuch_messages_destroy (messages);
>  
> -    if (first_message)
> -	fputs (format->results_null, stdout);
> -    else
> -	fputs (format->results_end, stdout);
> +    if(format == unstructured_text_printer) {
> +	if (first_message)
> +	    fputs (format_text.results_null, stdout);
> +	else
> +	    fputs (format_text.results_end, stdout);
> +    } else { /* structured output */
> +	format->pop(state, outermost_level);
> +    }
>  
>      return 0;
>  }
>  
>  static int
>  do_search_tags (notmuch_database_t *notmuch,
> -		const search_format_t *format,
> +		const structure_printer_t *format,
> +		void *state,
>  		notmuch_query_t *query)
>  {
>      notmuch_messages_t *messages = NULL;
>      notmuch_tags_t *tags;
>      const char *tag;
>      int first_tag = 1;
> +    int outermost_level = 0;
>  
>      /* should the following only special case if no excluded terms
>       * specified? */
> @@ -387,7 +603,11 @@ do_search_tags (notmuch_database_t *notmuch,
>      if (tags == NULL)
>  	return 1;
>  
> -    fputs (format->results_start, stdout);
> +    if(format == unstructured_text_printer) {
> +	fputs (format_text.results_start, stdout);
> +    } else { /* structured output */
> +	outermost_level = format->list(state);
> +    }
>  
>      for (;
>  	 notmuch_tags_valid (tags);
> @@ -395,10 +615,14 @@ do_search_tags (notmuch_database_t *notmuch,
>      {
>  	tag = notmuch_tags_get (tags);
>  
> -	if (! first_tag)
> -	    fputs (format->item_sep, stdout);
> +	if(format == unstructured_text_printer) {
> +	    if (! first_tag)
> +		fputs (format_text.item_sep, stdout);
>  
> -	format->item_id (tags, "", tag);
> +	    format_text.item_id (tags, "", tag);
> +	} else { /* structured output */
> +	    format->string(state, tag);
> +	}
>  
>  	first_tag = 0;
>      }
> @@ -408,10 +632,14 @@ do_search_tags (notmuch_database_t *notmuch,
>      if (messages)
>  	notmuch_messages_destroy (messages);
>  
> -    if (first_tag)
> -	fputs (format->results_null, stdout);
> -    else
> -	fputs (format->results_end, stdout);
> +    if(format == unstructured_text_printer) {
> +	if (first_tag)
> +	    fputs (format_text.results_null, stdout);
> +	else
> +	    fputs (format_text.results_end, stdout);
> +    } else { /* structured output */
> +	format->pop(state, outermost_level);
> +    }
>  
>      return 0;
>  }
> @@ -430,7 +658,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>      notmuch_query_t *query;
>      char *query_str;
>      notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;
> -    const search_format_t *format = &format_text;
> +    const structure_printer_t *format = unstructured_text_printer;
> +    void *state = NULL;
>      int opt_index, ret;
>      output_t output = OUTPUT_SUMMARY;
>      int offset = 0;
> @@ -457,11 +686,11 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>  				  { "files", OUTPUT_FILES },
>  				  { "tags", OUTPUT_TAGS },
>  				  { 0, 0 } } },
> -        { NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
> -          (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
> -                                  { "false", EXCLUDE_FALSE },
> -                                  { "flag", EXCLUDE_FLAG },
> -                                  { 0, 0 } } },
> +	{ NOTMUCH_OPT_KEYWORD, &exclude, "exclude", 'x',
> +	  (notmuch_keyword_t []){ { "true", EXCLUDE_TRUE },
> +				  { "false", EXCLUDE_FALSE },
> +				  { "flag", EXCLUDE_FLAG },
> +				  { 0, 0 } } },
>  	{ NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 },
>  	{ NOTMUCH_OPT_INT, &limit, "limit", 'L', 0  },
>  	{ 0, 0, 0, 0, 0 }
> @@ -475,10 +704,12 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>  
>      switch (format_sel) {
>      case NOTMUCH_FORMAT_TEXT:
> -	format = &format_text;
> +	format = unstructured_text_printer;
> +	state = 0;
>  	break;
>      case NOTMUCH_FORMAT_JSON:
> -	format = &format_json;
> +	format = &json_structure_printer;
> +	state = format->initial_state(format, stdout);
>  	break;
>      }
>  
> @@ -532,14 +763,14 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>      default:
>      case OUTPUT_SUMMARY:
>      case OUTPUT_THREADS:
> -	ret = do_search_threads (format, query, sort, output, offset, limit);
> +	ret = do_search_threads (format, state, query, sort, output, offset, limit);
>  	break;
>      case OUTPUT_MESSAGES:
>      case OUTPUT_FILES:
> -	ret = do_search_messages (format, query, output, offset, limit);
> +	ret = do_search_messages (format, state, query, output, offset, limit);
>  	break;
>      case OUTPUT_TAGS:
> -	ret = do_search_tags (notmuch, format, query);
> +	ret = do_search_tags (notmuch, format, state, query);
>  	break;
>      }
>  
> -- 
> 1.7.11.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch