[libvirt] [PATCH v5 0/3] vsh: Introduce new API for printing tables

[Daniel P. Berrangé]

On Fri, Aug 24, 2018 at 12:10:47PM +0200, Michal Privoznik wrote:
> On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> >> On 08/23/2018 05:53 PM, Simon Kobyda wrote:
> >>> Created new API for priting tables, mainly to solve alignment problems.
> >>> Implemented these test to virsh list. In the future, API may be
> >>> everywhere in virsh and virt-admin.
> >>> Also wrote basic tests for the new API, and corrected tests in virshtest
> >>> which are influenced by implementation of the API in virsh list.
> >>>
> >>> Changes in v5:
> >>> - cleanup and merged code for calculating zero-width, nonprintable and combined
> >>> character.
> >>> - replaced virBufferAddStr with virBufferAddChar in some places
> >>> - in tests moved code for setting correct locale
> >>> - fixed few leaks and unitialized values
> >>>
> >>> Changes in v4:
> >>> - fixed width calculation for zero-width, nonprintable and combined
> >>> character. (pulled some code from linux-util)
> >>> - added tests for cases mentioned above
> >>> - changed usage of vshControl variables. From now on PrintToStdout calls
> >>> PrintToString and then prints returned string to stdout
> >>>
> >>> Changes in v3:
> >>> - changed encoding of 3/3 patch, otherwise it cannot be applied
> >>>
> >>> Changes in v2:
> >>> - added tests
> >>> - fixed alignment for unicode character which span more spaces
> >>> - moved ncolumns check to vshTableRowAppend
> >>> - changed arguments for functions vshTablePrint, vshTablePrintToStdout,
> >>>     vshTablePrintToString
> >>>
> >>> Simon Kobyda (3):
> >>>   vsh: Add API for printing tables.
> >>>   virsh: Implement new table API for virsh list
> >>>   vsh: Added tests
> >>>
> >>>  tests/Makefile.am            |   8 +
> >>>  tests/virshtest.c            |  14 +-
> >>>  tests/vshtabletest.c         | 377 +++++++++++++++++++++++++++++
> >>>  tools/Makefile.am            |   4 +-
> >>>  tools/virsh-domain-monitor.c |  43 ++--
> >>>  tools/vsh-table.c            | 449 +++++++++++++++++++++++++++++++++++
> >>>  tools/vsh-table.h            |  42 ++++
> >>>  7 files changed, 910 insertions(+), 27 deletions(-)
> >>>  create mode 100644 tests/vshtabletest.c
> >>>  create mode 100644 tools/vsh-table.c
> >>>  create mode 100644 tools/vsh-table.h
> >>>
> >>
> >> ACKed and pushed.
> >>
> >> Now we can start converting the rest of the code to use vshTable APIs.
> > 
> > But first fix the build failures :-)
> > 
> > On CentOS / RHEL:
> > 
> > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > 
> > 
> >  4) testUnicode                                                       ... 
> > Offset 30
> > Expect [государство  
> > -----------------------------------------
> >  1    fedora28              running      
> >  2    🙊🙉🙈rhel7.5🙆🙆🙅]
> > Actual [                                                                                    государство  
> > -----------------------------------------------------------------------------------------------------------------------------
> >  1    fedora28                                                                                                  running      
> >  2    \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > 
> 
> Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> supposed to be fixed by adding -finput-charset= onto gcc command line.
> Haven't tested it though.
> 
> > 
> > 
> > On Mingw:
> > 
> > https://travis-ci.org/libvirt/libvirt/jobs/420024142
> > 
> > vsh-table.c: In function 'vshTableSafeEncode':
> > vsh-table.c:274:27: error: implicit declaration of function 'wcwidth' [-Werror=implicit-function-declaration]
> >                  *width += wcwidth(wc);
> >                            ^~~~~~~
> > vsh-table.c:274:27: error: nested extern declaration of 'wcwidth' [-Werror=nested-externs]
> 
> But this is weird. wcwidth() manpage says to include wchar.t which we
> are doing. Maybe _XOPEN_SOURCE macro is not defined?

It is quite simple - the function doesn't exist on mingw :-)

  $ grep -r wcwidth /usr/i686-w64-mingw32/sys-root/mingw/include/
  ...nothing...

Looks like gnulib can rescue us though

https://www.gnu.org/software/gnulib/manual/html_node/wcwidth.html

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 :|