Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-22 Thread Daniel P . Berrangé 
On Wed, Aug 22, 2018 at 12:50:12PM +0200, Simon Kobyda wrote:
> On Tue, 2018-08-21 at 11:46 +0100, Daniel P. Berrangé wrote:
> > On Tue, Aug 21, 2018 at 12:27:34PM +0200, Michal Privoznik wrote:
> > > On 08/21/2018 11:18 AM, Simon Kobyda wrote:
> > > > On Thu, 2018-08-16 at 12:28 +0100, Daniel P. Berrangé wrote:
> > > > > On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:
> > > > > > 
> > > > > 
> > > > > After asking around I have found the right solution that we
> > > > > need to
> > > > > use
> > > > > for measuring string width.  mbstowcs()/wcswidth() will get the
> > > > > answer
> > > > > wrong wrt zero-width characters, combining characters, non-
> > > > > printable
> > > > > characters, etc. We need to use the libunistring library:
> > > > > 
> > > > >   
> > > > > 
> https://www.gnu.org/software/libunistring/manual/libunistring.html#uniwidth_002eh
> > > > > 
> > > > > 
> > > > 
> > > > I've tried what you've suggested, but it seems that it doesn't
> > > > work
> > > > well with all unicode characters. I'm looking into the code of
> > > > the
> > > > library, and each function uN_strwidth calls function uN_width,
> > > > and
> > > > that function calls uc_width for calculation of width of
> > > > characters.
> > > > And if we look into the code of uc_width here:
> > > > 
> > > > 
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/uniwidth/width.c;h=269cfc77f50a3b9802e5fb5620ff8bcf95e05e40;hb=HEAD#l415
> > > > it seems that this library is limited only to certain unicodes,
> > > > e.g.:
> > > > hangul characters, angle brackets, CJK characters... But it
> > > > doesn't
> > > > cover all multiple-width characters. Example: I try to throw any
> > > > emoji
> > > > (e.g. , 呂, ), it returns width of 1 column for each charact
> > > > er, nevertheless these characters have width of 2 columns on
> > > > terminal.
> > > > 
> > > > BTW, it seems unistring library imports those funcions from
> > > > gnulib.
> > > 
> > > I guess the only option then is to try smartcols [1]. If it is good
> > > for
> > > util-linux it's going to be good for us too. Although, I'd prefer
> > > to
> > > have our own wrappers over their API.
> > > 
> > > https://github.com/karelzak/util-linux/tree/master/libsmartcols
> > 
> > The util-linux code does something that uses mbstowcs / wcwidth to
> > convert the characters and count their width, sort of like the
> > original
> > version of this patch. They have further code that decides to convert
> > certain unicode characters into "\xNN" escaped sequences, which
> > avoids
> > the problems I raised wrt non-printable strings.
> > 
> >https://github.com/karelzak/util-linux/blob/master/lib/mbsalign.c
> > 
> > So we could pull that helper API into our code, since its LGPL
> > loicensed.
> > I'm unclear if this correctly handles all the cases or not though as
> > there's no unit tests for it in util-linux AFACT.
> > 
> > Really the only way for us to be sure is to provide a unit test which
> > stresses our the code with a variety of unicode input strings.
> 
> About unit tests. Right now i've got tests for non-pritnable, zero-
> width, combining characters and opposite (rigth to left) writing.
> Anybody got any idea what else could be problematic with
> mbstowcs()/wcswidth(), and therefore tested?

I think that sounds reasonable enough for now - passing such tests would
already be massively better than the code  that exists today with strlen()

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-22 Thread Simon Kobyda 
On Tue, 2018-08-21 at 11:46 +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 21, 2018 at 12:27:34PM +0200, Michal Privoznik wrote:
> > On 08/21/2018 11:18 AM, Simon Kobyda wrote:
> > > On Thu, 2018-08-16 at 12:28 +0100, Daniel P. Berrangé wrote:
> > > > On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:
> > > > > 
> > > > 
> > > > After asking around I have found the right solution that we
> > > > need to
> > > > use
> > > > for measuring string width.  mbstowcs()/wcswidth() will get the
> > > > answer
> > > > wrong wrt zero-width characters, combining characters, non-
> > > > printable
> > > > characters, etc. We need to use the libunistring library:
> > > > 
> > > >   
> > > > 
https://www.gnu.org/software/libunistring/manual/libunistring.html#uniwidth_002eh
> > > > 
> > > > 
> > > 
> > > I've tried what you've suggested, but it seems that it doesn't
> > > work
> > > well with all unicode characters. I'm looking into the code of
> > > the
> > > library, and each function uN_strwidth calls function uN_width,
> > > and
> > > that function calls uc_width for calculation of width of
> > > characters.
> > > And if we look into the code of uc_width here:
> > > 
> > > 
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/uniwidth/width.c;h=269cfc77f50a3b9802e5fb5620ff8bcf95e05e40;hb=HEAD#l415
> > > it seems that this library is limited only to certain unicodes,
> > > e.g.:
> > > hangul characters, angle brackets, CJK characters... But it
> > > doesn't
> > > cover all multiple-width characters. Example: I try to throw any
> > > emoji
> > > (e.g. , 呂, ), it returns width of 1 column for each charact
> > > er, nevertheless these characters have width of 2 columns on
> > > terminal.
> > > 
> > > BTW, it seems unistring library imports those funcions from
> > > gnulib.
> > 
> > I guess the only option then is to try smartcols [1]. If it is good
> > for
> > util-linux it's going to be good for us too. Although, I'd prefer
> > to
> > have our own wrappers over their API.
> > 
> > https://github.com/karelzak/util-linux/tree/master/libsmartcols
> 
> The util-linux code does something that uses mbstowcs / wcwidth to
> convert the characters and count their width, sort of like the
> original
> version of this patch. They have further code that decides to convert
> certain unicode characters into "\xNN" escaped sequences, which
> avoids
> the problems I raised wrt non-printable strings.
> 
>https://github.com/karelzak/util-linux/blob/master/lib/mbsalign.c
> 
> So we could pull that helper API into our code, since its LGPL
> loicensed.
> I'm unclear if this correctly handles all the cases or not though as
> there's no unit tests for it in util-linux AFACT.
> 
> Really the only way for us to be sure is to provide a unit test which
> stresses our the code with a variety of unicode input strings.

About unit tests. Right now i've got tests for non-pritnable, zero-
width, combining characters and opposite (rigth to left) writing.
Anybody got any idea what else could be problematic with
mbstowcs()/wcswidth(), and therefore tested?

Simon Kobyda.

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

Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-21 Thread Daniel P . Berrangé 
On Tue, Aug 21, 2018 at 12:27:34PM +0200, Michal Privoznik wrote:
> On 08/21/2018 11:18 AM, Simon Kobyda wrote:
> > On Thu, 2018-08-16 at 12:28 +0100, Daniel P. Berrangé wrote:
> >> On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:
> >>>
> >>
> >> After asking around I have found the right solution that we need to
> >> use
> >> for measuring string width.  mbstowcs()/wcswidth() will get the
> >> answer
> >> wrong wrt zero-width characters, combining characters, non-printable
> >> characters, etc. We need to use the libunistring library:
> >>
> >>   
> >> https://www.gnu.org/software/libunistring/manual/libunistring.html#uniwidth_002eh
> >>
> >>
> > I've tried what you've suggested, but it seems that it doesn't work
> > well with all unicode characters. I'm looking into the code of the
> > library, and each function uN_strwidth calls function uN_width, and
> > that function calls uc_width for calculation of width of characters.
> > And if we look into the code of uc_width here:
> > 
> > http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/uniwidth/width.c;h=269cfc77f50a3b9802e5fb5620ff8bcf95e05e40;hb=HEAD#l415
> > it seems that this library is limited only to certain unicodes, e.g.:
> > hangul characters, angle brackets, CJK characters... But it doesn't
> > cover all multiple-width characters. Example: I try to throw any emoji
> > (e.g. , 呂, ), it returns width of 1 column for each charact
> > er, nevertheless these characters have width of 2 columns on terminal.
> > 
> > BTW, it seems unistring library imports those funcions from gnulib.
> 
> I guess the only option then is to try smartcols [1]. If it is good for
> util-linux it's going to be good for us too. Although, I'd prefer to
> have our own wrappers over their API.
> 
> https://github.com/karelzak/util-linux/tree/master/libsmartcols

The util-linux code does something that uses mbstowcs / wcwidth to
convert the characters and count their width, sort of like the original
version of this patch. They have further code that decides to convert
certain unicode characters into "\xNN" escaped sequences, which avoids
the problems I raised wrt non-printable strings.

   https://github.com/karelzak/util-linux/blob/master/lib/mbsalign.c

So we could pull that helper API into our code, since its LGPL loicensed.
I'm unclear if this correctly handles all the cases or not though as
there's no unit tests for it in util-linux AFACT.

Really the only way for us to be sure is to provide a unit test which
stresses our the code with a variety of unicode input strings.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-21 Thread Michal Privoznik 
On 08/21/2018 11:18 AM, Simon Kobyda wrote:
> On Thu, 2018-08-16 at 12:28 +0100, Daniel P. Berrangé wrote:
>> On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:
>>>
>>
>> After asking around I have found the right solution that we need to
>> use
>> for measuring string width.  mbstowcs()/wcswidth() will get the
>> answer
>> wrong wrt zero-width characters, combining characters, non-printable
>> characters, etc. We need to use the libunistring library:
>>
>>   
>> https://www.gnu.org/software/libunistring/manual/libunistring.html#uniwidth_002eh
>>
>>
> I've tried what you've suggested, but it seems that it doesn't work
> well with all unicode characters. I'm looking into the code of the
> library, and each function uN_strwidth calls function uN_width, and
> that function calls uc_width for calculation of width of characters.
> And if we look into the code of uc_width here:
> 
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/uniwidth/width.c;h=269cfc77f50a3b9802e5fb5620ff8bcf95e05e40;hb=HEAD#l415
> it seems that this library is limited only to certain unicodes, e.g.:
> hangul characters, angle brackets, CJK characters... But it doesn't
> cover all multiple-width characters. Example: I try to throw any emoji
> (e.g. , 呂, ), it returns width of 1 column for each charact
> er, nevertheless these characters have width of 2 columns on terminal.
> 
> BTW, it seems unistring library imports those funcions from gnulib.

I guess the only option then is to try smartcols [1]. If it is good for
util-linux it's going to be good for us too. Although, I'd prefer to
have our own wrappers over their API.

https://github.com/karelzak/util-linux/tree/master/libsmartcols

Michal

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

Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-21 Thread Simon Kobyda 
On Thu, 2018-08-16 at 12:28 +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:
> > 
> 
> After asking around I have found the right solution that we need to
> use
> for measuring string width.  mbstowcs()/wcswidth() will get the
> answer
> wrong wrt zero-width characters, combining characters, non-printable
> characters, etc. We need to use the libunistring library:
> 
>   
> https://www.gnu.org/software/libunistring/manual/libunistring.html#uniwidth_002eh
> 
> 
I've tried what you've suggested, but it seems that it doesn't work
well with all unicode characters. I'm looking into the code of the
library, and each function uN_strwidth calls function uN_width, and
that function calls uc_width for calculation of width of characters.
And if we look into the code of uc_width here:

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/uniwidth/width.c;h=269cfc77f50a3b9802e5fb5620ff8bcf95e05e40;hb=HEAD#l415
it seems that this library is limited only to certain unicodes, e.g.:
hangul characters, angle brackets, CJK characters... But it doesn't
cover all multiple-width characters. Example: I try to throw any emoji
(e.g. , 呂, ), it returns width of 1 column for each charact
er, nevertheless these characters have width of 2 columns on terminal.

BTW, it seems unistring library imports those funcions from gnulib.

Simon Kobyda.

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

Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-16 Thread Daniel P . Berrangé 
On Thu, Aug 16, 2018 at 02:05:45PM +0200, Ján Tomko wrote:
> On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:
> > It solves problems with alignment of columns. Width of each column
> > is calculated by its biggest cell. Should solve unicode bug.
> > In future, it may be implemented in virsh, virt-admin...
> > 
> > This API has 5 public functions:
> > - vshTableNew - adds new table and defines its header
> > - vshTableRowAppend - appends new row (for same number of columns as in
> > header)
> > - vshTablePrintToStdout
> > - vshTablePrintToString
> > - vshTableFree
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1574624
> > https://bugzilla.redhat.com/show_bug.cgi?id=1584630
> > 
> > Signed-off-by: Simon Kobyda 
> > ---
> > tools/Makefile.am |   4 +-
> > tools/vsh-table.c | 413 ++
> > tools/vsh-table.h |  42 +
> > 3 files changed, 458 insertions(+), 1 deletion(-)
> > create mode 100644 tools/vsh-table.c
> > create mode 100644 tools/vsh-table.h
> > 

> > +/**
> > + * vshTableGetColumnsWidths:
> > + * @table: table
> > + * @maxwidths: maximum count of characters for each columns
> > + * @widths: count of characters for each cell in the table
> > + *
> > + * Fill passed @maxwidths and @widths arrays with maximum number
> > + * of characters for columns and number of character per each
> > + * table cell, respectively.
> > + *
> > + * Handle unicode strings (user must have multibyte locale)
> > + */
> > +static int
> > +vshTableGetColumnsWidths(vshTablePtr table,
> > + size_t *maxwidths,
> > + size_t **widths,
> > + bool header)
> > +{
> > +int ret = -1;
> > +size_t i = 1;
> > +size_t j;
> > +size_t len;
> > +int tmp;
> > +wchar_t *wstr = NULL;
> > +size_t wstrlen;
> > +
> > +if (header)
> > +i = 0;
> > +else
> > +i = 1;
> > +for (; i < table->nrows; i++) {
> > +vshTableRowPtr row = table->rows[i];
> > +
> > +for (j = 0; j < row->ncells; j++) {
> 
> 
> > +/* strlen should return maximum possible length needed */
> > +wstrlen = strlen(row->cells[j]);
> > +VIR_FREE(wstr);
> > +if (VIR_ALLOC_N(wstr, wstrlen) < 0)
> > +goto cleanup;
> > +/* mbstowcs fails if machine is using singlebyte locale
> > + * and user tries to convert unicode(multibyte)
> > + * */
> > +if (mbstowcs(wstr, row->cells[j], wstrlen) ==
> > +(size_t) -1) {
> > +len = wstrlen;
> > +} else {
> > +tmp = wcswidth(wstr, wstrlen);
> > +if (tmp < 0)
> > +goto cleanup;
> > +len = (size_t)((unsigned)tmp);
> > +}
> 
> Whatever solution you use for converting a multi-byte string to a
> maximum count of characters, please put it in a separate function.
> That would make this function more readable.

Note that unfortnately libunistring is "GPLv2+ or LGPLv3+", so is not
compatible with libvirt.so

It is fine to use it from virsh since that's free to be GPLv2+ only,
but we mustn't link libunistring to libvirt.so.  So any helper func
would have to stay in the tools dir just for virsh/virt-admin


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-16 Thread Ján Tomko 

On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:

It solves problems with alignment of columns. Width of each column
is calculated by its biggest cell. Should solve unicode bug.
In future, it may be implemented in virsh, virt-admin...

This API has 5 public functions:
- vshTableNew - adds new table and defines its header
- vshTableRowAppend - appends new row (for same number of columns as in
header)
- vshTablePrintToStdout
- vshTablePrintToString
- vshTableFree

https://bugzilla.redhat.com/show_bug.cgi?id=1574624
https://bugzilla.redhat.com/show_bug.cgi?id=1584630

Signed-off-by: Simon Kobyda 
---
tools/Makefile.am |   4 +-
tools/vsh-table.c | 413 ++
tools/vsh-table.h |  42 +
3 files changed, 458 insertions(+), 1 deletion(-)
create mode 100644 tools/vsh-table.c
create mode 100644 tools/vsh-table.h

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 1452d984a0..f069167acc 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -144,7 +144,9 @@ libvirt_shell_la_LIBADD = \
$(READLINE_LIBS) \
../gnulib/lib/libgnu.la \
$(NULL)
-libvirt_shell_la_SOURCES = vsh.c vsh.h
+libvirt_shell_la_SOURCES = \
+   vsh.c vsh.h \
+   vsh-table.c vsh-table.h

virt_host_validate_SOURCES = \
virt-host-validate.c \
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
new file mode 100644
index 00..8842e4e4fd
--- /dev/null
+++ b/tools/vsh-table.c
@@ -0,0 +1,413 @@
+/*
+ * vsh-table.c: table printing helper
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Authors:
+ *   Simon Kobyda 
+ *
+ */
+
+#include 
+#include "vsh-table.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "viralloc.h"
+#include "virbuffer.h"
+#include "virstring.h"
+#include "virsh-util.h"
+
+typedef void (*vshPrintCB)(vshControl *ctl, const char *fmt, ...);
+
+struct _vshTableRow {
+char **cells;
+size_t ncells;
+};
+
+struct _vshTable {
+vshTableRowPtr *rows;
+size_t nrows;
+};
+
+static void
+vshTableRowFree(vshTableRowPtr row)
+{
+size_t i;
+
+if (!row)
+return;
+
+for (i = 0; i < row->ncells; i++)
+VIR_FREE(row->cells[i]);
+
+VIR_FREE(row->cells);
+VIR_FREE(row);
+}
+
+void
+vshTableFree(vshTablePtr table)
+{
+size_t i;
+
+if (!table)
+return;
+
+for (i = 0; i < table->nrows; i++)
+vshTableRowFree(table->rows[i]);
+VIR_FREE(table->rows);
+VIR_FREE(table);
+}
+
+/**
+ * vshTableRowNew:
+ * @arg: the first argument.
+ * @ap: list of variadic arguments
+ *
+ * Create a new row in the table. Each argument passed
+ * represents a cell in the row.
+ * Return: pointer to vshTableRowPtr row or NULL.
+ */
+static vshTableRowPtr
+vshTableRowNew(const char *arg, va_list ap)
+{
+vshTableRowPtr row = NULL;
+char *tmp = NULL;
+
+if (!arg) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Table row cannot be empty"));
+goto error;
+}
+
+if (VIR_ALLOC(row) < 0)
+goto error;
+
+while (arg) {
+if (VIR_STRDUP(tmp, arg) < 0)
+goto error;
+
+if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0)
+goto error;
+
+arg = va_arg(ap, const char *);
+}
+
+return row;
+
+ error:
+vshTableRowFree(row);
+return NULL;
+}
+
+/**
+ * vshTableNew:
+ * @arg: List of column names (NULL terminated)
+ *
+ * Create a new table.
+ *
+ * Returns: pointer to table or NULL.
+ */
+vshTablePtr
+vshTableNew(const char *arg, ...)
+{
+vshTablePtr table;
+vshTableRowPtr header = NULL;
+va_list ap;
+
+if (VIR_ALLOC(table) < 0)
+goto error;
+
+va_start(ap, arg);
+header = vshTableRowNew(arg, ap);
+va_end(ap);
+
+if (!header)
+goto error;
+
+if (VIR_APPEND_ELEMENT(table->rows, table->nrows, header) < 0)
+goto error;
+
+return table;
+ error:
+vshTableRowFree(header);
+vshTableFree(table);
+return NULL;
+}
+
+/**
+ * vshTableRowAppend:
+ * @table: table to append to
+ * @arg: cells of the row (NULL terminated)
+ *
+ * Append new row into the @table. The number of cells in the row has
+ * to be equal to the number of cells in the table header.
+ *
+ *

Re: [libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-16 Thread Daniel P . Berrangé 
On Thu, Aug 16, 2018 at 12:56:24PM +0200, Simon Kobyda wrote:
> It solves problems with alignment of columns. Width of each column
> is calculated by its biggest cell. Should solve unicode bug.
> In future, it may be implemented in virsh, virt-admin...
> 
> This API has 5 public functions:
> - vshTableNew - adds new table and defines its header
> - vshTableRowAppend - appends new row (for same number of columns as in
> header)
> - vshTablePrintToStdout
> - vshTablePrintToString
> - vshTableFree
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1574624
> https://bugzilla.redhat.com/show_bug.cgi?id=1584630
> 
> Signed-off-by: Simon Kobyda 
> ---
>  tools/Makefile.am |   4 +-
>  tools/vsh-table.c | 413 ++
>  tools/vsh-table.h |  42 +
>  3 files changed, 458 insertions(+), 1 deletion(-)
>  create mode 100644 tools/vsh-table.c
>  create mode 100644 tools/vsh-table.h
> 


> +/**
> + * vshTableGetColumnsWidths:
> + * @table: table
> + * @maxwidths: maximum count of characters for each columns
> + * @widths: count of characters for each cell in the table
> + *
> + * Fill passed @maxwidths and @widths arrays with maximum number
> + * of characters for columns and number of character per each
> + * table cell, respectively.
> + *
> + * Handle unicode strings (user must have multibyte locale)
> + */
> +static int
> +vshTableGetColumnsWidths(vshTablePtr table,
> + size_t *maxwidths,
> + size_t **widths,
> + bool header)
> +{
> +int ret = -1;
> +size_t i = 1;
> +size_t j;
> +size_t len;
> +int tmp;
> +wchar_t *wstr = NULL;
> +size_t wstrlen;
> +
> +if (header)
> +i = 0;
> +else
> +i = 1;
> +for (; i < table->nrows; i++) {
> +vshTableRowPtr row = table->rows[i];
> +
> +for (j = 0; j < row->ncells; j++) {
> +/* strlen should return maximum possible length needed */
> +wstrlen = strlen(row->cells[j]);
> +VIR_FREE(wstr);
> +if (VIR_ALLOC_N(wstr, wstrlen) < 0)
> +goto cleanup;
> +/* mbstowcs fails if machine is using singlebyte locale
> + * and user tries to convert unicode(multibyte)
> + * */
> +if (mbstowcs(wstr, row->cells[j], wstrlen) ==
> +(size_t) -1) {
> +len = wstrlen;
> +} else {
> +tmp = wcswidth(wstr, wstrlen);
> +if (tmp < 0)
> +goto cleanup;
> +len = (size_t)((unsigned)tmp);
> +}
> +widths[i][j] = len;
> +if (len > maxwidths[j])
> +maxwidths[j] = len;

After asking around I have found the right solution that we need to use
for measuring string width.  mbstowcs()/wcswidth() will get the answer
wrong wrt zero-width characters, combining characters, non-printable
characters, etc. We need to use the libunistring library:

  
https://www.gnu.org/software/libunistring/manual/libunistring.html#uniwidth_002eh


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH v3 1/3] vsh: Add API for printing tables.

2018-08-16 Thread Simon Kobyda 
It solves problems with alignment of columns. Width of each column
is calculated by its biggest cell. Should solve unicode bug.
In future, it may be implemented in virsh, virt-admin...

This API has 5 public functions:
- vshTableNew - adds new table and defines its header
- vshTableRowAppend - appends new row (for same number of columns as in
header)
- vshTablePrintToStdout
- vshTablePrintToString
- vshTableFree

https://bugzilla.redhat.com/show_bug.cgi?id=1574624
https://bugzilla.redhat.com/show_bug.cgi?id=1584630

Signed-off-by: Simon Kobyda 
---
 tools/Makefile.am |   4 +-
 tools/vsh-table.c | 413 ++
 tools/vsh-table.h |  42 +
 3 files changed, 458 insertions(+), 1 deletion(-)
 create mode 100644 tools/vsh-table.c
 create mode 100644 tools/vsh-table.h

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 1452d984a0..f069167acc 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -144,7 +144,9 @@ libvirt_shell_la_LIBADD = \
$(READLINE_LIBS) \
../gnulib/lib/libgnu.la \
$(NULL)
-libvirt_shell_la_SOURCES = vsh.c vsh.h
+libvirt_shell_la_SOURCES = \
+   vsh.c vsh.h \
+   vsh-table.c vsh-table.h
 
 virt_host_validate_SOURCES = \
virt-host-validate.c \
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
new file mode 100644
index 00..8842e4e4fd
--- /dev/null
+++ b/tools/vsh-table.c
@@ -0,0 +1,413 @@
+/*
+ * vsh-table.c: table printing helper
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Authors:
+ *   Simon Kobyda 
+ *
+ */
+
+#include 
+#include "vsh-table.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "viralloc.h"
+#include "virbuffer.h"
+#include "virstring.h"
+#include "virsh-util.h"
+
+typedef void (*vshPrintCB)(vshControl *ctl, const char *fmt, ...);
+
+struct _vshTableRow {
+char **cells;
+size_t ncells;
+};
+
+struct _vshTable {
+vshTableRowPtr *rows;
+size_t nrows;
+};
+
+static void
+vshTableRowFree(vshTableRowPtr row)
+{
+size_t i;
+
+if (!row)
+return;
+
+for (i = 0; i < row->ncells; i++)
+VIR_FREE(row->cells[i]);
+
+VIR_FREE(row->cells);
+VIR_FREE(row);
+}
+
+void
+vshTableFree(vshTablePtr table)
+{
+size_t i;
+
+if (!table)
+return;
+
+for (i = 0; i < table->nrows; i++)
+vshTableRowFree(table->rows[i]);
+VIR_FREE(table->rows);
+VIR_FREE(table);
+}
+
+/**
+ * vshTableRowNew:
+ * @arg: the first argument.
+ * @ap: list of variadic arguments
+ *
+ * Create a new row in the table. Each argument passed
+ * represents a cell in the row.
+ * Return: pointer to vshTableRowPtr row or NULL.
+ */
+static vshTableRowPtr
+vshTableRowNew(const char *arg, va_list ap)
+{
+vshTableRowPtr row = NULL;
+char *tmp = NULL;
+
+if (!arg) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Table row cannot be empty"));
+goto error;
+}
+
+if (VIR_ALLOC(row) < 0)
+goto error;
+
+while (arg) {
+if (VIR_STRDUP(tmp, arg) < 0)
+goto error;
+
+if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0)
+goto error;
+
+arg = va_arg(ap, const char *);
+}
+
+return row;
+
+ error:
+vshTableRowFree(row);
+return NULL;
+}
+
+/**
+ * vshTableNew:
+ * @arg: List of column names (NULL terminated)
+ *
+ * Create a new table.
+ *
+ * Returns: pointer to table or NULL.
+ */
+vshTablePtr
+vshTableNew(const char *arg, ...)
+{
+vshTablePtr table;
+vshTableRowPtr header = NULL;
+va_list ap;
+
+if (VIR_ALLOC(table) < 0)
+goto error;
+
+va_start(ap, arg);
+header = vshTableRowNew(arg, ap);
+va_end(ap);
+
+if (!header)
+goto error;
+
+if (VIR_APPEND_ELEMENT(table->rows, table->nrows, header) < 0)
+goto error;
+
+return table;
+ error:
+vshTableRowFree(header);
+vshTableFree(table);
+return NULL;
+}
+
+/**
+ * vshTableRowAppend:
+ * @table: table to append to
+ * @arg: cells of the row (NULL terminated)
+ *
+ * Append new row into the @table. The number of cells in the row has
+ * to be equal to the number of cells in the table header.
+ *
+ * Returns: 0 if succeeded, -1 if failed.
+ */
+int