On 12/01/2015 12:35 PM, Martin Kletzander wrote: > Clean up some duplicate code so it is easier to add new parameters. The > tests need to be adjusted because after this patch there will be > additional spaces at the end of lines in the output of virsh list, but > this affects only the table output that is meant to be read by users. > > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > --- > tests/virshtest.c | 14 +++++++------- > tools/virsh-domain-monitor.c | 38 +++++++++++++++++++------------------- > 2 files changed, 26 insertions(+), 26 deletions(-) > > diff --git a/tests/virshtest.c b/tests/virshtest.c > index 3fdae92b7834..5983b5b190d6 100644 > --- a/tests/virshtest.c > +++ b/tests/virshtest.c > @@ -98,9 +98,9 @@ static int testCompareListDefault(const void *data ATTRIBUTE_UNUSED) > { > const char *const argv[] = { VIRSH_DEFAULT, "list", NULL }; > const char *exp = "\ > - Id Name State\n\ > -----------------------------------------------------\n\ > - 1 test running\n\ > + Id Name State \n\ > +-------------------------------------------------\n\ > + 1 test running \n\ > \n"; > return testCompareOutputLit(exp, NULL, argv); > } > @@ -109,10 +109,10 @@ static int testCompareListCustom(const void *data ATTRIBUTE_UNUSED) > { > const char *const argv[] = { VIRSH_CUSTOM, "list", NULL }; > const char *exp = "\ > - Id Name State\n\ > -----------------------------------------------------\n\ > - 1 fv0 running\n\ > - 2 fc4 running\n\ > + Id Name State \n\ > +-------------------------------------------------\n\ > + 1 fv0 running \n\ > + 2 fc4 running \n\ [1]If only to make the output line up w/r/t \n... > \n"; > return testCompareOutputLit(exp, NULL, argv); > } > diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c > index db5bf4b2a566..24b902905968 100644 > --- a/tools/virsh-domain-monitor.c > +++ b/tools/virsh-domain-monitor.c > @@ -1914,16 +1914,18 @@ cmdList(vshControl *ctl, const vshCmd *cmd) > > /* print table header in legacy mode */ > if (optTable) { > + vshPrintExtra(ctl, " %-5s %-30s %-10s", _("Id"), _("Name"), _("State")); > + > if (optTitle) > - vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n", > - _("Id"), _("Name"), _("State"), _("Title"), > - "-----------------------------------------" > - "-----------------------------------------"); > - else > - vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n", > - _("Id"), _("Name"), _("State"), > - "-----------------------------------------" > - "-----------"); > + vshPrintExtra(ctl, " %-20s\n", _("Title")); ^^ I believe if Title were added there'd be an extra line once the following dashes are printed (end w/ \n, start with \n) > + > + vshPrintExtra(ctl, "\n%s", > + "-------------------------------------------------"); > + > + if (optTitle) > + vshPrintExtra(ctl, "%s", "---------------------"); It does seem in the expected virshtest.c output we're still off by 1. That is there is 1 more "-" than extra space after the State column. [1] Instead of counting dashes... dashcount = 45; /* Or some value - whatever it is */ ... if (optTitle) { ... dashcount += 20; } ,,, for (i = 0; i < dashcount; i++) buf[i] = '-'; vshPrintExtra(ctl, "%s\n", buf); ... where of course buf is appropriately defined ;-) A nit would be that the old output was 3 dashes longer. ACK with at least the fixup as a result of having Title on the line (and it would be nice to add a test for it too)! Whether you implement the '-' hack is up to you, but it may make future adjustments more simple... John > + > + vshPrintExtra(ctl, "\n"); > } > > for (i = 0; i < list->ndomains; i++) { > @@ -1945,23 +1947,21 @@ cmdList(vshControl *ctl, const vshCmd *cmd) > state = -2; > > if (optTable) { > + vshPrint(ctl, " %-5s %-30s %-10s", id_buf, > + virDomainGetName(dom), > + state == -2 ? _("saved") > + : virshDomainStateToString(state)); > + > if (optTitle) { > if (!(title = virshGetDomainDescription(ctl, dom, true, 0))) > goto cleanup; > > - vshPrint(ctl, " %-5s %-30s %-10s %-20s\n", id_buf, > - virDomainGetName(dom), > - state == -2 ? _("saved") > - : virshDomainStateToString(state), > - title); > + vshPrint(ctl, " %-20s", title); > > VIR_FREE(title); > - } else { > - vshPrint(ctl, " %-5s %-30s %s\n", id_buf, > - virDomainGetName(dom), > - state == -2 ? _("saved") > - : virshDomainStateToString(state)); > } > + > + vshPrint(ctl, "\n"); > } else if (optUUID) { > if (virDomainGetUUIDString(dom, uuid) < 0) { > vshError(ctl, "%s", _("Failed to get domain's UUID")); > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list