Repository navigation
Terminal: back color erase, and reset the pending wrap on cursor moves - #2900
Conversation
2e872b3 to
32edcf6
Compare
32edcf6 to
f9c9d44
Compare
|
Rebased on master (with #2894 in), one commit, f9c9d44; all terminal tests pass. Merging with #2894 turned up one more case, fixed here: erasing half of a wide character left the other half blank in the terminal's own colour. Both cells of a wide character are now erased in the current background, as the rest of the range, so the expectations of three wide character tests in VT100EmulatorBackendTest change from a space to an empty cell, and testEraseHalfOfWideKeepsBackground covers it. How to test, in a runtime Eclipse with this org.eclipse.terminal.control, Terminal view, local shell:
Unit tests: VT100EmulatorBackendTest (testEraseKeepsBackground, testEraseHalfOfWideKeepsBackground, testPendingWrapForgottenByCursorMove). |
3d80b66 to
7a94d12
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The change introduces quadratic erasure work with per-cell snapshot notifications and causes full reset to retain a non-default background instead of restoring the default.
2 open issues
What changed in this PR
This PR brings the terminal emulator closer to xterm behavior by preserving the current background during erasure and clearing pending wraps when the cursor moves.
Changes:
- Applies background-aware erasure to erase operations and cells exposed by character insertion or deletion.
- Clears pending-wrap state on vertical cursor movement, line feeds, and erasure.
- Adds regression tests for background preservation, wide-character erasure, and pending-wrap handling.
| File | Description |
|---|---|
| terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackendTest.java | Adds background and wrap tests; updates wide-character erase expectations. |
| terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackend.java | Centralizes background-aware erasure and resets pending wraps on movement. |
Review effort: Balanced
akurtakov
left a comment
There was a problem hiding this comment.
Thanks, back color erase is a real gap and the pending wrap reset on line moves matches DEC STD 070. Two things need fixing before this can go in; both are in Copilot's threads, with details below. After that there's a question about scrolling and a small nit, neither blocking.
7a94d12 to
dbabe3e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Whole-line erasure with the default background leaves deferred wrapping pending, causing subsequent text to move to the next row and incorrectly marking the erased row as wrapped.
2 open issues
1 resolved since last review
Review effort: Balanced
…on any cursor move Two places where the emulator departed from what programs written for xterm expect. Back color erase. Erasing wrote cells with no style, so they took the terminal's own background. Every colour terminal (xterm's "bce") erases in the background currently set, and programs rely on it: a run of a line is coloured by setting a background and erasing to the end of it, which is how a status line, a selection highlight or a table cell gets painted wider than its text. Without it such a run kept the terminal colour from its last character on. EL, ED and ECH now erase in the current background, and so do the cells that ICH opens and DCH uncovers, as in xterm; only the background carries over, not bold or underline. Erasing either half of a wide character erases both of its cells in that background, where it used to blank the other half with a space in the terminal's own colour; so does what is left of one that ICH or DCH splits. A full reset (ESC c) resets the style before it erases, so it leaves the default background. In the default background the cells are left unwritten, as before, so clearing costs no more than it did, and a range is filled in one call per line. Only erasing is covered here. The lines that scrolling brings in (a line feed at the bottom margin, IND, RI, IL, DL, SU, SD) still come in the terminal's own colour; xterm fills those with the background too, which is left for a follow-up. Pending wrap. With deferred wrapping (DECAWM as xterm does it) a character written to the last column leaves the cursor there with a wrap pending. DEC STD 070 lists what clears that state: CR, LF, IND, RI, NEL, the cursor movement sequences, and the erase, insert and delete sequences. The emulator cleared it only on a column move, so a line feed or a cursor-down after a full row still wrapped the next character a row further down and marked an unrelated row as wrapped - which showed up as unrelated rows joined when copying from a full screen program. Tests cover the backgrounds of each erase, of ICH/DCH, of half a wide character and after ESC c, and the reset of the pending wrap by LF, cursor line moves and ECH.
dbabe3e to
81c9981
Compare
|
@akurtakov all review points are addressed in 81c9981, including Copilot's follow-up (erasing a whole line in default colours also forgets a pending wrap). Filling the lines that scrolling brings in is left for a follow-up, as the commit message says. Could you take another look? |
akurtakov
left a comment
There was a problem hiding this comment.
Thanks. It's nice to not see visual glitches in midnight commander.

Two places where the emulator departed from what programs written for xterm expect.
Back color erase. Erasing wrote cells with no style, so they took the terminal's own background. Every colour terminal (xterm's "bce") erases in the background currently set, and programs rely on it: a run of a line is coloured by setting a background and erasing to the end of it, which is how a status line, a selection highlight or a table cell gets painted wider than its text. Without it such a run kept the terminal colour from its last character on. EL, ED and ECH now erase in the current background, and so do the cells that ICH opens and DCH uncovers, as in xterm; only the background carries over, not bold or underline.
Pending wrap. With deferred wrapping (DECAWM as xterm does it) a character written to the last column leaves the cursor there with a wrap pending. DEC STD 070 lists what clears that state: CR, LF, IND, RI, NEL, the cursor movement sequences, and the erase, insert and delete sequences. The emulator cleared it only on a column move, so a line feed or a cursor-down after a full row still wrapped the next character a row further down and marked an unrelated row as wrapped - which showed up as unrelated rows joined when copying from a full screen program.
Tests cover the backgrounds of each erase and of ICH/DCH, and the reset of the pending wrap by LF, cursor line moves and ECH.