Re: [libvirt] [PATCH 2/4] util: buffer: Add support for adding text blocks with indentation

[Martin Kletzander]

On Wed, Mar 25, 2015 at 01:00:17PM +0100, Peter Krempa wrote:

On Wed, Mar 25, 2015 at 10:32:11 +0100, Martin Kletzander wrote:

On Tue, Mar 24, 2015 at 03:03:21PM +0100, Peter Krempa wrote:
>The current auto-indentation buffer code applies indentation only on
>complete strings. To allow adding a string containing newlines and
>having it properly indented this patch adds virBufferAddStr.
>---
> src/libvirt_private.syms |  1 +
> src/util/virbuffer.c     | 38 ++++++++++++++++++++++++++++++++++++
> src/util/virbuffer.h     |  1 +
> tests/virbuftest.c       | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 90 insertions(+)
>
>diff --git a/tests/virbuftest.c b/tests/virbuftest.c
>index f964feb..067a77e 100644
>--- a/tests/virbuftest.c
>+++ b/tests/virbuftest.c
>@@ -310,6 +310,44 @@ static int testBufAddBuffer(const void *data 
ATTRIBUTE_UNUSED)
>     return ret;
> }
>
>+struct testBufAddStrData {
>+    const char *data;
>+    const char *expect;
>+};
>+
>+static int
>+testBufAddStr(const void *opaque ATTRIBUTE_UNUSED)
>+{
>+    const struct testBufAddStrData *data = opaque;
>+    virBuffer buf = VIR_BUFFER_INITIALIZER;
>+    char *actual;
>+    int ret = -1;
>+
>+    virBufferAddLit(&buf, "<c>\n");
>+    virBufferAdjustIndent(&buf, 2);
>+    virBufferAddStr(&buf, data->data);
>+    virBufferAdjustIndent(&buf, -2);
>+    virBufferAddLit(&buf, "</c>");
>+
>+    if (!(actual = virBufferContentAndReset(&buf))) {
>+        TEST_ERROR("buf is empty");
>+        goto cleanup;
>+    }
>+
>+    if (STRNEQ_NULLABLE(actual, data->expect)) {
>+        TEST_ERROR("testBufAddStr(): Strings don't match:\n"
>+                   "Expected:\n%s\nActual:\n%s\n",
>+                   data->expect, actual);

One more question though, virtTestDifferenceFull() doens't make sense
here?

Actually the output with virtTestDifferenceFull kind of sucks with short
documents.

I find it cleaner, but that's highly subjective ;) go ahead and push
it as-is, the ACK still stands.

The following part is with the existing code:
8) Buf: AddStr                                                       ... 
testBufAddStr(): Strings don't match:
Expected:
<c>
 <a/>
</c>
Actual:
<c>
 <a/></c>
FAILED
9) Buf: AddStr                                                       ... 
testBufAddStr(): Strings don't match:
Expected:
<c>
 <b>
   <a/>
 </b>
</c>
Actual:
<c>
 <b>  <a/></b></c>
FAILED


While this part is with virtTestDifference():
8) Buf: AddStr                                                       ... 
testBufAddStr(): Strings don't match:

Offset 0
Expect [<c>
 <a/>
</c>]
Actual [<c>
 <a/></c>]
                                                                     ... FAILED
9) Buf: AddStr                                                       ... 
testBufAddStr(): Strings don't match:

Offset 0
Expect [<c>
 <b>
   <a/>
 </b>
</c>]
Actual [<c>
 <b>  <a/></b></c>]
                                                                     ... FAILED

Peter

pgpcGrsPExUeS.pgp
Description: PGP signature

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