[libvirt] [PATCH 01/13] virsh: Implement vsh-table to iface-list

Simon Kobyda posted 13 patches 7 years, 4 months ago
There is a newer version of this series
[libvirt] [PATCH 01/13] virsh: Implement vsh-table to iface-list
Posted by Simon Kobyda 7 years, 4 months ago
Signed-off-by: Simon Kobyda <skobyda@redhat.com>
---
 tools/virsh-interface.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 50518c667b..3234845596 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -48,6 +48,7 @@
 #include "virutil.h"
 #include "virxml.h"
 #include "virstring.h"
+#include "vsh-table.h"
 
 virInterfacePtr
 virshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
@@ -356,6 +357,8 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     unsigned int flags = VIR_CONNECT_LIST_INTERFACES_ACTIVE;
     virshInterfaceListPtr list = NULL;
     size_t i;
+    bool ret = false;
+    vshTablePtr table = NULL;
 
     if (inactive)
         flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE;
@@ -366,21 +369,29 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     if (!(list = virshInterfaceListCollect(ctl, flags)))
         return false;
 
-    vshPrintExtra(ctl, " %-20s %-10s %s\n", _("Name"), _("State"),
-                  _("MAC Address"));
-    vshPrintExtra(ctl, "---------------------------------------------------\n");
+    table = vshTableNew("Name", "State", "MAC Address", NULL);
+    if (!table)
+        goto cleanup;
 
     for (i = 0; i < list->nifaces; i++) {
         virInterfacePtr iface = list->ifaces[i];
 
-        vshPrint(ctl, " %-20s %-10s %s\n",
-                 virInterfaceGetName(iface),
-                 virInterfaceIsActive(iface) ? _("active") : _("inactive"),
-                 virInterfaceGetMACString(iface));
+        if (vshTableRowAppend(table,
+                              virInterfaceGetName(iface),
+                              virInterfaceIsActive(iface) ? _("active")
+                              : _("inactive"),
+                              virInterfaceGetMACString(iface),
+                              NULL) < 0)
+            goto cleanup;
     }
 
+    vshTablePrintToStdout(table, ctl);
+
+    ret = true;
+ cleanup:
+    vshTableFree(table);
     virshInterfaceListFree(list);
-    return true;
+    return ret;
 }
 
 /*
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/13] virsh: Implement vsh-table to iface-list
Posted by Michal Privoznik 7 years, 4 months ago
On 09/18/2018 04:21 PM, Simon Kobyda wrote:
> Signed-off-by: Simon Kobyda <skobyda@redhat.com>
> ---
>  tools/virsh-interface.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
> index 50518c667b..3234845596 100644
> --- a/tools/virsh-interface.c
> +++ b/tools/virsh-interface.c
> @@ -48,6 +48,7 @@
>  #include "virutil.h"
>  #include "virxml.h"
>  #include "virstring.h"
> +#include "vsh-table.h"
>  
>  virInterfacePtr
>  virshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
> @@ -356,6 +357,8 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
>      unsigned int flags = VIR_CONNECT_LIST_INTERFACES_ACTIVE;
>      virshInterfaceListPtr list = NULL;
>      size_t i;
> +    bool ret = false;
> +    vshTablePtr table = NULL;
>  
>      if (inactive)
>          flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE;
> @@ -366,21 +369,29 @@ cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
>      if (!(list = virshInterfaceListCollect(ctl, flags)))
>          return false;
>  
> -    vshPrintExtra(ctl, " %-20s %-10s %s\n", _("Name"), _("State"),
> -                  _("MAC Address"));
> -    vshPrintExtra(ctl, "---------------------------------------------------\n");
> +    table = vshTableNew("Name", "State", "MAC Address", NULL);

This is not right. You need to keep the gettext function. If this was
merged as is then no translation would be done. You can find more info
in the commit I've just pushed:

https://www.redhat.com/archives/libvir-list/2018-September/msg00886.html

This applies to the rest of the patches too.

> +    if (!table)
> +        goto cleanup;
>  
>      for (i = 0; i < list->nifaces; i++) {
>          virInterfacePtr iface = list->ifaces[i];
>  
> -        vshPrint(ctl, " %-20s %-10s %s\n",
> -                 virInterfaceGetName(iface),
> -                 virInterfaceIsActive(iface) ? _("active") : _("inactive"),
> -                 virInterfaceGetMACString(iface));
> +        if (vshTableRowAppend(table,
> +                              virInterfaceGetName(iface),
> +                              virInterfaceIsActive(iface) ? _("active")
> +                              : _("inactive"),
> +                              virInterfaceGetMACString(iface),
> +                              NULL) < 0)
> +            goto cleanup;
>      }
>  
> +    vshTablePrintToStdout(table, ctl);
> +
> +    ret = true;
> + cleanup:
> +    vshTableFree(table);
>      virshInterfaceListFree(list);
> -    return true;
> +    return ret;
>  }
>  
>  /*
> 

ACK with that fixed.

Michal

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