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

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