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

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.