diff mbox

[v3,12/18] qapi: Add qobject_to_json_pretty_prefix()

Message ID 1461903820-3092-13-git-send-email-eblake@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Blake April 29, 2016, 4:23 a.m. UTC
The next patch will add pretty indentation to the JSON visitor.
But in order to support pretty output in the type_any() callback,
we need to prefix every line of the QObject visitor by the current
indentation in the JSON visitor.  Hence, a new function
qobject_to_json_pretty_indent(), and the old function becomes a
thin wrapper to the expanded behavior.

While at it, change 'pretty' to be a bool, to match its usage.

Note that the new simple_pretty() test is a bit sensitive to our
current notion of prettiness, as well as to the hash ordering in
QDict (most of the tests in check-qobject-json intentionally do
not compare the original string to the round-trip string, because
we liberally accept more input forms than the canonical form that
we output).

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v3: no change
v2: no change
---
 include/qapi/qmp/qobject-json.h |  1 +
 qobject/qobject-json.c          | 40 ++++++++++++++++++-------
 tests/check-qobject-json.c      | 65 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 11 deletions(-)

Comments

Markus Armbruster May 2, 2016, 1:56 p.m. UTC | #1
Eric Blake <eblake@redhat.com> writes:

> The next patch will add pretty indentation to the JSON visitor.
> But in order to support pretty output in the type_any() callback,
> we need to prefix every line of the QObject visitor by the current
> indentation in the JSON visitor.  Hence, a new function
> qobject_to_json_pretty_indent(), and the old function becomes a
> thin wrapper to the expanded behavior.
>
> While at it, change 'pretty' to be a bool, to match its usage.
>
> Note that the new simple_pretty() test is a bit sensitive to our
> current notion of prettiness, as well as to the hash ordering in
> QDict (most of the tests in check-qobject-json intentionally do
> not compare the original string to the round-trip string, because
> we liberally accept more input forms than the canonical form that
> we output).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v3: no change
> v2: no change
> ---
>  include/qapi/qmp/qobject-json.h |  1 +
>  qobject/qobject-json.c          | 40 ++++++++++++++++++-------
>  tests/check-qobject-json.c      | 65 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+), 11 deletions(-)
>
> diff --git a/include/qapi/qmp/qobject-json.h b/include/qapi/qmp/qobject-json.h
> index 02b1f2c..d420c71 100644
> --- a/include/qapi/qmp/qobject-json.h
> +++ b/include/qapi/qmp/qobject-json.h
> @@ -23,5 +23,6 @@ QObject *qobject_from_jsonv(const char *string, va_list *ap) GCC_FMT_ATTR(1, 0);
>
>  QString *qobject_to_json(const QObject *obj);
>  QString *qobject_to_json_pretty(const QObject *obj);
> +QString *qobject_to_json_pretty_prefix(const QObject *obj, const char *prefix);

Why a string prefix, and not indentation?

>
>  #endif /* QJSON_H */
[...]
Eric Blake May 2, 2016, 3:14 p.m. UTC | #2
On 05/02/2016 07:56 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> The next patch will add pretty indentation to the JSON visitor.
>> But in order to support pretty output in the type_any() callback,
>> we need to prefix every line of the QObject visitor by the current
>> indentation in the JSON visitor.  Hence, a new function
>> qobject_to_json_pretty_indent(), and the old function becomes a
>> thin wrapper to the expanded behavior.
>>

>>  QString *qobject_to_json(const QObject *obj);
>>  QString *qobject_to_json_pretty(const QObject *obj);
>> +QString *qobject_to_json_pretty_prefix(const QObject *obj, const char *prefix);
> 
> Why a string prefix, and not indentation?

Might be possible.  Would it be better as an integer for 'number of
spaces' or for 'number of indentation levels (where number of spaces per
indentation-level is not configurable)'?
Markus Armbruster May 3, 2016, 8:32 a.m. UTC | #3
Eric Blake <eblake@redhat.com> writes:

> On 05/02/2016 07:56 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> The next patch will add pretty indentation to the JSON visitor.
>>> But in order to support pretty output in the type_any() callback,
>>> we need to prefix every line of the QObject visitor by the current
>>> indentation in the JSON visitor.  Hence, a new function
>>> qobject_to_json_pretty_indent(), and the old function becomes a
>>> thin wrapper to the expanded behavior.
>>>
>
>>>  QString *qobject_to_json(const QObject *obj);
>>>  QString *qobject_to_json_pretty(const QObject *obj);
>>> +QString *qobject_to_json_pretty_prefix(const QObject *obj, const char *prefix);
>> 
>> Why a string prefix, and not indentation?
>
> Might be possible.  Would it be better as an integer for 'number of
> spaces' or for 'number of indentation levels (where number of spaces per
> indentation-level is not configurable)'?

I'd go with indentation levels.  The number of spaces per level is
already an implementation detail of the pretty printer.  Duplicating it
outside of it is not a good idea.
diff mbox

Patch

diff --git a/include/qapi/qmp/qobject-json.h b/include/qapi/qmp/qobject-json.h
index 02b1f2c..d420c71 100644
--- a/include/qapi/qmp/qobject-json.h
+++ b/include/qapi/qmp/qobject-json.h
@@ -23,5 +23,6 @@  QObject *qobject_from_jsonv(const char *string, va_list *ap) GCC_FMT_ATTR(1, 0);

 QString *qobject_to_json(const QObject *obj);
 QString *qobject_to_json_pretty(const QObject *obj);
+QString *qobject_to_json_pretty_prefix(const QObject *obj, const char *prefix);

 #endif /* QJSON_H */
diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c
index 963de07..4469835 100644
--- a/qobject/qobject-json.c
+++ b/qobject/qobject-json.c
@@ -70,12 +70,14 @@  QObject *qobject_from_jsonf(const char *string, ...)
 typedef struct ToJsonIterState
 {
     int indent;
-    int pretty;
+    bool pretty;
     int count;
     QString *str;
+    const char *prefix;
 } ToJsonIterState;

-static void to_json(const QObject *obj, QString *str, int pretty, int indent);
+static void to_json(const QObject *obj, QString *str, bool pretty, int indent,
+                    const char *prefix);

 static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
 {
@@ -86,13 +88,13 @@  static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
     }

     if (s->pretty) {
-        qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
+        qstring_append_format(s->str, "\n%s%*s", s->prefix, 4 * s->indent, "");
     }

     qstring_append_json_string(s->str, key);

     qstring_append(s->str, ": ");
-    to_json(obj, s->str, s->pretty, s->indent);
+    to_json(obj, s->str, s->pretty, s->indent, s->prefix);
     s->count++;
 }

@@ -105,14 +107,15 @@  static void to_json_list_iter(QObject *obj, void *opaque)
     }

     if (s->pretty) {
-        qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
+        qstring_append_format(s->str, "\n%s%*s", s->prefix, 4 * s->indent, "");
     }

-    to_json(obj, s->str, s->pretty, s->indent);
+    to_json(obj, s->str, s->pretty, s->indent, s->prefix);
     s->count++;
 }

-static void to_json(const QObject *obj, QString *str, int pretty, int indent)
+static void to_json(const QObject *obj, QString *str, bool pretty, int indent,
+                    const char *prefix)
 {
     switch (qobject_type(obj)) {
     case QTYPE_QNULL:
@@ -136,10 +139,11 @@  static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         s.str = str;
         s.indent = indent + 1;
         s.pretty = pretty;
+        s.prefix = prefix;
         qstring_append(str, "{");
         qdict_iter(val, to_json_dict_iter, &s);
         if (pretty) {
-            qstring_append_format(str, "\n%*s", 4 * indent, "");
+            qstring_append_format(str, "\n%s%*s", prefix, 4 * indent, "");
         }
         qstring_append(str, "}");
         break;
@@ -152,10 +156,11 @@  static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         s.str = str;
         s.indent = indent + 1;
         s.pretty = pretty;
+        s.prefix = prefix;
         qstring_append(str, "[");
         qlist_iter(val, (void *)to_json_list_iter, &s);
         if (pretty) {
-            qstring_append_format(str, "\n%*s", 4 * indent, "");
+            qstring_append_format(str, "\n%s%*s", prefix, 4 * indent, "");
         }
         qstring_append(str, "]");
         break;
@@ -186,7 +191,7 @@  QString *qobject_to_json(const QObject *obj)
 {
     QString *str = qstring_new();

-    to_json(obj, str, 0, 0);
+    to_json(obj, str, false, 0, "");

     return str;
 }
@@ -195,7 +200,20 @@  QString *qobject_to_json_pretty(const QObject *obj)
 {
     QString *str = qstring_new();

-    to_json(obj, str, 1, 0);
+    to_json(obj, str, true, 0, "");
+
+    return str;
+}
+
+/*
+ * Pretty-print JSON representing obj, inserting prefix after every newline.
+ * Note that prefix is NOT applied before the first character.
+ */
+QString *qobject_to_json_pretty_prefix(const QObject *obj, const char *prefix)
+{
+    QString *str = qstring_new();
+
+    to_json(obj, str, true, 0, prefix);

     return str;
 }
diff --git a/tests/check-qobject-json.c b/tests/check-qobject-json.c
index d889501..96bb017 100644
--- a/tests/check-qobject-json.c
+++ b/tests/check-qobject-json.c
@@ -1388,6 +1388,70 @@  static void simple_whitespace(void)
     }
 }

+static void simple_pretty(void)
+{
+    int i;
+    struct {
+        const char *encoded;
+        LiteralQObject decoded;
+    } test_cases[] = {
+        {
+            /* The first line is intentionally not prefixed. */
+            .encoded =
+            "[\n"
+            "        43,\n"
+            "        42\n"
+            "    ]",
+            .decoded = QLIT_QLIST(((LiteralQObject[]){
+                        QLIT_QINT(43),
+                        QLIT_QINT(42),
+                        { }
+                    })),
+        },
+        {
+            .encoded =
+            "[\n"
+            "        43,\n"
+            "        {\n"
+            "            \"a\": 32,\n"
+            "            \"h\": \"b\"\n"
+            "        },\n"
+            "        [\n"
+            "        ],\n"
+            "        42\n"
+            "    ]",
+            .decoded = QLIT_QLIST(((LiteralQObject[]){
+                        QLIT_QINT(43),
+                        QLIT_QDICT(((LiteralQDictEntry[]){
+                                    { "a", QLIT_QINT(32) },
+                                    { "h", QLIT_QSTR("b") },
+                                    { } })),
+                        QLIT_QLIST(((LiteralQObject[]){
+                                    { } })),
+                        QLIT_QINT(42),
+                        { }
+                    })),
+        },
+        { }
+    };
+
+    for (i = 0; test_cases[i].encoded; i++) {
+        QObject *obj;
+        QString *str;
+
+        obj = qobject_from_json(test_cases[i].encoded);
+        g_assert(obj != NULL);
+        g_assert(qobject_type(obj) == QTYPE_QLIST);
+
+        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+
+        str = qobject_to_json_pretty_prefix(obj, "    ");
+        qobject_decref(obj);
+        g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].encoded);
+        QDECREF(str);
+    }
+}
+
 static void simple_varargs(void)
 {
     QObject *embedded_obj;
@@ -1525,6 +1589,7 @@  int main(int argc, char **argv)
     g_test_add_func("/lists/simple_list", simple_list);

     g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
+    g_test_add_func("/whitespace/pretty", simple_pretty);

     g_test_add_func("/varargs/simple_varargs", simple_varargs);