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: <craven@gmx.net>
>
> 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