diff --git a/src/basic/string-util.c b/src/basic/string-util.c index efd690b076..26602ac771 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -414,8 +414,10 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne bool has_ansi_seq = string_has_ansi_sequence(s, old_length); - /* If no multibyte characters or ANSI sequences, use ascii_ellipsize_mem for speed */ - if (!has_ansi_seq && ascii_is_valid_n(s, old_length)) + /* If no multibyte characters or ANSI sequences, use ascii_ellipsize_mem for speed. It equates + * bytes with character cells, which holds for ASCII except for the tab, which + * unichar_console_width() counts as 8 cells. */ + if (!has_ansi_seq && ascii_is_valid_n(s, old_length) && !memchr(s, '\t', old_length)) return ascii_ellipsize_mem(s, old_length, new_length, percent); x = (new_length - 1) * percent / 100; @@ -436,7 +438,7 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne char32_t c; assert_se(utf8_encoded_to_unichar(i, &c) == r); - int w = unichar_iswide(c) ? 2 : 1; + int w = unichar_console_width(c); if (k + w > x) break; @@ -467,7 +469,7 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne if (!tt) return NULL; - w = unichar_iswide(c) ? 2 : 1; + w = unichar_console_width(c); if (k + w > new_length) break; diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 88d76b67f0..518ef5bd25 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2079,18 +2079,23 @@ static char* align_string_mem(const char *str, const char *url, size_t new_lengt } else clickable_length = old_length; - /* Determine current width on screen */ + /* Determine current width on screen. Use the same helper the measuring pass uses, so that this + * function cannot keep a second, more permissive idea of both validity and width of its own. + * Note that our caller only gets here after utf8_console_width() succeeded on the very same + * string, so the invalid case below should not be reachable in practice. */ p = str; while (p < str + old_length) { - char32_t c; - - if (utf8_encoded_to_unichar(p, &c) < 0) { - p++, w++; /* count invalid chars as 1 */ + int n = utf8_char_console_width(p); + if (n < 0) { + /* Count an invalid sequence as one character of width one. Skip all of it, so + * that its continuation bytes are not charged a cell each of their own. */ + p = utf8_next_char(p); + w++; continue; } p = utf8_next_char(p); - w += unichar_iswide(c) ? 2 : 1; + w += n; } /* Already wider than the target, if so, don't do anything */ diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index 8a0f93fb6d..0b3ec62953 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -13,6 +13,7 @@ #include "terminal-util.h" #include "tests.h" #include "time-util.h" +#include "utf8.h" TEST(issue_9549) { _cleanup_(table_unrefp) Table *table = NULL; @@ -67,6 +68,59 @@ TEST(invalid_utf8_cell) { } } +TEST(tab_in_cell) { + _cleanup_(table_unrefp) Table *table = NULL; + _cleanup_free_ char *formatted = NULL; + + /* A tab takes up eight character cells, so the padding align_string_mem() adds has to be + * computed with unichar_console_width() too, or the column comes out too wide. + * + * Note that unichar_console_width() charges a tab a flat eight cells, while a terminal + * advances to the next multiple of eight instead. The two only agree if the tab itself starts + * on a multiple of eight, hence the eight characters in front of it here: that way the + * expected output below is what a terminal really renders, too. */ + + ASSERT_NOT_NULL((table = table_new("name", "value"))); + ASSERT_OK(table_add_many(table, + TABLE_STRING, "aaaaaaaa\tb", + TABLE_STRING, "1", + TABLE_STRING, "wide enough to pad the tab cell", + TABLE_STRING, "2")); + + table_set_width(table, 40); + ASSERT_OK(table_format(table, &formatted)); + + printf("%s\n", formatted); + ASSERT_STREQ(formatted, + "NAME VALUE\n" + "aaaaaaaa\tb 1\n" + "wide enough to pad the tab cell 2\n"); + + /* And the case the two passes have to agree on: a cell whose tab makes it the widest one in + * its column, in a table pinned narrower than that. The cell drives the requested column width + * and is then handed to ellipsize(), which used to believe the tab was a single cell and so + * returned something that still overflowed the column it had just been asked to fit into. */ + + _cleanup_(table_unrefp) Table *narrow = NULL; + _cleanup_free_ char *narrow_formatted = NULL; + _cleanup_strv_free_ char **lines = NULL; + + ASSERT_NOT_NULL((narrow = table_new("name", "value"))); + ASSERT_OK(table_add_many(narrow, + TABLE_STRING, "aaaaaaaa\tbbbbbbbb", + TABLE_STRING, "1", + TABLE_STRING, "short", + TABLE_STRING, "2")); + + table_set_width(narrow, 20); + ASSERT_OK(table_format(narrow, &narrow_formatted)); + + printf("%s\n", narrow_formatted); + ASSERT_NOT_NULL((lines = strv_split_newlines(narrow_formatted))); + STRV_FOREACH(l, lines) + ASSERT_LE(utf8_console_width(*l), (size_t) 20); +} + TEST(multiline) { _cleanup_(table_unrefp) Table *table = NULL; _cleanup_free_ char *formatted = NULL; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 79b8175c34..732517541f 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -9,6 +9,7 @@ #include "string-util.h" #include "strv.h" #include "tests.h" +#include "utf8.h" #include "version.h" TEST(ellipsize_mem_ansi_short) { @@ -22,6 +23,48 @@ TEST(ellipsize_mem_ansi_short) { assert_se(c); } +TEST(ellipsize_mem_tab) { + /* unichar_console_width() counts a tab as eight character cells. Callers such as + * format-table.c measure a field with utf8_console_width() and then ask ellipsize() to fit it + * into that many columns, so ellipsize() must not count a tab as a single cell: the result + * would still overflow the column that the measurement had just asked us to fit. */ + + unsigned percent; + + FOREACH_STRING(x, + "\t", + "a\tb", + "ab\tcd", + "\t\t\t", + "a\tb\tc\td\te", + "a\t你\tb") /* tabs mixed with a wide character */ + for (size_t w = 1; w <= 24; w++) + /* Sweep the ellipsis placement too: it decides how much of the string the + * forward loop takes and how much is left to the backward one, and the two + * meet at quite different places for an eight cell tab than for a single cell + * character. */ + FOREACH_ARGUMENT(percent, 0u, 30u, 50u, 90u, 100u) { + _cleanup_free_ char *e = ASSERT_NOT_NULL(ellipsize(x, w, percent)); + + ASSERT_LE(utf8_console_width(e), w); + + /* And don't truncate what already fits. */ + if (utf8_console_width(x) <= w) + ASSERT_STREQ(e, x); + } + + /* Pin a couple of concrete results, so that a change to the assumed tab width or to where the + * ellipsis lands shows up in the diff. Note that these are in terms of the flat eight cells + * unichar_console_width() charges a tab, which is what our callers measure with; a terminal + * instead advances to the next multiple of eight, and hence needs one column less for "a\tb" + * at the start of a line. */ + _cleanup_free_ char *a = ASSERT_NOT_NULL(ellipsize("a\tb", 10, 50)); + ASSERT_STREQ(a, "a\tb"); + + _cleanup_free_ char *b = ASSERT_NOT_NULL(ellipsize("a\tb", 9, 50)); + ASSERT_STREQ(b, "a…b"); +} + TEST(xsprintf) { char buf[5];