Re: [libvirt] [PATCH 1/9] conf: eliminate hardcoded indent from domain xml
On 03/06/2014 07:15 PM, Eric Blake wrote:
On 03/06/2014 08:24 AM, Laine Stump wrote:
> if (def->label) {
> virBufferAddLit(buf, ">\n");
> - virBufferEscapeString(buf, " <label>%s</label>\n",
> + virBufferAdjustIndent(buf, 2);
> + virBufferEscapeString(buf, "<label>%s</label>\n",
> def->label);
> + virBufferAdjustIndent(buf, -2);
> virBufferAddLit(buf, "</seclabel>\n");
For small hunks like this where all the context is local, this just adds
lines of code. I'm almost of the opinion that it's easier to have each
function start with no indent, and any strings it prints locally are
correctly indented (allowing leading whitespace), while any helper
functions it calls are surrounded by AdjustIndent (since the helper
function also prints at a local top level).
Yeah, I thought of that too, but it made it easier to verify, as you say
below, and once I got going it was difficult to pick a cutoff point, so
I just did it 100%.
The clincher is that our testsuite validates that we didn't break
any
output :) ACK.
Definitely I wouldn't have been able to make these changes with any
level of confidence without the tests (and yes, they did catch a few
things I'd missed, e.g. accidentally adding indent instead of removing).