John Snow <js...@redhat.com> writes:
> On 02/23/2016 05:51 AM, Daniel P. Berrange wrote:
>> Pretty printing of JSON responses is important to be able to understand
>> large responses from query commands in particular. Unfortunately this
>> was broken during the addition of the verbose flag in
>>
>> commit 1ceca07e48ead0dd2e41576c81d40e6a91cafefd
>> Author: John Snow <js...@redhat.com>
>> Date: Wed Apr 29 15:14:04 2015 -0400
>>
>> scripts: qmp-shell: Add verbose flag
>>
>> This is because that change turned the python data structure into a
>> formatted JSON string before the pretty print was given it. So we're
>> just pretty printing a string, which is a no-op.
>>
>
> Mea Culpa.
>
>> The original pretty printer would output python objects.
>>
>> (QEMU) query-chardev
>> { u'return': [ { u'filename': u'vc',
>> u'frontend-open': False,
>> u'label': u'parallel0'},
>> { u'filename': u'vc',
>> u'frontend-open': True,
>> u'label': u'serial0'},
>> { u'filename': u'unix:/tmp/qemp,server',
>> u'frontend-open': True,
>> u'label': u'compat_monitor0'}]}
>>
>> This fixes the problem by switching to outputting pretty formatted JSON
>> text instead. This has the added benefit that the pretty printed output
>> is now valid JSON text. Due to the way the verbose flag was handled, the
>> pretty printing now applies to the command sent, as well as its response:
>>
>> (QEMU) query-chardev
>> {
>> "execute": "query-chardev",
>> "arguments": {}
>> }
>> {
>> "return": [
>> {
>> "frontend-open": false,
>> "label": "parallel0",
>> "filename": "vc"
>> },
>> {
>> "frontend-open": true,
>> "label": "serial0",
>> "filename": "vc"
>> },
>> {
>> "frontend-open": true,
>> "label": "compat_monitor0",
>> "filename": "unix:/tmp/qmp,server"
>> }
>> ]
>> }
>>
>
> That's good news as far as I am concerned.
>
>> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
>> ---
>> scripts/qmp/qmp-shell | 23 ++++++++++-------------
>> 1 file changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>> index 7a402ed..0373b24 100755
>> --- a/scripts/qmp/qmp-shell
>> +++ b/scripts/qmp/qmp-shell
>> @@ -70,7 +70,6 @@ import json
>> import ast
>> import readline
>> import sys
>> -import pprint
>>
>> class QMPCompleter(list):
>> def complete(self, text, state):
>> @@ -103,11 +102,11 @@ class FuzzyJSON(ast.NodeTransformer):
>> # TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and
>> # _execute_cmd()). Let's design a better one.
>
> ^ Heh ^
>
>> class QMPShell(qmp.QEMUMonitorProtocol):
>> - def __init__(self, address, pp=None):
>> + def __init__(self, address, pretty=False):
>> qmp.QEMUMonitorProtocol.__init__(self, self.__get_address(address))
>> self._greeting = None
>> self._completer = None
>> - self._pp = pp
>> + self._pretty = pretty
>> self._transmode = False
>> self._actions = list()
>>
>> @@ -231,11 +230,11 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>> return qmpcmd
>>
>> def _print(self, qmp):
>> - jsobj = json.dumps(qmp)
>> - if self._pp is not None:
>> - self._pp.pprint(jsobj)
>> - else:
>> - print str(jsobj)
>> + indent = None
>> + if self._pretty:
>> + indent = 4
>> + jsobj = json.dumps(qmp, indent=indent)
>> + print str(jsobj)
>>
>> def _execute_cmd(self, cmdline):
>> try:
>> @@ -377,7 +376,7 @@ def main():
>> addr = ''
>> qemu = None
>> hmp = False
>> - pp = None
>> + pretty = False
>> verbose = False
>>
>> try:
>> @@ -387,9 +386,7 @@ def main():
>> fail_cmdline(arg)
>> hmp = True
>> elif arg == "-p":
>> - if pp is not None:
>> - fail_cmdline(arg)
>> - pp = pprint.PrettyPrinter(indent=4)
>> + pretty = True
>
> We now accept any number of -p arguments. That's probably fine.
I'd call that a bug fix.
>> elif arg == "-v":
>> verbose = True
>> else:
>> @@ -398,7 +395,7 @@ def main():
>> if hmp:
>> qemu = HMPShell(arg)
>> else:
>> - qemu = QMPShell(arg, pp)
>> + qemu = QMPShell(arg, pretty)
>> addr = arg
>>
>> if qemu is None:
>>
>
> Reviewed-by: John Snow <js...@redhat.com>
Applied to my qapi-next branch, thanks!
