From 3c3c17923788d9450f7289abc235d7623252207b Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 10 Aug 2024 10:38:02 +0100 Subject: [PATCH] Don't set term->wrapnext when not in auto-wrapping mode. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user sent a transcript from a curses-based tool 'ncmpc', which carefully disables terminal autowrap when printing a character in the bottom right corner of the display, and then turns it back on again. After that, it expects that sending the backspace character really moves the cursor back a space, instead of clearing the wrapnext flag. But in PuTTY, we set the wrapnext flag even if we're not in wrapping mode - it just doesn't _do_ anything when the next character is sent. But it remains set, and still affects backspace. So the display is corrupted by this change of expectation. (Specifically, ncmpc is printing a time display [m:ss] in the very bottom right, so it disables wrap in order to print the final ']'. Then the next thing it needs to do is to update the low-order digit of the seconds field, so it sends \b as the simplest way to get to that character. The effect on the display is that the updated seconds digit appears where the ] was, instead of overwriting the old seconds digit.) This is a tradeoff in desirable behaviours. The point of having a backspace operation cancel the wrapnext flag and not actually move the cursor is to preserve the invariant that sending 'x', backspace, 'y' causes the y to overprint the x, even if that happens near the end of the terminal's line length. In non-wrapping mode that invariant was bound to break _eventually_, but with this change, it breaks one character earlier than before. However, I think that's less bad than breaking the expectations of curses-based full-screen applications, especially since the _main_ need for that invariant arises from naïve applications that don't want to have to think about the terminal width at all - and those applications generally run in _wrapping_ mode, where it's possible to continue the invariant across multiple lines in any case. --- terminal/terminal.c | 36 ++++++++++++++++++++++++++---------- test/test_terminal.c | 32 ++++++++++++++------------------ 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/terminal/terminal.c b/terminal/terminal.c index 90150044..08df6e59 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -1796,8 +1796,13 @@ void term_reconfig(Terminal *term, Conf *conf) conf_free(term->conf); term->conf = conf_copy(conf); - if (reset_wrap) + if (reset_wrap) { term->alt_wrap = term->wrap = conf_get_bool(term->conf, CONF_wrap_mode); + if (!term->wrap) + term->wrapnext = false; + if (!term->alt_wrap) + term->alt_wnext = false; + } if (reset_decom) term->alt_om = term->dec_om = conf_get_bool(term->conf, CONF_dec_om); if (reset_bce) { @@ -3093,6 +3098,8 @@ static void toggle_mode(Terminal *term, int mode, int query, bool state) break; case 7: /* DECAWM: auto wrap */ term->wrap = state; + if (!term->wrap) + term->wrapnext = false; break; case 8: /* DECARM: auto key repeat */ term->repeat_off = !state; @@ -3425,15 +3432,23 @@ static void term_display_graphic_char(Terminal *term, unsigned long c) term->curs.x++; if (term->curs.x >= linecols) { term->curs.x = linecols - 1; - term->wrapnext = true; - if (term->wrap && term->vt52_mode) { - cline->lattr |= LATTR_WRAPPED; - if (term->curs.y == term->marg_b) - scroll(term, term->marg_t, term->marg_b, 1, true); - else if (term->curs.y < term->rows - 1) - term->curs.y++; - term->curs.x = 0; - term->wrapnext = false; + + if (term->wrap) { + if (!term->vt52_mode) { + /* Set the wrapnext flag, so that the next character + * wraps, but this one doesn't. */ + term->wrapnext = true; + } else { + /* VT52 mode expects simpler handling, and we just + * wrap straight away. */ + cline->lattr |= LATTR_WRAPPED; + if (term->curs.y == term->marg_b) + scroll(term, term->marg_t, term->marg_b, 1, true); + else if (term->curs.y < term->rows - 1) + term->curs.y++; + term->curs.x = 0; + term->wrapnext = false; + } } } seen_disp_event(term); @@ -5655,6 +5670,7 @@ static void term_out(Terminal *term, bool called_from_term_data) case 'w': /* Autowrap off */ /* compatibility(ATARI) */ term->wrap = false; + term->wrapnext = false; break; case 'R': diff --git a/test/test_terminal.c b/test/test_terminal.c index 9fdca6cc..5ebc1204 100644 --- a/test/test_terminal.c +++ b/test/test_terminal.c @@ -384,7 +384,8 @@ static void test_wrap(Mock *mk) static void test_nonwrap(Mock *mk) { - /* Test behaviour when printing characters hit end of line without wrap */ + /* Test behaviour when printing characters hit end of line without wrap. + * The wrapnext flag is never set in this mode. */ mk->ucsdata->line_codepage = CP_UTF8; /* Print 'abc' without enough space for the c */ @@ -398,27 +399,22 @@ static void test_nonwrap(Mock *mk) IEQUAL(mk->term->curs.y, 0); IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_termchar(mk->term, 78, 0).chr, CSET_ASCII | 'a'); - /* The 'b' prints, leaving the cursor where it is with wrapnext set */ + /* The 'b' prints, leaving the cursor where it is */ term_datapl(mk->term, PTRLEN_LITERAL("b")); IEQUAL(mk->term->curs.x, 79); IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 1); + IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_lineattr(mk->term, 0), 0); IEQUAL(get_termchar(mk->term, 79, 0).chr, CSET_ASCII | 'b'); - /* The 'c' overwrites the b, leaving wrapnext still set */ + /* The 'c' overwrites the b */ term_datapl(mk->term, PTRLEN_LITERAL("c")); IEQUAL(mk->term->curs.x, 79); IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 1); + IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_lineattr(mk->term, 0), 0); IEQUAL(get_termchar(mk->term, 78, 0).chr, CSET_ASCII | 'a'); IEQUAL(get_termchar(mk->term, 79, 0).chr, CSET_ASCII | 'c'); - /* So backspacing clears wrapnext, leaving us on the c */ - term_datapl(mk->term, PTRLEN_LITERAL("\b")); - IEQUAL(mk->term->curs.x, 79); - IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 0); - /* And backspacing again returns the cursor to the a */ + /* Since wrapnext was never set, backspacing returns us to the a */ term_datapl(mk->term, PTRLEN_LITERAL("\b")); IEQUAL(mk->term->curs.x, 78); IEQUAL(mk->term->curs.y, 0); @@ -428,18 +424,18 @@ static void test_nonwrap(Mock *mk) mk->term->curs.x = 78; mk->term->curs.y = 0; mk->term->wrap = false; - /* The DW character goes directly to the wrapnext state */ + /* The DW character occupies the rightmost two columns */ term_datapl(mk->term, PTRLEN_LITERAL("\xEA\xB0\x80")); IEQUAL(mk->term->curs.x, 79); IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 1); + IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_termchar(mk->term, 78, 0).chr, 0xAC00); IEQUAL(get_termchar(mk->term, 79, 0).chr, UCSWIDE); /* The 'c' must overprint the RHS of the DW char, clearing the LHS */ term_datapl(mk->term, PTRLEN_LITERAL("c")); IEQUAL(mk->term->curs.x, 79); IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 1); + IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_lineattr(mk->term, 0), 0); IEQUAL(get_termchar(mk->term, 78, 0).chr, CSET_ASCII | ' '); IEQUAL(get_termchar(mk->term, 79, 0).chr, CSET_ASCII | 'c'); @@ -459,7 +455,7 @@ static void test_nonwrap(Mock *mk) term_datapl(mk->term, PTRLEN_LITERAL("\xEA\xB0\x80")); IEQUAL(mk->term->curs.x, 79); IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 1); + IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_lineattr(mk->term, 0), 0); IEQUAL(get_termchar(mk->term, 78, 0).chr, CSET_ASCII | 'a'); IEQUAL(get_termchar(mk->term, 79, 0).chr, 0xFFFD); @@ -469,18 +465,18 @@ static void test_nonwrap(Mock *mk) mk->term->curs.x = 78; mk->term->curs.y = 0; mk->term->wrap = false; - /* First DW character goes directly to the wrapnext state */ + /* First DW character occupies the rightmost columns */ term_datapl(mk->term, PTRLEN_LITERAL("\xEA\xB0\x80")); IEQUAL(mk->term->curs.x, 79); IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 1); + IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_termchar(mk->term, 78, 0).chr, 0xAC00); IEQUAL(get_termchar(mk->term, 79, 0).chr, UCSWIDE); /* Second DW char becomes U+FFFD, overwriting RHS of the first one */ term_datapl(mk->term, PTRLEN_LITERAL("\xEA\xB0\x81")); IEQUAL(mk->term->curs.x, 79); IEQUAL(mk->term->curs.y, 0); - IEQUAL(mk->term->wrapnext, 1); + IEQUAL(mk->term->wrapnext, 0); IEQUAL(get_lineattr(mk->term, 0), 0); IEQUAL(get_termchar(mk->term, 78, 0).chr, CSET_ASCII | ' '); IEQUAL(get_termchar(mk->term, 79, 0).chr, 0xFFFD);