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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Peter

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]