[PATCH v8 0/3] notmuch-search: Structured Output Formatters

[PATCH v8 0/3] notmuch-search: Structured Output Formatters

[craven]

From: <craven@gmx.net>

Currently there is no easy way to add support for different structured
formatters (like JSON). For example, adding support for S-Expressions
would result in code duplication.

This patch series amends the situation by introducing structured
formatters, which allow different implementations of structures like
lists, maps, strings and numbers.

The new code in sprinter.h and sprinter-json.c can be used instead of
the current ad-hoc output in all parts of notmuch, a patch for
notmuch-search.c is included.

In a later patch, all other parts of notmuch should be adapted to the
structured formatters, and the creation of formatters should be
centralised (to make adding new formatters easier).

A "structured" formatter is provided for notmuch-search that prints the
current text format. This removes almost all the special-casing from
notmuch-search.c.

Changes versus v7 of this patch:
- added {} around "else" blocks (as mentioned in
  id:20120722160843.GC31834@mit.edu)
- added fallback to INTERNAL_ERROR (which should never be called) in
  notmuch-search.c if format is unknown (as mentioned in
  id:m2r4s694ly.fsf@guru.guru-group.fi).

Summary: 

Peter Feigl (3):
  Add support for structured output formatters.
  Add structured output formatter for JSON and plain text (but don't
    use them yet).
  Use the structured formatters in notmuch-search.c.

 Makefile.local   |   2 +
 notmuch-search.c | 304 ++++++++++++++---------------------------------
 sprinter-json.c  | 187 +++++++++++++++++++++++++++++
 sprinter-text.c  | 126 ++++++++++++++++++++
 sprinter.h       |  68 +++++++++++
 test/json        |  34 +++---
 6 files changed, 489 insertions(+), 232 deletions(-)
 create mode 100644 sprinter-json.c
 create mode 100644 sprinter-text.c
 create mode 100644 sprinter.h

-- 
1.7.11.2

[PATCH v8 1/3] Add support for structured output formatters.

[craven]

From: <craven@gmx.net>

This patch adds a new struct type sprinter_t, which is used for
structured formatting, e.g. JSON or S-Expressions. The structure printer
is heavily based on code from Austin Clements
(id:87d34hsdx8.fsf@awakening.csail.mit.edu).

It includes the following functions:

    /* Start a new map/dictionary structure. This should be followed by
     * a sequence of alternating calls to map_key and one of the
     * value-printing functions until the map is ended by end.
     */
    void (*begin_map) (struct sprinter *);

    /* Start a new list/array structure.
     */
    void (*begin_list) (struct sprinter *);

    /* End the last opened list or map structure.
     */
    void (*end) (struct sprinter *);

    /* Print one string/integer/boolean/null element (possibly inside a
     * list or map, followed or preceded by separators).
     * For string, the char * must be UTF-8 encoded.
     */
    void (*string) (struct sprinter *, const char *);
    void (*integer) (struct sprinter *, int);
    void (*boolean) (struct sprinter *, notmuch_bool_t);
    void (*null) (struct sprinter *);

    /* Print the key of a map's key/value pair. The char * must be UTF-8
     * encoded.
     */
    void (*map_key) (struct sprinter *, const char *);

    /* Insert a separator (usually extra whitespace) for improved
     * readability without affecting the abstract syntax of the
     * structure being printed.
     * For JSON, this could simply be a line break.
     */
    void (*separator) (struct sprinter *);

    /* Set the current string prefix. This only affects the text
     * printer, which will print this string, followed by a colon,
     * before any string. For other printers, this does nothing.
     */
    void (*set_prefix) (struct sprinter *, const char *);

To support the plain text format properly, the following additional
function must also be implemented:

    /* Set the current string prefix. This only affects the text
     * printer, which will print this string, followed by a colon,
     * before any string. For other printers, this does nothing.
     */
    void (*set_prefix) (struct sprinter *, const char *);

The structure also contains a flag that should be set to FALSE in all
custom printers and to TRUE in the plain text formatter.

    /* True if this is the special-cased plain text printer.
     */
    notmuch_bool_t is_text_printer;

The printer can (and should) use internal state to insert delimiters
and syntax at the correct places.

Example:

format->begin_map(format);
format->map_key(format, "foo");
format->begin_list(format);
format->integer(format, 1);
format->integer(format, 2);
format->integer(format, 3);
format->end(format);
format->map_key(format, "bar");
format->begin_map(format);
format->map_key(format, "baaz");
format->string(format, "hello world");
format->end(format);
format->end(format);

would output JSON as follows:

{"foo": [1, 2, 3], "bar": { "baaz": "hello world"}}
---
 sprinter.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/sprinter.h b/sprinter.h
new file mode 100644
index 0000000..77dc26f
--- /dev/null
+++ b/sprinter.h
@@ -0,0 +1,58 @@
+#ifndef NOTMUCH_SPRINTER_H
+#define NOTMUCH_SPRINTER_H
+
+/* Necessary for notmuch_bool_t */
+#include "notmuch-client.h"
+
+/* Structure printer interface. This is used to create output
+ * structured as maps (with key/value pairs), lists and primitives
+ * (strings, integers and booleans).
+ */
+typedef struct sprinter {
+    /* Start a new map/dictionary structure. This should be followed by
+     * a sequence of alternating calls to map_key and one of the
+     * value-printing functions until the map is ended by end.
+     */
+    void (*begin_map) (struct sprinter *);
+
+    /* Start a new list/array structure.
+     */
+    void (*begin_list) (struct sprinter *);
+
+    /* End the last opened list or map structure.
+     */
+    void (*end) (struct sprinter *);
+
+    /* Print one string/integer/boolean/null element (possibly inside a
+     * list or map, followed or preceded by separators).
+     * For string, the char * must be UTF-8 encoded.
+     */
+    void (*string) (struct sprinter *, const char *);
+    void (*integer) (struct sprinter *, int);
+    void (*boolean) (struct sprinter *, notmuch_bool_t);
+    void (*null) (struct sprinter *);
+
+    /* Print the key of a map's key/value pair. The char * must be UTF-8
+     * encoded.
+     */
+    void (*map_key) (struct sprinter *, const char *);
+
+    /* Insert a separator (usually extra whitespace) for improved
+     * readability without affecting the abstract syntax of the
+     * structure being printed.
+     * For JSON, this could simply be a line break.
+     */
+    void (*separator) (struct sprinter *);
+
+    /* Set the current string prefix. This only affects the text
+     * printer, which will print this string, followed by a colon,
+     * before any string. For other printers, this does nothing.
+     */
+    void (*set_prefix) (struct sprinter *, const char *);
+
+    /* True if this is the special-cased plain text printer.
+     */
+    notmuch_bool_t is_text_printer;
+} sprinter_t;
+
+#endif // NOTMUCH_SPRINTER_H
-- 
1.7.11.2

[PATCH v8 2/3] Add structured output formatter for JSON and plain text (but don't use them yet).

[craven]

From: <craven@gmx.net>

Using the new structured printer support in sprinter.h, implement
sprinter_json_create, which returns a new JSON structured output
formatter. The formatter prints output similar to the existing JSON, but
with differences in whitespace (mostly newlines, --output=summary prints
the entire message summary on one line, not split across multiple lines).

Also implement a "structured" formatter for plain text that prints
prefixed strings, to be used with notmuch-search.c plain text output.
---
 Makefile.local  |   2 +
 sprinter-json.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++
 sprinter-text.c | 126 ++++++++++++++++++++++++++++++++
 sprinter.h      |  10 +++
 4 files changed, 325 insertions(+)

diff --git a/Makefile.local b/Makefile.local
index a890df2..296995d 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -290,6 +290,8 @@ notmuch_client_srcs =		\
 	notmuch-show.c		\
 	notmuch-tag.c		\
 	notmuch-time.c		\
+	sprinter-json.c		\
+	sprinter-text.c		\
 	query-string.c		\
 	mime-node.c		\
 	crypto.c		\
diff --git a/sprinter-json.c b/sprinter-json.c
new file mode 100644
index 0000000..4649655
--- /dev/null
+++ b/sprinter-json.c
@@ -0,0 +1,187 @@
+#include <stdbool.h>
+#include <stdio.h>
+#include <talloc.h>
+#include "sprinter.h"
+
+struct sprinter_json {
+    struct sprinter vtable;
+    FILE *stream;
+    /* Top of the state stack, or NULL if the printer is not currently
+     * inside any aggregate types. */
+    struct json_state *state;
+
+    /* A flag to signify that a separator should be inserted in the
+     * output as soon as possible.
+     */
+    notmuch_bool_t insert_separator;
+};
+
+struct json_state {
+    struct json_state *parent;
+    /* True if nothing has been printed in this aggregate yet.
+     * Suppresses the comma before a value. */
+    notmuch_bool_t first;
+    /* The character that closes the current aggregate. */
+    char close;
+};
+
+/* Helper function to set up the stream to print a value.  If this
+ * value follows another value, prints a comma. */
+static struct sprinter_json *
+json_begin_value (struct sprinter *sp)
+{
+    struct sprinter_json *spj = (struct sprinter_json *) sp;
+
+    if (spj->state) {
+	if (! spj->state->first) {
+	    fputc (',', spj->stream);
+	    if (spj->insert_separator) {
+		fputc ('\n', spj->stream);
+		spj->insert_separator = FALSE;
+	    } else {
+		fputc (' ', spj->stream);
+	    }
+	} else {
+	    spj->state->first = FALSE;
+	}
+    }
+    return spj;
+}
+
+/* Helper function to begin an aggregate type.  Prints the open
+ * character and pushes a new state frame. */
+static void
+json_begin_aggregate (struct sprinter *sp, char open, char close)
+{
+    struct sprinter_json *spj = json_begin_value (sp);
+    struct json_state *state = talloc (spj, struct json_state);
+
+    fputc (open, spj->stream);
+    state->parent = spj->state;
+    state->first = TRUE;
+    state->close = close;
+    spj->state = state;
+}
+
+static void
+json_begin_map (struct sprinter *sp)
+{
+    json_begin_aggregate (sp, '{', '}');
+}
+
+static void
+json_begin_list (struct sprinter *sp)
+{
+    json_begin_aggregate (sp, '[', ']');
+}
+
+static void
+json_end (struct sprinter *sp)
+{
+    struct sprinter_json *spj = (struct sprinter_json *) sp;
+    struct json_state *state = spj->state;
+
+    fputc (spj->state->close, spj->stream);
+    spj->state = state->parent;
+    talloc_free (state);
+    if (spj->state == NULL)
+	fputc ('\n', spj->stream);
+}
+
+static void
+json_string (struct sprinter *sp, const char *val)
+{
+    static const char *const escapes[] = {
+	['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
+	['\f'] = "\\f",  ['\n'] = "\\n",  ['\t'] = "\\t"
+    };
+    struct sprinter_json *spj = json_begin_value (sp);
+
+    fputc ('"', spj->stream);
+    for (; *val; ++val) {
+	unsigned char ch = *val;
+	if (ch < ARRAY_SIZE (escapes) && escapes[ch])
+	    fputs (escapes[ch], spj->stream);
+	else if (ch >= 32)
+	    fputc (ch, spj->stream);
+	else
+	    fprintf (spj->stream, "\\u%04x", ch);
+    }
+    fputc ('"', spj->stream);
+}
+
+static void
+json_integer (struct sprinter *sp, int val)
+{
+    struct sprinter_json *spj = json_begin_value (sp);
+
+    fprintf (spj->stream, "%d", val);
+}
+
+static void
+json_boolean (struct sprinter *sp, notmuch_bool_t val)
+{
+    struct sprinter_json *spj = json_begin_value (sp);
+
+    fputs (val ? "true" : "false", spj->stream);
+}
+
+static void
+json_null (struct sprinter *sp)
+{
+    struct sprinter_json *spj = json_begin_value (sp);
+
+    fputs ("null", spj->stream);
+}
+
+static void
+json_map_key (struct sprinter *sp, const char *key)
+{
+    struct sprinter_json *spj = (struct sprinter_json *) sp;
+
+    json_string (sp, key);
+    fputs (": ", spj->stream);
+    spj->state->first = TRUE;
+}
+
+static void
+json_set_prefix (unused (struct sprinter *sp), unused (const char *name))
+{
+}
+
+static void
+json_separator (struct sprinter *sp)
+{
+    struct sprinter_json *spj = (struct sprinter_json *) sp;
+
+    spj->insert_separator = TRUE;
+}
+
+struct sprinter *
+sprinter_json_create (const void *ctx, FILE *stream)
+{
+    static const struct sprinter_json template = {
+	.vtable = {
+	    .begin_map = json_begin_map,
+	    .begin_list = json_begin_list,
+	    .end = json_end,
+	    .string = json_string,
+	    .integer = json_integer,
+	    .boolean = json_boolean,
+	    .null = json_null,
+	    .map_key = json_map_key,
+	    .separator = json_separator,
+	    .set_prefix = json_set_prefix,
+	    .is_text_printer = FALSE,
+	}
+    };
+    struct sprinter_json *res;
+
+    res = talloc (ctx, struct sprinter_json);
+    if (! res)
+	return NULL;
+
+    *res = template;
+    res->stream = stream;
+    return &res->vtable;
+}
diff --git a/sprinter-text.c b/sprinter-text.c
new file mode 100644
index 0000000..b208840
--- /dev/null
+++ b/sprinter-text.c
@@ -0,0 +1,126 @@
+#include <stdbool.h>
+#include <stdio.h>
+#include <talloc.h>
+#include "sprinter.h"
+
+/* "Structured printer" interface for unstructured text printing.
+ * Note that --output=summary is dispatched and formatted in
+ * notmuch-search.c, the code in this file is only used for all other
+ * output types.
+ */
+
+struct sprinter_text {
+    struct sprinter vtable;
+    FILE *stream;
+
+    /* The current prefix to be printed with string/integer/boolean
+     * data.
+     */
+    const char *current_prefix;
+
+    /* A flag to indicate if this is the first tag. Used in list of tags
+     * for summary.
+     */
+    notmuch_bool_t first_tag;
+};
+
+static void
+text_string (struct sprinter *sp, const char *val)
+{
+    struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+    if (sptxt->current_prefix != NULL)
+	fprintf (sptxt->stream, "%s:", sptxt->current_prefix);
+
+    fputs(val, sptxt->stream);
+}
+
+static void
+text_integer (struct sprinter *sp, int val)
+{
+    struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+    fprintf (sptxt->stream, "%d", val);
+}
+
+static void
+text_boolean (struct sprinter *sp, notmuch_bool_t val)
+{
+    struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+    fputs (val ? "true" : "false", sptxt->stream);
+}
+
+static void
+text_separator (struct sprinter *sp)
+{
+    struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+    fputc ('\n', sptxt->stream);
+}
+
+static void
+text_set_prefix (struct sprinter *sp, const char *prefix)
+{
+    struct sprinter_text *sptxt = (struct sprinter_text *) sp;
+
+    sptxt->current_prefix = prefix;
+}
+
+/* The structure functions begin_map, begin_list, end and map_key
+ * don't do anything in the text formatter.
+ */
+
+static void
+text_begin_map (unused (struct sprinter *sp))
+{
+}
+
+static void
+text_begin_list (unused (struct sprinter *sp))
+{
+}
+
+static void
+text_end (unused (struct sprinter *sp))
+{
+}
+
+static void
+text_null (unused (struct sprinter *sp))
+{
+}
+
+static void
+text_map_key (unused (struct sprinter *sp), unused (const char *key))
+{
+}
+
+struct sprinter *
+sprinter_text_create (const void *ctx, FILE *stream)
+{
+    static const struct sprinter_text template = {
+	.vtable = {
+	    .begin_map = text_begin_map,
+	    .begin_list = text_begin_list,
+	    .end = text_end,
+	    .string = text_string,
+	    .integer = text_integer,
+	    .boolean = text_boolean,
+	    .null = text_null,
+	    .map_key = text_map_key,
+	    .separator = text_separator,
+	    .set_prefix = text_set_prefix,
+	    .is_text_printer = TRUE,
+	},
+    };
+    struct sprinter_text *res;
+
+    res = talloc (ctx, struct sprinter_text);
+    if (! res)
+	return NULL;
+
+    *res = template;
+    res->stream = stream;
+    return &res->vtable;
+}
diff --git a/sprinter.h b/sprinter.h
index 77dc26f..6680d41 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -55,4 +55,14 @@ typedef struct sprinter {
     notmuch_bool_t is_text_printer;
 } sprinter_t;
 
+
+/* Create a new unstructured printer that emits the default text format
+ * for "notmuch search". */
+struct sprinter *
+sprinter_text_create (const void *ctx, FILE *stream);
+
+/* Create a new structure printer that emits JSON. */
+struct sprinter *
+sprinter_json_create (const void *ctx, FILE *stream);
+
 #endif // NOTMUCH_SPRINTER_H
-- 
1.7.11.2

[PATCH v8 3/3] Use the structured formatters in notmuch-search.c.

[craven]

From: <craven@gmx.net>

This patch switches from the current ad-hoc printer to the structured
formatters in sprinter.h, sprinter-text.c and sprinter-json.c.

The JSON tests are changed slightly in order to make them PASS for the
new structured output formatter.

The text tests pass without adaptation.
---
 notmuch-search.c | 304 ++++++++++++++---------------------------------
 test/json        |  34 +++---
 2 files changed, 106 insertions(+), 232 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 3be296d..830c4e4 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -19,6 +19,7 @@
  */
 
 #include "notmuch-client.h"
+#include "sprinter.h"
 
 typedef enum {
     OUTPUT_SUMMARY,
@@ -28,92 +29,6 @@ typedef enum {
     OUTPUT_TAGS
 } output_t;
 
-typedef struct search_format {
-    const char *results_start;
-    const char *item_start;
-    void (*item_id) (const void *ctx,
-		     const char *item_type,
-		     const char *item_id);
-    void (*thread_summary) (const void *ctx,
-			    const char *thread_id,
-			    const time_t date,
-			    const int matched,
-			    const int total,
-			    const char *authors,
-			    const char *subject);
-    const char *tag_start;
-    const char *tag;
-    const char *tag_sep;
-    const char *tag_end;
-    const char *item_sep;
-    const char *item_end;
-    const char *results_end;
-    const char *results_null;
-} search_format_t;
-
-static void
-format_item_id_text (const void *ctx,
-		     const char *item_type,
-		     const char *item_id);
-
-static void
-format_thread_text (const void *ctx,
-		    const char *thread_id,
-		    const time_t date,
-		    const int matched,
-		    const int total,
-		    const char *authors,
-		    const char *subject);
-static const search_format_t format_text = {
-    "",
-	"",
-	    format_item_id_text,
-	    format_thread_text,
-	    " (",
-		"%s", " ",
-	    ")", "\n",
-	"",
-    "\n",
-    "",
-};
-
-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)
-{
-    printf ("%s%s", item_type, item_id);
-}
-
 static char *
 sanitize_string (const void *ctx, const char *str)
 {
@@ -131,72 +46,8 @@ sanitize_string (const void *ctx, const char *str)
     return out;
 }
 
-static void
-format_thread_text (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 %12s [%d/%d] %s; %s",
-	    thread_id,
-	    notmuch_time_relative_date (ctx, date),
-	    matched,
-	    total,
-	    sanitize_string (ctx_quote, authors),
-	    sanitize_string (ctx_quote, subject));
-
-    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 (sprinter_t *format,
 		   notmuch_query_t *query,
 		   notmuch_sort_t sort,
 		   output_t output,
@@ -207,7 +58,6 @@ do_search_threads (const search_format_t *format,
     notmuch_threads_t *threads;
     notmuch_tags_t *tags;
     time_t date;
-    int first_thread = 1;
     int i;
 
     if (offset < 0) {
@@ -220,14 +70,12 @@ do_search_threads (const search_format_t *format,
     if (threads == NULL)
 	return 1;
 
-    fputs (format->results_start, stdout);
+    format->begin_list (format);
 
     for (i = 0;
 	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
 	 notmuch_threads_move_to_next (threads), i++)
     {
-	int first_tag = 1;
-
 	thread = notmuch_threads_get (threads);
 
 	if (i < offset) {
@@ -235,60 +83,97 @@ do_search_threads (const search_format_t *format,
 	    continue;
 	}
 
-	if (! first_thread)
-	    fputs (format->item_sep, stdout);
-
 	if (output == OUTPUT_THREADS) {
-	    format->item_id (thread, "thread:",
-			     notmuch_thread_get_thread_id (thread));
+	    format->set_prefix (format, "thread");
+	    format->string (format,
+			    notmuch_thread_get_thread_id (thread));
+	    format->separator (format);
 	} else { /* output == OUTPUT_SUMMARY */
-	    fputs (format->item_start, stdout);
+	    void *ctx_quote = talloc_new (thread);
+	    const char *authors = notmuch_thread_get_authors (thread);
+	    const char *subject = notmuch_thread_get_subject (thread);
+	    const char *thread_id = notmuch_thread_get_thread_id (thread);
+	    int matched = notmuch_thread_get_matched_messages (thread);
+	    int total = notmuch_thread_get_total_messages (thread);
+	    const char *relative_date = NULL;
+	    notmuch_bool_t first_tag = TRUE;
+
+	    format->begin_map (format);
 
 	    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));
+	    relative_date = notmuch_time_relative_date (ctx_quote, date);
+
+	    if (format->is_text_printer) {
+                /* Special case for the text formatter */
+		printf ("thread:%s %12s [%d/%d] %s; %s (",
+			thread_id,
+			relative_date,
+			matched,
+			total,
+			sanitize_string (ctx_quote, authors),
+			sanitize_string (ctx_quote, subject));
+	    } else { /* Structured Output */
+		format->map_key (format, "thread");
+		format->string (format, thread_id);
+		format->map_key (format, "timestamp");
+		format->integer (format, date);
+		format->map_key (format, "date_relative");
+		format->string (format, relative_date);
+		format->map_key (format, "matched");
+		format->integer (format, matched);
+		format->map_key (format, "total");
+		format->integer (format, total);
+		format->map_key (format, "authors");
+		format->string (format, authors);
+		format->map_key (format, "subject");
+		format->string (format, subject);
+	    }
+
+	    talloc_free (ctx_quote);
 
-	    fputs (format->tag_start, stdout);
+	    format->map_key (format, "tags");
+	    format->begin_list (format);
 
 	    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));
-		first_tag = 0;
+		const char *tag = notmuch_tags_get (tags);
+
+		if (format->is_text_printer) {
+                  /* Special case for the text formatter */
+		    if (first_tag)
+			first_tag = FALSE;
+		    else
+			fputc (' ', stdout);
+		    fputs (tag, stdout);
+		} else { /* Structured Output */
+		    format->string (format, tag);
+		}
 	    }
 
-	    fputs (format->tag_end, stdout);
+	    if (format->is_text_printer)
+		printf (")");
 
-	    fputs (format->item_end, stdout);
+	    format->end (format);
+	    format->end (format);
+	    format->separator (format);
 	}
 
-	first_thread = 0;
-
 	notmuch_thread_destroy (thread);
     }
 
-    if (first_thread)
-	fputs (format->results_null, stdout);
-    else
-	fputs (format->results_end, stdout);
+    format->end (format);
 
     return 0;
 }
 
 static int
-do_search_messages (const search_format_t *format,
+do_search_messages (sprinter_t *format,
 		    notmuch_query_t *query,
 		    output_t output,
 		    int offset,
@@ -297,7 +182,6 @@ do_search_messages (const search_format_t *format,
     notmuch_message_t *message;
     notmuch_messages_t *messages;
     notmuch_filenames_t *filenames;
-    int first_message = 1;
     int i;
 
     if (offset < 0) {
@@ -310,7 +194,7 @@ do_search_messages (const search_format_t *format,
     if (messages == NULL)
 	return 1;
 
-    fputs (format->results_start, stdout);
+    format->begin_list (format);
 
     for (i = 0;
 	 notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit);
@@ -328,24 +212,17 @@ 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);
-
-		format->item_id (message, "",
-				 notmuch_filenames_get (filenames));
-
-		first_message = 0;
+		format->string (format, notmuch_filenames_get (filenames));
+		format->separator (format);
 	    }
 	    
 	    notmuch_filenames_destroy( filenames );
 
 	} else { /* output == OUTPUT_MESSAGES */
-	    if (! first_message)
-		fputs (format->item_sep, stdout);
-
-	    format->item_id (message, "id:",
-			     notmuch_message_get_message_id (message));
-	    first_message = 0;
+	    format->set_prefix (format, "id");
+	    format->string (format,
+			    notmuch_message_get_message_id (message));
+	    format->separator (format);
 	}
 
 	notmuch_message_destroy (message);
@@ -353,23 +230,19 @@ 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);
+    format->end (format);
 
     return 0;
 }
 
 static int
 do_search_tags (notmuch_database_t *notmuch,
-		const search_format_t *format,
+		sprinter_t *format,
 		notmuch_query_t *query)
 {
     notmuch_messages_t *messages = NULL;
     notmuch_tags_t *tags;
     const char *tag;
-    int first_tag = 1;
 
     /* should the following only special case if no excluded terms
      * specified? */
@@ -387,7 +260,7 @@ do_search_tags (notmuch_database_t *notmuch,
     if (tags == NULL)
 	return 1;
 
-    fputs (format->results_start, stdout);
+    format->begin_list (format);
 
     for (;
 	 notmuch_tags_valid (tags);
@@ -395,12 +268,9 @@ do_search_tags (notmuch_database_t *notmuch,
     {
 	tag = notmuch_tags_get (tags);
 
-	if (! first_tag)
-	    fputs (format->item_sep, stdout);
-
-	format->item_id (tags, "", tag);
+	format->string (format, tag);
+	format->separator (format);
 
-	first_tag = 0;
     }
 
     notmuch_tags_destroy (tags);
@@ -408,10 +278,7 @@ 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);
+    format->end (format);
 
     return 0;
 }
@@ -430,7 +297,7 @@ 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;
+    sprinter_t *format = NULL;
     int opt_index, ret;
     output_t output = OUTPUT_SUMMARY;
     int offset = 0;
@@ -475,11 +342,14 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
 
     switch (format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	format = &format_text;
+	format = sprinter_text_create (ctx, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	format = &format_json;
+	format = sprinter_json_create (ctx, stdout);
 	break;
+    default:
+	/* this should never happen */
+	INTERNAL_ERROR("no output format selected");
     }
 
     config = notmuch_config_open (ctx, NULL, NULL);
@@ -546,5 +416,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
 
+    talloc_free (format);
+
     return ret;
 }
diff --git a/test/json b/test/json
index 6439788..337b3f5 100755
--- a/test/json
+++ b/test/json
@@ -9,15 +9,16 @@ test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"e
 
 test_begin_subtest "Search message: json"
 add_message "[subject]=\"json-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"json-search-message\""
-output=$(notmuch search --format=json "json-search-message" | notmuch_search_sanitize)
+output=$(notmuch search --format=json "json-search-message" | notmuch_json_show_sanitize | notmuch_search_sanitize)
 test_expect_equal "$output" "[{\"thread\": \"XXX\",
-\"timestamp\": 946728000,
-\"date_relative\": \"2000-01-01\",
-\"matched\": 1,
-\"total\": 1,
-\"authors\": \"Notmuch Test Suite\",
-\"subject\": \"json-search-subject\",
-\"tags\": [\"inbox\", \"unread\"]}]"
+ \"timestamp\": 946728000,
+ \"date_relative\": \"2000-01-01\",
+ \"matched\": 1,
+ \"total\": 1,
+ \"authors\": \"Notmuch Test Suite\",
+ \"subject\": \"json-search-subject\",
+ \"tags\": [\"inbox\",
+ \"unread\"]}]"
 
 test_begin_subtest "Show message: json, utf-8"
 add_message "[subject]=\"json-show-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-show-méssage\""
@@ -39,14 +40,15 @@ test_expect_equal "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\":
 
 test_begin_subtest "Search message: json, utf-8"
 add_message "[subject]=\"json-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
-output=$(notmuch search --format=json "jsön-search-méssage" | notmuch_search_sanitize)
+output=$(notmuch search --format=json "jsön-search-méssage" | notmuch_json_show_sanitize | notmuch_search_sanitize)
 test_expect_equal "$output" "[{\"thread\": \"XXX\",
-\"timestamp\": 946728000,
-\"date_relative\": \"2000-01-01\",
-\"matched\": 1,
-\"total\": 1,
-\"authors\": \"Notmuch Test Suite\",
-\"subject\": \"json-search-utf8-body-sübjéct\",
-\"tags\": [\"inbox\", \"unread\"]}]"
+ \"timestamp\": 946728000,
+ \"date_relative\": \"2000-01-01\",
+ \"matched\": 1,
+ \"total\": 1,
+ \"authors\": \"Notmuch Test Suite\",
+ \"subject\": \"json-search-utf8-body-sübjéct\",
+ \"tags\": [\"inbox\",
+ \"unread\"]}]"
 
 test_done
-- 
1.7.11.2

Re: [PATCH v8 0/3] notmuch-search: Structured Output Formatters

[Austin Clements]

LGTM

Quoth craven@gmx.net on Jul 23 at 12:39 pm:
> From: <craven@gmx.net>
> 
> Currently there is no easy way to add support for different structured
> formatters (like JSON). For example, adding support for S-Expressions
> would result in code duplication.
> 
> This patch series amends the situation by introducing structured
> formatters, which allow different implementations of structures like
> lists, maps, strings and numbers.
> 
> The new code in sprinter.h and sprinter-json.c can be used instead of
> the current ad-hoc output in all parts of notmuch, a patch for
> notmuch-search.c is included.
> 
> In a later patch, all other parts of notmuch should be adapted to the
> structured formatters, and the creation of formatters should be
> centralised (to make adding new formatters easier).
> 
> A "structured" formatter is provided for notmuch-search that prints the
> current text format. This removes almost all the special-casing from
> notmuch-search.c.
> 
> Changes versus v7 of this patch:
> - added {} around "else" blocks (as mentioned in
>   id:20120722160843.GC31834@mit.edu)
> - added fallback to INTERNAL_ERROR (which should never be called) in
>   notmuch-search.c if format is unknown (as mentioned in
>   id:m2r4s694ly.fsf@guru.guru-group.fi).
> 
> Summary: 
> 
> Peter Feigl (3):
>   Add support for structured output formatters.
>   Add structured output formatter for JSON and plain text (but don't
>     use them yet).
>   Use the structured formatters in notmuch-search.c.
> 
>  Makefile.local   |   2 +
>  notmuch-search.c | 304 ++++++++++++++---------------------------------
>  sprinter-json.c  | 187 +++++++++++++++++++++++++++++
>  sprinter-text.c  | 126 ++++++++++++++++++++
>  sprinter.h       |  68 +++++++++++
>  test/json        |  34 +++---
>  6 files changed, 489 insertions(+), 232 deletions(-)
>  create mode 100644 sprinter-json.c
>  create mode 100644 sprinter-text.c
>  create mode 100644 sprinter.h
> 

Re: [PATCH v8 0/3] notmuch-search: Structured Output Formatters

[Tomi Ollila]

On Mon, Jul 23 2012, craven@gmx.net wrote:

> From: <craven@gmx.net>
>
> Currently there is no easy way to add support for different structured
> formatters (like JSON). For example, adding support for S-Expressions
> would result in code duplication.
>
> This patch series amends the situation by introducing structured
> formatters, which allow different implementations of structures like
> lists, maps, strings and numbers.

LGTM

Tomi

>
> The new code in sprinter.h and sprinter-json.c can be used instead of
> the current ad-hoc output in all parts of notmuch, a patch for
> notmuch-search.c is included.
>
> In a later patch, all other parts of notmuch should be adapted to the
> structured formatters, and the creation of formatters should be
> centralised (to make adding new formatters easier).
>
> A "structured" formatter is provided for notmuch-search that prints the
> current text format. This removes almost all the special-casing from
> notmuch-search.c.
>
> Changes versus v7 of this patch:
> - added {} around "else" blocks (as mentioned in
>   id:20120722160843.GC31834@mit.edu)
> - added fallback to INTERNAL_ERROR (which should never be called) in
>   notmuch-search.c if format is unknown (as mentioned in
>   id:m2r4s694ly.fsf@guru.guru-group.fi).
>
> Summary: 
>
> Peter Feigl (3):
>   Add support for structured output formatters.
>   Add structured output formatter for JSON and plain text (but don't
>     use them yet).
>   Use the structured formatters in notmuch-search.c.
>
>  Makefile.local   |   2 +
>  notmuch-search.c | 304 ++++++++++++++---------------------------------
>  sprinter-json.c  | 187 +++++++++++++++++++++++++++++
>  sprinter-text.c  | 126 ++++++++++++++++++++
>  sprinter.h       |  68 +++++++++++
>  test/json        |  34 +++---
>  6 files changed, 489 insertions(+), 232 deletions(-)
>  create mode 100644 sprinter-json.c
>  create mode 100644 sprinter-text.c
>  create mode 100644 sprinter.h
>
> -- 
> 1.7.11.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v8 0/3] notmuch-search: Structured Output Formatters

[David Bremner]

craven@gmx.net writes:

> This patch series amends the situation by introducing structured
> formatters, which allow different implementations of structures like
> lists, maps, strings and numbers.
>

pushed, 

d