From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36374) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btv6s-0004pP-RO for qemu-devel@nongnu.org; Tue, 11 Oct 2016 07:20:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btv6p-0001jG-4w for qemu-devel@nongnu.org; Tue, 11 Oct 2016 07:20:37 -0400 Received: from mail-lf0-x243.google.com ([2a00:1450:4010:c07::243]:33174) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btv6o-0001iZ-Ag for qemu-devel@nongnu.org; Tue, 11 Oct 2016 07:20:34 -0400 Received: by mail-lf0-x243.google.com with SMTP id l131so458450lfl.0 for ; Tue, 11 Oct 2016 04:20:34 -0700 (PDT) MIME-Version: 1.0 References: <1476105837-9861-1-git-send-email-eblake@redhat.com> <1476105837-9861-13-git-send-email-eblake@redhat.com> In-Reply-To: <1476105837-9861-13-git-send-email-eblake@redhat.com> From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Tue, 11 Oct 2016 11:20:22 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 12/15] qapi: Support pretty printing in JSON output visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: armbru@redhat.com, Michael Roth On Mon, Oct 10, 2016 at 5:44 PM Eric Blake wrote: > Add pretty printing, where the format intentionally matches that of > qobject_to_json() (a later patch will then rework qobject-json.c to > work on top of the JSON visitor). The trickiest part is probably > that the testsuite now has to honor parameterization on whether > pretty printing is enabled. > > Signed-off-by: Eric Blake > > --- > v6: tweak commit message, use of 'bool pretty' in testsuite > [no v5 due to series split] > 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 | 124 > +++++++++++++++++++++++++------------ > 3 files changed, 101 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 805154a..7d12879 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 =3D 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 =3D 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 =3D 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 =3D g_malloc0(sizeof(*v)); > + v->pretty =3D pretty; > v->result =3D result; > *result =3D NULL; > v->str =3D qstring_new(); > diff --git a/tests/test-json-output-visitor.c > b/tests/test-json-output-visitor.c > index 3c77a61..849cf2b 100644 > --- a/tests/test-json-output-visitor.c > +++ b/tests/test-json-output-visitor.c > @@ -24,13 +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 =3D json_output_visitor_new(&data->str); > + bool pretty =3D *(bool *)arg; > + > Have you considered GPOINTER_TO_INT/GINT_TO_POINTER instead of pointers to the stack frame? either way, it's fine. > + data->pretty =3D pretty; > + data->ov =3D json_output_visitor_new(pretty, &data->str); > g_assert(data->ov); > } > > @@ -52,8 +56,10 @@ static const char *visitor_get(TestOutputVisitorData > *data) > > static void visitor_reset(TestOutputVisitorData *data) > { > + bool pretty =3D data->pretty; > + > visitor_output_teardown(data, NULL); > - visitor_output_setup(data, NULL); > + visitor_output_setup(data, &pretty); > } > > static void test_visitor_out_int(TestOutputVisitorData *data, > @@ -161,8 +167,9 @@ static void > test_visitor_out_struct(TestOutputVisitorData *data, > } > > static void test_visitor_out_struct_nested(TestOutputVisitorData *data, > - const void *unused) > + const void *arg) > { > + bool pretty =3D *(bool *)arg; > int64_t value =3D 42; > UserDefTwo *ud2; > const char *string =3D "user def string"; > @@ -192,27 +199,51 @@ static void > test_visitor_out_struct_nested(TestOutputVisitorData *data, > visit_type_UserDefTwo(data->ov, "unused", &ud2, &error_abort); > > out =3D visitor_get(data); > - g_assert_cmpstr(out, =3D=3D, > - "{" > - "\"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, =3D=3D, > + "{\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, =3D=3D, > + "{" > + "\"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\"" > + "}" > + "}" > + "}"); > + } > qapi_free_UserDefTwo(ud2); > } > > @@ -346,36 +377,49 @@ static void > test_visitor_out_null(TestOutputVisitorData *data, > g_assert_cmpstr(out, =3D=3D, "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 =3D false; > + bool pretty =3D 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", &prett= y, > + 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(); > > -- > 2.7.4 > > > -- Marc-Andr=C3=A9 Lureau