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 <mark@jamsek.dev>
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 <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68