Return-Path: X-Original-To: notmuch@notmuchmail.org Delivered-To: notmuch@notmuchmail.org Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 68BFD431FAF for ; Thu, 12 Jul 2012 17:02:42 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id O-SDe8Ki+tco for ; Thu, 12 Jul 2012 17:02:39 -0700 (PDT) Received: from dmz-mailsec-scanner-2.mit.edu (DMZ-MAILSEC-SCANNER-2.MIT.EDU [18.9.25.13]) by olra.theworths.org (Postfix) with ESMTP id B3DEE431FAE for ; Thu, 12 Jul 2012 17:02:38 -0700 (PDT) X-AuditID: 1209190d-b7fd56d000000933-78-4fff659e682e Received: from mailhub-auth-2.mit.edu ( [18.7.62.36]) by dmz-mailsec-scanner-2.mit.edu (Symantec Messaging Gateway) with SMTP id C7.25.02355.E956FFF4; Thu, 12 Jul 2012 20:02:38 -0400 (EDT) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id q6D02bsI005845; Thu, 12 Jul 2012 20:02:37 -0400 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) (authenticated bits=0) (User authenticated as amdragon@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q6D02ZQW000240 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Thu, 12 Jul 2012 20:02:36 -0400 (EDT) Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) (envelope-from ) id 1SpTL8-00056E-UV; Thu, 12 Jul 2012 20:02:35 -0400 Date: Thu, 12 Jul 2012 20:02:34 -0400 From: Austin Clements To: craven@gmx.net Subject: Re: [PATCH v4 3/3] Use the structured format printer for JSON in notmuch search. Message-ID: <20120713000234.GK7332@mit.edu> References: <87d34hsdx8.fsf@awakening.csail.mit.edu> <1342079004-5300-1-git-send-email-craven@gmx.net> <1342079004-5300-4-git-send-email-craven@gmx.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1342079004-5300-4-git-send-email-craven@gmx.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprLKsWRmVeSWpSXmKPExsUixG6nojsv9b+/wbQXrBZ7G9oZLa7fnMns wOSxeNN+No9nq24xBzBFcdmkpOZklqUW6dslcGXMX3mGtWDLJ8aKS99uszQwth1m7GLk5JAQ MJE4MvUMC4QtJnHh3no2EFtIYB+jxJ1Ddl2MXED2BkaJnwuXMEMkTjJJ/NxvBJFYwiix+OIy sA4WAVWJN6f3gxWxCWhIbNu/HGyDiICQxKQvr8A2MAtIS3z73cwEYgsLREtMmHYZzOYV0Jb4 tfIbK8TQOYwS3QceMkMkBCVOznwC1awjsXPrHaBlHGCDlv/jgAjLSzRvnQ1WzilgJzHn0kaw clEBFYkpJ7exTWAUnoVk0iwkk2YhTJqFZNICRpZVjLIpuVW6uYmZOcWpybrFyYl5ealFukZ6 uZkleqkppZsYQbHAKcm7g/HdQaVDjAIcjEo8vL9i//sLsSaWFVfmHmKU5GBSEuWNSQIK8SXl p1RmJBZnxBeV5qQWH2KU4GBWEuFdZw+U401JrKxKLcqHSUlzsCiJ815JuekvJJCeWJKanZpa kFoEk5Xh4FCS4N2eAtQoWJSanlqRlplTgpBm4uAEGc4DNPwWSA1vcUFibnFmOkT+FKOilDjv EpCEAEgiozQPrheWql4xigO9Isw7H6SKB5jm4LpfAQ1mAho86+c/kMEliQgpqQZGh5gAlkOX T7TPm/7u37l9fHzcHg9Xu+yccq7srOymXtHaJZ/aFE+c//7FWzdSgF0zUn5lc8CkqHcnOLdM N5bqUdy254+0vm6DtNiv2zdFRBWPqxnvmhk7sdk+g++mxJO/8hNfWKgYvF/3f+PDR7Xr0rz/ f9ksWjRhZVmDpPOXF2VMe/w1Guo2K7EUZyQaajEXFScCABrTpOcwAwAA Cc: notmuch@notmuchmail.org X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Jul 2012 00:02:42 -0000 This is fantastic. It simplifies the code a lot, and I think it opens up opportunities to simplify it even further. Detailed comments are below, but first one general comment. For the text format, I wonder if most of the special case code would go away with a stub sprinter that did nothing for most operations and for string printed the string followed by a newline (or maybe the newline would be printed by the "frame" method or whatever you end up calling it). I believe this would unify all of the code in do_search_tags and do_search_files between the two formats. For do_search_threads, you'll still need some special-casing to format the summary line, but I think it would unify all of the framing code. (If this does work out, some of my comments below will be irrelevant.) Quoth craven@gmx.net on Jul 12 at 9:43 am: > This patch switches from the current ad-hoc printer to the structured > output formatter in sprinter.h. > > It removes search_format_t, replaces it by sprinter_t and inlines the > text printer where necessary. > > The tests are changed (only whitespaces and regular expressions) in > order to make them PASS for the new structured output formatter. > --- > notmuch-search.c | 349 ++++++++++++++++++++++------------------------------- > test/json | 20 +-- > test/search-output | 270 +++++++++++++++++++++-------------------- > 3 files changed, 288 insertions(+), 351 deletions(-) > > diff --git a/notmuch-search.c b/notmuch-search.c > index 3be296d..b853f5f 100644 > --- a/notmuch-search.c > +++ b/notmuch-search.c > @@ -19,6 +19,7 @@ > */ > > #include "notmuch-client.h" > +#include "sprinter.h" > > typedef enum { > OUTPUT_SUMMARY, > @@ -28,91 +29,9 @@ typedef enum { > OUTPUT_TAGS > } output_t; > > -typedef struct search_format { > - const char *results_start; > - const char *item_start; > - void (*item_id) (const void *ctx, > - const char *item_type, > - const char *item_id); > - void (*thread_summary) (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject); > - const char *tag_start; > - const char *tag; > - const char *tag_sep; > - const char *tag_end; > - const char *item_sep; > - const char *item_end; > - const char *results_end; > - const char *results_null; > -} search_format_t; > - > -static void > -format_item_id_text (const void *ctx, > - const char *item_type, > - const char *item_id); > - > -static void > -format_thread_text (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject); > -static const search_format_t format_text = { > - "", > - "", > - format_item_id_text, > - format_thread_text, > - " (", > - "%s", " ", > - ")", "\n", > - "", > - "\n", > - "", > -}; > - > -static void > -format_item_id_json (const void *ctx, > - const char *item_type, > - const char *item_id); > - > -static void > -format_thread_json (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject); > - > -/* Any changes to the JSON format should be reflected in the file > - * devel/schemata. */ > -static const search_format_t format_json = { > - "[", > - "{", > - format_item_id_json, > - format_thread_json, > - "\"tags\": [", > - "\"%s\"", ", ", > - "]", ",\n", > - "}", > - "]\n", > - "]\n", > -}; > - > -static void > -format_item_id_text (unused (const void *ctx), > - const char *item_type, > - const char *item_id) > -{ > - printf ("%s%s", item_type, item_id); > -} > +static const char * text_item_sep = "\n"; > +static const char * text_results_null = ""; > +static const char * text_results_end = "\n"; Given that you're special-casing the text format anyway, I think it's actually more readable if you hard-code these in the appropriate places below. > > static char * > sanitize_string (const void *ctx, const char *str) > @@ -131,72 +50,8 @@ sanitize_string (const void *ctx, const char *str) > return out; > } > > -static void > -format_thread_text (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject) > -{ > - void *ctx_quote = talloc_new (ctx); > - > - printf ("thread:%s %12s [%d/%d] %s; %s", > - thread_id, > - notmuch_time_relative_date (ctx, date), > - matched, > - total, > - sanitize_string (ctx_quote, authors), > - sanitize_string (ctx_quote, subject)); > - > - talloc_free (ctx_quote); > -} > - > -static void > -format_item_id_json (const void *ctx, > - unused (const char *item_type), > - const char *item_id) > -{ > - void *ctx_quote = talloc_new (ctx); > - > - printf ("%s", json_quote_str (ctx_quote, item_id)); > - > - talloc_free (ctx_quote); > - > -} > - > -static void > -format_thread_json (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject) > -{ > - void *ctx_quote = talloc_new (ctx); > - > - printf ("\"thread\": %s,\n" > - "\"timestamp\": %ld,\n" > - "\"date_relative\": \"%s\",\n" > - "\"matched\": %d,\n" > - "\"total\": %d,\n" > - "\"authors\": %s,\n" > - "\"subject\": %s,\n", > - json_quote_str (ctx_quote, thread_id), > - date, > - notmuch_time_relative_date (ctx, date), > - matched, > - total, > - json_quote_str (ctx_quote, authors), > - json_quote_str (ctx_quote, subject)); > - > - talloc_free (ctx_quote); > -} > - > static int > -do_search_threads (const search_format_t *format, > +do_search_threads (sprinter_t *format, > notmuch_query_t *query, > notmuch_sort_t sort, > output_t output, > @@ -220,7 +75,9 @@ do_search_threads (const search_format_t *format, > if (threads == NULL) > return 1; > > - fputs (format->results_start, stdout); > + if (format != sprinter_text) { > + format->begin_list (format); > + } > > for (i = 0; > notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit); > @@ -235,43 +92,96 @@ do_search_threads (const search_format_t *format, > continue; > } > > - if (! first_thread) > - fputs (format->item_sep, stdout); > + if (format == sprinter_text && ! first_thread) > + fputs (text_item_sep, stdout); I think you can drop this (and the first_thread variable) if you print ")\n" instead of ")" down below. This roundabout first_thread stuff was necessary to support the JSON format because it uses separators between results; however, the text format is much simpler because it just has the terminating newline after each result. Hence, you can just bake printing the newline in with printing the result and you never have to worry about whether it's the first result of if you didn't print any results. > > if (output == OUTPUT_THREADS) { > - format->item_id (thread, "thread:", > - notmuch_thread_get_thread_id (thread)); > + const char *thread_id = notmuch_thread_get_thread_id (thread); > + if (format == sprinter_text) > + printf ("thread:%s", thread_id); > + else { > + format->string (format, thread_id); > + format->frame (format); > + } > + > } else { /* output == OUTPUT_SUMMARY */ > - fputs (format->item_start, stdout); > + const char *authors = notmuch_thread_get_authors (thread); > + const char *subject = notmuch_thread_get_subject (thread); > + const char *thread_id = notmuch_thread_get_thread_id (thread); > + int matched = notmuch_thread_get_matched_messages (thread); > + int total = notmuch_thread_get_total_messages (thread); > + const char *relative_date = NULL; > + > + if (format != sprinter_text) > + format->begin_map (format); > + Extra blank line. > > if (sort == NOTMUCH_SORT_OLDEST_FIRST) > date = notmuch_thread_get_oldest_date (thread); > else > date = notmuch_thread_get_newest_date (thread); > > - format->thread_summary (thread, > - notmuch_thread_get_thread_id (thread), > - date, > - notmuch_thread_get_matched_messages (thread), > - notmuch_thread_get_total_messages (thread), > - notmuch_thread_get_authors (thread), > - notmuch_thread_get_subject (thread)); > + void *ctx_quote = talloc_new (thread); > + relative_date = > + notmuch_time_relative_date (ctx_quote, date); > + > + if (format == sprinter_text) { > + printf ("thread:%s %12s [%d/%d] %s; %s", > + thread_id, > + relative_date, > + matched, > + total, > + sanitize_string (ctx_quote, authors), > + sanitize_string (ctx_quote, subject)); > + } else { > + format->map_key (format, "thread"); > + format->string (format, thread_id); > + format->map_key (format, "timestamp"); > + format->integer (format, date); > + format->map_key (format, "date_relative"); > + format->string (format, relative_date); > + format->map_key (format, "matched"); > + format->integer (format, matched); > + format->map_key (format, "total"); > + format->integer (format, total); > + format->map_key (format, "authors"); > + format->string (format, authors); > + format->map_key (format, "subject"); > + format->string (format, subject); > + } > + > + talloc_free (ctx_quote); > > - fputs (format->tag_start, stdout); > + if (format == sprinter_text) { > + fputs (" (", stdout); > + } else { > + format->map_key (format, "tags"); > + format->begin_list (format); > + } > > for (tags = notmuch_thread_get_tags (thread); > notmuch_tags_valid (tags); > notmuch_tags_move_to_next (tags)) > { > - if (! first_tag) > - fputs (format->tag_sep, stdout); > - printf (format->tag, notmuch_tags_get (tags)); > + const char *tag = notmuch_tags_get (tags); > + if (format == sprinter_text) { > + if (! first_tag) > + fputs (" ", stdout); > + fputs (tag, stdout); > + } else { > + format->string (format, tag); > + } > + > first_tag = 0; > } > > - fputs (format->tag_end, stdout); > - > - fputs (format->item_end, stdout); > + if (format == sprinter_text) { > + fputs (")", stdout); > + } else { > + format->end (format); > + format->end (format); > + format->frame (format); > + } > } > > first_thread = 0; > @@ -279,16 +189,20 @@ do_search_threads (const search_format_t *format, > notmuch_thread_destroy (thread); > } > > - if (first_thread) > - fputs (format->results_null, stdout); > - else > - fputs (format->results_end, stdout); > + if (format == sprinter_text) > + if (first_thread) > + fputs (text_results_null, stdout); > + else > + fputs (text_results_end, stdout); You can drop this, too, if you print ")\n" instead of ")" above. > + else { > + format->end (format); > + } > > return 0; > } > > static int > -do_search_messages (const search_format_t *format, > +do_search_messages (sprinter_t *format, > notmuch_query_t *query, > output_t output, > int offset, > @@ -310,7 +224,8 @@ do_search_messages (const search_format_t *format, > if (messages == NULL) > return 1; > > - fputs (format->results_start, stdout); > + if (format != sprinter_text) > + format->begin_list (format); > > for (i = 0; > notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit); > @@ -328,23 +243,36 @@ do_search_messages (const search_format_t *format, > notmuch_filenames_valid (filenames); > notmuch_filenames_move_to_next (filenames)) > { > - if (! first_message) > - fputs (format->item_sep, stdout); > + const char *filenames_str = notmuch_filenames_get (filenames); > + > + if (format == sprinter_text && ! first_message) > + fputs (text_item_sep, stdout); Likewise, you can remove this, the first_message logic for OUTPUT_MESSAGES, and the first_message logic at the end of this function if you print a newline after filenames_str and message_id. > > - format->item_id (message, "", > - notmuch_filenames_get (filenames)); > + if (format == sprinter_text) > + fputs (filenames_str, stdout); > + else { > + format->string (format, filenames_str); > + format->frame (format); > + } > > first_message = 0; > } > - > - notmuch_filenames_destroy( filenames ); > + > + notmuch_filenames_destroy (filenames); No need to reformat this. > > } else { /* output == OUTPUT_MESSAGES */ > - if (! first_message) > - fputs (format->item_sep, stdout); > + const char *message_id = notmuch_message_get_message_id (message); > + > + if (format == sprinter_text && ! first_message) > + fputs (text_item_sep, stdout); > + > + if (format == sprinter_text) > + printf ("id:%s", message_id); > + else { > + format->string (format, message_id); > + format->frame (format); > + } > > - format->item_id (message, "id:", > - notmuch_message_get_message_id (message)); > first_message = 0; > } > > @@ -353,17 +281,21 @@ do_search_messages (const search_format_t *format, > > notmuch_messages_destroy (messages); > > - if (first_message) > - fputs (format->results_null, stdout); > - else > - fputs (format->results_end, stdout); > + if (format == sprinter_text) > + if (first_message) > + fputs (text_results_null, stdout); > + else > + fputs (text_results_end, stdout); > + else { > + format->end (format); > + } > > return 0; > } > > static int > do_search_tags (notmuch_database_t *notmuch, > - const search_format_t *format, > + sprinter_t *format, > notmuch_query_t *query) > { > notmuch_messages_t *messages = NULL; > @@ -387,7 +319,8 @@ do_search_tags (notmuch_database_t *notmuch, > if (tags == NULL) > return 1; > > - fputs (format->results_start, stdout); > + if (format != sprinter_text) > + format->begin_list (format); > > for (; > notmuch_tags_valid (tags); > @@ -395,10 +328,15 @@ do_search_tags (notmuch_database_t *notmuch, > { > tag = notmuch_tags_get (tags); > > - if (! first_tag) > - fputs (format->item_sep, stdout); > + if (format == sprinter_text && ! first_tag) > + fputs (text_item_sep, stdout); Same thing about dropping the first_tag logic in this function if you print a newline after tag below. > > - format->item_id (tags, "", tag); > + if (format == sprinter_text) > + fputs (tag, stdout); > + else { > + format->string (format, tag); > + format->frame (format); > + } > > first_tag = 0; > } > @@ -408,10 +346,14 @@ do_search_tags (notmuch_database_t *notmuch, > if (messages) > notmuch_messages_destroy (messages); > > - if (first_tag) > - fputs (format->results_null, stdout); > - else > - fputs (format->results_end, stdout); > + if (format == sprinter_text) > + if (first_tag) > + fputs (text_results_null, stdout); > + else > + fputs (text_results_end, stdout); > + else { > + format->end (format); > + } > > return 0; > } > @@ -430,7 +372,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) > notmuch_query_t *query; > char *query_str; > notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST; > - const search_format_t *format = &format_text; > + sprinter_t *format = NULL; /* the default output is text */ > int opt_index, ret; > output_t output = OUTPUT_SUMMARY; > int offset = 0; > @@ -475,10 +417,10 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) > > switch (format_sel) { > case NOTMUCH_FORMAT_TEXT: > - format = &format_text; > + format = NULL; > break; > case NOTMUCH_FORMAT_JSON: > - format = &format_json; > + format = sprinter_json_new (ctx, stdout); > break; > } > > @@ -546,5 +488,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) > notmuch_query_destroy (query); > notmuch_database_destroy (notmuch); > > + if (format != sprinter_text) > + talloc_free(format); Missing space before argument list. > + > return ret; > } > diff --git a/test/json b/test/json > index 6439788..88b8a6d 100755 > --- a/test/json > +++ b/test/json > @@ -10,14 +10,8 @@ test_expect_equal "$output" "[[[{\"id\": \"${gen_msg_id}\", \"match\": true, \"e > test_begin_subtest "Search message: json" > add_message "[subject]=\"json-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"json-search-message\"" > output=$(notmuch search --format=json "json-search-message" | notmuch_search_sanitize) If you additionally pipe this through notmuch_json_show_sanitize (or maybe define a similar function for search), then the output will be much closer to the output currently in the test. > -test_expect_equal "$output" "[{\"thread\": \"XXX\", > -\"timestamp\": 946728000, > -\"date_relative\": \"2000-01-01\", > -\"matched\": 1, > -\"total\": 1, > -\"authors\": \"Notmuch Test Suite\", > -\"subject\": \"json-search-subject\", > -\"tags\": [\"inbox\", \"unread\"]}]" > +test_expect_equal "$output" "[{\"thread\": \"XXX\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"matched\": 1, \"total\": 1, \"authors\": \"Notmuch Test Suite\", \"subject\": \"json-search-subject\", \"tags\": [\"inbox\", \"unread\"]} > +]" > > 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\"" > @@ -40,13 +34,7 @@ test_expect_equal "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": > test_begin_subtest "Search message: json, utf-8" > add_message "[subject]=\"json-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\"" > output=$(notmuch search --format=json "jsön-search-méssage" | notmuch_search_sanitize) Same here. > -test_expect_equal "$output" "[{\"thread\": \"XXX\", > -\"timestamp\": 946728000, > -\"date_relative\": \"2000-01-01\", > -\"matched\": 1, > -\"total\": 1, > -\"authors\": \"Notmuch Test Suite\", > -\"subject\": \"json-search-utf8-body-sübjéct\", > -\"tags\": [\"inbox\", \"unread\"]}]" > +test_expect_equal "$output" "[{\"thread\": \"XXX\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"matched\": 1, \"total\": 1, \"authors\": \"Notmuch Test Suite\", \"subject\": \"json-search-utf8-body-sübjéct\", \"tags\": [\"inbox\", \"unread\"]} > +]" > > test_done > diff --git a/test/search-output b/test/search-output > index 8b57a43..f2650f7 100755 > --- a/test/search-output > +++ b/test/search-output > @@ -37,30 +37,31 @@ test_expect_equal_file OUTPUT EXPECTED > test_begin_subtest "--output=threads --format=json" > notmuch search --format=json --output=threads '*' | sed -e s/\".*\"/\"THREADID\"/ >OUTPUT > cat <EXPECTED > -["THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID", > -"THREADID"] > +["THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +, "THREADID" > +] Hmm. It would be nice if json_frame printed this in the way people usually format JSON and the test currently expects. Doing this isn't entirely straightforward because the caller could call frame and then immediately call end, so you can't just print ",\n"... You could replace json_state.first with /* The separator to print before the next value. */ const char *sep; json_begin_value would fputs sep and set it to ", ", json_begin_aggregate would set it to "", and json_map_key would set it to ": " and not fputs(": ", spj->stream) itself. json_frame could then be something like static void json_frame(struct sprinter *sp) { struct json_state *state = ((struct sprinter_json*)sp)->state; if (!state) return; if (state->sep[0] == ',') state->sep = ",\n"; else if (state->sep[0] == ':') state->sep = ":\n"; } I think that would work. > EOF > test_expect_equal_file OUTPUT EXPECTED > > @@ -125,58 +126,59 @@ test_expect_equal_file OUTPUT EXPECTED > test_begin_subtest "--output=messages --format=json" > notmuch search --format=json --output=messages '*' >OUTPUT > cat <EXPECTED > -["4EFC743A.3060609@april.org", > -"877h1wv7mg.fsf@inf-8657.int-evry.fr", > -"1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk", > -"877htoqdbo.fsf@yoom.home.cworth.org", > -"878we4qdqf.fsf@yoom.home.cworth.org", > -"87aaykqe24.fsf@yoom.home.cworth.org", > -"87bpj0qeng.fsf@yoom.home.cworth.org", > -"87fx8cqf8v.fsf@yoom.home.cworth.org", > -"87hbssqfix.fsf@yoom.home.cworth.org", > -"87iqd8qgiz.fsf@yoom.home.cworth.org", > -"87k4xoqgnl.fsf@yoom.home.cworth.org", > -"87ocn0qh6d.fsf@yoom.home.cworth.org", > -"87pr7gqidx.fsf@yoom.home.cworth.org", > -"867hto2p0t.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me", > -"1258532999-9316-1-git-send-email-keithp@keithp.com", > -"86aayk2rbj.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me", > -"86d43g2w3y.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me", > -"ddd65cda0911172214t60d22b63hcfeb5a19ab54a39b@mail.gmail.com", > -"86einw2xof.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me", > -"736613.51770.qm@web113505.mail.gq1.yahoo.com", > -"1258520223-15328-1-git-send-email-jan@ryngle.com", > -"ddd65cda0911171950o4eea4389v86de9525e46052d3@mail.gmail.com", > -"1258510940-7018-1-git-send-email-stewart@flamingspork.com", > -"yunzl6kd1w0.fsf@aiko.keithp.com", > -"yun1vjwegii.fsf@aiko.keithp.com", > -"yun3a4cegoa.fsf@aiko.keithp.com", > -"1258509400-32511-1-git-send-email-stewart@flamingspork.com", > -"1258506353-20352-1-git-send-email-stewart@flamingspork.com", > -"20091118010116.GC25380@dottiness.seas.harvard.edu", > -"20091118005829.GB25380@dottiness.seas.harvard.edu", > -"20091118005040.GA25380@dottiness.seas.harvard.edu", > -"cf0c4d610911171623q3e27a0adx802e47039b57604b@mail.gmail.com", > -"1258500222-32066-1-git-send-email-ingmar@exherbo.org", > -"20091117232137.GA7669@griffis1.net", > -"20091118002059.067214ed@hikari", > -"1258498485-sup-142@elly", > -"f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com", > -"f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com", > -"1258496327-12086-1-git-send-email-jan@ryngle.com", > -"1258493565-13508-1-git-send-email-keithp@keithp.com", > -"yunaayketfm.fsf@aiko.keithp.com", > -"yunbpj0etua.fsf@aiko.keithp.com", > -"1258491078-29658-1-git-send-email-dottedmag@dottedmag.net", > -"87fx8can9z.fsf@vertex.dottedmag", > -"20091117203301.GV3165@dottiness.seas.harvard.edu", > -"87lji4lx9v.fsf@yoom.home.cworth.org", > -"cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com", > -"87iqd9rn3l.fsf@vertex.dottedmag", > -"20091117190054.GU3165@dottiness.seas.harvard.edu", > -"87lji5cbwo.fsf@yoom.home.cworth.org", > -"1258471718-6781-2-git-send-email-dottedmag@dottedmag.net", > -"1258471718-6781-1-git-send-email-dottedmag@dottedmag.net"] > +["4EFC743A.3060609@april.org" > +, "877h1wv7mg.fsf@inf-8657.int-evry.fr" > +, "1258544095-16616-1-git-send-email-chris@chris-wilson.co.uk" > +, "877htoqdbo.fsf@yoom.home.cworth.org" > +, "878we4qdqf.fsf@yoom.home.cworth.org" > +, "87aaykqe24.fsf@yoom.home.cworth.org" > +, "87bpj0qeng.fsf@yoom.home.cworth.org" > +, "87fx8cqf8v.fsf@yoom.home.cworth.org" > +, "87hbssqfix.fsf@yoom.home.cworth.org" > +, "87iqd8qgiz.fsf@yoom.home.cworth.org" > +, "87k4xoqgnl.fsf@yoom.home.cworth.org" > +, "87ocn0qh6d.fsf@yoom.home.cworth.org" > +, "87pr7gqidx.fsf@yoom.home.cworth.org" > +, "867hto2p0t.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me" > +, "1258532999-9316-1-git-send-email-keithp@keithp.com" > +, "86aayk2rbj.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me" > +, "86d43g2w3y.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me" > +, "ddd65cda0911172214t60d22b63hcfeb5a19ab54a39b@mail.gmail.com" > +, "86einw2xof.fsf@fortitudo.i-did-not-set--mail-host-address--so-tickle-me" > +, "736613.51770.qm@web113505.mail.gq1.yahoo.com" > +, "1258520223-15328-1-git-send-email-jan@ryngle.com" > +, "ddd65cda0911171950o4eea4389v86de9525e46052d3@mail.gmail.com" > +, "1258510940-7018-1-git-send-email-stewart@flamingspork.com" > +, "yunzl6kd1w0.fsf@aiko.keithp.com" > +, "yun1vjwegii.fsf@aiko.keithp.com" > +, "yun3a4cegoa.fsf@aiko.keithp.com" > +, "1258509400-32511-1-git-send-email-stewart@flamingspork.com" > +, "1258506353-20352-1-git-send-email-stewart@flamingspork.com" > +, "20091118010116.GC25380@dottiness.seas.harvard.edu" > +, "20091118005829.GB25380@dottiness.seas.harvard.edu" > +, "20091118005040.GA25380@dottiness.seas.harvard.edu" > +, "cf0c4d610911171623q3e27a0adx802e47039b57604b@mail.gmail.com" > +, "1258500222-32066-1-git-send-email-ingmar@exherbo.org" > +, "20091117232137.GA7669@griffis1.net" > +, "20091118002059.067214ed@hikari" > +, "1258498485-sup-142@elly" > +, "f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com" > +, "f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com" > +, "1258496327-12086-1-git-send-email-jan@ryngle.com" > +, "1258493565-13508-1-git-send-email-keithp@keithp.com" > +, "yunaayketfm.fsf@aiko.keithp.com" > +, "yunbpj0etua.fsf@aiko.keithp.com" > +, "1258491078-29658-1-git-send-email-dottedmag@dottedmag.net" > +, "87fx8can9z.fsf@vertex.dottedmag" > +, "20091117203301.GV3165@dottiness.seas.harvard.edu" > +, "87lji4lx9v.fsf@yoom.home.cworth.org" > +, "cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" > +, "87iqd9rn3l.fsf@vertex.dottedmag" > +, "20091117190054.GU3165@dottiness.seas.harvard.edu" > +, "87lji5cbwo.fsf@yoom.home.cworth.org" > +, "1258471718-6781-2-git-send-email-dottedmag@dottedmag.net" > +, "1258471718-6781-1-git-send-email-dottedmag@dottedmag.net" > +] > EOF > test_expect_equal_file OUTPUT EXPECTED > > @@ -242,59 +244,60 @@ test_expect_equal_file OUTPUT EXPECTED > test_begin_subtest "--output=files --format=json" > notmuch search --format=json --output=files '*' | sed -e "s,$MAIL_DIR,MAIL_DIR," >OUTPUT > cat <EXPECTED > -["MAIL_DIR/cur/52:2,", > -"MAIL_DIR/cur/53:2,", > -"MAIL_DIR/cur/50:2,", > -"MAIL_DIR/cur/49:2,", > -"MAIL_DIR/cur/48:2,", > -"MAIL_DIR/cur/47:2,", > -"MAIL_DIR/cur/46:2,", > -"MAIL_DIR/cur/45:2,", > -"MAIL_DIR/cur/44:2,", > -"MAIL_DIR/cur/43:2,", > -"MAIL_DIR/cur/42:2,", > -"MAIL_DIR/cur/41:2,", > -"MAIL_DIR/cur/40:2,", > -"MAIL_DIR/cur/39:2,", > -"MAIL_DIR/cur/38:2,", > -"MAIL_DIR/cur/37:2,", > -"MAIL_DIR/cur/36:2,", > -"MAIL_DIR/cur/35:2,", > -"MAIL_DIR/cur/34:2,", > -"MAIL_DIR/cur/33:2,", > -"MAIL_DIR/cur/32:2,", > -"MAIL_DIR/cur/31:2,", > -"MAIL_DIR/cur/30:2,", > -"MAIL_DIR/cur/29:2,", > -"MAIL_DIR/cur/28:2,", > -"MAIL_DIR/cur/27:2,", > -"MAIL_DIR/cur/26:2,", > -"MAIL_DIR/cur/25:2,", > -"MAIL_DIR/cur/24:2,", > -"MAIL_DIR/cur/23:2,", > -"MAIL_DIR/cur/22:2,", > -"MAIL_DIR/cur/21:2,", > -"MAIL_DIR/cur/19:2,", > -"MAIL_DIR/cur/18:2,", > -"MAIL_DIR/cur/51:2,", > -"MAIL_DIR/cur/20:2,", > -"MAIL_DIR/cur/17:2,", > -"MAIL_DIR/cur/16:2,", > -"MAIL_DIR/cur/15:2,", > -"MAIL_DIR/cur/14:2,", > -"MAIL_DIR/cur/13:2,", > -"MAIL_DIR/cur/12:2,", > -"MAIL_DIR/cur/11:2,", > -"MAIL_DIR/cur/10:2,", > -"MAIL_DIR/cur/09:2,", > -"MAIL_DIR/cur/08:2,", > -"MAIL_DIR/cur/06:2,", > -"MAIL_DIR/cur/05:2,", > -"MAIL_DIR/cur/04:2,", > -"MAIL_DIR/cur/03:2,", > -"MAIL_DIR/cur/07:2,", > -"MAIL_DIR/cur/02:2,", > -"MAIL_DIR/cur/01:2,"] > +["MAIL_DIR/cur/52:2," > +, "MAIL_DIR/cur/53:2," > +, "MAIL_DIR/cur/50:2," > +, "MAIL_DIR/cur/49:2," > +, "MAIL_DIR/cur/48:2," > +, "MAIL_DIR/cur/47:2," > +, "MAIL_DIR/cur/46:2," > +, "MAIL_DIR/cur/45:2," > +, "MAIL_DIR/cur/44:2," > +, "MAIL_DIR/cur/43:2," > +, "MAIL_DIR/cur/42:2," > +, "MAIL_DIR/cur/41:2," > +, "MAIL_DIR/cur/40:2," > +, "MAIL_DIR/cur/39:2," > +, "MAIL_DIR/cur/38:2," > +, "MAIL_DIR/cur/37:2," > +, "MAIL_DIR/cur/36:2," > +, "MAIL_DIR/cur/35:2," > +, "MAIL_DIR/cur/34:2," > +, "MAIL_DIR/cur/33:2," > +, "MAIL_DIR/cur/32:2," > +, "MAIL_DIR/cur/31:2," > +, "MAIL_DIR/cur/30:2," > +, "MAIL_DIR/cur/29:2," > +, "MAIL_DIR/cur/28:2," > +, "MAIL_DIR/cur/27:2," > +, "MAIL_DIR/cur/26:2," > +, "MAIL_DIR/cur/25:2," > +, "MAIL_DIR/cur/24:2," > +, "MAIL_DIR/cur/23:2," > +, "MAIL_DIR/cur/22:2," > +, "MAIL_DIR/cur/21:2," > +, "MAIL_DIR/cur/19:2," > +, "MAIL_DIR/cur/18:2," > +, "MAIL_DIR/cur/51:2," > +, "MAIL_DIR/cur/20:2," > +, "MAIL_DIR/cur/17:2," > +, "MAIL_DIR/cur/16:2," > +, "MAIL_DIR/cur/15:2," > +, "MAIL_DIR/cur/14:2," > +, "MAIL_DIR/cur/13:2," > +, "MAIL_DIR/cur/12:2," > +, "MAIL_DIR/cur/11:2," > +, "MAIL_DIR/cur/10:2," > +, "MAIL_DIR/cur/09:2," > +, "MAIL_DIR/cur/08:2," > +, "MAIL_DIR/cur/06:2," > +, "MAIL_DIR/cur/05:2," > +, "MAIL_DIR/cur/04:2," > +, "MAIL_DIR/cur/03:2," > +, "MAIL_DIR/cur/07:2," > +, "MAIL_DIR/cur/02:2," > +, "MAIL_DIR/cur/01:2," > +] > EOF > test_expect_equal_file OUTPUT EXPECTED > > @@ -311,10 +314,11 @@ test_expect_equal_file OUTPUT EXPECTED > test_begin_subtest "--output=tags --format=json" > notmuch search --format=json --output=tags '*' >OUTPUT > cat <EXPECTED > -["attachment", > -"inbox", > -"signed", > -"unread"] > +["attachment" > +, "inbox" > +, "signed" > +, "unread" > +] > EOF > test_expect_equal_file OUTPUT EXPECTED >