headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 00/13] Convert notmuch show to use structure printers

Relative to version 2 [0], this version fixes a duplicate typedef of
struct sprinter that was not standards compliant and failed to compile
on older gccs [1] (thanks to Tomi for discovering that and figuring
out what was going on).

The diff relative to version 2 follows.

[0] id:"1343449754-9010-1-git-send-email-amdragon@..."

[1] http://stackoverflow.com/questions/8594954/repeated-typedefs-invalid-in-c-but-valid-in-c

diff --git a/notmuch-client.h b/notmuch-client.h
index de31aa1..ae9344b 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
 <at>  <at>  -66,12 +66,12  <at>  <at>  typedef GMimeCipherContext notmuch_crypto_context_t;
 #define STRINGIFY_(s) #s

 typedef struct mime_node mime_node_t;
-typedef struct sprinter sprinter_t;
+struct sprinter;
 struct notmuch_show_params;

 typedef struct notmuch_show_format {
-    sprinter_t *(*new_sprinter) (const void *ctx, FILE *stream);
-    notmuch_status_t (*part) (const void *ctx, sprinter_t *sprinter,
+    struct sprinter *(*new_sprinter) (const void *ctx, FILE *stream);
+    notmuch_status_t (*part) (const void *ctx, struct sprinter *sprinter,
 			      struct mime_node *node, int indent,
 			      const struct notmuch_show_params *params);
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 10/13] show: Convert envelope format_part_json to use sprinter

---
 notmuch-show.c |   57 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index afbd9d0..b9d9f5d 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
 <at>  <at>  -109,35 +109,45  <at>  <at>  _get_one_line_summary (const void *ctx, notmuch_message_t *message)
 			    from, relative_date, tags);
 }

+/* Emit a sequence of key/value pairs for the metadata of message.
+ * The caller should begin a map before calling this. */
 static void
-format_message_json (const void *ctx, notmuch_message_t *message)
+format_message_json (sprinter_t *sp, notmuch_message_t *message)
 {
+    void *local = talloc_new (NULL);
     notmuch_tags_t *tags;
-    int first = 1;
-    void *ctx_quote = talloc_new (ctx);
     time_t date;
     const char *relative_date;

+    sp->map_key (sp, "id");
+    sp->string (sp, notmuch_message_get_message_id (message));
+
+    sp->map_key (sp, "match");
+    sp->boolean (sp, notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH));
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 08/13] show: Convert format_part_sigstatus_json to use sprinter

---
 notmuch-show.c |  118 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 61 insertions(+), 57 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 9852119..3ff32df 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
 <at>  <at>  -337,134 +337,138  <at>  <at>  signer_status_to_string (GMimeSignerStatus x)

 #ifdef GMIME_ATLEAST_26
 static void
-format_part_sigstatus_json (mime_node_t *node)
+format_part_sigstatus_json (sprinter_t *sp, mime_node_t *node)
 {
     GMimeSignatureList *siglist = node->sig_list;

-    printf ("[");
+    sp->begin_list (sp);

     if (!siglist) {
-	printf ("]");
+	sp->end (sp);
 	return;
     }

-    void *ctx_quote = talloc_new (NULL);
     int i;
     for (i = 0; i < g_mime_signature_list_length (siglist); i++) {
 	GMimeSignature *signature = g_mime_signature_list_get_signature (siglist, i);
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 05/13] reply: Create a JSON sprinter

---
 notmuch-reply.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index de21f3b..e42ba79 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
 <at>  <at>  -22,6 +22,7  <at>  <at> 

 #include "notmuch-client.h"
 #include "gmime-filter-headers.h"
+#include "sprinter.h"

 static void
 show_reply_headers (GMimeMessage *message)
 <at>  <at>  -596,6 +597,7  <at>  <at>  notmuch_reply_format_json(void *ctx,
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     mime_node_t *node;
+    sprinter_t *sp;

     if (notmuch_query_count_messages (query) != 1) {
 	fprintf (stderr, "Error: search term did not match precisely one message.\n");
 <at>  <at>  -611,6 +613,8  <at>  <at>  notmuch_reply_format_json(void *ctx,
     if (!reply)
 	return 1;

+    sp = sprinter_json_create (ctx, stdout);
+
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 03/13] sprinter: Add a string_len method

This method allows callers to output strings with specific lengths.
It's useful both for strings with embedded NULs (which JSON can
represent, though parser support is apparently spotty), and
non-terminated strings.
---
 sprinter-json.c |   16 ++++++++++++++--
 sprinter-text.c |   11 +++++++++--
 sprinter.h      |    9 ++++++---
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/sprinter-json.c b/sprinter-json.c
index 4649655..c9b6835 100644
--- a/sprinter-json.c
+++ b/sprinter-json.c
 <at>  <at>  -88,8 +88,13  <at>  <at>  json_end (struct sprinter *sp)
 	fputc ('\n', spj->stream);
 }

+/* This implementation supports embedded NULs as allowed by the JSON
+ * specification and Unicode.  Support for *parsing* embedded NULs
+ * varies, but is generally not a problem outside of C-based parsers
+ * (Python's json module and Emacs' json.el take embedded NULs in
+ * stride). */
 static void
-json_string (struct sprinter *sp, const char *val)
+json_string_len (struct sprinter *sp, const char *val, size_t len)
 {
     static const char *const escapes[] = {
 	['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
 <at>  <at>  -98,7 +103,7  <at>  <at>  json_string (struct sprinter *sp, const char *val)
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 02/13] test: Remove unnecessary JSON canonicalization

Format canonicalization of JSON output is no longer necessary, so
remove it.  Value canonicalization (e.g., normalizing thread IDs) is
still necessary, so all of the sanitization functions remain.
---
 test/json         |    4 ++--
 test/maildir-sync |    1 -
 test/multipart    |   40 ++++++++++------------------------------
 3 files changed, 12 insertions(+), 33 deletions(-)

diff --git a/test/json b/test/json
index d86ee46..ac8fa8e 100755
--- a/test/json
+++ b/test/json
 <at>  <at>  -18,7 +18,7  <at>  <at>  test_expect_equal_json "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true

 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_json_show_sanitize | notmuch_search_sanitize)
+output=$(notmuch search --format=json "json-search-message" | notmuch_search_sanitize)
 test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"timestamp\": 946728000,
  \"date_relative\": \"2000-01-01\",
 <at>  <at>  -49,7 +49,7  <at>  <at>  test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"exclud

 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_json_show_sanitize | notmuch_search_sanitize)
+output=$(notmuch search --format=json "jsön-search-méssage" | notmuch_search_sanitize)
 test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 06/13] show: Feed the sprinter down to part formatters

There are several levels of function calls between where we create the
sprinter and the call to the part formatter in show_message. This
feeds the sprinter through all of them and into the part formatters.
---
 notmuch-client.h |    6 ++++--
 notmuch-reply.c  |    2 +-
 notmuch-show.c   |   50 +++++++++++++++++++++++++++++---------------------
 3 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index bbc0a11..112574c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
 <at>  <at>  -66,12 +66,13  <at>  <at>  typedef GMimeCipherContext notmuch_crypto_context_t;
 #define STRINGIFY_(s) #s

 typedef struct mime_node mime_node_t;
+struct sprinter;
 struct notmuch_show_params;

 typedef struct notmuch_show_format {
     struct sprinter *(*new_sprinter) (const void *ctx, FILE *stream);
     const char *message_set_start;
-    notmuch_status_t (*part) (const void *ctx,
+    notmuch_status_t (*part) (const void *ctx, struct sprinter *sprinter,
 			      struct mime_node *node, int indent,
 			      const struct notmuch_show_params *params);
     const char *message_set_sep;
 <at>  <at>  -178,7 +179,8  <at>  <at>  notmuch_status_t
 show_one_part (const char *filename, int part);
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 09/13] show: Convert non-envelope format_part_json to use sprinter

---
 notmuch-show.c |   74 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 28 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 3ff32df..afbd9d0 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
 <at>  <at>  -588,7 +588,6  <at>  <at>  format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	return;
     }

-    void *local = talloc_new (ctx);
     /* The disposition and content-type metadata are associated with
      * the envelope for message parts */
     GMimeObject *meta = node->envelope_part ?
 <at>  <at>  -597,31 +596,41  <at>  <at>  format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
     const char *cid = g_mime_object_get_content_id (meta);
     const char *filename = GMIME_IS_PART (node->part) ?
 	g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
-    const char *terminator = "";
+    int nclose = 0;
     int i;

-    if (!first)
-	printf (", ");
+    sp->begin_map (sp);

-    printf ("{\"id\": %d", node->part_num);
+    sp->map_key (sp, "id");
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 01/13] test: Uniformly canonicalize actual and expected JSON

Previously, we used a variety of ad-hoc canonicalizations for JSON
output in the test suite, but were ultimately very sensitive to JSON
irrelevancies such as whitespace.  This introduces a new test
comparison function, test_expect_equal_json, that first pretty-prints
*both* the actual and expected JSON and the compares the result.

The current implementation of this simply uses Python's json.tool to
perform pretty-printing (with a fallback to the identity function if
parsing fails).  However, since the interface it introduces is
semantically high-level, we could swap in other mechanisms in the
future, such as another pretty-printer or something that does not
re-order object keys (if we decide that we care about that).

In general, this patch does not remove the existing ad-hoc
canonicalization because it does no harm.  We do have to remove the
newline-after-comma rule from notmuch_json_show_sanitize and
filter_show_json because it results in invalid JSON that cannot be
pretty-printed.

Most of this patch simply replaces test_expect_equal and
test_expect_equal_file with test_expect_equal_json.  It changes the
expected JSON in a few places where sanitizers had placed newlines
after commas inside strings.
---
 test/crypto        |   37 +++++++++++++++----------------------
 test/json          |   14 +++++++-------
 test/maildir-sync  |   11 ++++-------
 test/multipart     |   34 +++++++++++++++-------------------
 test/search-output |    2 +-
 test/test-lib.sh   |   17 +++++++++++++----
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 07/13] show: Convert format_headers_json to use sprinter

This no longer requires a talloc context (not that it really did
before since it didn't return anything), so we remove its context
argument.
---
 notmuch-client.h |    3 ++-
 notmuch-reply.c  |    2 +-
 notmuch-show.c   |   58 ++++++++++++++++++++++++++----------------------------
 3 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 112574c..b11caff 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
 <at>  <at>  -183,7 +183,8  <at>  <at>  format_part_json (const void *ctx, struct sprinter *sp, mime_node_t *node,
 		  notmuch_bool_t first, notmuch_bool_t output_body);

 void
-format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply);
+format_headers_json (struct sprinter *sp, GMimeMessage *message,
+		     notmuch_bool_t reply);

 typedef enum {
     NOTMUCH_SHOW_TEXT_PART_REPLY = 1 << 0,
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 07d4452..fa6665f 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
 <at>  <at>  -617,7 +617,7  <at>  <at>  notmuch_reply_format_json(void *ctx,

     /* The headers of the reply message we've created */
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 04/13] show: Associate an sprinter with each format

This associates an sprinter constructor with each show format and uses
this to construct the appropriate sprinter.  Currently nothing is done
with this sprinter, but the following patches will weave it through
the layers of notmuch show.
---
 notmuch-client.h |    1 +
 notmuch-show.c   |    9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index f930798..bbc0a11 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
 <at>  <at>  -69,6 +69,7  <at>  <at>  typedef struct mime_node mime_node_t;
 struct notmuch_show_params;

 typedef struct notmuch_show_format {
+    struct sprinter *(*new_sprinter) (const void *ctx, FILE *stream);
     const char *message_set_start;
     notmuch_status_t (*part) (const void *ctx,
 			      struct mime_node *node, int indent,
diff --git a/notmuch-show.c b/notmuch-show.c
index d3419e4..d04943f 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
 <at>  <at>  -20,12 +20,14  <at>  <at> 

 #include "notmuch-client.h"
 #include "gmime-filter-reply.h"
+#include "sprinter.h"
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 13/13] show: Remove now unused fields from notmuch_show_format

The message_set_{begin,sep,end} and null_message fields are no longer
used because we now use the structure printer provided by the format.
---
 notmuch-client.h |    4 ----
 notmuch-show.c   |    4 ----
 2 files changed, 8 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index b11caff..ae9344b 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
 <at>  <at>  -71,13 +71,9  <at>  <at>  struct notmuch_show_params;

 typedef struct notmuch_show_format {
     struct sprinter *(*new_sprinter) (const void *ctx, FILE *stream);
-    const char *message_set_start;
     notmuch_status_t (*part) (const void *ctx, struct sprinter *sprinter,
 			      struct mime_node *node, int indent,
 			      const struct notmuch_show_params *params);
-    const char *message_set_sep;
-    const char *message_set_end;
-    const char *null_message;
 } notmuch_show_format_t;

 typedef struct notmuch_crypto {
diff --git a/notmuch-show.c b/notmuch-show.c
index 89bf2e7..3556293 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
 <at>  <at>  -37,11 +37,7  <at>  <at>  format_part_json_entry (const void *ctx, sprinter_t *sp, mime_node_t *node,
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 12/13] show: Convert do_show to use sprinter

---
 notmuch-show.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index ec3e861..89bf2e7 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
 <at>  <at>  -961,11 +961,9  <at>  <at>  do_show (void *ctx,
     notmuch_threads_t *threads;
     notmuch_thread_t *thread;
     notmuch_messages_t *messages;
-    int first_toplevel = 1;
     notmuch_status_t status, res = NOTMUCH_STATUS_SUCCESS;

-    if (format->message_set_start)
-	fputs (format->message_set_start, stdout);
+    sp->begin_list (sp);

     for (threads = notmuch_query_search_threads (query);
 	 notmuch_threads_valid (threads);
 <at>  <at>  -979,10 +977,6  <at>  <at>  do_show (void *ctx,
 	    INTERNAL_ERROR ("Thread %s has no toplevel messages.\n",
 			    notmuch_thread_get_thread_id (thread));

-	if (!first_toplevel && format->message_set_sep)
-	    fputs (format->message_set_sep, stdout);
-	first_toplevel = 0;
-
 	status = show_messages (ctx, format, sp, messages, 0, params);
(Continue reading)

Permalink | Reply |
headers
Austin Clements | 3 Aug 2012 03:14
Picon
Favicon

[PATCH v3 11/13] show: Convert show_message to use sprinter

Unlike the previous patches, this function is used for all formats.
However, for formats other than the JSON format, the sprinter methods
used by show_message are all no-ops, so this code continues to
function correctly for all of the formats.

Converting show_message eliminates show_null_message in the process,
since this maps directly to an sprinter method.
---
 notmuch-show.c |   31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index b9d9f5d..ec3e861 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
 <at>  <at>  -842,15 +842,6  <at>  <at>  format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
 }

 static notmuch_status_t
-show_null_message (const notmuch_show_format_t *format)
-{
-    /* Output a null message. Currently empty for all formats except Json */
-    if (format->null_message)
-	printf ("%s", format->null_message);
-    return NOTMUCH_STATUS_SUCCESS;
-}
-
-static notmuch_status_t
 show_message (void *ctx,
 	      const notmuch_show_format_t *format,
(Continue reading)

Permalink | Reply |
headers
Tomi Ollila | 3 Aug 2012 11:35
X-Face
Picon
Picon
Favicon

Re: [PATCH v3 00/13] Convert notmuch show to use structure printers

On Fri, Aug 03 2012, Austin Clements <amdragon@...> wrote:

> Relative to version 2 [0], this version fixes a duplicate typedef of
> struct sprinter that was not standards compliant and failed to compile
> on older gccs [1] (thanks to Tomi for discovering that and figuring
> out what was going on).

LGTM.

Tomi

> The diff relative to version 2 follows.
>
> [0] id:"1343449754-9010-1-git-send-email-amdragon@..."
>
> [1] http://stackoverflow.com/questions/8594954/repeated-typedefs-invalid-in-c-but-valid-in-c
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index de31aa1..ae9344b 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
>  <at>  <at>  -66,12 +66,12  <at>  <at>  typedef GMimeCipherContext notmuch_crypto_context_t;
>  #define STRINGIFY_(s) #s
>  
>  typedef struct mime_node mime_node_t;
> -typedef struct sprinter sprinter_t;
> +struct sprinter;
>  struct notmuch_show_params;
>  
>  typedef struct notmuch_show_format {
(Continue reading)

Permalink | Reply |
headers
David Bremner | 4 Aug 2012 01:48

Re: [PATCH v3 00/13] Convert notmuch show to use structure printers

Austin Clements <amdragon@...> writes:

> Relative to version 2 [0], this version fixes a duplicate typedef of
> struct sprinter that was not standards compliant and failed to compile
> on older gccs [1] (thanks to Tomi for discovering that and figuring
> out what was going on).

pushed.

d
Permalink | Reply |

Gmane