From 557b0841b7b4d03c85f3e627dd9f20d977974605 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 May 2020 14:46:37 +0200 Subject: [PATCH 1/7] format-table: display an empty strv cell the same way as an empty cell An empty list conceptually is very close to null, hence let's show it the same way, i.e. using the defined "empty" string, and greyed out. --- src/shared/format-table.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index aac8ac47fa..cef96343ab 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -1272,6 +1272,9 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas case TABLE_STRV: { char *p; + if (strv_isempty(d->strv)) + return strempty(t->empty_string); + p = strv_join(d->strv, "\n"); if (!p) return NULL; @@ -1694,6 +1697,20 @@ static char *align_string_mem(const char *str, const char *url, size_t new_lengt return ret; } +static bool table_data_isempty(TableData *d) { + assert(d); + + if (d->type == TABLE_EMPTY) + return true; + + /* Let's also consider an empty strv as truly empty. */ + if (d->type == TABLE_STRV) + return strv_isempty(d->strv); + + /* Note that an empty string we do not consider empty here! */ + return false; +} + static const char* table_data_color(TableData *d) { assert(d); @@ -1701,7 +1718,7 @@ static const char* table_data_color(TableData *d) { return d->color; /* Let's implicitly color all "empty" cells in grey, in case an "empty_string" is set that is not empty */ - if (d->type == TABLE_EMPTY) + if (table_data_isempty(d)) return ansi_grey(); return NULL; From 0773357ad1e3d9dbc10343137c0eccf402849d24 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 May 2020 14:47:46 +0200 Subject: [PATCH 2/7] systemctl: show empty cells as '-' This is mostly relevant for "systemct list-sockets" which can have empty cells for the activating unit. --- src/systemctl/systemctl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 03b4d1b783..5c32147f14 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -425,6 +425,8 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { if (arg_full) table_set_width(table, 0); + (void) table_set_empty_string(table, "-"); + for (u = unit_infos; unit_infos && u < unit_infos + c; u++) { _cleanup_free_ char *j = NULL; const char *on_underline = "", *on_loaded = "", *on_active = ""; @@ -1021,6 +1023,8 @@ static int output_sockets_list(struct socket_info *socket_infos, unsigned cs) { if (arg_full) table_set_width(table, 0); + (void) table_set_empty_string(table, "-"); + if (cs) { for (s = socket_infos; s < socket_infos + cs; s++) { _cleanup_free_ char *j = NULL, *activates = NULL; @@ -1265,6 +1269,8 @@ static int output_timers_list(struct timer_info *timer_infos, unsigned n) { if (arg_full) table_set_width(table, 0); + (void) table_set_empty_string(table, "-"); + if (n > 0) { for (t = timer_infos; t < timer_infos + n; t++) { _cleanup_free_ char *j = NULL, *activates = NULL; @@ -1478,6 +1484,8 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { if (arg_full) table_set_width(table, 0); + (void) table_set_empty_string(table, "-"); + for (const UnitFileList *u = units; u < units + c; u++) { const char *on_underline = NULL, *on_unit_color = NULL, *id; bool underline; @@ -2031,6 +2039,8 @@ static int output_machines_list(struct machine_info *machine_infos, unsigned n) if (arg_full) table_set_width(table, 0); + (void) table_set_empty_string(table, "-"); + for (m = machine_infos; m < machine_infos + n; m++) { _cleanup_free_ char *mname = NULL; const char *on_state = "", *on_failed = ""; @@ -2351,6 +2361,8 @@ static int output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n if (arg_full) table_set_width(table, 0); + (void) table_set_empty_string(table, "-"); + for (j = jobs; j < jobs + n; j++) { if (streq(j->state, "running")) on = ansi_highlight(); From bc04bb0d29623669eac0b514987128016b0c8b47 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 May 2020 14:48:14 +0200 Subject: [PATCH 3/7] systemctl: change column name in "list-sockets" out from "units" to "unit" All our other tables call the field in singular, hence let's do so here, too. --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 5c32147f14..673f29bcaa 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1008,7 +1008,7 @@ static int output_sockets_list(struct socket_info *socket_infos, unsigned cs) { const char *on, *off; int r; - table = table_new("listen", "type", "units", "activates"); + table = table_new("listen", "type", "unit", "activates"); if (!table) return log_oom(); From 50098d87fbe677bd43cb7329b2e32b039401f35c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 May 2020 14:48:54 +0200 Subject: [PATCH 4/7] systemctl: let's tweak how we synthesize a cell for activating unit Let's create a string cell for the unit if possible (since there can only be one unit right now, and the JSON alternative output then generates a string instead of an array for us), an empty cell if empty. --- src/systemctl/systemctl.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 673f29bcaa..07b0fdd2c5 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1027,7 +1027,7 @@ static int output_sockets_list(struct socket_info *socket_infos, unsigned cs) { if (cs) { for (s = socket_infos; s < socket_infos + cs; s++) { - _cleanup_free_ char *j = NULL, *activates = NULL; + _cleanup_free_ char *j = NULL; const char *path; if (s->machine) { @@ -1038,17 +1038,25 @@ static int output_sockets_list(struct socket_info *socket_infos, unsigned cs) { } else path = s->path; - activates = strv_join(s->triggered, ", "); - if (!activates) - return log_oom(); - r = table_add_many(table, TABLE_STRING, path, TABLE_STRING, s->type, - TABLE_STRING, s->id, - TABLE_STRING, activates); + TABLE_STRING, s->id); if (r < 0) return table_log_add_error(r); + + if (strv_isempty(s->triggered)) + r = table_add_cell(table, NULL, TABLE_EMPTY, NULL); + else if (strv_length(s->triggered) == 1) + r = table_add_cell(table, NULL, TABLE_STRING, s->triggered[0]); + else + /* This should never happen, currently our socket units can only trigger a + * single unit. But let's handle this anyway, who knows what the future + * brings? */ + r = table_add_cell(table, NULL, TABLE_STRV, s->triggered); + if (r < 0) + return table_log_add_error(r); + } on = ansi_highlight(); From 45d82c3f1c47b79898b03ee89062d502e720b8d5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 May 2020 14:56:37 +0200 Subject: [PATCH 5/7] systemctl: ensure underline for "list-unit-files" empty cells --- src/basic/terminal-util.h | 2 ++ src/systemctl/systemctl.c | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h index c7570c7a61..67634b4d46 100644 --- a/src/basic/terminal-util.h +++ b/src/basic/terminal-util.h @@ -28,6 +28,7 @@ #define ANSI_HIGHLIGHT_YELLOW4 "\x1B[0;1;38;5;100m" /* Underlined */ +#define ANSI_GREY_UNDERLINE "\x1B[0;4;38;5;245m" #define ANSI_HIGHLIGHT_RED_UNDERLINE "\x1B[0;1;4;31m" #define ANSI_HIGHLIGHT_GREEN_UNDERLINE "\x1B[0;1;4;32m" #define ANSI_HIGHLIGHT_YELLOW_UNDERLINE "\x1B[0;1;4;38;5;185m" @@ -138,6 +139,7 @@ DEFINE_ANSI_FUNC(highlight_grey, HIGHLIGHT_GREY); DEFINE_ANSI_FUNC_UNDERLINE(underline, UNDERLINE, NORMAL); DEFINE_ANSI_FUNC_UNDERLINE(highlight_underline, HIGHLIGHT_UNDERLINE, HIGHLIGHT); +DEFINE_ANSI_FUNC_UNDERLINE(grey_underline, GREY_UNDERLINE, GREY); DEFINE_ANSI_FUNC_UNDERLINE(highlight_red_underline, HIGHLIGHT_RED_UNDERLINE, HIGHLIGHT_RED); DEFINE_ANSI_FUNC_UNDERLINE(highlight_green_underline, HIGHLIGHT_GREEN_UNDERLINE, HIGHLIGHT_GREEN); DEFINE_ANSI_FUNC_UNDERLINE(highlight_yellow_underline, HIGHLIGHT_YELLOW_UNDERLINE, HIGHLIGHT_YELLOW); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 07b0fdd2c5..807a497427 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1546,8 +1546,9 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { TABLE_STRING, unit_preset_str, TABLE_SET_COLOR, strempty(on_preset_color)); } else - r = table_add_many(table, TABLE_EMPTY); - + r = table_add_many(table, + TABLE_EMPTY, + TABLE_SET_COLOR, underline ? ansi_grey_underline() : ansi_grey()); if (r < 0) return table_log_add_error(r); } From b0395c11768967e2c463c5721163ebaa3a277593 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 May 2020 15:36:50 +0200 Subject: [PATCH 6/7] format-table: support coloring the gap right of each cell individually This is useful for underlining a whole row of cells: previously we couldn't underline the gap. Now we can. --- src/shared/format-table.c | 76 ++++++++++++++++++++++++++++++++++----- src/shared/format-table.h | 3 ++ 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index cef96343ab..f12ad2d038 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -73,6 +73,7 @@ typedef struct TableData { bool uppercase; /* Uppercase string on display */ const char *color; /* ANSI color string to use for this cell. When written to terminal should not move cursor. Will automatically be reset after the cell */ + const char *rgap_color; /* The ANSI color to use for the gap right of this cell. Usually used to underline entire rows in a gapless fashion */ char *url; /* A URL to use for a clickable hyperlink */ char *formatted; /* A cached textual representation of the cell data, before ellipsation/alignment */ @@ -334,7 +335,7 @@ static bool table_data_matches( return false; /* If a color/url/uppercase flag is set, refuse to merge */ - if (d->color) + if (d->color || d->rgap_color) return false; if (d->url) return false; @@ -542,6 +543,7 @@ static int table_dedup_cell(Table *t, TableCell *cell) { return -ENOMEM; nd->color = od->color; + nd->rgap_color = od->rgap_color; nd->url = TAKE_PTR(curl); nd->uppercase = od->uppercase; @@ -671,6 +673,20 @@ int table_set_color(Table *t, TableCell *cell, const char *color) { return 0; } +int table_set_rgap_color(Table *t, TableCell *cell, const char *color) { + int r; + + assert(t); + assert(cell); + + r = table_dedup_cell(t, cell); + if (r < 0) + return r; + + table_get_data(t, cell)->rgap_color = empty_to_null(color); + return 0; +} + int table_set_url(Table *t, TableCell *cell, const char *url) { _cleanup_free_ char *copy = NULL; int r; @@ -744,6 +760,7 @@ int table_update(Table *t, TableCell *cell, TableDataType type, const void *data return -ENOMEM; nd->color = od->color; + nd->rgap_color = od->rgap_color; nd->url = TAKE_PTR(curl); nd->uppercase = od->uppercase; @@ -952,6 +969,25 @@ int table_add_many_internal(Table *t, TableDataType first_type, ...) { break; } + case TABLE_SET_RGAP_COLOR: { + const char *c = va_arg(ap, const char*); + r = table_set_rgap_color(t, last_cell, c); + break; + } + + case TABLE_SET_BOTH_COLORS: { + const char *c = va_arg(ap, const char*); + + r = table_set_color(t, last_cell, c); + if (r < 0) { + va_end(ap); + return r; + } + + r = table_set_rgap_color(t, last_cell, c); + break; + } + case TABLE_SET_URL: { const char *u = va_arg(ap, const char*); r = table_set_url(t, last_cell, u); @@ -1724,6 +1760,15 @@ static const char* table_data_color(TableData *d) { return NULL; } +static const char* table_data_rgap_color(TableData *d) { + assert(d); + + if (d->rgap_color) + return d->rgap_color; + + return NULL; +} + int table_print(Table *t, FILE *f) { size_t n_rows, *minimum_width, *maximum_width, display_columns, *requested_width, i, j, table_minimum_width, table_maximum_width, table_requested_width, table_effective_width, @@ -1975,12 +2020,13 @@ int table_print(Table *t, FILE *f) { row = t->data + i * t->n_columns; do { + const char *gap_color = NULL; more_sublines = false; for (j = 0; j < display_columns; j++) { _cleanup_free_ char *buffer = NULL, *extracted = NULL; bool lines_truncated = false; - const char *field; + const char *field, *color = NULL; TableData *d; size_t l; @@ -2059,23 +2105,35 @@ int table_print(Table *t, FILE *f) { field = buffer; } - if (row == t->data) /* underline header line fully, including the column separator */ - fputs(ansi_underline(), f); + if (colors_enabled()) { + if (gap_color) + fputs(gap_color, f); + else if (row == t->data) /* underline header line fully, including the column separator */ + fputs(ansi_underline(), f); + } if (j > 0) - fputc(' ', f); /* column separator */ + fputc(' ', f); /* column separator left of cell */ - if (table_data_color(d) && colors_enabled()) { - if (row == t->data) /* first undo header underliner */ + if (colors_enabled()) { + color = table_data_color(d); + + /* Undo gap color */ + if (gap_color || (color && row == t->data)) fputs(ANSI_NORMAL, f); - fputs(table_data_color(d), f); + if (color) + fputs(color, f); + else if (gap_color && row == t->data) /* underline header line cell */ + fputs(ansi_underline(), f); } fputs(field, f); - if (colors_enabled() && (table_data_color(d) || row == t->data)) + if (colors_enabled() && (color || row == t->data)) fputs(ANSI_NORMAL, f); + + gap_color = table_data_rgap_color(d); } fputc('\n', f); diff --git a/src/shared/format-table.h b/src/shared/format-table.h index 62f1ed740d..20b4ebd39b 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -47,6 +47,8 @@ typedef enum TableDataType { TABLE_SET_ALIGN_PERCENT, TABLE_SET_ELLIPSIZE_PERCENT, TABLE_SET_COLOR, + TABLE_SET_RGAP_COLOR, + TABLE_SET_BOTH_COLORS, TABLE_SET_URL, TABLE_SET_UPPERCASE, @@ -89,6 +91,7 @@ int table_set_weight(Table *t, TableCell *cell, unsigned weight); int table_set_align_percent(Table *t, TableCell *cell, unsigned percent); int table_set_ellipsize_percent(Table *t, TableCell *cell, unsigned percent); int table_set_color(Table *t, TableCell *cell, const char *color); +int table_set_rgap_color(Table *t, TableCell *cell, const char *color); int table_set_url(Table *t, TableCell *cell, const char *url); int table_set_uppercase(Table *t, TableCell *cell, bool b); From 1fab5797432ff84e6685825d228a1b3d54d662a7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 May 2020 15:37:33 +0200 Subject: [PATCH 7/7] systemctl: underline whole rows, gapless Just some tweaking of the output. --- src/systemctl/systemctl.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 807a497427..7258db2ede 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -440,14 +440,15 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { } if (STR_IN_SET(u->load_state, "error", "not-found", "bad-setting", "masked") && !arg_plain) { - on_circle = ansi_highlight_yellow(); + on_circle = underline ? ansi_highlight_yellow_underline() : ansi_highlight_yellow(); circle = true; on_loaded = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); } else if (streq(u->active_state, "failed") && !arg_plain) { - on_circle = ansi_highlight_red(); + on_circle = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); circle = true; on_active = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); } else { + on_circle = on_underline; on_active = on_underline; on_loaded = on_underline; } @@ -463,19 +464,19 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) { r = table_add_many(table, TABLE_STRING, circle ? special_glyph(SPECIAL_GLYPH_BLACK_CIRCLE) : " ", - TABLE_SET_COLOR, on_circle, + TABLE_SET_BOTH_COLORS, on_circle, TABLE_STRING, id, - TABLE_SET_COLOR, on_active, + TABLE_SET_BOTH_COLORS, on_active, TABLE_STRING, u->load_state, - TABLE_SET_COLOR, on_loaded, + TABLE_SET_BOTH_COLORS, on_loaded, TABLE_STRING, u->active_state, - TABLE_SET_COLOR, on_active, + TABLE_SET_BOTH_COLORS, on_active, TABLE_STRING, u->sub_state, - TABLE_SET_COLOR, on_active, + TABLE_SET_BOTH_COLORS, on_active, TABLE_STRING, u->job_id ? u->job_type: "", - TABLE_SET_COLOR, u->job_id ? on_underline : "", + TABLE_SET_BOTH_COLORS, u->job_id ? on_underline : "", TABLE_STRING, u->description, - TABLE_SET_COLOR, on_underline); + TABLE_SET_BOTH_COLORS, on_underline); if (r < 0) return table_log_add_error(r); @@ -1521,9 +1522,9 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { r = table_add_many(table, TABLE_STRING, id, - TABLE_SET_COLOR, strempty(on_underline), + TABLE_SET_BOTH_COLORS, strempty(on_underline), TABLE_STRING, unit_file_state_to_string(u->state), - TABLE_SET_COLOR, strempty(on_unit_color)); + TABLE_SET_BOTH_COLORS, strempty(on_unit_color)); if (r < 0) return table_log_add_error(r); @@ -1544,11 +1545,11 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { r = table_add_many(table, TABLE_STRING, unit_preset_str, - TABLE_SET_COLOR, strempty(on_preset_color)); + TABLE_SET_BOTH_COLORS, strempty(on_preset_color)); } else r = table_add_many(table, TABLE_EMPTY, - TABLE_SET_COLOR, underline ? ansi_grey_underline() : ansi_grey()); + TABLE_SET_BOTH_COLORS, underline ? ansi_grey_underline() : ansi_grey()); if (r < 0) return table_log_add_error(r); } @@ -2378,7 +2379,6 @@ static int output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n else on = ""; - r = table_add_many(table, TABLE_UINT, j->id, TABLE_STRING, j->name,