From 05276bda5cbc7fbacb71cf3b32997a9a62efbfd7 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 19 Apr 2023 14:20:58 +0100 Subject: [PATCH] Make backspace take account of LATTR_WRAPPED2. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suppose an application tries to print a double-width character starting in the rightmost column of the screen, so that we apply our emergency fix of wrapping to the next line immediately and printing the character in the first two columns. Suppose they then backspace twice, taking the cursor to the RHS and then the LHS of that character. What should happen if they backspace a third time? Our previous behaviour was to completely ignore the unusual situation, and do the same thing we'd do in any other backspace from column 0: anti-wrap the cursor to the last column of the previous line, leaving it in the empty character cell that was skipped when the DW char couldn't be printed in it. But I think this isn't the best response, because it breaks the invariant that printing N columns' worth of graphic characters and then backspacing N times should leave the cursor on the first of those characters. If I print "a가" (for example) and then backspace three times, I want the cursor on the a, _even_ if weird line wrapping behaviour happened somewhere in that sequence. (Rationale: this helps naïve terminal applications which don't even know what the terminal width is, and aren't tracking their absolute x position. In particular, the simplistic line-based input systems that appear in OS kernels and our own lineedit.c will want to emit a fixed number of backspace-space-backspace sequences to delete characters previously entered on to the line by the user. They still need to check the wcwidth of the characters they're emitting, so that they can BSB twice for a DW character or 0 times for a combining one, but it would be *hugely* more awkward for them to ask the terminal where the cursor is so that they can take account of difficult line wraps!) We already have the ability to _recognise_ this situation: on a line that was wrapped in this unusual way, we set the LATTR_WRAPPED2 line attribute flag, to prevent the empty rightmost column from injecting an unwanted space into copy-pastes from the terminal. Now we also use the same flag to cause the backspace control character to do something interesting. This was the fix that inspired me to start writing test_terminal, because I knew it was touching a delicate area. However, in the course of writing this fix and its tests, I encountered two (!) further bugs, which I'll fix in followup commits! (cherry picked from commit 9ba742ad9f44ffddf053b77064e4b2694b2a1a37) --- terminal/terminal.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/terminal/terminal.c b/terminal/terminal.c index 34a8b2f2..f1e8c7c0 100644 --- a/terminal/terminal.c +++ b/terminal/terminal.c @@ -3965,14 +3965,35 @@ static void term_out(Terminal *term, bool called_from_term_data) break; } case '\b': /* BS: Back space */ - if (term->curs.x == 0 && (term->curs.y == 0 || !term->wrap)) - /* do nothing */ ; - else if (term->curs.x == 0 && term->curs.y > 0) + if (term->curs.x == 0 && (term->curs.y == 0 || !term->wrap)) { + /* do nothing */ + } else if (term->curs.x == 0 && term->curs.y > 0) { term->curs.x = term->cols - 1, term->curs.y--; - else if (term->wrapnext) + + /* + * If the line we've just wrapped back on to had the + * LATTR_WRAPPED2 flag set, it means that the line wrapped + * because a double-width character was printed with the + * cursor in the rightmost column, and the best handling + * available was to leave that column empty and move the + * whole character to the next line. In that situation, + * backspacing needs to put the cursor on the previous + * _logical_ character, i.e. skip the empty space left by + * the wrapping. This arranges that if an application + * unaware of the terminal width or cursor position prints + * a number of printing characters and then tries to return + * to a particular one of them by emitting the right number + * of backspaces, it's still the right number even if a + * line break appeared in a maximally awkward position. + */ + termline *ldata = scrlineptr(term->curs.y); + if (term->curs.x > 0 && (ldata->lattr & LATTR_WRAPPED2)) + term->curs.x--; + } else if (term->wrapnext) { term->wrapnext = false; - else + } else { term->curs.x--; + } seen_disp_event(term); break; case '\016': /* LS1: Locking-shift one */