From cc132791d7aad58e6d69d74d048e04687004bb17 Mon Sep 17 00:00:00 2001 From: John Campion Jr Date: Tue, 1 Sep 2026 21:29:07 -0400 Subject: [PATCH] Keep a line's double-size attribute when only its text is erased EraseLineCells reset LineAttribute to Normal on any whole-line erase. That reset exists for ED -- vttest erases the display between screens, and an attribute carried across it doubled whatever the next screen wrote -- but it also caught EL and ECH, which erase a line without ending it. The attribute is a property of the LINE, not of the text on it, and DEC keeps it across both. vttest's double-size test (menu 4) is built to catch exactly this. Row 14 is the only line on that screen carrying an EL, and it arrives after the DECDHL and before the text: ESC[14;2H ESC#6 ESC#5 ESC#4 ESC#3 ESC[2K, then the line. Dropping the attribute there drew row 14 at normal size and left the double-height bottom half on row 15 with no top half above it. Make the reset opt-in with resetLineAttribute, passed only from the three ED loops and from EraseWholeScreen (DECCOLM's clear). EL 0/1/2 and ECH now leave the attribute standing. Tests cover both halves of the pair. The ED reset had no test of its own, which is what let a narrowing like this look free. Co-Authored-By: Claude Opus 5 --- src/XTerm.NET.Tests/VtTestBehaviourTests.cs | 51 ++++++++++++++++++++- src/XTerm.NET/InputHandler.Csi.cs | 11 +++-- src/XTerm.NET/InputHandler.Protection.cs | 29 ++++++++---- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/src/XTerm.NET.Tests/VtTestBehaviourTests.cs b/src/XTerm.NET.Tests/VtTestBehaviourTests.cs index f28afb5..440dcf3 100644 --- a/src/XTerm.NET.Tests/VtTestBehaviourTests.cs +++ b/src/XTerm.NET.Tests/VtTestBehaviourTests.cs @@ -1,4 +1,4 @@ -using XTerm.Options; +using XTerm.Options; namespace XTerm.Tests; @@ -370,6 +370,55 @@ public void A_full_erase_keeps_the_width_of_a_line_whose_text_survived() Assert.Equal(XTerm.Buffer.LineAttribute.DoubleWidth, terminal.Buffer.Lines[0]!.LineAttribute); } + /// + /// Erasing a line does not erase its DECDWL/DECDHL attribute. vttest menu 4. + /// + /// + /// The attribute belongs to the LINE, not to the text sitting on it, so clearing the text + /// does not end it. vttest's double-size screen is built to catch a terminal that thinks + /// otherwise: row 14 is the only line on that screen carrying an EL, and it arrives AFTER + /// the DECDHL and BEFORE the text -- ESC[14;2H ESC#6 ESC#5 ESC#4 ESC#3 ESC[2K, then the + /// line. Dropping the attribute there drew row 14 at normal size and left the double-height + /// bottom half on row 15 with no top half above it, which is what the screen looked like. + /// + /// ECH is the same case reached by a different sequence, and is here for the same reason: + /// at column 0 with a count covering the line it erases just as much as EL 2 does. + /// + [Theory] + [InlineData("[2K")] // EL, whole line + [InlineData("[0K")] // EL, to the right -- from column 0, the whole line + [InlineData("[1K")] // EL, to the left + [InlineData("[80X")] // ECH, wider than the line + public void Erasing_a_line_keeps_the_line_attribute(string erase) + { + var terminal = Sized(20, 3); + + terminal.Write($"{Esc}[1;1H{Esc}#3{Esc}{erase}another such line"); + + Assert.Equal("another such line", terminal.GetLine(0)); + Assert.Equal(XTerm.Buffer.LineAttribute.DoubleHeightTop, terminal.Buffer.Lines[0]!.LineAttribute); + } + + /// + /// Erasing the DISPLAY does take the line attribute with it. + /// + /// + /// The other half of the pair above, and the reason the reset exists at all: vttest erases + /// the display between screens, so an attribute carried across ED doubled whatever the next + /// screen wrote. Pinned here because the EL fix works by narrowing the reset to ED, and a + /// narrowing is exactly the kind of change that overshoots. + /// + [Fact] + public void Erasing_the_display_clears_the_line_attribute() + { + var terminal = Sized(20, 3); + + terminal.Write($"{Esc}[1;1H{Esc}#3doubled"); + terminal.Write($"{Esc}[2J"); + + Assert.Equal(XTerm.Buffer.LineAttribute.Normal, terminal.Buffer.Lines[0]!.LineAttribute); + } + /// DECSCPP declines a width it does not define rather than rounding to one. [Theory] [InlineData(81)] diff --git a/src/XTerm.NET/InputHandler.Csi.cs b/src/XTerm.NET/InputHandler.Csi.cs index 13994fe..1e15eb9 100644 --- a/src/XTerm.NET/InputHandler.Csi.cs +++ b/src/XTerm.NET/InputHandler.Csi.cs @@ -1,4 +1,4 @@ -using System.Globalization; +using System.Globalization; using System.Runtime.CompilerServices; using System.Text; using XTerm.Buffer; @@ -386,7 +386,8 @@ private void EraseInDisplay(Params parameters, bool selective = false) EraseInLine(parameters, selective); // Current line from cursor for (int i = _buffer.Y + 1; i < _terminal.Rows; i++) { - EraseLineCells(_buffer.Lines[_buffer.YBase + i], 0, _terminal.Cols, selective); + EraseLineCells(_buffer.Lines[_buffer.YBase + i], 0, _terminal.Cols, selective, + resetLineAttribute: true); BreakWrapFromAbove(i); if (hasBlocks) EraseBlocksHangingOver(_buffer.YBase + i, 0, _terminal.Cols); @@ -395,7 +396,8 @@ private void EraseInDisplay(Params parameters, bool selective = false) case 1: // Erase above for (int i = 0; i < _buffer.Y; i++) { - EraseLineCells(_buffer.Lines[_buffer.YBase + i], 0, _terminal.Cols, selective); + EraseLineCells(_buffer.Lines[_buffer.YBase + i], 0, _terminal.Cols, selective, + resetLineAttribute: true); BreakWrapFromAbove(i); if (hasBlocks) EraseBlocksHangingOver(_buffer.YBase + i, 0, _terminal.Cols); @@ -406,7 +408,8 @@ private void EraseInDisplay(Params parameters, bool selective = false) case 2: // Erase all — the visible screen only; the scrollback is kept for (int i = 0; i < _terminal.Rows; i++) { - EraseLineCells(_buffer.Lines[_buffer.YBase + i], 0, _terminal.Cols, selective); + EraseLineCells(_buffer.Lines[_buffer.YBase + i], 0, _terminal.Cols, selective, + resetLineAttribute: true); BreakWrapFromAbove(i); if (hasBlocks) EraseBlocksHangingOver(_buffer.YBase + i, 0, _terminal.Cols); diff --git a/src/XTerm.NET/InputHandler.Protection.cs b/src/XTerm.NET/InputHandler.Protection.cs index 6468327..c12e54c 100644 --- a/src/XTerm.NET/InputHandler.Protection.cs +++ b/src/XTerm.NET/InputHandler.Protection.cs @@ -42,7 +42,8 @@ public partial class InputHandler internal void EraseWholeScreen() { for (var row = 0; row < _terminal.Rows; row++) - EraseLineCells(_buffer.Lines[_buffer.YBase + row], 0, _terminal.Cols, selective: false); + EraseLineCells(_buffer.Lines[_buffer.YBase + row], 0, _terminal.Cols, selective: false, + resetLineAttribute: true); } /// DECSCA. 1 protects what is written next; 0 and 2 stop protecting. @@ -73,7 +74,12 @@ internal void StartProtectedArea() /// , honouring whichever protection applies: guarded cells always /// survive, and DECSCA-protected cells survive the SELECTIVE erases. /// - private void EraseLineCells(BufferLine? line, int start, int end, bool selective) + /// + /// Whether an erase that takes the whole line may also take its DECDWL/DECDHL attribute. + /// Only ED asks for this; see the note on wholeLine below. + /// + private void EraseLineCells(BufferLine? line, int start, int end, bool selective, + bool resetLineAttribute = false) { if (line is null) return; @@ -81,18 +87,25 @@ private void EraseLineCells(BufferLine? line, int start, int end, bool selective var blank = BufferCell.Space; blank.Attributes = GetEraseAttributes(); - // A line erased in full goes back to single width. The attribute describes how the - // line is DRAWN, and an erased line has nothing left to draw at double size -- so - // carrying it forward only doubles whatever is written next. vttest's double-size - // test made that visible: it erases the display between screens, so every screen - // after it stayed doubled. + // A line erased in full by ED goes back to single width. The attribute describes how + // the line is DRAWN, and a line ED has cleared has nothing left to draw at double size + // -- so carrying it forward only doubles whatever is written next. vttest's double-size + // test made that visible: it erases the display between screens, so every screen after + // it stayed doubled. + // + // ED ONLY, which is what resetLineAttribute carries in. EL and ECH erase a line without + // ending it, and the attribute is a property of the line rather than of the text on it: + // DEC keeps it across both, and vttest's double-size test is built to catch a terminal + // that does not. It sets DECDHL on row 14, sends EL 2, and only then writes the text -- + // so a terminal that dropped the attribute here drew that row at normal size, leaving + // the sheared bottom half on row 15 with no top half above it. // // Full and non-selective only, and only when nothing SURVIVED the erase. A partial // erase leaves text that is still meant to be double; a selective erase exists to // leave protected text standing; and under ISO protection a guarded cell survives // even a plain erase, which is why this is decided after the walk below rather than // before it -- resizing the line under surviving text is the same mistake in reverse. - var wholeLine = !selective && start == 0 && end >= _terminal.Cols; + var wholeLine = resetLineAttribute && !selective && start == 0 && end >= _terminal.Cols; if (!_protectionUsed || _protectionMode == ProtectionOff) {