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 9D7D8431FB6 for ; Fri, 13 Jul 2012 19:09:59 -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 uXg3NFNtSJQX for ; Fri, 13 Jul 2012 19:09:57 -0700 (PDT) Received: from dmz-mailsec-scanner-7.mit.edu (DMZ-MAILSEC-SCANNER-7.MIT.EDU [18.7.68.36]) by olra.theworths.org (Postfix) with ESMTP id A22E8431FAE for ; Fri, 13 Jul 2012 19:09:57 -0700 (PDT) X-AuditID: 12074424-b7f2a6d0000008bf-16-5000d4f548e4 Received: from mailhub-auth-1.mit.edu ( [18.9.21.35]) by dmz-mailsec-scanner-7.mit.edu (Symantec Messaging Gateway) with SMTP id D6.1C.02239.5F4D0005; Fri, 13 Jul 2012 22:09:57 -0400 (EDT) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id q6E29ukA022868; Fri, 13 Jul 2012 22:09:56 -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 q6E29tcc009849 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Fri, 13 Jul 2012 22:09:55 -0400 (EDT) Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) (envelope-from ) id 1Sprnu-0000f4-Sr; Fri, 13 Jul 2012 22:09:54 -0400 Date: Fri, 13 Jul 2012 22:09:54 -0400 From: Austin Clements To: Peter Feigl Subject: Re: [PATCH v5 3/3] Use the structured formatters in notmuch-search.c. Message-ID: <20120714020954.GD31670@mit.edu> References: <20120710191331.GE7332@mit.edu> <1342167097-25012-1-git-send-email-craven@gmx.net> <1342167097-25012-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: <1342167097-25012-4-git-send-email-craven@gmx.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprDKsWRmVeSWpSXmKPExsUixCmqrPv1CkOAwcqtHBZ7G9oZLa7fnMns wOSxeNN+No9nq24xBzBFcdmkpOZklqUW6dslcGX8vXiSuWBKXcXnm2tZGxjnxXcxcnBICJhI rP4j2MXICWSKSVy4t56ti5GLQ0hgH6PEp9bPUM4GRomVDyayQjgnmSRmbbgK5SxhlPjx/Asz SD+LgKrEmpmbWUBsNgENiW37lzOC2CICChLP1jWB2cwC0hLffjczgdjCAn4SFxb1soHYvAI6 Ept372GEGDqVUWLeit+MEAlBiZMzn7BANOtI7Nx6hw3kbpBBy/9xQITlJZq3zga7gVPAXuLq 6s3sILaogIrElJPb2CYwCs9CMmkWkkmzECbNQjJpASPLKkbZlNwq3dzEzJzi1GTd4uTEvLzU Il1zvdzMEr3UlNJNjKBIYHdR2cHYfEjpEKMAB6MSD2+SP0OAEGtiWXFl7iFGSQ4mJVHeWReB QnxJ+SmVGYnFGfFFpTmpxYcYJTiYlUR4zduAcrwpiZVVqUX5MClpDhYlcd7rKTf9hQTSE0tS s1NTC1KLYLIyHBxKEryFwIgXEixKTU+tSMvMKUFIM3FwggznARreAlLDW1yQmFucmQ6RP8Wo KCXOGwKSEABJZJTmwfXCEtUrRnGgV4R540GqeIBJDq77FdBgJqDBs37+8wcaXJKIkJJqYDT5 zDhXLax3wr8rR6Mn7fv3RcXMPXfe9DunHMucw3dNL3T7tKZ9W+LNpfHNp80t98+awbA77zO/ v5IOf0BGftLFKx1zF0xe8KfqSK3bH3/vGfy/vJvub5i5ZottlV7GDE3pPmPJG86LlPa4PE2y l5P+dSZa/rZ28PulWf9X7GFqfvS+pzRrSq0SS3FGoqEWc1FxIgDICgKQLwMAAA== 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: Sat, 14 Jul 2012 02:09:59 -0000 Just one comment below. Otherwise this patch LGTM (though some of my comments on the previous patch will affect this one). It's nice to see all of that old newline logic disappear. Quoth Peter Feigl on Jul 13 at 10:11 am: > From: > > This patch switches from the current ad-hoc printer to the structured > output formatter in sprinter.h, sprinter-text-search.c and > sprinter-json.c. > > The JSON tests are changed slightly in order to make them PASS for the > new structured output formatter. > > The Text tests pass without adaptation. > --- > notmuch-search.c | 292 +++++++++++-------------------------------------------- > test/json | 18 +--- > 2 files changed, 58 insertions(+), 252 deletions(-) > > diff --git a/notmuch-search.c b/notmuch-search.c > index 3be296d..99fddac 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,175 +29,8 @@ typedef enum { > OUTPUT_TAGS > } output_t; > > -typedef struct search_format { > - const char *results_start; > - const char *item_start; > - void (*item_id) (const void *ctx, > - const char *item_type, > - const char *item_id); > - void (*thread_summary) (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject); > - const char *tag_start; > - const char *tag; > - const char *tag_sep; > - const char *tag_end; > - const char *item_sep; > - const char *item_end; > - const char *results_end; > - const char *results_null; > -} search_format_t; > - > -static void > -format_item_id_text (const void *ctx, > - const char *item_type, > - const char *item_id); > - > -static void > -format_thread_text (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject); > -static const search_format_t format_text = { > - "", > - "", > - format_item_id_text, > - format_thread_text, > - " (", > - "%s", " ", > - ")", "\n", > - "", > - "\n", > - "", > -}; > - > -static void > -format_item_id_json (const void *ctx, > - const char *item_type, > - const char *item_id); > - > -static void > -format_thread_json (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject); > - > -/* Any changes to the JSON format should be reflected in the file > - * devel/schemata. */ > -static const search_format_t format_json = { > - "[", > - "{", > - format_item_id_json, > - format_thread_json, > - "\"tags\": [", > - "\"%s\"", ", ", > - "]", ",\n", > - "}", > - "]\n", > - "]\n", > -}; > - > -static void > -format_item_id_text (unused (const void *ctx), > - const char *item_type, > - const char *item_id) > -{ > - printf ("%s%s", item_type, item_id); > -} > - > -static char * > -sanitize_string (const void *ctx, const char *str) > -{ > - char *out, *loop; > - > - if (NULL == str) > - return NULL; > - > - loop = out = talloc_strdup (ctx, str); > - > - for (; *loop; loop++) { > - if ((unsigned char)(*loop) < 32) > - *loop = '?'; > - } > - return out; > -} > - > -static void > -format_thread_text (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject) > -{ > - void *ctx_quote = talloc_new (ctx); > - > - printf ("thread:%s %12s [%d/%d] %s; %s", > - thread_id, > - notmuch_time_relative_date (ctx, date), > - matched, > - total, > - sanitize_string (ctx_quote, authors), > - sanitize_string (ctx_quote, subject)); > - > - talloc_free (ctx_quote); > -} > - > -static void > -format_item_id_json (const void *ctx, > - unused (const char *item_type), > - const char *item_id) > -{ > - void *ctx_quote = talloc_new (ctx); > - > - printf ("%s", json_quote_str (ctx_quote, item_id)); > - > - talloc_free (ctx_quote); > - > -} > - > -static void > -format_thread_json (const void *ctx, > - const char *thread_id, > - const time_t date, > - const int matched, > - const int total, > - const char *authors, > - const char *subject) > -{ > - void *ctx_quote = talloc_new (ctx); > - > - printf ("\"thread\": %s,\n" > - "\"timestamp\": %ld,\n" > - "\"date_relative\": \"%s\",\n" > - "\"matched\": %d,\n" > - "\"total\": %d,\n" > - "\"authors\": %s,\n" > - "\"subject\": %s,\n", > - json_quote_str (ctx_quote, thread_id), > - date, > - notmuch_time_relative_date (ctx, date), > - matched, > - total, > - json_quote_str (ctx_quote, authors), > - json_quote_str (ctx_quote, subject)); > - > - talloc_free (ctx_quote); > -} > - > static int > -do_search_threads (const search_format_t *format, > +do_search_threads (sprinter_t *format, > notmuch_query_t *query, > notmuch_sort_t sort, > output_t output, > @@ -207,7 +41,6 @@ do_search_threads (const search_format_t *format, > notmuch_threads_t *threads; > notmuch_tags_t *tags; > time_t date; > - int first_thread = 1; > int i; > > if (offset < 0) { > @@ -220,14 +53,12 @@ do_search_threads (const search_format_t *format, > if (threads == NULL) > return 1; > > - fputs (format->results_start, stdout); > + format->begin_list (format); > > for (i = 0; > notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit); > notmuch_threads_move_to_next (threads), i++) > { > - int first_tag = 1; > - > thread = notmuch_threads_get (threads); > > if (i < offset) { > @@ -235,60 +66,65 @@ do_search_threads (const search_format_t *format, > continue; > } > > - if (! first_thread) > - fputs (format->item_sep, stdout); > - > if (output == OUTPUT_THREADS) { > - format->item_id (thread, "thread:", > - notmuch_thread_get_thread_id (thread)); > + format->set_prefix (format, "thread"); > + format->string (format, > + notmuch_thread_get_thread_id (thread)); > + format->separator (format); > } else { /* output == OUTPUT_SUMMARY */ > - fputs (format->item_start, stdout); > + void *ctx_quote = talloc_new (thread); > + > + format->begin_map (format); > > if (sort == NOTMUCH_SORT_OLDEST_FIRST) > date = notmuch_thread_get_oldest_date (thread); > else > date = notmuch_thread_get_newest_date (thread); > > - format->thread_summary (thread, > - notmuch_thread_get_thread_id (thread), > - date, > - notmuch_thread_get_matched_messages (thread), > - notmuch_thread_get_total_messages (thread), > - notmuch_thread_get_authors (thread), > - notmuch_thread_get_subject (thread)); > - > - fputs (format->tag_start, stdout); > + format->map_key (format, "thread"); > + format->string (format, notmuch_thread_get_thread_id (thread)); > + format->map_key (format, "timestamp"); > + format->integer (format, date); > + format->map_key (format, "date_relative"); > + format->string (format, notmuch_time_relative_date (ctx_quote, date)); > + format->map_key (format, "matched"); > + format->integer (format, notmuch_thread_get_matched_messages (thread)); > + format->map_key (format, "total"); > + format->integer (format, notmuch_thread_get_total_messages (thread)); > + format->map_key (format, "authors"); > + format->string (format, notmuch_thread_get_authors (thread)); > + format->map_key (format, "subject"); > + format->string (format, notmuch_thread_get_subject (thread)); > + > + talloc_free (ctx_quote); > + > + format->map_key (format, "tags"); > + format->begin_list (format); > > for (tags = notmuch_thread_get_tags (thread); > notmuch_tags_valid (tags); > notmuch_tags_move_to_next (tags)) > { > - if (! first_tag) > - fputs (format->tag_sep, stdout); > - printf (format->tag, notmuch_tags_get (tags)); > - first_tag = 0; > - } > + const char *tag = notmuch_tags_get (tags); > > - fputs (format->tag_end, stdout); > + format->string (format, tag); > + } > > - fputs (format->item_end, stdout); > + format->end (format); > + format->end (format); > + format->separator (format); > } > > - first_thread = 0; > - > notmuch_thread_destroy (thread); > } > > - if (first_thread) > - fputs (format->results_null, stdout); > - else > - fputs (format->results_end, stdout); > + format->end (format); > > return 0; > } > > static int > -do_search_messages (const search_format_t *format, > +do_search_messages (sprinter_t *format, > notmuch_query_t *query, > output_t output, > int offset, > @@ -297,7 +133,6 @@ do_search_messages (const search_format_t *format, > notmuch_message_t *message; > notmuch_messages_t *messages; > notmuch_filenames_t *filenames; > - int first_message = 1; > int i; > > if (offset < 0) { > @@ -310,7 +145,7 @@ do_search_messages (const search_format_t *format, > if (messages == NULL) > return 1; > > - fputs (format->results_start, stdout); > + format->begin_list (format); > > for (i = 0; > notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit); > @@ -328,24 +163,17 @@ do_search_messages (const search_format_t *format, > notmuch_filenames_valid (filenames); > notmuch_filenames_move_to_next (filenames)) > { > - if (! first_message) > - fputs (format->item_sep, stdout); > - > - format->item_id (message, "", > - notmuch_filenames_get (filenames)); > - > - first_message = 0; > + format->string (format, notmuch_filenames_get (filenames)); > + format->separator (format); > } > - > + > notmuch_filenames_destroy( filenames ); > > } else { /* output == OUTPUT_MESSAGES */ > - if (! first_message) > - fputs (format->item_sep, stdout); > - > - format->item_id (message, "id:", > - notmuch_message_get_message_id (message)); > - first_message = 0; > + format->set_prefix (format, "id"); > + format->string (format, > + notmuch_message_get_message_id (message)); > + format->separator (format); > } > > notmuch_message_destroy (message); > @@ -353,23 +181,19 @@ do_search_messages (const search_format_t *format, > > notmuch_messages_destroy (messages); > > - if (first_message) > - fputs (format->results_null, stdout); > - else > - fputs (format->results_end, stdout); > + format->end (format); > > return 0; > } > > static int > do_search_tags (notmuch_database_t *notmuch, > - const search_format_t *format, > + sprinter_t *format, > notmuch_query_t *query) > { > notmuch_messages_t *messages = NULL; > notmuch_tags_t *tags; > const char *tag; > - int first_tag = 1; > > /* should the following only special case if no excluded terms > * specified? */ > @@ -387,7 +211,7 @@ do_search_tags (notmuch_database_t *notmuch, > if (tags == NULL) > return 1; > > - fputs (format->results_start, stdout); > + format->begin_list (format); > > for (; > notmuch_tags_valid (tags); > @@ -395,12 +219,9 @@ do_search_tags (notmuch_database_t *notmuch, > { > tag = notmuch_tags_get (tags); > > - if (! first_tag) > - fputs (format->item_sep, stdout); > + format->string (format, tag); > + format->separator (format); > > - format->item_id (tags, "", tag); > - > - first_tag = 0; > } > > notmuch_tags_destroy (tags); > @@ -408,10 +229,7 @@ do_search_tags (notmuch_database_t *notmuch, > if (messages) > notmuch_messages_destroy (messages); > > - if (first_tag) > - fputs (format->results_null, stdout); > - else > - fputs (format->results_end, stdout); > + format->end (format); > > return 0; > } > @@ -430,7 +248,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) > notmuch_query_t *query; > char *query_str; > notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST; > - const search_format_t *format = &format_text; > + sprinter_t *format = NULL; > int opt_index, ret; > output_t output = OUTPUT_SUMMARY; > int offset = 0; > @@ -475,10 +293,10 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) > > switch (format_sel) { > case NOTMUCH_FORMAT_TEXT: > - format = &format_text; > + format = sprinter_text_search_create (ctx, stdout); > break; > case NOTMUCH_FORMAT_JSON: > - format = &format_json; > + format = sprinter_json_create (ctx, stdout); > break; > } > > @@ -546,5 +364,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) > notmuch_query_destroy (query); > notmuch_database_destroy (notmuch); > > + talloc_free (format); > + > return ret; > } > diff --git a/test/json b/test/json > index 6439788..f0ebf08 100755 > --- a/test/json > +++ b/test/json > @@ -10,14 +10,7 @@ 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 pipe this through notmuch_json_show_sanitize, it'll make the test output much cleaner and closer to the original output (probably not exactly the same, unfortunately). > -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 +33,6 @@ 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