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

[Michal Privoznik]

On 09/18/2018 04:21 PM, Simon Kobyda wrote:
> Signed-off-by: Simon Kobyda <skobyda at 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