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

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

[Austin Clements]

This patch series converts notmuch show to use the new structure
printer infrastructure, cleaning up the code and paving the way to
easily support other structure types.  There are a lot of patches, but
most of them are small and the conversion is very mechanical.

The first three patches set up prerequisites by making the test
infrastructure more resilient to irrelevant JSON changes and by
introducing a new method to the sprinter.  The next three patches
introduce sprinters to notmuch show and feed them through all of its
layers, though they remain unused.  The remaining patches work their
way back up from the leaves of the JSON printer all the way to
do_show, converting each layer to use the sprinter.  By doing this
conversion from the leaves of the structure up, the output remains
valid and the tests continue to pass at every step.

The full series has been tested with both GMime 2.4 and GMime 2.6.

The first two patches together and the third patch alone stand on
their own and could be pushed before the rest of the series if
desired.

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

[Austin Clements]

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 +++++++++++++----
 6 files changed, 55 insertions(+), 60 deletions(-)

diff --git a/test/crypto b/test/crypto
index be752b1..5dd14c4 100755
--- a/test/crypto
+++ b/test/crypto
@@ -51,8 +51,7 @@ expected='[[[{"id": "XXXXX",
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
- "Date": "Sat,
- 01 Jan 2000 12:00:00 +0000"},
+ "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "sigstatus": [{"status": "good",
  "fingerprint": "'$FINGERPRINT'",
@@ -64,7 +63,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 
@@ -85,8 +84,7 @@ expected='[[[{"id": "XXXXX",
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
- "Date": "Sat,
- 01 Jan 2000 12:00:00 +0000"},
+ "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "sigstatus": [{"status": "good",
  "fingerprint": "'$FINGERPRINT'",
@@ -99,7 +97,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 
@@ -119,8 +117,7 @@ expected='[[[{"id": "XXXXX",
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
- "Date": "Sat,
- 01 Jan 2000 12:00:00 +0000"},
+ "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "sigstatus": [{"status": "error",
  "keyid": "'$(echo $FINGERPRINT | cut -c 25-)'",
@@ -132,7 +129,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
@@ -193,8 +190,7 @@ expected='[[[{"id": "XXXXX",
  "headers": {"Subject": "test encrypted message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
- "Date": "Sat,
- 01 Jan 2000 12:00:00 +0000"},
+ "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "encstatus": [{"status": "good"}],
  "sigstatus": [],
@@ -210,7 +206,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/octet-stream",
  "filename": "TESTATTACHMENT"}]}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 
@@ -221,7 +217,7 @@ output=$(notmuch show --format=json --part=4 --decrypt subject:"test encrypted m
 expected='{"id": 4,
  "content-type": "text/plain",
  "content": "This is a test encrypted message.\n"}'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 
@@ -248,8 +244,7 @@ expected='[[[{"id": "XXXXX",
  "headers": {"Subject": "test encrypted message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
- "Date": "Sat,
- 01 Jan 2000 12:00:00 +0000"},
+ "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "encstatus": [{"status": "bad"}],
  "content-type": "multipart/encrypted",
@@ -258,7 +253,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/octet-stream"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
@@ -283,8 +278,7 @@ expected='[[[{"id": "XXXXX",
  "headers": {"Subject": "test encrypted message 002",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
- "Date": "Sat,
- 01 Jan 2000 12:00:00 +0000"},
+ "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "encstatus": [{"status": "good"}],
  "sigstatus": [{"status": "good",
@@ -298,7 +292,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is another test encrypted message.\n"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 
@@ -338,8 +332,7 @@ expected='[[[{"id": "XXXXX",
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
- "Date": "Sat,
- 01 Jan 2000 12:00:00 +0000"},
+ "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
  "body": [{"id": 1,
  "sigstatus": [{"status": "error",
  "keyid": "6D92612D94E46381",
@@ -351,7 +344,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal_json \
     "$output" \
     "$expected"
 
diff --git a/test/json b/test/json
index 831e105..d86ee46 100755
--- a/test/json
+++ b/test/json
@@ -5,21 +5,21 @@ test_description="--format=json output"
 test_begin_subtest "Show message: json"
 add_message "[subject]=\"json-show-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"json-show-message\""
 output=$(notmuch show --format=json "json-show-message")
-test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
+test_expect_equal_json "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
 
 # This should be the same output as above.
 test_begin_subtest "Show message: json --body=true"
 output=$(notmuch show --format=json --body=true "json-show-message")
-test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
+test_expect_equal_json "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"json-show-message\n\"}]}, []]]]"
 
 test_begin_subtest "Show message: json --body=false"
 output=$(notmuch show --format=json --body=false "json-show-message")
-test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}}, []]]]"
+test_expect_equal_json "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}}, []]]]"
 
 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)
-test_expect_equal "$output" "[{\"thread\": \"XXX\",
+test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"timestamp\": 946728000,
  \"date_relative\": \"2000-01-01\",
  \"matched\": 1,
@@ -32,7 +32,7 @@ test_expect_equal "$output" "[{\"thread\": \"XXX\",
 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\""
 output=$(notmuch show --format=json "jsön-show-méssage")
-test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-utf8-body-sübjéct\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"jsön-show-méssage\n\"}]}, []]]]"
+test_expect_equal_json "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"excluded\": false, \"filename\": \"${gen_msg_filename}\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\",\"unread\"], \"headers\": {\"Subject\": \"json-show-utf8-body-sübjéct\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"text/plain\", \"content\": \"jsön-show-méssage\n\"}]}, []]]]"
 
 test_begin_subtest "Show message: json, inline attachment filename"
 subject='json-show-inline-attachment-filename'
@@ -45,12 +45,12 @@ emacs_deliver_message \
      (insert \"Message-ID: <$id>\n\")"
 output=$(notmuch show --format=json "id:$id")
 filename=$(notmuch search --output=files "id:$id")
-test_expect_equal "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"test_suite@notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"filename\": \"README\"}]}]}, []]]]"
+test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"test_suite@notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"filename\": \"README\"}]}]}, []]]]"
 
 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)
-test_expect_equal "$output" "[{\"thread\": \"XXX\",
+test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"timestamp\": 946728000,
  \"date_relative\": \"2000-01-01\",
  \"matched\": 1,
diff --git a/test/maildir-sync b/test/maildir-sync
index 01348d3..b748d04 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -4,11 +4,9 @@ test_description="maildir synchronization"
 
 . ./test-lib.sh
 
-# Much easier to examine differences if the "notmuch show
-# --format=json" output includes some newlines. Also, need to avoid
-# including the local value of MAIL_DIR in the result.
+# Avoid including the local value of MAIL_DIR in the result.
 filter_show_json() {
-    sed -e 's/, /,\n/g'  | sed -e "s|${MAIL_DIR}/|MAIL_DIR/|"
+    sed -e "s|${MAIL_DIR}/|MAIL_DIR/|"
     echo
 }
 
@@ -44,7 +42,7 @@ test_expect_equal "$output" "adding-replied-tag:2,RS"
 
 test_begin_subtest "notmuch show works with renamed file (without notmuch new)"
 output=$(notmuch show --format=json id:${gen_msg_id} | filter_show_json)
-test_expect_equal "$output" '[[[{"id": "adding-replied-tag@notmuch-test-suite",
+test_expect_equal_json "$output" '[[[{"id": "adding-replied-tag@notmuch-test-suite",
 "match": true,
 "excluded": false,
 "filename": "MAIL_DIR/cur/adding-replied-tag:2,RS",
@@ -54,8 +52,7 @@ test_expect_equal "$output" '[[[{"id": "adding-replied-tag@notmuch-test-suite",
 "headers": {"Subject": "Adding replied tag",
 "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
 "To": "Notmuch Test Suite <test_suite@notmuchmail.org>",
-"Date": "Fri,
-05 Jan 2001 15:43:57 +0000"},
+"Date": "Fri, 05 Jan 2001 15:43:57 +0000"},
 "body": [{"id": 1,
 "content-type": "text/plain",
 "content": "This is just a test message (#3)\n"}]},
diff --git a/test/multipart b/test/multipart
index 72d3927..3ccf27f 100755
--- a/test/multipart
+++ b/test/multipart
@@ -334,7 +334,7 @@ cat <<EOF >EXPECTED
 {"id": 8, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}]}, 
 {"id": 9, "content-type": "application/pgp-signature"}]}]}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=1, message body"
 notmuch show --format=json --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -351,7 +351,7 @@ cat <<EOF >EXPECTED
 {"id": 8, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}]}, 
 {"id": 9, "content-type": "application/pgp-signature"}]}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=2, multipart/mixed"
 notmuch show --format=json --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -366,7 +366,7 @@ cat <<EOF >EXPECTED
 {"id": 7, "content-type": "text/plain", "filename": "attachment", "content": "This is a text attachment.\n"}, 
 {"id": 8, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}]}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=3, rfc822 part"
 notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -378,7 +378,7 @@ cat <<EOF >EXPECTED
 {"id": 5, "content-type": "text/html"}, 
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=4, rfc822's multipart/alternative"
 notmuch show --format=json --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -389,7 +389,7 @@ cat <<EOF >EXPECTED
 {"id": 5, "content-type": "text/html"}, 
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=5, rfc822's html part"
 notmuch show --format=json --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -398,7 +398,7 @@ cat <<EOF >EXPECTED
 
 {"id": 5, "content-type": "text/html"}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=6, rfc822's text part"
 notmuch show --format=json --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -407,7 +407,7 @@ cat <<EOF >EXPECTED
 
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=7, inline attachment"
 notmuch show --format=json --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -416,7 +416,7 @@ cat <<EOF >EXPECTED
 
 {"id": 7, "content-type": "text/plain", "filename": "attachment", "content": "This is a text attachment.\n"}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=8, plain text part"
 notmuch show --format=json --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -425,7 +425,7 @@ cat <<EOF >EXPECTED
 
 {"id": 8, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=9, pgp signature (unverified)"
 notmuch show --format=json --part=9 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
@@ -434,7 +434,7 @@ cat <<EOF >EXPECTED
 
 {"id": 9, "content-type": "application/pgp-signature"}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_expect_success \
     "--format=json --part=10, no part, expect error" \
@@ -617,8 +617,7 @@ notmuch reply --format=json 'id:87liy5ap00.fsf@yoom.home.cworth.org' | notmuch_j
 cat <<EOF >EXPECTED
 {"reply-headers": {"Subject": "Re: Multipart message",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
- "To": "Carl Worth <cworth@cworth.org>,
- cworth@cworth.org",
+ "To": "Carl Worth <cworth@cworth.org>, cworth@cworth.org",
  "In-reply-to": "<87liy5ap00.fsf@yoom.home.cworth.org>",
  "References": " <87liy5ap00.fsf@yoom.home.cworth.org>"},
  "original": {"id": "XXXXX",
@@ -631,8 +630,7 @@ cat <<EOF >EXPECTED
  "headers": {"Subject": "Multipart message",
  "From": "Carl Worth <cworth@cworth.org>",
  "To": "cworth@cworth.org",
- "Date": "Fri,
- 05 Jan 2001 15:43:57 +0000"},
+ "Date": "Fri, 05 Jan 2001 15:43:57 +0000"},
  "body": [{"id": 1,
  "content-type": "multipart/signed",
  "content": [{"id": 2,
@@ -642,16 +640,14 @@ cat <<EOF >EXPECTED
  "content": [{"headers": {"Subject": "html message",
  "From": "Carl Worth <cworth@cworth.org>",
  "To": "cworth@cworth.org",
- "Date": "Fri,
- 05 Jan 2001 15:42:57 +0000"},
+ "Date": "Fri, 05 Jan 2001 15:42:57 +0000"},
  "body": [{"id": 4,
  "content-type": "multipart/alternative",
  "content": [{"id": 5,
  "content-type": "text/html"},
  {"id": 6,
  "content-type": "text/plain",
- "content": "This is an embedded message,
- with a multipart/alternative part.\n"}]}]}]},
+ "content": "This is an embedded message, with a multipart/alternative part.\n"}]}]}]},
  {"id": 7,
  "content-type": "text/plain",
  "filename": "YYYYY",
@@ -662,7 +658,7 @@ cat <<EOF >EXPECTED
  {"id": 9,
  "content-type": "application/pgp-signature"}]}]}}
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "'notmuch show --part' does not corrupt a part with CRLF pair"
 notmuch show --format=raw --part=3 id:base64-part-with-crlf > crlf.out
diff --git a/test/search-output b/test/search-output
index 8b57a43..c2a87eb 100755
--- a/test/search-output
+++ b/test/search-output
@@ -62,7 +62,7 @@ cat <<EOF >EXPECTED
 "THREADID",
 "THREADID"]
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--output=messages"
 notmuch search --output=messages '*' >OUTPUT
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 06aaea2..791d2dc 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -512,6 +512,16 @@ test_expect_equal_file ()
     fi
 }
 
+# Like test_expect_equal, but arguments are JSON expressions to be
+# canonicalized before diff'ing.  If an argument cannot be parsed, it
+# is used unchanged so that there's something to diff against.
+test_expect_equal_json () {
+    output=$(echo "$1" | python -mjson.tool || echo "$1")
+    expected=$(echo "$2" | python -mjson.tool || echo "$2")
+    shift 2
+    test_expect_equal "$output" "$expected" "$@"
+}
+
 test_emacs_expect_t () {
 	test "$#" = 2 && { prereq=$1; shift; } || prereq=
 	test "$#" = 1 ||
@@ -565,10 +575,9 @@ notmuch_show_sanitize_all ()
 
 notmuch_json_show_sanitize ()
 {
-    sed -e 's|, |,\n |g' | \
-	sed \
-	-e 's|"id": "[^"]*",|"id": "XXXXX",|' \
-	-e 's|"filename": "[^"]*",|"filename": "YYYYY",|'
+    sed \
+	-e 's|"id": "[^"]*",|"id": "XXXXX",|g' \
+	-e 's|"filename": "[^"]*",|"filename": "YYYYY",|g'
 }
 
 # End of notmuch helper functions
-- 
1.7.10

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

[Austin Clements]

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
@@ -18,7 +18,7 @@ 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\",
@@ -49,7 +49,7 @@ 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\",
  \"timestamp\": 946728000,
  \"date_relative\": \"2000-01-01\",
diff --git a/test/maildir-sync b/test/maildir-sync
index b748d04..cd7d241 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -7,7 +7,6 @@ test_description="maildir synchronization"
 # Avoid including the local value of MAIL_DIR in the result.
 filter_show_json() {
     sed -e "s|${MAIL_DIR}/|MAIL_DIR/|"
-    echo
 }
 
 # Create the expected maildir structure
diff --git a/test/multipart b/test/multipart
index 3ccf27f..0527f84 100755
--- a/test/multipart
+++ b/test/multipart
@@ -319,10 +319,8 @@ test_expect_success \
     "notmuch show --format=text --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "--format=json --part=0, full message"
-notmuch show --format=json --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": "87liy5ap00.fsf@yoom.home.cworth.org", "match": true, "excluded": false, "filename": "${MAIL_DIR}/multipart", "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["attachment","inbox","signed","unread"], "headers": {"Subject": "Multipart message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:43:57 +0000"}, "body": [
 {"id": 1, "content-type": "multipart/signed", "content": [
 {"id": 2, "content-type": "multipart/mixed", "content": [
@@ -337,10 +335,8 @@ EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=1, message body"
-notmuch show --format=json --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 1, "content-type": "multipart/signed", "content": [
 {"id": 2, "content-type": "multipart/mixed", "content": [
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"Subject": "html message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
@@ -354,10 +350,8 @@ EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=2, multipart/mixed"
-notmuch show --format=json --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 2, "content-type": "multipart/mixed", "content": [
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"Subject": "html message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
 {"id": 4, "content-type": "multipart/alternative", "content": [
@@ -369,10 +363,8 @@ EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=3, rfc822 part"
-notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"Subject": "html message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
 {"id": 4, "content-type": "multipart/alternative", "content": [
 {"id": 5, "content-type": "text/html"}, 
@@ -381,10 +373,8 @@ EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=4, rfc822's multipart/alternative"
-notmuch show --format=json --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 4, "content-type": "multipart/alternative", "content": [
 {"id": 5, "content-type": "text/html"}, 
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}]}
@@ -392,46 +382,36 @@ EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=5, rfc822's html part"
-notmuch show --format=json --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 5, "content-type": "text/html"}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=6, rfc822's text part"
-notmuch show --format=json --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=6 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 6, "content-type": "text/plain", "content": "This is an embedded message, with a multipart/alternative part.\n"}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=7, inline attachment"
-notmuch show --format=json --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=7 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 7, "content-type": "text/plain", "filename": "attachment", "content": "This is a text attachment.\n"}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=8, plain text part"
-notmuch show --format=json --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 8, "content-type": "text/plain", "content": "And this message is signed.\n\n-Carl\n"}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
 test_begin_subtest "--format=json --part=9, pgp signature (unverified)"
-notmuch show --format=json --part=9 'id:87liy5ap00.fsf@yoom.home.cworth.org' | sed 's|{"id":|\n{"id":|g' >OUTPUT
-echo >>OUTPUT # expect *no* newline at end of output
+notmuch show --format=json --part=9 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-
 {"id": 9, "content-type": "application/pgp-signature"}
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
-- 
1.7.10

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

[Austin Clements]

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 |   11 +++++++++--
 sprinter-text.c |   11 +++++++++--
 sprinter.h      |    1 +
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/sprinter-json.c b/sprinter-json.c
index 4649655..2587ca6 100644
--- a/sprinter-json.c
+++ b/sprinter-json.c
@@ -89,7 +89,7 @@ json_end (struct sprinter *sp)
 }
 
 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",
@@ -98,7 +98,7 @@ json_string (struct sprinter *sp, const char *val)
     struct sprinter_json *spj = json_begin_value (sp);
 
     fputc ('"', spj->stream);
-    for (; *val; ++val) {
+    for (; len; ++val, --len) {
 	unsigned char ch = *val;
 	if (ch < ARRAY_SIZE (escapes) && escapes[ch])
 	    fputs (escapes[ch], spj->stream);
@@ -111,6 +111,12 @@ json_string (struct sprinter *sp, const char *val)
 }
 
 static void
+json_string (struct sprinter *sp, const char *val)
+{
+    json_string_len (sp, val, strlen (val));
+}
+
+static void
 json_integer (struct sprinter *sp, int val)
 {
     struct sprinter_json *spj = json_begin_value (sp);
@@ -166,6 +172,7 @@ sprinter_json_create (const void *ctx, FILE *stream)
 	    .begin_list = json_begin_list,
 	    .end = json_end,
 	    .string = json_string,
+	    .string_len = json_string_len,
 	    .integer = json_integer,
 	    .boolean = json_boolean,
 	    .null = json_null,
diff --git a/sprinter-text.c b/sprinter-text.c
index b208840..dfa54b5 100644
--- a/sprinter-text.c
+++ b/sprinter-text.c
@@ -25,14 +25,20 @@ struct sprinter_text {
 };
 
 static void
-text_string (struct sprinter *sp, const char *val)
+text_string_len (struct sprinter *sp, const char *val, size_t len)
 {
     struct sprinter_text *sptxt = (struct sprinter_text *) sp;
 
     if (sptxt->current_prefix != NULL)
 	fprintf (sptxt->stream, "%s:", sptxt->current_prefix);
 
-    fputs(val, sptxt->stream);
+    fwrite (val, len, 1, sptxt->stream);
+}
+
+static void
+text_string (struct sprinter *sp, const char *val)
+{
+    text_string_len (sp, val, strlen (val));
 }
 
 static void
@@ -105,6 +111,7 @@ sprinter_text_create (const void *ctx, FILE *stream)
 	    .begin_list = text_begin_list,
 	    .end = text_end,
 	    .string = text_string,
+	    .string_len = text_string_len,
 	    .integer = text_integer,
 	    .boolean = text_boolean,
 	    .null = text_null,
diff --git a/sprinter.h b/sprinter.h
index 6680d41..826a852 100644
--- a/sprinter.h
+++ b/sprinter.h
@@ -28,6 +28,7 @@ typedef struct sprinter {
      * For string, the char * must be UTF-8 encoded.
      */
     void (*string) (struct sprinter *, const char *);
+    void (*string_len) (struct sprinter *, const char *, size_t);
     void (*integer) (struct sprinter *, int);
     void (*boolean) (struct sprinter *, notmuch_bool_t);
     void (*null) (struct sprinter *);
-- 
1.7.10

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

[Austin Clements]

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 |    2 ++
 notmuch-show.c   |    9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index f930798..e4172a2 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -66,9 +66,11 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #define STRINGIFY_(s) #s
 
 typedef struct mime_node mime_node_t;
+typedef struct sprinter sprinter_t;
 struct notmuch_show_params;
 
 typedef struct notmuch_show_format {
+    sprinter_t *(*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
@@ -20,12 +20,14 @@
 
 #include "notmuch-client.h"
 #include "gmime-filter-reply.h"
+#include "sprinter.h"
 
 static notmuch_status_t
 format_part_text (const void *ctx, mime_node_t *node,
 		  int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_text = {
+    .new_sprinter = sprinter_text_create,
     .part = format_part_text,
 };
 
@@ -34,6 +36,7 @@ format_part_json_entry (const void *ctx, mime_node_t *node,
 			int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_json = {
+    .new_sprinter = sprinter_json_create,
     .message_set_start = "[",
     .part = format_part_json_entry,
     .message_set_sep = ", ",
@@ -46,6 +49,7 @@ format_part_mbox (const void *ctx, mime_node_t *node,
 		  int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_mbox = {
+    .new_sprinter = sprinter_text_create,
     .part = format_part_mbox,
 };
 
@@ -55,6 +59,7 @@ format_part_raw (unused (const void *ctx), mime_node_t *node,
 		 unused (const notmuch_show_params_t *params));
 
 static const notmuch_show_format_t format_raw = {
+    .new_sprinter = sprinter_text_create,
     .part = format_part_raw,
 };
 
@@ -1003,6 +1008,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     char *query_string;
     int opt_index, ret;
     const notmuch_show_format_t *format = &format_text;
+    sprinter_t *sprinter;
     notmuch_show_params_t params = {
 	.part = -1,
 	.omit_excluded = TRUE,
@@ -1130,6 +1136,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
+    /* Create structure printer. */
+    sprinter = format->new_sprinter(ctx, stdout);
+
     /* If a single message is requested we do not use search_excludes. */
     if (params.part >= 0)
 	ret = do_show_single (ctx, query, format, &params);
-- 
1.7.10

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

[Austin Clements]

---
 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
@@ -22,6 +22,7 @@
 
 #include "notmuch-client.h"
 #include "gmime-filter-headers.h"
+#include "sprinter.h"
 
 static void
 show_reply_headers (GMimeMessage *message)
@@ -596,6 +597,7 @@ 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");
@@ -611,6 +613,8 @@ notmuch_reply_format_json(void *ctx,
     if (!reply)
 	return 1;
 
+    sp = sprinter_json_create (ctx, stdout);
+
     /* The headers of the reply message we've created */
     printf ("{\"reply-headers\": ");
     format_headers_json (ctx, reply, TRUE);
-- 
1.7.10

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

[Austin Clements]

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 |    5 +++--
 notmuch-reply.c  |    2 +-
 notmuch-show.c   |   50 +++++++++++++++++++++++++++++---------------------
 3 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index e4172a2..8eeedf7 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -72,7 +72,7 @@ struct notmuch_show_params;
 typedef struct notmuch_show_format {
     sprinter_t *(*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, sprinter_t *sprinter,
 			      struct mime_node *node, int indent,
 			      const struct notmuch_show_params *params);
     const char *message_set_sep;
@@ -179,7 +179,8 @@ notmuch_status_t
 show_one_part (const char *filename, int part);
 
 void
-format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first, notmuch_bool_t output_body);
+format_part_json (const void *ctx, sprinter_t *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);
diff --git a/notmuch-reply.c b/notmuch-reply.c
index e42ba79..07d4452 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -624,7 +624,7 @@ notmuch_reply_format_json(void *ctx,
     /* Start the original */
     printf (", \"original\": ");
 
-    format_part_json (ctx, node, TRUE, TRUE);
+    format_part_json (ctx, sp, node, TRUE, TRUE);
 
     /* End */
     printf ("}\n");
diff --git a/notmuch-show.c b/notmuch-show.c
index d04943f..b258f65 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -23,7 +23,7 @@
 #include "sprinter.h"
 
 static notmuch_status_t
-format_part_text (const void *ctx, mime_node_t *node,
+format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 		  int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_text = {
@@ -32,7 +32,7 @@ static const notmuch_show_format_t format_text = {
 };
 
 static notmuch_status_t
-format_part_json_entry (const void *ctx, mime_node_t *node,
+format_part_json_entry (const void *ctx, sprinter_t *sp, mime_node_t *node,
 			int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_json = {
@@ -45,7 +45,7 @@ static const notmuch_show_format_t format_json = {
 };
 
 static notmuch_status_t
-format_part_mbox (const void *ctx, mime_node_t *node,
+format_part_mbox (const void *ctx, sprinter_t *sp, mime_node_t *node,
 		  int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_mbox = {
@@ -54,7 +54,7 @@ static const notmuch_show_format_t format_mbox = {
 };
 
 static notmuch_status_t
-format_part_raw (unused (const void *ctx), mime_node_t *node,
+format_part_raw (unused (const void *ctx), sprinter_t *sp, mime_node_t *node,
 		 unused (int indent),
 		 unused (const notmuch_show_params_t *params));
 
@@ -471,7 +471,7 @@ format_part_sigstatus_json (mime_node_t *node)
 #endif
 
 static notmuch_status_t
-format_part_text (const void *ctx, mime_node_t *node,
+format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 		  int indent, const notmuch_show_params_t *params)
 {
     /* The disposition and content-type metadata are associated with
@@ -553,7 +553,7 @@ format_part_text (const void *ctx, mime_node_t *node,
     }
 
     for (i = 0; i < node->nchildren; i++)
-	format_part_text (ctx, mime_node_child (node, i), indent, params);
+	format_part_text (ctx, sp, mime_node_child (node, i), indent, params);
 
     if (GMIME_IS_MESSAGE (node->part))
 	printf ("\fbody}\n");
@@ -564,7 +564,8 @@ format_part_text (const void *ctx, mime_node_t *node,
 }
 
 void
-format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first, notmuch_bool_t output_body)
+format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
+		  notmuch_bool_t first, notmuch_bool_t output_body)
 {
     /* Any changes to the JSON format should be reflected in the file
      * devel/schemata. */
@@ -578,7 +579,7 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first, notm
 
 	if (output_body) {
 	    printf (", \"body\": [");
-	    format_part_json (ctx, mime_node_child (node, 0), first, TRUE);
+	    format_part_json (ctx, sp, mime_node_child (node, 0), first, TRUE);
 	    printf ("]");
 	}
 	printf ("}");
@@ -659,16 +660,17 @@ format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first, notm
     talloc_free (local);
 
     for (i = 0; i < node->nchildren; i++)
-	format_part_json (ctx, mime_node_child (node, i), i == 0, TRUE);
+	format_part_json (ctx, sp, mime_node_child (node, i), i == 0, TRUE);
 
     printf ("%s}", terminator);
 }
 
 static notmuch_status_t
-format_part_json_entry (const void *ctx, mime_node_t *node, unused (int indent),
+format_part_json_entry (const void *ctx, sprinter_t *sp,
+			mime_node_t *node, unused (int indent),
 			const notmuch_show_params_t *params)
 {
-    format_part_json (ctx, node, TRUE, params->output_body);
+    format_part_json (ctx, sp, node, TRUE, params->output_body);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
@@ -679,7 +681,8 @@ format_part_json_entry (const void *ctx, mime_node_t *node, unused (int indent),
  * http://qmail.org/qmail-manual-html/man5/mbox.html
  */
 static notmuch_status_t
-format_part_mbox (const void *ctx, mime_node_t *node, unused (int indent),
+format_part_mbox (const void *ctx, unused (sprinter_t *sp), mime_node_t *node,
+		  unused (int indent),
 		  unused (const notmuch_show_params_t *params))
 {
     notmuch_message_t *message = node->envelope_file;
@@ -730,8 +733,8 @@ format_part_mbox (const void *ctx, mime_node_t *node, unused (int indent),
 }
 
 static notmuch_status_t
-format_part_raw (unused (const void *ctx), mime_node_t *node,
-		 unused (int indent),
+format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
+		 mime_node_t *node, unused (int indent),
 		 unused (const notmuch_show_params_t *params))
 {
     if (node->envelope_file) {
@@ -819,6 +822,7 @@ show_null_message (const notmuch_show_format_t *format)
 static notmuch_status_t
 show_message (void *ctx,
 	      const notmuch_show_format_t *format,
+	      sprinter_t *sp,
 	      notmuch_message_t *message,
 	      int indent,
 	      notmuch_show_params_t *params)
@@ -832,7 +836,7 @@ show_message (void *ctx,
 	goto DONE;
     part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
     if (part)
-	status = format->part (local, part, indent, params);
+	status = format->part (local, sp, part, indent, params);
   DONE:
     talloc_free (local);
     return status;
@@ -841,6 +845,7 @@ show_message (void *ctx,
 static notmuch_status_t
 show_messages (void *ctx,
 	       const notmuch_show_format_t *format,
+	       sprinter_t *sp,
 	       notmuch_messages_t *messages,
 	       int indent,
 	       notmuch_show_params_t *params)
@@ -874,7 +879,7 @@ show_messages (void *ctx,
 	next_indent = indent;
 
 	if ((match && (!excluded || !params->omit_excluded)) || params->entire_thread) {
-	    status = show_message (ctx, format, message, indent, params);
+	    status = show_message (ctx, format, sp, message, indent, params);
 	    if (status && !res)
 		res = status;
 	    next_indent = indent + 1;
@@ -886,7 +891,7 @@ show_messages (void *ctx,
 	    fputs (format->message_set_sep, stdout);
 
 	status = show_messages (ctx,
-				format,
+				format, sp,
 				notmuch_message_get_replies (message),
 				next_indent,
 				params);
@@ -910,6 +915,7 @@ static int
 do_show_single (void *ctx,
 		notmuch_query_t *query,
 		const notmuch_show_format_t *format,
+		sprinter_t *sp,
 		notmuch_show_params_t *params)
 {
     notmuch_messages_t *messages;
@@ -930,7 +936,8 @@ do_show_single (void *ctx,
 
     notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH, 1);
 
-    return show_message (ctx, format, message, 0, params) != NOTMUCH_STATUS_SUCCESS;
+    return show_message (ctx, format, sp, message, 0, params)
+	!= NOTMUCH_STATUS_SUCCESS;
 }
 
 /* Formatted output of threads */
@@ -938,6 +945,7 @@ static int
 do_show (void *ctx,
 	 notmuch_query_t *query,
 	 const notmuch_show_format_t *format,
+	 sprinter_t *sp,
 	 notmuch_show_params_t *params)
 {
     notmuch_threads_t *threads;
@@ -965,7 +973,7 @@ do_show (void *ctx,
 	    fputs (format->message_set_sep, stdout);
 	first_toplevel = 0;
 
-	status = show_messages (ctx, format, messages, 0, params);
+	status = show_messages (ctx, format, sp, messages, 0, params);
 	if (status && !res)
 	    res = status;
 
@@ -1141,7 +1149,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 
     /* If a single message is requested we do not use search_excludes. */
     if (params.part >= 0)
-	ret = do_show_single (ctx, query, format, &params);
+	ret = do_show_single (ctx, query, format, sprinter, &params);
     else {
 	/* We always apply set the exclude flag. The
 	 * exclude=true|false option controls whether or not we return
@@ -1160,7 +1168,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	    params.omit_excluded = FALSE;
 	}
 
-	ret = do_show (ctx, query, format, &params);
+	ret = do_show (ctx, query, format, sprinter, &params);
     }
 
     notmuch_crypto_cleanup (&params.crypto);
-- 
1.7.10

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

[Austin Clements]

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 8eeedf7..21c3231 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -183,7 +183,8 @@ format_part_json (const void *ctx, sprinter_t *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 (sprinter_t *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
@@ -617,7 +617,7 @@ notmuch_reply_format_json(void *ctx,
 
     /* The headers of the reply message we've created */
     printf ("{\"reply-headers\": ");
-    format_headers_json (ctx, reply, TRUE);
+    format_headers_json (sp, reply, TRUE);
     g_object_unref (G_OBJECT (reply));
     reply = NULL;
 
diff --git a/notmuch-show.c b/notmuch-show.c
index b258f65..9852119 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -199,48 +199,46 @@ _is_from_line (const char *line)
 }
 
 void
-format_headers_json (const void *ctx, GMimeMessage *message, notmuch_bool_t reply)
+format_headers_json (sprinter_t *sp, GMimeMessage *message,
+		     notmuch_bool_t reply)
 {
-    void *local = talloc_new (ctx);
     InternetAddressList *recipients;
     const char *recipients_string;
 
-    printf ("{%s: %s",
-	    json_quote_str (local, "Subject"),
-	    json_quote_str (local, g_mime_message_get_subject (message)));
-    printf (", %s: %s",
-	    json_quote_str (local, "From"),
-	    json_quote_str (local, g_mime_message_get_sender (message)));
+    sp->begin_map (sp);
+
+    sp->map_key (sp, "Subject");
+    sp->string (sp, g_mime_message_get_subject (message));
+
+    sp->map_key (sp, "From");
+    sp->string (sp, g_mime_message_get_sender (message));
+
     recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
     recipients_string = internet_address_list_to_string (recipients, 0);
-    if (recipients_string)
-	printf (", %s: %s",
-		json_quote_str (local, "To"),
-		json_quote_str (local, recipients_string));
+    if (recipients_string) {
+	sp->map_key (sp, "To");
+	sp->string (sp, recipients_string);
+    }
+
     recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
     recipients_string = internet_address_list_to_string (recipients, 0);
-    if (recipients_string)
-	printf (", %s: %s",
-		json_quote_str (local, "Cc"),
-		json_quote_str (local, recipients_string));
+    if (recipients_string) {
+	sp->map_key (sp, "Cc");
+	sp->string (sp, recipients_string);
+    }
 
     if (reply) {
-	printf (", %s: %s",
-		json_quote_str (local, "In-reply-to"),
-		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "In-reply-to")));
+	sp->map_key (sp, "In-reply-to");
+	sp->string (sp, g_mime_object_get_header (GMIME_OBJECT (message), "In-reply-to"));
 
-	printf (", %s: %s",
-		json_quote_str (local, "References"),
-		json_quote_str (local, g_mime_object_get_header (GMIME_OBJECT (message), "References")));
+	sp->map_key (sp, "References");
+	sp->string (sp, g_mime_object_get_header (GMIME_OBJECT (message), "References"));
     } else {
-	printf (", %s: %s",
-		json_quote_str (local, "Date"),
-		json_quote_str (local, g_mime_message_get_date_as_string (message)));
+	sp->map_key (sp, "Date");
+	sp->string (sp, g_mime_message_get_date_as_string (message));
     }
 
-    printf ("}");
-
-    talloc_free (local);
+    sp->end (sp);
 }
 
 /* Write a MIME text part out to the given stream.
@@ -575,7 +573,7 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	format_message_json (ctx, node->envelope_file);
 
 	printf ("\"headers\": ");
-	format_headers_json (ctx, GMIME_MESSAGE (node->part), FALSE);
+	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
 
 	if (output_body) {
 	    printf (", \"body\": [");
@@ -651,7 +649,7 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
     } else if (GMIME_IS_MESSAGE (node->part)) {
 	printf (", \"content\": [{");
 	printf ("\"headers\": ");
-	format_headers_json (local, GMIME_MESSAGE (node->part), FALSE);
+	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
 
 	printf (", \"body\": [");
 	terminator = "]}]";
-- 
1.7.10

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

[Austin Clements]

---
 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
@@ -337,134 +337,138 @@ 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);
 
-	if (i > 0)
-	    printf (", ");
-
-	printf ("{");
+	sp->begin_map (sp);
 
 	/* status */
 	GMimeSignatureStatus status = g_mime_signature_get_status (signature);
-	printf ("\"status\": %s",
-		json_quote_str (ctx_quote,
-				signature_status_to_string (status)));
+	sp->map_key (sp, "status");
+	sp->string (sp, signature_status_to_string (status));
 
 	GMimeCertificate *certificate = g_mime_signature_get_certificate (signature);
 	if (status == GMIME_SIGNATURE_STATUS_GOOD) {
-	    if (certificate)
-		printf (", \"fingerprint\": %s", json_quote_str (ctx_quote, g_mime_certificate_get_fingerprint (certificate)));
+	    if (certificate) {
+		sp->map_key (sp, "fingerprint");
+		sp->string (sp, g_mime_certificate_get_fingerprint (certificate));
+	    }
 	    /* these dates are seconds since the epoch; should we
 	     * provide a more human-readable format string? */
 	    time_t created = g_mime_signature_get_created (signature);
-	    if (created != -1)
-		printf (", \"created\": %d", (int) created);
+	    if (created != -1) {
+		sp->map_key (sp, "created");
+		sp->integer (sp, created);
+	    }
 	    time_t expires = g_mime_signature_get_expires (signature);
-	    if (expires > 0)
-		printf (", \"expires\": %d", (int) expires);
+	    if (expires > 0) {
+		sp->map_key (sp, "expires");
+		sp->integer (sp, expires);
+	    }
 	    /* output user id only if validity is FULL or ULTIMATE. */
 	    /* note that gmime is using the term "trust" here, which
 	     * is WRONG.  It's actually user id "validity". */
 	    if (certificate) {
 		const char *name = g_mime_certificate_get_name (certificate);
 		GMimeCertificateTrust trust = g_mime_certificate_get_trust (certificate);
-		if (name && (trust == GMIME_CERTIFICATE_TRUST_FULLY || trust == GMIME_CERTIFICATE_TRUST_ULTIMATE))
-		    printf (", \"userid\": %s", json_quote_str (ctx_quote, name));
+		if (name && (trust == GMIME_CERTIFICATE_TRUST_FULLY || trust == GMIME_CERTIFICATE_TRUST_ULTIMATE)) {
+		    sp->map_key (sp, "userid");
+		    sp->string (sp, name);
+		}
 	    }
 	} else if (certificate) {
 	    const char *key_id = g_mime_certificate_get_key_id (certificate);
-	    if (key_id)
-		printf (", \"keyid\": %s", json_quote_str (ctx_quote, key_id));
+	    if (key_id) {
+		sp->map_key (sp, "keyid");
+		sp->string (sp, key_id);
+	    }
 	}
 
 	GMimeSignatureError errors = g_mime_signature_get_errors (signature);
 	if (errors != GMIME_SIGNATURE_ERROR_NONE) {
-	    printf (", \"errors\": %d", errors);
+	    sp->map_key (sp, "errors");
+	    sp->integer (sp, errors);
 	}
 
-	printf ("}");
+	sp->end (sp);
      }
 
-    printf ("]");
-
-    talloc_free (ctx_quote);
+    sp->end (sp);
 }
 #else
 static void
-format_part_sigstatus_json (mime_node_t *node)
+format_part_sigstatus_json (sprinter_t *sp, mime_node_t *node)
 {
     const GMimeSignatureValidity* validity = node->sig_validity;
 
-    printf ("[");
+    sp->begin_list (sp);
 
     if (!validity) {
-	printf ("]");
+	sp->end (sp);
 	return;
     }
 
     const GMimeSigner *signer = g_mime_signature_validity_get_signers (validity);
-    int first = 1;
-    void *ctx_quote = talloc_new (NULL);
-
     while (signer) {
-	if (first)
-	    first = 0;
-	else
-	    printf (", ");
-
-	printf ("{");
+	sp->begin_map (sp);
 
 	/* status */
-	printf ("\"status\": %s",
-		json_quote_str (ctx_quote,
-				signer_status_to_string (signer->status)));
+	sp->map_key (sp, "status");
+	sp->string (sp, signer_status_to_string (signer->status));
 
 	if (signer->status == GMIME_SIGNER_STATUS_GOOD)
 	{
-	    if (signer->fingerprint)
-		printf (", \"fingerprint\": %s", json_quote_str (ctx_quote, signer->fingerprint));
+	    if (signer->fingerprint) {
+		sp->map_key (sp, "fingerprint");
+		sp->string (sp, signer->fingerprint);
+	    }
 	    /* these dates are seconds since the epoch; should we
 	     * provide a more human-readable format string? */
-	    if (signer->created)
-		printf (", \"created\": %d", (int) signer->created);
-	    if (signer->expires)
-		printf (", \"expires\": %d", (int) signer->expires);
+	    if (signer->created) {
+		sp->map_key (sp, "created");
+		sp->integer (sp, signer->created);
+	    }
+	    if (signer->expires) {
+		sp->map_key (sp, "expires");
+		sp->integer (sp, signer->expires);
+	    }
 	    /* output user id only if validity is FULL or ULTIMATE. */
 	    /* note that gmime is using the term "trust" here, which
 	     * is WRONG.  It's actually user id "validity". */
 	    if ((signer->name) && (signer->trust)) {
-		if ((signer->trust == GMIME_SIGNER_TRUST_FULLY) || (signer->trust == GMIME_SIGNER_TRUST_ULTIMATE))
-		    printf (", \"userid\": %s", json_quote_str (ctx_quote, signer->name));
+		if ((signer->trust == GMIME_SIGNER_TRUST_FULLY) || (signer->trust == GMIME_SIGNER_TRUST_ULTIMATE)) {
+		    sp->map_key (sp, "userid");
+		    sp->string (sp, signer->name);
+		}
            }
        } else {
-           if (signer->keyid)
-               printf (", \"keyid\": %s", json_quote_str (ctx_quote, signer->keyid));
+           if (signer->keyid) {
+	       sp->map_key (sp, "keyid");
+	       sp->string (sp, signer->keyid);
+	   }
        }
        if (signer->errors != GMIME_SIGNER_ERROR_NONE) {
-           printf (", \"errors\": %d", signer->errors);
+	   sp->map_key (sp, "errors");
+	   sp->integer (sp, signer->errors);
        }
 
-       printf ("}");
+       sp->end (sp);
        signer = signer->next;
     }
 
-    printf ("]");
-
-    talloc_free (ctx_quote);
+    sp->end (sp);
 }
 #endif
 
@@ -607,7 +611,7 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
 
     if (node->verify_attempted) {
 	printf (", \"sigstatus\": ");
-	format_part_sigstatus_json (node);
+	format_part_sigstatus_json (sp, node);
     }
 
     printf (", \"content-type\": %s",
-- 
1.7.10

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

[Austin Clements]

---
 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
@@ -588,7 +588,6 @@ 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 ?
@@ -597,31 +596,41 @@ 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");
+    sp->integer (sp, node->part_num);
 
-    if (node->decrypt_attempted)
-	printf (", \"encstatus\": [{\"status\": \"%s\"}]",
-		node->decrypt_success ? "good" : "bad");
+    if (node->decrypt_attempted) {
+	sp->map_key (sp, "encstatus");
+	sp->begin_list (sp);
+	sp->begin_map (sp);
+	sp->map_key (sp, "status");
+	sp->string (sp, node->decrypt_success ? "good" : "bad");
+	sp->end (sp);
+	sp->end (sp);
+    }
 
     if (node->verify_attempted) {
-	printf (", \"sigstatus\": ");
+	sp->map_key (sp, "sigstatus");
 	format_part_sigstatus_json (sp, node);
     }
 
-    printf (", \"content-type\": %s",
-	    json_quote_str (local, g_mime_content_type_to_string (content_type)));
+    sp->map_key (sp, "content-type");
+    sp->string (sp, g_mime_content_type_to_string (content_type));
 
-    if (cid)
-	printf (", \"content-id\": %s", json_quote_str (local, cid));
+    if (cid) {
+	sp->map_key (sp, "content-id");
+	sp->string (sp, cid);
+    }
 
-    if (filename)
-	printf (", \"filename\": %s", json_quote_str (local, filename));
+    if (filename) {
+	sp->map_key (sp, "filename");
+	sp->string (sp, filename);
+    }
 
     if (GMIME_IS_PART (node->part)) {
 	/* For non-HTML text parts, we include the content in the
@@ -636,35 +645,44 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	if (g_mime_content_type_is_type (content_type, "text", "html")) {
 	    const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset");
 
-	    if (content_charset != NULL)
-		printf (", \"content-charset\": %s", json_quote_str (local, content_charset));
+	    if (content_charset != NULL) {
+		sp->map_key (sp, "content-charset");
+		sp->string (sp, content_charset);
+	    }
 	} else if (g_mime_content_type_is_type (content_type, "text", "*")) {
 	    GMimeStream *stream_memory = g_mime_stream_mem_new ();
 	    GByteArray *part_content;
 	    show_text_part_content (node->part, stream_memory, 0);
 	    part_content = g_mime_stream_mem_get_byte_array (GMIME_STREAM_MEM (stream_memory));
-
-	    printf (", \"content\": %s", json_quote_chararray (local, (char *) part_content->data, part_content->len));
+	    sp->map_key (sp, "content");
+	    sp->string_len (sp, (char *) part_content->data, part_content->len);
 	    g_object_unref (stream_memory);
 	}
     } else if (GMIME_IS_MULTIPART (node->part)) {
-	printf (", \"content\": [");
-	terminator = "]";
+	sp->map_key (sp, "content");
+	sp->begin_list (sp);
+	nclose = 1;
     } else if (GMIME_IS_MESSAGE (node->part)) {
-	printf (", \"content\": [{");
-	printf ("\"headers\": ");
+	sp->map_key (sp, "content");
+	sp->begin_list (sp);
+	sp->begin_map (sp);
+
+	sp->map_key (sp, "headers");
 	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
 
-	printf (", \"body\": [");
-	terminator = "]}]";
+	sp->map_key (sp, "body");
+	sp->begin_list (sp);
+	nclose = 3;
     }
 
-    talloc_free (local);
-
     for (i = 0; i < node->nchildren; i++)
 	format_part_json (ctx, sp, mime_node_child (node, i), i == 0, TRUE);
 
-    printf ("%s}", terminator);
+    /* Close content structures */
+    for (i = 0; i < nclose; i++)
+	sp->end (sp);
+    /* Close part map */
+    sp->end (sp);
 }
 
 static notmuch_status_t
-- 
1.7.10

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

[Austin Clements]

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

diff --git a/notmuch-show.c b/notmuch-show.c
index afbd9d0..fa1e6e9 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -110,34 +110,44 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
 }
 
 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;
 
     date = notmuch_message_get_date (message);
-    relative_date = notmuch_time_relative_date (ctx, date);
+    relative_date = notmuch_time_relative_date (local, 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));
+
+    sp->map_key (sp, "excluded");
+    sp->boolean (sp, notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED));
+
+    sp->map_key (sp, "filename");
+    sp->string (sp, notmuch_message_get_filename (message));
+
+    sp->map_key (sp, "timestamp");
+    date = notmuch_message_get_date (message);
+    sp->integer (sp, date);
 
-    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
-	    json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
-	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
-	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
-	    json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
-	    date, relative_date);
+    sp->map_key (sp, "date_relative");
+    sp->string (sp, relative_date);
 
+    sp->map_key (sp, "tags");
+    sp->begin_list (sp);
     for (tags = notmuch_message_get_tags (message);
 	 notmuch_tags_valid (tags);
 	 notmuch_tags_move_to_next (tags))
-    {
-         printf("%s%s", first ? "" : ",",
-               json_quote_str (ctx_quote, notmuch_tags_get (tags)));
-         first = 0;
-    }
-    printf("], ");
-    talloc_free (ctx_quote);
+	sp->string (sp, notmuch_tags_get (tags));
+    sp->end (sp);
+
+    talloc_free (local);
 }
 
 /* Extract just the email address from the contents of a From:
@@ -573,18 +583,19 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
      * devel/schemata. */
 
     if (node->envelope_file) {
-	printf ("{");
-	format_message_json (ctx, node->envelope_file);
+	sp->begin_map (sp);
+	format_message_json (sp, node->envelope_file);
 
-	printf ("\"headers\": ");
+	sp->map_key (sp, "headers");
 	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
 
 	if (output_body) {
-	    printf (", \"body\": [");
+	    sp->map_key (sp, "body");
+	    sp->begin_list (sp);
 	    format_part_json (ctx, sp, mime_node_child (node, 0), first, TRUE);
-	    printf ("]");
+	    sp->end (sp);
 	}
-	printf ("}");
+	sp->end (sp);
 	return;
     }
 
-- 
1.7.10

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

[Austin Clements]

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 fa1e6e9..5ee2156 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -842,15 +842,6 @@ 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,
 	      sprinter_t *sp,
@@ -884,23 +875,16 @@ show_messages (void *ctx,
     notmuch_message_t *message;
     notmuch_bool_t match;
     notmuch_bool_t excluded;
-    int first_set = 1;
     int next_indent;
     notmuch_status_t status, res = NOTMUCH_STATUS_SUCCESS;
 
-    if (format->message_set_start)
-	fputs (format->message_set_start, stdout);
+    sp->begin_list (sp);
 
     for (;
 	 notmuch_messages_valid (messages);
 	 notmuch_messages_move_to_next (messages))
     {
-	if (!first_set && format->message_set_sep)
-	    fputs (format->message_set_sep, stdout);
-	first_set = 0;
-
-	if (format->message_set_start)
-	    fputs (format->message_set_start, stdout);
+	sp->begin_list (sp);
 
 	message = notmuch_messages_get (messages);
 
@@ -915,12 +899,9 @@ show_messages (void *ctx,
 		res = status;
 	    next_indent = indent + 1;
 	} else {
-	    status = show_null_message (format);
+	    sp->null (sp);
 	}
 
-	if (!status && format->message_set_sep)
-	    fputs (format->message_set_sep, stdout);
-
 	status = show_messages (ctx,
 				format, sp,
 				notmuch_message_get_replies (message),
@@ -931,12 +912,10 @@ show_messages (void *ctx,
 
 	notmuch_message_destroy (message);
 
-	if (format->message_set_end)
-	    fputs (format->message_set_end, stdout);
+	sp->end (sp);
     }
 
-    if (format->message_set_end)
-	fputs (format->message_set_end, stdout);
+    sp->end (sp);
 
     return res;
 }
-- 
1.7.10

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

[Austin Clements]

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

diff --git a/notmuch-show.c b/notmuch-show.c
index 5ee2156..a0da48a 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -961,11 +961,9 @@ 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);
@@ -979,10 +977,6 @@ 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);
 	if (status && !res)
 	    res = status;
@@ -991,8 +985,7 @@ do_show (void *ctx,
 
     }
 
-    if (format->message_set_end)
-	fputs (format->message_set_end, stdout);
+    sp->end (sp);
 
     return res != NOTMUCH_STATUS_SUCCESS;
 }
-- 
1.7.10

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

[Austin Clements]

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 21c3231..de31aa1 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -71,13 +71,9 @@ struct notmuch_show_params;
 
 typedef struct notmuch_show_format {
     sprinter_t *(*new_sprinter) (const void *ctx, FILE *stream);
-    const char *message_set_start;
     notmuch_status_t (*part) (const void *ctx, sprinter_t *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 a0da48a..f5bc098 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -37,11 +37,7 @@ format_part_json_entry (const void *ctx, sprinter_t *sp, mime_node_t *node,
 
 static const notmuch_show_format_t format_json = {
     .new_sprinter = sprinter_json_create,
-    .message_set_start = "[",
     .part = format_part_json_entry,
-    .message_set_sep = ", ",
-    .message_set_end = "]",
-    .null_message = "null"
 };
 
 static notmuch_status_t
-- 
1.7.10

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

[Mark Walters]

Hi

I have read this series (apart from the test changes) and it is both
very nice to review and looks good. It builds and all tests pass at all
the interesting partial stages and when fully applied. I have a few very
minor comments.

On Wed, 25 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> 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 |   11 +++++++++--
>  sprinter-text.c |   11 +++++++++--
>  sprinter.h      |    1 +
>  3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/sprinter-json.c b/sprinter-json.c
> index 4649655..2587ca6 100644
> --- a/sprinter-json.c
> +++ b/sprinter-json.c
> @@ -89,7 +89,7 @@ json_end (struct sprinter *sp)
>  }
>  
>  static void
> -json_string (struct sprinter *sp, const char *val)
> +json_string_len (struct sprinter *sp, const char *val, size_t len)
>  {

I think this function could do with a comment along the lines of the
commit message. It might be nice to document somewhere where/when we
might actually have nulls in an encoded string (are they allowed in
bodies, headers etc).

Actually, do we know that the json emacs parser handles nulls correctly?

Best wishes

Mark
>      static const char *const escapes[] = {
>  	['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
> @@ -98,7 +98,7 @@ json_string (struct sprinter *sp, const char *val)
>      struct sprinter_json *spj = json_begin_value (sp);
>  
>      fputc ('"', spj->stream);
> -    for (; *val; ++val) {
> +    for (; len; ++val, --len) {
>  	unsigned char ch = *val;
>  	if (ch < ARRAY_SIZE (escapes) && escapes[ch])
>  	    fputs (escapes[ch], spj->stream);
> @@ -111,6 +111,12 @@ json_string (struct sprinter *sp, const char *val)
>  }
>  
>  static void
> +json_string (struct sprinter *sp, const char *val)
> +{
> +    json_string_len (sp, val, strlen (val));
> +}
> +
> +static void
>  json_integer (struct sprinter *sp, int val)
>  {
>      struct sprinter_json *spj = json_begin_value (sp);
> @@ -166,6 +172,7 @@ sprinter_json_create (const void *ctx, FILE *stream)
>  	    .begin_list = json_begin_list,
>  	    .end = json_end,
>  	    .string = json_string,
> +	    .string_len = json_string_len,
>  	    .integer = json_integer,
>  	    .boolean = json_boolean,
>  	    .null = json_null,
> diff --git a/sprinter-text.c b/sprinter-text.c
> index b208840..dfa54b5 100644
> --- a/sprinter-text.c
> +++ b/sprinter-text.c
> @@ -25,14 +25,20 @@ struct sprinter_text {
>  };
>  
>  static void
> -text_string (struct sprinter *sp, const char *val)
> +text_string_len (struct sprinter *sp, const char *val, size_t len)
>  {
>      struct sprinter_text *sptxt = (struct sprinter_text *) sp;
>  
>      if (sptxt->current_prefix != NULL)
>  	fprintf (sptxt->stream, "%s:", sptxt->current_prefix);
>  
> -    fputs(val, sptxt->stream);
> +    fwrite (val, len, 1, sptxt->stream);
> +}
> +
> +static void
> +text_string (struct sprinter *sp, const char *val)
> +{
> +    text_string_len (sp, val, strlen (val));
>  }
>  
>  static void
> @@ -105,6 +111,7 @@ sprinter_text_create (const void *ctx, FILE *stream)
>  	    .begin_list = text_begin_list,
>  	    .end = text_end,
>  	    .string = text_string,
> +	    .string_len = text_string_len,
>  	    .integer = text_integer,
>  	    .boolean = text_boolean,
>  	    .null = text_null,
> diff --git a/sprinter.h b/sprinter.h
> index 6680d41..826a852 100644
> --- a/sprinter.h
> +++ b/sprinter.h
> @@ -28,6 +28,7 @@ typedef struct sprinter {
>       * For string, the char * must be UTF-8 encoded.
>       */
>      void (*string) (struct sprinter *, const char *);
> +    void (*string_len) (struct sprinter *, const char *, size_t);
>      void (*integer) (struct sprinter *, int);
>      void (*boolean) (struct sprinter *, notmuch_bool_t);
>      void (*null) (struct sprinter *);
> -- 
> 1.7.10
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Mark Walters]

On Wed, 25 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> ---
>  notmuch-show.c |   57 +++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index afbd9d0..fa1e6e9 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -110,34 +110,44 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
>  }
>  
>  static void
> -format_message_json (const void *ctx, notmuch_message_t *message)
> +format_message_json (sprinter_t *sp, notmuch_message_t *message)
>  {

It might be nice to have a comment for this function (and other similar ones) saying
what it prints: is it a sequence of key: value pairs, or a map or an
array etc. On the other hand it might be that it is best as it is where
you just read the code to see.

> +    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;
>  
>      date = notmuch_message_get_date (message);
> -    relative_date = notmuch_time_relative_date (ctx, date);
> +    relative_date = notmuch_time_relative_date (local, date);

This makes the diff very easy to read but it might be nicer to have
these two assignments further down where they are used.

Best wishes

Mark

> +
> +    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));
> +
> +    sp->map_key (sp, "excluded");
> +    sp->boolean (sp, notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED));
> +
> +    sp->map_key (sp, "filename");
> +    sp->string (sp, notmuch_message_get_filename (message));
> +
> +    sp->map_key (sp, "timestamp");
> +    date = notmuch_message_get_date (message);
> +    sp->integer (sp, date);
>  
> -    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> -	    json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
> -	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
> -	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
> -	    json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
> -	    date, relative_date);
> +    sp->map_key (sp, "date_relative");
> +    sp->string (sp, relative_date);
>  
> +    sp->map_key (sp, "tags");
> +    sp->begin_list (sp);
>      for (tags = notmuch_message_get_tags (message);
>  	 notmuch_tags_valid (tags);
>  	 notmuch_tags_move_to_next (tags))
> -    {
> -         printf("%s%s", first ? "" : ",",
> -               json_quote_str (ctx_quote, notmuch_tags_get (tags)));
> -         first = 0;
> -    }
> -    printf("], ");
> -    talloc_free (ctx_quote);
> +	sp->string (sp, notmuch_tags_get (tags));
> +    sp->end (sp);
> +
> +    talloc_free (local);
>  }
>  
>  /* Extract just the email address from the contents of a From:
> @@ -573,18 +583,19 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
>       * devel/schemata. */
>  
>      if (node->envelope_file) {
> -	printf ("{");
> -	format_message_json (ctx, node->envelope_file);
> +	sp->begin_map (sp);
> +	format_message_json (sp, node->envelope_file);
>  
> -	printf ("\"headers\": ");
> +	sp->map_key (sp, "headers");
>  	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
>  
>  	if (output_body) {
> -	    printf (", \"body\": [");
> +	    sp->map_key (sp, "body");
> +	    sp->begin_list (sp);
>  	    format_part_json (ctx, sp, mime_node_child (node, 0), first, TRUE);
> -	    printf ("]");
> +	    sp->end (sp);
>  	}
> -	printf ("}");
> +	sp->end (sp);
>  	return;
>      }
>  
> -- 
> 1.7.10
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Tomi Ollila]

On Wed, Jul 25 2012, Austin Clements <amdragon@MIT.EDU> wrote:

> 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).


The whole series looks good to me and was easy to read through.
The use of Python json.tool is pretty reasonable; It is shipped in
Python 2.6+ (For example perl does not ship json components by default,
making it more complicated to set up for this purpose). If that is too
"much", then we could also use tr -d '[:space:]' as an intermediate
solution (but that eats whitespace in quoted content too...)

I'll be afk for a few days then I test this... 

Tomi

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

[Austin Clements]

Quoth Mark Walters on Jul 25 at  7:57 pm:
> 
> Hi
> 
> I have read this series (apart from the test changes) and it is both
> very nice to review and looks good. It builds and all tests pass at all
> the interesting partial stages and when fully applied. I have a few very
> minor comments.
> 
> On Wed, 25 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> > 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 |   11 +++++++++--
> >  sprinter-text.c |   11 +++++++++--
> >  sprinter.h      |    1 +
> >  3 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/sprinter-json.c b/sprinter-json.c
> > index 4649655..2587ca6 100644
> > --- a/sprinter-json.c
> > +++ b/sprinter-json.c
> > @@ -89,7 +89,7 @@ json_end (struct sprinter *sp)
> >  }
> >  
> >  static void
> > -json_string (struct sprinter *sp, const char *val)
> > +json_string_len (struct sprinter *sp, const char *val, size_t len)
> >  {
> 
> I think this function could do with a comment along the lines of the
> commit message. It might be nice to document somewhere where/when we

Will do.  I put a comment on this function explaining how it handles
NULs, as well as a comment on the generic vtable pointer explaining
string_len.

> might actually have nulls in an encoded string (are they allowed in
> bodies, headers etc).

Interestingly, RFC822, while being limited to 7-bit ASCII, explicitly
did allow NULs [RFC822, CHAR non-terminal].  RFC2822 explicitly
obsoletes them [RFC2822 4 or appendix B], which means consumers should
support them, but generators must not generate them.  As usual, MIME
makes things more complicated by supporting a "binary" content
transfer encoding that does permits NULs.  However, good luck getting
that through SMTP business; SMTP theoretically allows any ASCII
control character, but the standard specifically warns that control
characters other than SP, HT, CR, and LF should be avoided [RFC2821
4.1.1.4].  Even if you've negotiated the 8BITMIME extension [RFC1652],
you're limited by what MIME's "8bit" content transfer encoding
permits, which differs from the "binary" content transfer encoding by
disallowing (guess what) NULs and long lines.

In other words: yes.

> Actually, do we know that the json emacs parser handles nulls correctly?

It does.  Emacs strings are clean, so json.el doesn't have to do
special anything to support them.

> Best wishes
> 
> Mark
> >      static const char *const escapes[] = {
> >  	['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
> > @@ -98,7 +98,7 @@ json_string (struct sprinter *sp, const char *val)
> >      struct sprinter_json *spj = json_begin_value (sp);
> >  
> >      fputc ('"', spj->stream);
> > -    for (; *val; ++val) {
> > +    for (; len; ++val, --len) {
> >  	unsigned char ch = *val;
> >  	if (ch < ARRAY_SIZE (escapes) && escapes[ch])
> >  	    fputs (escapes[ch], spj->stream);
> > @@ -111,6 +111,12 @@ json_string (struct sprinter *sp, const char *val)
> >  }
> >  
> >  static void
> > +json_string (struct sprinter *sp, const char *val)
> > +{
> > +    json_string_len (sp, val, strlen (val));
> > +}
> > +
> > +static void
> >  json_integer (struct sprinter *sp, int val)
> >  {
> >      struct sprinter_json *spj = json_begin_value (sp);
> > @@ -166,6 +172,7 @@ sprinter_json_create (const void *ctx, FILE *stream)
> >  	    .begin_list = json_begin_list,
> >  	    .end = json_end,
> >  	    .string = json_string,
> > +	    .string_len = json_string_len,
> >  	    .integer = json_integer,
> >  	    .boolean = json_boolean,
> >  	    .null = json_null,
> > diff --git a/sprinter-text.c b/sprinter-text.c
> > index b208840..dfa54b5 100644
> > --- a/sprinter-text.c
> > +++ b/sprinter-text.c
> > @@ -25,14 +25,20 @@ struct sprinter_text {
> >  };
> >  
> >  static void
> > -text_string (struct sprinter *sp, const char *val)
> > +text_string_len (struct sprinter *sp, const char *val, size_t len)
> >  {
> >      struct sprinter_text *sptxt = (struct sprinter_text *) sp;
> >  
> >      if (sptxt->current_prefix != NULL)
> >  	fprintf (sptxt->stream, "%s:", sptxt->current_prefix);
> >  
> > -    fputs(val, sptxt->stream);
> > +    fwrite (val, len, 1, sptxt->stream);
> > +}
> > +
> > +static void
> > +text_string (struct sprinter *sp, const char *val)
> > +{
> > +    text_string_len (sp, val, strlen (val));
> >  }
> >  
> >  static void
> > @@ -105,6 +111,7 @@ sprinter_text_create (const void *ctx, FILE *stream)
> >  	    .begin_list = text_begin_list,
> >  	    .end = text_end,
> >  	    .string = text_string,
> > +	    .string_len = text_string_len,
> >  	    .integer = text_integer,
> >  	    .boolean = text_boolean,
> >  	    .null = text_null,
> > diff --git a/sprinter.h b/sprinter.h
> > index 6680d41..826a852 100644
> > --- a/sprinter.h
> > +++ b/sprinter.h
> > @@ -28,6 +28,7 @@ typedef struct sprinter {
> >       * For string, the char * must be UTF-8 encoded.
> >       */
> >      void (*string) (struct sprinter *, const char *);
> > +    void (*string_len) (struct sprinter *, const char *, size_t);
> >      void (*integer) (struct sprinter *, int);
> >      void (*boolean) (struct sprinter *, notmuch_bool_t);
> >      void (*null) (struct sprinter *);
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> 

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.

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

[Austin Clements]

Quoth Mark Walters on Jul 25 at  8:03 pm:
> On Wed, 25 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> > ---
> >  notmuch-show.c |   57 +++++++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 34 insertions(+), 23 deletions(-)
> >
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index afbd9d0..fa1e6e9 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -110,34 +110,44 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message)
> >  }
> >  
> >  static void
> > -format_message_json (const void *ctx, notmuch_message_t *message)
> > +format_message_json (sprinter_t *sp, notmuch_message_t *message)
> >  {
> 
> It might be nice to have a comment for this function (and other
> similar ones) saying what it prints: is it a sequence of key: value
> pairs, or a map or an array etc. On the other hand it might be that
> it is best as it is where you just read the code to see.

Ah, good point.  I don't think this is so necessary for the other
functions, but this one is weird because it requires the caller to
begin the map (every other function simply emits a value, so it
doesn't depend on the caller's context).

> > +    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;
> >  
> >      date = notmuch_message_get_date (message);
> > -    relative_date = notmuch_time_relative_date (ctx, date);
> > +    relative_date = notmuch_time_relative_date (local, date);
> 
> This makes the diff very easy to read but it might be nicer to have
> these two assignments further down where they are used.

Done, and it is nicer.  (Apparently I had the 'date' assignment both
here and below!)

> Best wishes
> 
> Mark
> 
> > +
> > +    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));
> > +
> > +    sp->map_key (sp, "excluded");
> > +    sp->boolean (sp, notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED));
> > +
> > +    sp->map_key (sp, "filename");
> > +    sp->string (sp, notmuch_message_get_filename (message));
> > +
> > +    sp->map_key (sp, "timestamp");
> > +    date = notmuch_message_get_date (message);
> > +    sp->integer (sp, date);
> >  
> > -    printf ("\"id\": %s, \"match\": %s, \"excluded\": %s, \"filename\": %s, \"timestamp\": %ld, \"date_relative\": \"%s\", \"tags\": [",
> > -	    json_quote_str (ctx_quote, notmuch_message_get_message_id (message)),
> > -	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? "true" : "false",
> > -	    notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? "true" : "false",
> > -	    json_quote_str (ctx_quote, notmuch_message_get_filename (message)),
> > -	    date, relative_date);
> > +    sp->map_key (sp, "date_relative");
> > +    sp->string (sp, relative_date);
> >  
> > +    sp->map_key (sp, "tags");
> > +    sp->begin_list (sp);
> >      for (tags = notmuch_message_get_tags (message);
> >  	 notmuch_tags_valid (tags);
> >  	 notmuch_tags_move_to_next (tags))
> > -    {
> > -         printf("%s%s", first ? "" : ",",
> > -               json_quote_str (ctx_quote, notmuch_tags_get (tags)));
> > -         first = 0;
> > -    }
> > -    printf("], ");
> > -    talloc_free (ctx_quote);
> > +	sp->string (sp, notmuch_tags_get (tags));
> > +    sp->end (sp);
> > +
> > +    talloc_free (local);
> >  }
> >  
> >  /* Extract just the email address from the contents of a From:
> > @@ -573,18 +583,19 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node,
> >       * devel/schemata. */
> >  
> >      if (node->envelope_file) {
> > -	printf ("{");
> > -	format_message_json (ctx, node->envelope_file);
> > +	sp->begin_map (sp);
> > +	format_message_json (sp, node->envelope_file);
> >  
> > -	printf ("\"headers\": ");
> > +	sp->map_key (sp, "headers");
> >  	format_headers_json (sp, GMIME_MESSAGE (node->part), FALSE);
> >  
> >  	if (output_body) {
> > -	    printf (", \"body\": [");
> > +	    sp->map_key (sp, "body");
> > +	    sp->begin_list (sp);
> >  	    format_part_json (ctx, sp, mime_node_child (node, 0), first, TRUE);
> > -	    printf ("]");
> > +	    sp->end (sp);
> >  	}
> > -	printf ("}");
> > +	sp->end (sp);
> >  	return;
> >      }
> >