[libvirt] [PATCHv2 07/13] snapshot: simplify indentation of sysinfo

[Daniel P. Berrange]

On Thu, Oct 20, 2011 at 03:03:25PM +0200, Peter Krempa wrote:
> On 10/20/2011 12:20 PM, Daniel P. Berrange wrote:
> >On Thu, Oct 20, 2011 at 12:14:57PM +0200, Peter Krempa wrote:
> >>On 09/29/2011 06:22 PM, Eric Blake wrote:
> >>>The improvements to virBuffer, along with a paradigm shift to pass
> >>>the original buffer through rather than creating a second buffer,
> >>>allow us to shave off quite a few lines of code.
> >>>
> >>>* src/util/sysinfo.h (virSysinfoFormat): Alter signature.
> >>>* src/util/sysinfo.c (virSysinfoFormat, virSysinfoBIOSFormat)
> >>>(virSysinfoSystemFormat, virSysinfoProcessorFormat)
> >>>(virSysinfoMemoryFormat): Change indentation parameter.
> >>>* src/conf/domain_conf.c (virDomainSysinfoDefFormat): Adjust
> >>>caller.
> >>>* src/qemu/qemu_driver.c (qemuGetSysinfo): Likewise.
> >>>---
> >>>  src/conf/domain_conf.c |   12 +-
> >>>  src/qemu/qemu_driver.c |    9 +-
> >>>  src/util/sysinfo.c     |  399 ++++++++++++++++--------------------------------
> >>>  src/util/sysinfo.h     |    3 +-
> >>>  4 files changed, 147 insertions(+), 276 deletions(-)
> >>>
> >>I'd squash in the attached patch, but it's not necessary as it gets
> >>rid of non automatic indentation whitespace, but makes the code look
> >>cleaner :)
> >
> >I'm not entirely convinced this is a good idea.  This means that
> >when looking at the code, it is no longer obvious what the nesting
> >of XML elements is supposed to be - they are all the level.
> >
> >I see the value of the automatic indentation code, being to allow
> >us to embed 1 XML document, inside another XML document. eg domain
> >conf XML, inside QEMU state XML.
> >
> >I don't think we should use it to remove indentation in all our
> >code.
> >
> 
> Oh well, maybe I got too far with removing indentation, but I think
> that functions outputing XML should have a consistent default
> indentation (0 or 2 spaces), so when embedding them in more complex
> XML documents as sub-elemets, we will not have to check wether this
> function is at col 0 relative from the caller or on column 2. I
> agree that it's not necessary to change everything, but it'd be nice
> to have a consistent way to do this.

The only sane option is for any function generating an embeddable XML
document to default to zero top level indent.

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 :|