[libvirt] [PATCHv2 04/13] virbuf: add auto-indentation support

Eric Blake eblake at redhat.com
Thu Oct 20 21:35:04 UTC 2011


On 10/19/2011 08:31 PM, Hai Dong Li wrote:
> This email is just for your attention. I'm relatively new to work in a
> community, so I didn't pay much attention to the readability of the
> comments last email. It seems comments lie in a large patch like this is
> easily to be omitted. So I cut the codes, leave codes associated with
> the comments.

Yes, trimming to just relevant context is a must for any high-volume 
patch list.  Also, separate your replies from the quoted material by 
blank lines, so it stands out better (visually, I find it easier to spot 
replies that appear in isolation, by scanning just the left column; not 
to mention that some mailers corrupt long lines on quoted replies where 
a long single-line paragraph in the original turns into a wrapped 
multi-line text with the first line quoted but subsequent lines 
unquoted; adding whitespace before your reply makes it obvious that you 
made the comment, rather than your mailer reformatting things).


>> + virBufferAdjustIndent(buf, -2);
>> + if (virBufferGetIndent(buf, false) != 1 ||
>> + virBufferGetIndent(buf, true) != 1 ||
>> + virBufferError(buf)) {
>> + TEST_ERROR("Wrong indentation");
>> + ret = -1;
>> + }
> So now buf->indent is 1. Go to the next step, the indent is given -2
> again, see what will happen.
> if virBufferAdjustIndent failed to check the indent overflow, the
> buf->indent will be -1,too, so it may avoid the check
> (virBufferGetIndent(buf, false) != -1) and (virBufferGetIndent(buf,
> true) != -1).
>> + virBufferAdjustIndent(buf, -2);
> So I think -3 may be better.

Good idea; I've folded that into my patch.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list