From: Mark Jamsek Subject: Re: tog test harness To: Christian Weisgerber Cc: gameoftrees@openbsd.org Date: Tue, 18 Apr 2023 12:33:39 +1000 On 23-04-17 09:41PM, Christian Weisgerber wrote: > Mark Jamsek: > > > > > + * Extract visible substring of line y from the curses screen > > > > + * and strip trailing whitespace. If vline is set and locale is > > > > + * UTF-8, overwrite line[vline] with '|' because the ACS_VLINE > > > > + * character is written out as 'x'. Write the line to file f. > > > > + */ > > > > The locale is only relevant in this case because we only use the ACS_* > > characters when in a utf8 locale-- > > Why? That doesn't make sense. The ACS feature in ncurses is > specifically designed to exploit a feature of the DEC VT100 and its > successors that has been available since 1978, well before UTF-8. > ncurses has some fallback code that uses ASCII replacement characters > if the termcap/terminfo entry doesn't enable ACS. IIRC, this was a hueristic used to determine whether tog was being run in the console because there they are rendered as '?' so we use | and - instead. I'm not certain about this, however, as I didn't originally write that logic so I could be wrong about the "why" but this is definitely what we do (see tog.c:969:view_border()). > > > Yes, you should use a standardized $TERM. I mean what happens for > > > TERM=dumb? I'm not sure what a good choice is. "xterm" feels like > > > a moving target. > > > > I was going back and forth on this and ultimately went the other way! > > But I'm happy to defer to your better judgement on this; "dumb" seems to > > work so maybe run with that? > > Huh, I thought curses would refuse to work with "dumb"! > The problem is finding some sort of stable, generally available > terminal type. "vt220"? I was surprised too! But, "vt220" also works :) The below diff removes the screendump defines and writes | and - unconditionally when capturing screen dumps. We define TERM=vt220 in the regress setup script so this can more readily be adapted rather than hardcoding it in the newterm() call. ----------------------------------------------- commit 192c3791a14d64b25b81cee0c8139f442d7a67f1 (main) from: Mark Jamsek date: Tue Apr 18 02:26:42 2023 UTC tog regress: zap needless defines and use "vt220" TERM Also, overwrite - and | unconditionally when capturing screen dumps. Tweaks suggested by naddy@ M regress/tog/common.sh | 1+ 0- M tog/tog.c | 7+ 10- 2 files changed, 8 insertions(+), 10 deletions(-) diff 989289cabaf1c0a5d169b5bc196d19d68cad2bf9 192c3791a14d64b25b81cee0c8139f442d7a67f1 commit - 989289cabaf1c0a5d169b5bc196d19d68cad2bf9 commit + 192c3791a14d64b25b81cee0c8139f442d7a67f1 blob - e03be4c0dcc342c5c4bee7432c3f25219682d085 blob + ecee2678155e9ca794a20e40f84cf1d8c0be6de1 --- regress/tog/common.sh +++ regress/tog/common.sh @@ -19,6 +19,7 @@ export LC_ALL=C.UTF-8 unset TOG_VIEW_SPLIT_MODE unset LC_ALL +export TERM=vt220 export LC_ALL=C.UTF-8 export COLUMNS=80 export LINES=24 blob - 0237a28c7228d9cc3bca3f728f24c3a608686ebf blob + 95caf80a266562fd16c873f6b4b9d1e4bf245efb --- tog/tog.c +++ tog/tog.c @@ -622,9 +622,7 @@ static int using_mock_io; } tog_io; static int using_mock_io; -#define TOG_SCREEN_DUMP "SCREENDUMP" -#define TOG_SCREEN_DUMP_LEN (sizeof(TOG_SCREEN_DUMP) - 1) -#define TOG_KEY_SCRDUMP SHRT_MIN +#define TOG_KEY_SCRDUMP SHRT_MIN /* * We implement two types of views: parent views and child views. @@ -1450,9 +1448,9 @@ strip_trailing_ws(char *str, int *n) /* * Extract visible substring of line y from the curses screen - * and strip trailing whitespace. If vline is set and locale is - * UTF-8, overwrite line[vline] with '|' because the ACS_VLINE - * character is written out as 'x'. Write the line to file f. + * and strip trailing whitespace. If vline is set, overwrite + * line[vline] with '|' because the ACS_VLINE character is + * written out as 'x'. Write the line to file f. */ static const struct got_error * view_write_line(FILE *f, int y, int vline) @@ -1471,7 +1469,7 @@ view_write_line(FILE *f, int y, int vline) */ strip_trailing_ws(line, &r); - if (vline > 0 && got_locale_is_utf8()) + if (vline > 0) line[vline] = '|'; w = fprintf(f, "%s\n", line); @@ -1521,7 +1519,7 @@ screendump(struct tog_view *view) view->child->begin_y : view->begin_y; for (i = 0; i < view->lines; i++) { - if (hline && got_locale_is_utf8() && i == hline - 1) { + if (hline && i == hline - 1) { int c; /* ACS_HLINE writes out as 'q', overwrite it */ @@ -1655,7 +1653,7 @@ tog_read_script_key(FILE *script, int *ch, int *done) *ch = KEY_DOWN; else if (strncasecmp(line, "KEY_UP", 6) == 0) *ch = KEY_UP; - else if (strncasecmp(line, TOG_SCREEN_DUMP, TOG_SCREEN_DUMP_LEN) == 0) + else if (strncasecmp(line, "SCREENDUMP", 10) == 0) *ch = TOG_KEY_SCRDUMP; else *ch = *line; @@ -4205,7 +4203,6 @@ init_mock_term(const char *test_script_path) goto done; } - /* use local TERM so we test in different environments */ if (newterm(NULL, tog_io.cout, tog_io.cin) == NULL) err = got_error_msg(GOT_ERR_IO, "newterm: failed to initialise curses"); -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68