[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 25/28] qapi: Support pretty printing in JSON
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4 25/28] qapi: Support pretty printing in JSON output visitor |
Date: |
Fri, 03 Jun 2016 09:56:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Similar to pretty printing in the QObject visitor. The trickiest
> part is probably that the testsuite now has to honor parameterization
> on whether pretty printing is enabled.
Worth mentioning that the pretty-printing matches the one in
qobject-json.c?
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v4: rebase to earlier changes, defer type_any() to later
> v3: rebase to earlier changes
> v2: rebase to earlier changes
> ---
> include/qapi/json-output-visitor.h | 5 +-
> qapi/json-output-visitor.c | 15 ++++-
> tests/test-json-output-visitor.c | 122
> +++++++++++++++++++++++++------------
> 3 files changed, 99 insertions(+), 43 deletions(-)
>
> diff --git a/include/qapi/json-output-visitor.h
> b/include/qapi/json-output-visitor.h
> index 41c79f4..94c9e0f 100644
> --- a/include/qapi/json-output-visitor.h
> +++ b/include/qapi/json-output-visitor.h
> @@ -21,8 +21,11 @@ typedef struct JsonOutputVisitor JsonOutputVisitor;
> * If everything else succeeds, pass @result to visit_complete() to
> * collect the result of the visit.
> *
> + * If @pretty, make the output legible with newlines and indentation;
> + * otherwise the output uses a single line.
> + *
> * For now, this cannot be used to visit the 'any' type.
> */
> -Visitor *json_output_visitor_new(char **result);
> +Visitor *json_output_visitor_new(bool pretty, char **result);
>
> #endif
> diff --git a/qapi/json-output-visitor.c b/qapi/json-output-visitor.c
> index 7010ff8..881c9ee 100644
> --- a/qapi/json-output-visitor.c
> +++ b/qapi/json-output-visitor.c
> @@ -17,6 +17,7 @@
> struct JsonOutputVisitor {
> Visitor visitor;
> QString *str;
> + bool pretty;
> bool comma;
> unsigned int depth;
> char **result;
> @@ -30,10 +31,13 @@ static JsonOutputVisitor *to_jov(Visitor *v)
> static void json_output_name(JsonOutputVisitor *jov, const char *name)
> {
> if (jov->comma) {
> - qstring_append(jov->str, ", ");
> + qstring_append(jov->str, jov->pretty ? "," : ", ");
> } else {
> jov->comma = true;
> }
> + if (jov->pretty && jov->depth) {
> + qstring_append_printf(jov->str, "\n%*s", 4 * jov->depth, "");
> + }
> if (name && jov->depth) {
> qstring_append_json_string(jov->str, name);
> qstring_append(jov->str, ": ");
> @@ -57,6 +61,9 @@ static void json_output_end_struct(Visitor *v, void **obj)
>
> assert(jov->depth);
> jov->depth--;
> + if (jov->pretty) {
> + qstring_append_printf(jov->str, "\n%*s", 4 * jov->depth, "");
> + }
> qstring_append(jov->str, "}");
> jov->comma = true;
> }
> @@ -85,6 +92,9 @@ static void json_output_end_list(Visitor *v, void **obj)
>
> assert(jov->depth);
> jov->depth--;
> + if (jov->pretty) {
> + qstring_append_printf(jov->str, "\n%*s", 4 * jov->depth, "");
> + }
> qstring_append(jov->str, "]");
> jov->comma = true;
> }
> @@ -165,11 +175,12 @@ static void json_output_free(Visitor *v)
> g_free(jov);
> }
>
> -Visitor *json_output_visitor_new(char **result)
> +Visitor *json_output_visitor_new(bool pretty, char **result)
> {
> JsonOutputVisitor *v;
>
> v = g_malloc0(sizeof(*v));
> + v->pretty = pretty;
> v->result = result;
> *result = NULL;
> v->str = qstring_new();
> diff --git a/tests/test-json-output-visitor.c
> b/tests/test-json-output-visitor.c
> index 3c77a61..5073715 100644
> --- a/tests/test-json-output-visitor.c
> +++ b/tests/test-json-output-visitor.c
> @@ -24,14 +24,17 @@
>
> typedef struct TestOutputVisitorData {
> Visitor *ov;
> + bool pretty;
> char *str;
> } TestOutputVisitorData;
>
> static void visitor_output_setup(TestOutputVisitorData *data,
> - const void *unused)
> + const void *arg)
> {
> - data->ov = json_output_visitor_new(&data->str);
> + const bool *pretty = arg;
Could do bool pretty = *(bool *)arg. Matter of taste. Same elsewhere.
Blank line between declarations and statements, please.
> + data->ov = json_output_visitor_new(*pretty, &data->str);
> g_assert(data->ov);
> + data->pretty = *pretty;
I'd put this line before data->ov = ...
> }
>
> static void visitor_output_teardown(TestOutputVisitorData *data,
> @@ -52,8 +55,9 @@ static const char *visitor_get(TestOutputVisitorData *data)
>
> static void visitor_reset(TestOutputVisitorData *data)
> {
> + bool pretty = data->pretty;
Blank line between declarations and statements, please.
> visitor_output_teardown(data, NULL);
> - visitor_output_setup(data, NULL);
> + visitor_output_setup(data, &pretty);
We could pass &data->pretty (teardown leaves it alone), but I guess not
relying on that is slightly cleaner.
> }
>
> static void test_visitor_out_int(TestOutputVisitorData *data,
> @@ -161,8 +165,9 @@ static void test_visitor_out_struct(TestOutputVisitorData
> *data,
> }
>
> static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
> - const void *unused)
> + const void *arg)
> {
> + const bool *pretty = arg;
> int64_t value = 42;
> UserDefTwo *ud2;
> const char *string = "user def string";
> @@ -192,27 +197,51 @@ static void
> test_visitor_out_struct_nested(TestOutputVisitorData *data,
> visit_type_UserDefTwo(data->ov, "unused", &ud2, &error_abort);
>
> out = visitor_get(data);
> - g_assert_cmpstr(out, ==,
> - "{"
> - "\"string0\": \"forty two\", "
> - "\"dict1\": {"
> - "\"string1\": \"forty three\", "
> - "\"dict2\": {"
> - "\"userdef\": {"
> - "\"integer\": 42, "
> - "\"string\": \"user def string\""
> - "}, "
> - "\"string\": \"forty four\""
> - "}, "
> - "\"dict3\": {"
> - "\"userdef\": {"
> - "\"integer\": 42, "
> - "\"string\": \"user def string\""
> - "}, "
> - "\"string\": \"forty five\""
> - "}"
> - "}"
> - "}");
> + if (*pretty) {
> + g_assert_cmpstr(out, ==,
> + "{\n"
> + " \"string0\": \"forty two\",\n"
> + " \"dict1\": {\n"
> + " \"string1\": \"forty three\",\n"
> + " \"dict2\": {\n"
> + " \"userdef\": {\n"
> + " \"integer\": 42,\n"
> + " \"string\": \"user def string\"\n"
> + " },\n"
> + " \"string\": \"forty four\"\n"
> + " },\n"
> + " \"dict3\": {\n"
> + " \"userdef\": {\n"
> + " \"integer\": 42,\n"
> + " \"string\": \"user def string\"\n"
> + " },\n"
> + " \"string\": \"forty five\"\n"
> + " }\n"
> + " }\n"
> + "}");
> + } else {
> + g_assert_cmpstr(out, ==,
> + "{"
> + "\"string0\": \"forty two\", "
> + "\"dict1\": {"
> + "\"string1\": \"forty three\", "
> + "\"dict2\": {"
> + "\"userdef\": {"
> + "\"integer\": 42, "
> + "\"string\": \"user def string\""
> + "}, "
> + "\"string\": \"forty four\""
> + "}, "
> + "\"dict3\": {"
> + "\"userdef\": {"
> + "\"integer\": 42, "
> + "\"string\": \"user def string\""
> + "}, "
> + "\"string\": \"forty five\""
> + "}"
> + "}"
> + "}");
> + }
The not-pretty case is unchanged. Good.
> qapi_free_UserDefTwo(ud2);
> }
>
> @@ -346,36 +375,49 @@ static void test_visitor_out_null(TestOutputVisitorData
> *data,
> g_assert_cmpstr(out, ==, "null");
> }
>
> -static void output_visitor_test_add(const char *testpath,
> +static void output_visitor_test_add(const char *testpath, bool *data,
> void (*test_func)(TestOutputVisitorData
> *,
> const void *))
> {
> - g_test_add(testpath, TestOutputVisitorData, NULL, visitor_output_setup,
> + g_test_add(testpath, TestOutputVisitorData, data, visitor_output_setup,
> test_func, visitor_output_teardown);
> }
>
> int main(int argc, char **argv)
> {
> + bool plain = false;
> + bool pretty = true;
> +
> g_test_init(&argc, &argv, NULL);
>
> - output_visitor_test_add("/visitor/json/int", test_visitor_out_int);
> - output_visitor_test_add("/visitor/json/bool", test_visitor_out_bool);
> - output_visitor_test_add("/visitor/json/number", test_visitor_out_number);
> - output_visitor_test_add("/visitor/json/string", test_visitor_out_string);
> - output_visitor_test_add("/visitor/json/enum", test_visitor_out_enum);
> - output_visitor_test_add("/visitor/json/enum-errors",
> + output_visitor_test_add("/visitor/json/int", &plain,
> + test_visitor_out_int);
> + output_visitor_test_add("/visitor/json/bool", &plain,
> + test_visitor_out_bool);
> + output_visitor_test_add("/visitor/json/number", &plain,
> + test_visitor_out_number);
> + output_visitor_test_add("/visitor/json/string", &plain,
> + test_visitor_out_string);
> + output_visitor_test_add("/visitor/json/enum", &plain,
> + test_visitor_out_enum);
> + output_visitor_test_add("/visitor/json/enum-errors", &plain,
> test_visitor_out_enum_errors);
> - output_visitor_test_add("/visitor/json/struct", test_visitor_out_struct);
> - output_visitor_test_add("/visitor/json/struct-nested",
> + output_visitor_test_add("/visitor/json/struct", &plain,
> + test_visitor_out_struct);
> + output_visitor_test_add("/visitor/json/struct-nested", &plain,
> test_visitor_out_struct_nested);
> - output_visitor_test_add("/visitor/json/struct-errors",
> + output_visitor_test_add("/visitor/json-pretty/struct-nested", &pretty,
> + test_visitor_out_struct_nested);
> + output_visitor_test_add("/visitor/json/struct-errors", &plain,
> test_visitor_out_struct_errors);
> - output_visitor_test_add("/visitor/json/list", test_visitor_out_list);
> - output_visitor_test_add("/visitor/json/union-flat",
> + output_visitor_test_add("/visitor/json/list", &plain,
> + test_visitor_out_list);
> + output_visitor_test_add("/visitor/json/union-flat", &plain,
> test_visitor_out_union_flat);
> - output_visitor_test_add("/visitor/json/alternate",
> + output_visitor_test_add("/visitor/json/alternate", &plain,
> test_visitor_out_alternate);
> - output_visitor_test_add("/visitor/json/null", test_visitor_out_null);
> + output_visitor_test_add("/visitor/json/null", &plain,
> + test_visitor_out_null);
>
> g_test_run();
- Re: [Qemu-devel] [PATCH v4 25/28] qapi: Support pretty printing in JSON output visitor,
Markus Armbruster <=