From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36231) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bAK89-0001RZ-70 for qemu-devel@nongnu.org; Tue, 07 Jun 2016 12:45:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bAK87-0002Ax-Tg for qemu-devel@nongnu.org; Tue, 07 Jun 2016 12:45:29 -0400 Date: Tue, 7 Jun 2016 17:45:17 +0100 From: "Daniel P. Berrange" Message-ID: <20160607164517.GV20196@redhat.com> Reply-To: "Daniel P. Berrange" References: <1465294275-8733-1-git-send-email-berrange@redhat.com> <1465294275-8733-5-git-send-email-berrange@redhat.com> <5756F1CC.5010408@redhat.com> <20160607162042.GU20196@redhat.com> <5756F904.1090804@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5756F904.1090804@redhat.com> Subject: Re: [Qemu-devel] [PATCH v1 4/6] qapi: add a text output visitor for pretty printing types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Kevin Wolf , Max Reitz , Markus Armbruster , Michael Roth On Tue, Jun 07, 2016 at 10:40:36AM -0600, Eric Blake wrote: > On 06/07/2016 10:20 AM, Daniel P. Berrange wrote: > > On Tue, Jun 07, 2016 at 10:09:48AM -0600, Eric Blake wrote: > >> On 06/07/2016 04:11 AM, Daniel P. Berrange wrote: > >>> The current approach for pretty-printing QAPI types is to > >>> convert them to JSON using the QMP output visitor and then > >>> pretty-print the JSON document. This has an unfixable problem > >>> that structs get their keys printed out in random order, since > >>> JSON dicts do not contain any key ordering information. > >>> > >>> To address this, introduce a text output visitor that can > >>> directly pretty print a QAPI type into a string. > >>> > >>> Signed-off-by: Daniel P. Berrange > >>> --- > >>> include/qapi/text-output-visitor.h | 73 ++++++++++++ > >>> include/qapi/visitor-impl.h | 5 +- > >>> include/qapi/visitor.h | 5 +- > >>> qapi/Makefile.objs | 1 + > >>> qapi/opts-visitor.c | 5 +- > >>> qapi/qapi-dealloc-visitor.c | 4 +- > >>> qapi/qapi-visit-core.c | 9 +- > >>> qapi/qmp-input-visitor.c | 5 +- > >>> qapi/qmp-output-visitor.c | 4 +- > >>> qapi/string-input-visitor.c | 5 +- > >>> qapi/string-output-visitor.c | 5 +- > >> > >> Why can't we enhance the existing string-output-visitor to handle structs? > > > > string-output-visitor seems to be doing something very > > different from this. In particular it only ever seems > > to output the values, never the field names. So if we > > did enhance string-output-visitor, we'd basically have > > to make all of its code conditional to output in one > > style or the other style, at which point I didn't think > > it was really buying us anything vs a new visitor. > > That is, it was always doing a top-level visit of a scalar or array of > scalars, and nothing else. It may still be something that can be merged. > Maybe I should take a rough shot at it, since I have ideas on how to use > a common handler for name/list index (and do nothing at the top level), > then the rest of each callback is independent from what name prefix, if > any, was output. On the other hand, I guess the way intList is handled > (compacting it into a single list, instead of each element of the list), > may indeed be a reason to keep it as two visitors. If you want to have a shot, go ahead. I put this patch at the end of my series, since it wasn't a blocking issue to resolve the struct field ordering, just a "nice to have", so no rush. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|