libsmartcols patch
Forwarding patch from https://launchpad.net/bugs/1406133:
src/table_print.c +53 -28
@@ -151,6 +151,32 @@return bytes; } +/* returns pointer to the end of used data */ +static int line_ascii_art_to_buffer (struct libscols_table *tb, +struct libscols_line *ln, +struct libscols_buffer *buf) +{ +const char *art; +int rc; + +assert (ln); +assert (buf); + +if (!ln->parent) +return 0 ; + + rc =line_ascii_art_to_buffer (tb, ln->parent, buf); +if (rc) +return rc; + +if (list_entry_is_last (&ln->ln_children, &ln->parent->ln_branch)) + art =" " ; +else + art = tb->symbols->vert; + +return buffer_append_data (buf, art); +} +#define is_last_column (_tb, _cl) \ list_entry_is_last(&(_cl)->cl_columns, &(_tb)->tb_columns)
@@ -258,8 +284,33 @@if (len > width && !scols_column_is_trunc (cl)) {fputs (linesep (tb), tb->out);for (i =0 ; i <= (size_t ) cl->seqnum; i++) { -struct libscols_column *x =scols_table_get_column (tb, i); -fprintf (tb->out,"%*s " , -((int )x->width)," " ); +size_t len_pad =0 ;/* in screen cells as opposed to bytes */ +struct libscols_column * cl_pad =scols_table_get_column (tb, i); +if (ln &&scols_column_is_tree (cl_pad)) { +if (!ln->parent) {/* Root cell needs special treatment. */ +if (!list_empty (&ln->ln_branch)) {/* only print symbols->vert if followed by something */ +fputs (tb->symbols->vert, tb->out); + len_pad =mbs_safe_width (tb->symbols->vert); + } + }else {/* use the same draw function as though we were intending to draw an L-shape */ +struct libscols_buffer * b_art =new_buffer (1024 );/* TODO: FIXME proper size */ +char * data =0 ; + +if (b_art) { +line_ascii_art_to_buffer (tb, ln, b_art);/* whatever the rc, len_pad will be sensible */ + data =buffer_get_safe_data (b_art, &len_pad); +if (data && len_pad) { +fputs (data, tb->out); + } + +free_buffer (b_art); + } + } + } +for (; len_pad <= cl_pad->width; ++len_pad) + { +fputc (' ' , tb->out); + } } }else fputs (colsep (tb), tb->out);/* columns separator */
@@ -268,32 +319,6 @@return 0 ; } -/* returns pointer to the end of used data */ -static int line_ascii_art_to_buffer (struct libscols_table *tb, -struct libscols_line *ln, -struct libscols_buffer *buf) -{ -const char *art; -int rc; - -assert (ln); -assert (buf); - -if (!ln->parent) -return 0 ; - - rc =line_ascii_art_to_buffer (tb, ln->parent, buf); -if (rc) -return rc; - -if (list_entry_is_last (&ln->ln_children, &ln->parent->ln_branch)) - art =" " ; -else - art = tb->symbols->vert; - -return buffer_append_data (buf, art); -} - static int cell_to_buffer(struct libscols_table *tb, struct libscols_line *ln, struct libscols_column *cl,
Karel Zak
maintainerQuoted 1 line · Phillip Susi
Forwarding patch from https://launchpad.net/bugs/1406133:
Applied with some changes, note that the patch does not resolve situation when the tree is not the first column, for example:
$ findmnt -o SOURCE,FSTYPE,OPTIONS,TARGET
but it's fine. I have tried to implement it for for trees after the line break and then the output is pretty unreadable.
Karel
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
src/table_print.c line 163
Quoted 5 lines · Phillip Susi
src/table_print.c +2
-0+assert (ln); +assert (buf);
general note: libraries should never assert/abort/exit. if this were internal, then you could make a case about the utilities being ok with that, but it looks like the intention is for people to include the header and link against it from outside of util-linux.
-mike
Karel Zak
maintainerQuoted 8 lines · Mike Frysinger
src/table_print.c line 163
Quoted 5 lines · Phillip Susi
src/table_print.c +2
-0+assert (ln); +assert (buf);general note: libraries should never assert/abort/exit.
what about segfault?
We use assert to detect fatal internal library bugs, for mistakes in applications (e.g. incomplete arguments) it returns -EINVAL.
Its' possible to remove CONFIG_LIBSMARTCOLS_ASSERT from smartcolsP.h, but I don't think the library is already well tested.
Karel
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
Quoted 11 lines · Karel Zak
Quoted 8 lines · Mike Frysinger
src/table_print.c line 163
Quoted 5 lines · Phillip Susi
src/table_print.c +2
-0+assert (ln); +assert (buf);general note: libraries should never assert/abort/exit.
what about segfault?
that's a red herring. you can't guarantee your codebase is bug free, but that doesn't mean encouraging a policy of committing suicide is a good thing.
Quoted 2 lines
We use assert to detect fatal internal library bugs, for mistakes in applications (e.g. incomplete arguments) it returns
-EINVAL.
libsmartcols/src/table_print.c:
int scols_print_table(struct libscols_table *tb)
{
...
assert(tb);
if (!tb)
return -1;looks like over-eagerness has already broken the public API ;).
Quoted 2 lines
Its' possible to remove
CONFIG_LIBSMARTCOLS_ASSERTfromsmartcolsP.h, but I don't think the library is already well tested.
odd ... why is that reimplementing the wheel ? the standard assert.h already supports NDEBUG for turning off asserts.
i see there's a bunch of such knobs in the codebase. time for a --{en,dis}able-asserts configure flag ? you could delete all the existing CONFIG_xxx_ASSERT defines and simply rely on configure adding -DNDEBUG to CPPFLAGS.
-mike
Karel Zak
maintainerQuoted 6 lines · Mike Frysinger
Quoted 3 lines
Quoted 1 line
general note: libraries should never assert/abort/exit.
what about segfault?
that's a red herring. you can't guarantee your codebase is bug free, but that doesn't mean encouraging a policy of committing suicide is a good thing.
suicide + usable message is better than segfault
Quoted 10 lines
Quoted 2 lines
We use assert to detect fatal internal library bugs, for mistakes in applications (e.g. incomplete arguments) it returns
-EINVAL.
libsmartcols/src/table_print.c:
intscols_print_table(struct libscols_table *tb)
{
...assert(tb); if (!tb) return -1;
this is mistake, fixed in libsmartcols and libmount, I'll later fix it in libfdisk too.
Quoted 10 lines
Quoted 2 lines
Its' possible to remove
CONFIG_LIBSMARTCOLS_ASSERTfromsmartcolsP.h, but I don't think the library is already well tested.odd ... why is that reimplementing the wheel ? the standard
assert.halready supports NDEBUG for turning off asserts.i see there's a bunch of such knobs in the codebase. time for a --{en,dis}able-asserts configure flag ? you could delete all the existing
CONFIG_xxx_ASSERTdefines and simply rely on configure adding -DNDEBUG to CPPFLAGS.
Maybe, the idea has been to keep it in our hands and force people to use asserts for the "not yet well tested" libs.
Karel
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com