Re: [libvirt] [PATCH 1/9] conf: eliminate hardcoded indent from all xml
On 03/06/2014 10:32 PM, Eric Blake wrote:
On 03/06/2014 08:24 AM, Laine Stump wrote:
[0/9 in the subject line]
> Many of the domain xml format functions (including all of the device
> format functions) had hard-coded spaces, which made for incorrect
> indentation when those functions were called in a different context
> (for example, commit 2122cf39 added <interface> XML into the document
> provided to a network hook script, and in this case it should have
> been indented by 2 spaces, but was instead indented by 6 spaces).
>
> In that patch I mentioned doing a followup patch to make the device
> xml formatters more consistent. After doing that patch, it felt
> incomplete to not give the same treatment to the entire directory.
>
> The one downside to this series is that it may create merge conflicts
> during backports, but fortunately the conflicts should all be fairly
> easy to resolve.
Missing from this series:
qemuDomainObjPrivateXMLFormat in qemu_domain.c
qemuMigrationCookieGraphicsXMLFormat in qemu_migration.c
Ah, I didn't think to look in directories other than conf.
probably others. Also, it would be a good idea to add a syntax check to
cfg.mk. I'd suggest a rule something like:
sc_forbid_manual_xml_indent:
@prohibit='virBuffer.*" +<' \
halt='use virBufferIndent when indenting xml' \
$(_sc_search_regexp)
... and adding in that rule reveals a few more files that still have spaces.
Sounds like something for me to do while I'm sitting in airports
tomorrow :-)
As I mentioned in the real 1/9, using local indentation is more compact
than using virBufferIndent, but makes it harder to enforce - so if we go
with your patches, a syntax check rule that enforces things will make it
so we don't slip into old habits.
I also had idle thoughts of doing away with virBufferAdjustIndent
entirely, and adding an "xml pretty print" option to
virBufferContentsAndReset, but that's probably too inefficient in
comparison to what it does now. :-)