1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00

Make backspace take account of LATTR_WRAPPED2.

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 9ba742ad9f)
This commit is contained in:
Simon Tatham 2023-04-19 14:20:58 +01:00
parent 407bf88a95
commit 05276bda5c

View File

@ -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 */