| Message ID | 20221011041108.5933-1-xdavidwuph@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] client: handle wide chars for table rows | expand |
Hi Pinghao, On Tue, 2022-10-11 at 12:11 +0800, Pinghao Wu wrote: > Find out printing width of wide chars via wcwidth(). > --- > v3: > Code simplification > Unify handling of color escape / char width a bit > > client/display.c | 30 +++++++++++++++++++++--------- > client/main.c | 3 +++ > 2 files changed, 24 insertions(+), 9 deletions(-) Just a couple of style issues and error checks, otherwise looks good to me. There are some compiler errors due to -Werror, make sure you compile with this turned on. I use ./bootstrap-configure (may need -- disable-manual-pages if you don't have the packages). ``` client/display.c: In function 'next_line': client/display.c:408:31: error: comparison of integer expressions of different signedness: 'unsigned int' and 'int' [-Werror=sign-compare] 408 | while (i <= *max && i != s_len) { | ^~ client/display.c:420:25: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] 420 | wchar_t w; | ^~~~~~~ cc1: all warnings being treated as errors make[1]: *** [Makefile:2409: client/display.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1588: all] Error 2 > > diff --git a/client/display.c b/client/display.c > index b729ad4c..5ec80789 100644 > --- a/client/display.c > +++ b/client/display.c > @@ -30,6 +30,7 @@ > #include <sys/stat.h> > #include <sys/ioctl.h> > #include <unistd.h> > +#include <wchar.h> > > #include <readline/history.h> > #include <readline/readline.h> > @@ -398,21 +399,32 @@ static unsigned int color_end(char *s) > */ > static char* next_line(char *s, unsigned int *max, char **color_out) > { > - unsigned int i; > + unsigned int i = 0; > int last_space = -1; > int last_color = -1; > + int s_len = strlen(s); This should be unsigned int > > /* Find the last space before 'max', as well as any color */ > - for (i = 0; i <= *max && s[i] != '\0'; i++) { > - if (s[i] == ' ') > - last_space = i; > - else if (s[i] == 0x1b) { > + while (i <= *max && i != s_len) { > + int sequence_len; > + int sequence_columns; > + > + if (s[i] == 0x1b) { > + sequence_len = color_end(s + i); > /* color escape won't count for column width > */ > - *max += color_end(s + i); > + sequence_columns = 0; > last_color = i; > - /* Add width for non-codepoint UTF-8 bytes */ > - } else if (((uint8_t)s[i] >> 6) == 2) > - *max += 1; > + } else { > + if (s[i] == ' ') > + last_space = i; > + wchar_t w; Move this declaration up with sequence_len/sequence_columns > + sequence_len = l_utf8_get_codepoint(&s[i], > s_len - i, &w); Probably should check the return here and treat as a single byte/character if this fails. In reality this should never happen (utf8 is validated on the IWD side) but just to be safe... This line also exceeds 80 characters. > + sequence_columns = wcwidth(w); > + } > + > + /* Compensate max bytes */ > + *max += sequence_len - sequence_columns; > + i += sequence_len; > } > > /* Reached the end of the string within the column bounds */ > diff --git a/client/main.c b/client/main.c > index df5c0a61..7e8dead4 100644 > --- a/client/main.c > +++ b/client/main.c > @@ -25,6 +25,7 @@ > #endif > > #include <errno.h> > +#include <locale.h> > #include <signal.h> > #include <ell/ell.h> > > @@ -50,6 +51,8 @@ int main(int argc, char *argv[]) > { > bool all_done; > > + setlocale(LC_CTYPE, ""); > + > if (!l_main_init()) > return EXIT_FAILURE; >
diff --git a/client/display.c b/client/display.c index b729ad4c..5ec80789 100644 --- a/client/display.c +++ b/client/display.c @@ -30,6 +30,7 @@ #include <sys/stat.h> #include <sys/ioctl.h> #include <unistd.h> +#include <wchar.h> #include <readline/history.h> #include <readline/readline.h> @@ -398,21 +399,32 @@ static unsigned int color_end(char *s) */ static char* next_line(char *s, unsigned int *max, char **color_out) { - unsigned int i; + unsigned int i = 0; int last_space = -1; int last_color = -1; + int s_len = strlen(s); /* Find the last space before 'max', as well as any color */ - for (i = 0; i <= *max && s[i] != '\0'; i++) { - if (s[i] == ' ') - last_space = i; - else if (s[i] == 0x1b) { + while (i <= *max && i != s_len) { + int sequence_len; + int sequence_columns; + + if (s[i] == 0x1b) { + sequence_len = color_end(s + i); /* color escape won't count for column width */ - *max += color_end(s + i); + sequence_columns = 0; last_color = i; - /* Add width for non-codepoint UTF-8 bytes */ - } else if (((uint8_t)s[i] >> 6) == 2) - *max += 1; + } else { + if (s[i] == ' ') + last_space = i; + wchar_t w; + sequence_len = l_utf8_get_codepoint(&s[i], s_len - i, &w); + sequence_columns = wcwidth(w); + } + + /* Compensate max bytes */ + *max += sequence_len - sequence_columns; + i += sequence_len; } /* Reached the end of the string within the column bounds */ diff --git a/client/main.c b/client/main.c index df5c0a61..7e8dead4 100644 --- a/client/main.c +++ b/client/main.c @@ -25,6 +25,7 @@ #endif #include <errno.h> +#include <locale.h> #include <signal.h> #include <ell/ell.h> @@ -50,6 +51,8 @@ int main(int argc, char *argv[]) { bool all_done; + setlocale(LC_CTYPE, ""); + if (!l_main_init()) return EXIT_FAILURE;