Re: [PATCH v5 1/3] Add support for structured output formatters.

Subject: Re: [PATCH v5 1/3] Add support for structured output formatters.

Date: Fri, 13 Jul 2012 21:46:29 -0400

To: Peter Feigl

Cc: notmuch@notmuchmail.org

From: Austin Clements


Quoth Peter Feigl on Jul 13 at 10:11 am:
> From: <craven@gmx.net>
> 
> This patch adds a new type sprinter_t, which is used for structured
> formatting, e.g. JSON or S-Expressions. The structure printer is the
> code from Austin Clements (id:87d34hsdx8.fsf@awakening.csail.mit.edu)
> with minor modifications.
> 
> The structure printer contains the following function pointers:
> 
> /* 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 *);
> 
> /* Set the prefix of the next component. This is purely for
>  * debugging purposes and for the unstructured text formatter.
>  */
> void (*set_prefix)(struct sprinter *, const char *);
> 
> /* 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 *);
> 
> 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 | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 sprinter.h
> 
> diff --git a/sprinter.h b/sprinter.h
> new file mode 100644
> index 0000000..c9cd6a6
> --- /dev/null
> +++ b/sprinter.h
> @@ -0,0 +1,50 @@
> +#ifndef NOTMUCH_SPRINTER_H
> +#define NOTMUCH_SPRINTER_H
> +
> +/* Necessary for notmuch_bool_t */
> +#include "notmuch-client.h"
> +
> +/* Structure printer interface */
> +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 *);

It looks like you lost the spaces before the argument lists, which
your previous version had.  Was this from uncrustify?  The few other
places that notmuch had function pointers do put a space here.

> +
> +    /* Start a new list/array structure.
> +     */
> +    void (*begin_list)(struct sprinter *);
> +
> +    /* End the last opened list or map structure.
> +     */
> +    void (*end)(struct sprinter *);
> +
> +    /* Set the prefix of the next component. This is purely for
> +     * debugging purposes and for the unstructured text formatter.

This should probably be more specific about what this operation does
and under what circumstances.  It also makes it sound like it only
applies to the next component, which I think is not the case.  Maybe

/* 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 *);
> +
> +    /* 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 *);
> +} sprinter_t;
> +
> +#endif // NOTMUCH_SPRINTER_H

Thread: