From b42fa156238f8da8543485a3c3319b532836fc41 Mon Sep 17 00:00:00 2001 From: John Campion Jr Date: Mon, 31 Aug 2026 20:44:09 -0400 Subject: [PATCH 1/3] Give the status line a row to be written to DECSSDT and DECSASD were parsed, stored so DECRQSS could report them, and otherwise ignored -- which was worse than not implementing them at all. A program asked whether its status line had been accepted, was told yes, wrote to it, and the text went wherever the cursor happened to be: into the middle of its own display. vttest's simple status-line test shows it as one row reading TEXT IN THE STATUS LINEThere should be TEXT IN THE STATUS LINE. There is a row now. DECSSDT 2 creates it, DECSASD 1 sends what is written next into it, DECSASD 0 hands the cursor back. The buffer swap is the alternate screen's, for the same reason: the input handler writes through whichever buffer it was given, so redirecting output is a matter of handing it a different one rather than teaching every write about status lines. What the two controls now refuse is as much of the fix as what they accept. DECSASD 1 is declined unless a HOST-WRITABLE status line exists -- type 0 has no row and the indicator's contents belong to the terminal -- and the value stored for DECRQSS is the one that was ACCEPTED, so a program is no longer told yes and then given nowhere to put its text. Each display keeps its own cursor, or a program that writes a status message loses its place mid-screen. Removing the status line hands the cursor back first, or the row stops existing while it still has it. RIS takes the whole thing with everything else it undoes. The row is deliberately not one of Rows. An application told it has N rows must have N rows it can write to, and every size report is computed from that count -- so the status line is exposed as a property for a host to draw below the text area, with a change event shaped like TitleChanged. That event is raised at the BATCH boundary, and the first version of this asked the question on the print path instead. Per CLAUDE.md's first section: a per-character test for a state that is almost never on pays a compare for every character the terminal ever prints, to catch a message that arrives once a second. Being selected is enough to count as changed -- while DECSASD has the status line, every printed character goes into it by definition. 2159 passed, 0 failed. Co-Authored-By: Claude Fable 5 --- src/XTerm.NET.Tests/StatusLineTests.cs | 187 +++++++++++++++++++++++++ src/XTerm.NET/InputHandler.cs | 8 ++ src/XTerm.NET/Terminal.cs | 159 +++++++++++++++++++++ 3 files changed, 354 insertions(+) create mode 100644 src/XTerm.NET.Tests/StatusLineTests.cs diff --git a/src/XTerm.NET.Tests/StatusLineTests.cs b/src/XTerm.NET.Tests/StatusLineTests.cs new file mode 100644 index 0000000..7815299 --- /dev/null +++ b/src/XTerm.NET.Tests/StatusLineTests.cs @@ -0,0 +1,187 @@ +using XTerm.Options; + +namespace XTerm.Tests; + +/// +/// The DEC status line: an extra row a program selects with DECSSDT and writes to with DECSASD. +/// +/// +/// Both controls used to be parsed, stored for DECRQSS, and otherwise ignored — which was +/// worse than not implementing them. DECRQSS answered with the stored value, so a program asking +/// whether its status line had been accepted was told yes, wrote to it, and the text went wherever +/// the cursor happened to be: into the middle of the application's own display. vttest's simple +/// status-line test shows it as TEXT IN THE STATUS LINEThere should be TEXT IN THE STATUS LINE +/// on a single row. +/// The row is deliberately NOT one of the terminal's Rows. An application told it has +/// N rows must have N rows it can write to, and every size report is computed from that count. +/// +public class StatusLineTests +{ + private static Terminal Fresh() => new(new TerminalOptions { Cols = 40, Rows = 5 }); + + private static string Csi(string body) => "\u001b[" + body; + + /// DECSSDT: 0 none, 1 indicator, 2 host-writable. + private static string Type(int t) => Csi($"{t}$~"); + + /// DECSASD: 0 main display, 1 status line. + private static string Select(int d) => Csi($"{d}$}}"); + + private static string Row(Terminal t, int row) => + (t.Buffer.GetLine(t.Buffer.ViewportY + row)?.TranslateToString(true) ?? "").TrimEnd(); + + private static string StatusText(Terminal t) => + (t.StatusLine?.TranslateToString(true) ?? "").TrimEnd(); + + [Fact] + public void There_is_no_status_line_until_a_program_asks_for_one() + { + var terminal = Fresh(); + + Assert.Null(terminal.StatusLine); + Assert.Equal(0, terminal.StatusDisplayType); + Assert.False(terminal.StatusLineActive); + } + + [Fact] + public void Text_written_to_the_status_line_stays_out_of_the_display() + { + // vttest's own sequence, and the bug it exposed: the capitals belong on a row of their own. + var terminal = Fresh(); + + terminal.Write("There should be TEXT IN THE STATUS LINE"); + terminal.Write(Type(2)); + terminal.Write(Select(1)); + terminal.Write("TEXT IN THE STATUS LINE"); + terminal.Write(Select(0)); + + Assert.Equal("There should be TEXT IN THE STATUS LINE", Row(terminal, 0)); + Assert.Equal("TEXT IN THE STATUS LINE", StatusText(terminal)); + } + + [Fact] + public void The_cursor_comes_back_to_where_the_program_left_it() + { + // Each display has its own cursor. Losing the application's is how a program that writes a + // status message finds itself continuing in the wrong place. + var terminal = Fresh(); + + terminal.Write("abc"); + var x = terminal.Buffer.X; + var y = terminal.Buffer.Y; + + terminal.Write(Type(2) + Select(1) + "status" + Select(0)); + + Assert.Equal(x, terminal.Buffer.X); + Assert.Equal(y, terminal.Buffer.Y); + + terminal.Write("def"); + Assert.Equal("abcdef", Row(terminal, 0)); + } + + [Fact] + public void Selecting_the_status_line_is_refused_when_there_is_not_one() + { + // The failure this control exists to stop. Honouring the selection with no row to write to + // puts the text in the display; refusing keeps the program's own screen intact. + var terminal = Fresh(); + + terminal.Write("intact"); + terminal.Write(Select(1)); + + Assert.False(terminal.StatusLineActive); + + terminal.Write("!"); + Assert.Equal("intact!", Row(terminal, 0)); + } + + [Fact] + public void The_indicator_type_is_not_writable_by_the_program() + { + // Type 1 is the terminal's own indicator; its contents are not the application's to set. + var terminal = Fresh(); + + terminal.Write("intact"); + terminal.Write(Type(1)); + terminal.Write(Select(1)); + + Assert.False(terminal.StatusLineActive); + Assert.NotNull(terminal.StatusLine); + + terminal.Write("!"); + Assert.Equal("intact!", Row(terminal, 0)); + } + + [Fact] + public void Removing_the_status_line_hands_the_cursor_back_first() + { + // Otherwise the row stops existing while it still has the cursor, and everything written + // afterwards goes nowhere with no way to recover. + var terminal = Fresh(); + + terminal.Write("abc" + Type(2) + Select(1) + "status"); + Assert.True(terminal.StatusLineActive); + + terminal.Write(Type(0)); + + Assert.False(terminal.StatusLineActive); + Assert.Null(terminal.StatusLine); + + terminal.Write("def"); + Assert.Equal("abcdef", Row(terminal, 0)); + } + + [Fact] + public void The_status_row_is_not_one_of_the_terminals_rows() + { + var terminal = Fresh(); + var rows = terminal.Rows; + + terminal.Write(Type(2)); + + Assert.Equal(rows, terminal.Rows); + } + + [Fact] + public void The_row_follows_the_screens_width() + { + var terminal = Fresh(); + terminal.Write(Type(2)); + + terminal.Resize(100, 5); + + Assert.Equal(100, terminal.StatusLine!.Length); + } + + [Fact] + public void A_change_is_announced_once_per_batch() + { + // A status line is written a character at a time like anything else; a host that repaints + // on the event must not repaint once per character of one message. + var terminal = Fresh(); + terminal.Write(Type(2) + Select(1)); + + var changes = 0; + terminal.StatusLineChanged += (_, _) => changes++; + + terminal.Write("hello"); + + Assert.Equal(1, changes); + } + + [Fact] + public void RIS_takes_the_status_line_with_everything_else() + { + var terminal = Fresh(); + terminal.Write(Type(2) + Select(1) + "status"); + + // \u001b, not \x1b: a \x escape is VARIABLE length, so "\x1bc" is the single + // character U+01BC rather than ESC followed by c. The sequence gets printed instead + // of dispatched, and the assertion then fails for a reason that is not the code. + terminal.Write("\u001bc"); + + Assert.Null(terminal.StatusLine); + Assert.Equal(0, terminal.StatusDisplayType); + Assert.False(terminal.StatusLineActive); + } +} diff --git a/src/XTerm.NET/InputHandler.cs b/src/XTerm.NET/InputHandler.cs index 12bda2c..43e1d08 100644 --- a/src/XTerm.NET/InputHandler.cs +++ b/src/XTerm.NET/InputHandler.cs @@ -757,11 +757,19 @@ public void HandleCsi(string identifier, Params parameters) break; case CsiCommand.SelectActiveStatusDisplay: + // Stored for DECRQSS as before, and now acted on. Storing the ACCEPTED value + // rather than the requested one is the point: DECSASD 1 is refused unless a + // host-writable status line exists, and reporting back a selection that was + // refused is what told a program its text had somewhere to go when it did not. _activeStatusDisplay = parameters.GetParam(0, 0); + _terminal.SetActiveStatusDisplay(_activeStatusDisplay); + _activeStatusDisplay = _terminal.StatusLineActive ? 1 : 0; break; case CsiCommand.SelectStatusDisplayType: _statusDisplayType = parameters.GetParam(0, 0); + _terminal.SetStatusDisplayType(_statusDisplayType); + _statusDisplayType = _terminal.StatusDisplayType; break; case CsiCommand.RequestTerminalParameters: diff --git a/src/XTerm.NET/Terminal.cs b/src/XTerm.NET/Terminal.cs index ac05504..2465c66 100644 --- a/src/XTerm.NET/Terminal.cs +++ b/src/XTerm.NET/Terminal.cs @@ -63,6 +63,152 @@ public class Terminal : IDisposable /// DECCOLM's current answer: whether the screen is in 132-column mode. public bool ColumnMode132 { get; private set; } + // ---------------------------------------------------------------- DEC status line + + /// The status line's single row, or when there is not one. + /// + /// Present only while DECSSDT has selected a type that HAS a row -- host-writable (2), and + /// the indicator (1), whose contents the terminal would own. Null for type 0, which is the + /// default and means the display has no status line at all. + /// A host draws this below the text area. It is deliberately not one of the terminal's + /// : an application told it has N rows must have N rows it can write to, and + /// every size report is computed from that count. + /// + public Buffer.BufferLine? StatusLine => _statusDisplayType == 0 ? null : _statusBuffer?.GetLine(0); + + /// + /// DECSSDT's current answer: 0 none, 1 indicator, 2 host-writable. + /// + public int StatusDisplayType => _statusDisplayType; + + /// + /// DECSASD's current answer: whether writes are going to the status line rather than the screen. + /// + public bool StatusLineActive => _statusLineActive; + + /// Raised when the status line's contents, type or cursor change. + /// + /// Shaped like because a host consumes it the same way: something + /// small changed that only the host can show. Raised on the write path, so a host that repaints + /// on it repaints once per batch rather than per character. + /// + public event EventHandler? StatusLineChanged; + + private Buffer.TerminalBuffer? _statusBuffer; + private int _statusDisplayType; + private bool _statusLineActive; + + // Where the cursor was on the ordinary display while the status line has it. DEC gives each + // display its own cursor: DECSASD 1 must not move the application's, and DECSASD 0 must put it + // back exactly, or a program that writes a status message loses its place mid-screen. + private int _cursorXBeforeStatus, _cursorYBeforeStatus; + + internal void RaiseStatusLineChanged() => StatusLineChanged?.Invoke(this, EventArgs.Empty); + + private bool _statusLineDirty; + + /// Announces a status-line change, at most once for the batch just parsed. + /// + /// Asked HERE rather than on the print path, which is the hottest code in the parser: a + /// per-character test for a state that is almost never on would pay a compare for every + /// character of ordinary output to catch a status message that arrives once a second. The + /// batch boundary asks the same question once. + /// Being selected is enough to count as changed. While DECSASD has the status line, every + /// printed character goes into it by definition -- so a batch parsed in that state either wrote + /// to the row or was the batch that selected it, and both are things a host wants to repaint + /// for. The cost of not tracking it precisely is a repaint of an unchanged row; the cost of + /// tracking it precisely is paid by every character the terminal ever prints. + /// + private void FlushStatusLineChange() + { + if (!_statusLineDirty && !_statusLineActive) + return; + + _statusLineDirty = false; + RaiseStatusLineChanged(); + } + + /// + /// DECSSDT ($ ~). Chooses whether there is a status line and who owns it. + /// + /// + /// Type 0 removes it. Leaving the status line SELECTED while removing its row would leave + /// writes going nowhere with no way back, so this returns the cursor to the main display first + /// -- the same care DECSASD 0 takes. + /// Type 1 is the indicator: the terminal owns the contents, not the application. The row + /// exists so a host can show something there, and writes are not routed to it. + /// + public void SetStatusDisplayType(int type) + { + if (type is < 0 or > 2) + return; + + if (type == _statusDisplayType) + return; + + if (_statusLineActive) + SetActiveStatusDisplay(0); + + _statusDisplayType = type; + + if (type == 0) + { + _statusBuffer = null; + } + else + { + _statusBuffer ??= new Buffer.TerminalBuffer(Cols, 1, 0, hasScrollback: false); + _statusBuffer.Resize(Cols, 1); + } + + RaiseStatusLineChanged(); + } + + /// + /// DECSASD ($ }). Sends what is written next to the status line (1) or the main display (0). + /// + /// + /// Refused unless DECSSDT has selected the HOST-WRITABLE type. Type 0 has no row, and the + /// indicator's contents belong to the terminal -- honouring the selection for either would put + /// the application's text somewhere it can never be seen, which is the failure this whole + /// control exists to stop: text meant for a status line landing in the middle of the screen. + /// The buffer swap is the alternate screen's, for the same reason: the input handler + /// writes through whichever buffer it was given, so redirecting output is a matter of giving it + /// a different one rather than teaching every write about status lines. + /// + public void SetActiveStatusDisplay(int display) + { + var wantStatus = display == 1; + + if (wantStatus && _statusDisplayType != 2) + return; + + if (wantStatus == _statusLineActive) + return; + + if (wantStatus) + { + _cursorXBeforeStatus = _buffer.X; + _cursorYBeforeStatus = _buffer.Y; + + _statusBuffer ??= new Buffer.TerminalBuffer(Cols, 1, 0, hasScrollback: false); + _statusBuffer.Resize(Cols, 1); + _statusLineActive = true; + _buffer = _statusBuffer; + _inputHandler.SetBuffer(_buffer); + _buffer.SetCursor(0, 0); + } + else + { + _statusLineActive = false; + _buffer = _usingAltBuffer ? _altBuffer! : _normalBuffer!; + _inputHandler.SetBuffer(_buffer); + _buffer.SetCursor(_cursorXBeforeStatus, _cursorYBeforeStatus); + } + + RaiseStatusLineChanged(); + } + /// /// DECSET/DECRST 3. Switching clears the screen, homes the cursor and resets the margins -- /// the DEC behaviour programs rely on for a clean slate -- and resizes the grid to 132 or 80 @@ -690,6 +836,7 @@ public void Write(string data) return; _parser.Parse(data); + FlushStatusLineChange(); } /// @@ -707,6 +854,7 @@ public void Write(ReadOnlySpan data) return; _parser.Parse(data); + FlushStatusLineChange(); } /// @@ -824,6 +972,11 @@ public void Resize(int cols, int rows) _normalBuffer?.Resize(cols, rows); _altBuffer?.Resize(cols, rows); + // One row always, only ever as wide as the screen. It is not part of the grid, so the row + // count never reaches it -- but a status line narrower than the display would clip its own + // text after a widen, and wider would hold text nothing can show. + _statusBuffer?.Resize(cols, 1); + Resized?.Invoke(this, new TerminalEvents.ResizeEventArgs(cols, rows)); // After the host has been told, not before. The spec requires the report to follow the @@ -938,6 +1091,12 @@ public void Reset() var shapeBefore = PointerShape; + // The status line goes with everything else RIS undoes: its type, its selection and its + // contents. Returning the cursor to the main display FIRST, so the reset below is applied + // to the screen rather than to a row that is about to stop existing. + SetActiveStatusDisplay(0); + SetStatusDisplayType(0); + // Reset to normal buffer if (_usingAltBuffer) { From ee551372952407c08d09f7073a1d57d0c9ae2f70 Mon Sep 17 00:00:00 2001 From: John Campion Jr Date: Mon, 31 Aug 2026 20:56:24 -0400 Subject: [PATCH 2/3] Address the review: cursor, routing, batching, reporting, zero size Six findings, all real, all mine. The batching the comment described was not happening. The dirty flag was never set and both setters raised synchronously, so one write carrying DECSSDT, DECSASD and a message emitted three events -- two from the setters and one from the flush. The setters now record instead of raising while a batch is being parsed, and a host calling the API outside one is told immediately, because outside a batch there is no end to wait for. Selecting the status line homed its cursor every time, which contradicted the per-display cursor the same method documents: a program writing half a message, stepping back to the screen and returning began again at column one and overwrote what it had written. The buffer starts at (0, 0), which is where a first selection finds it, and later selections leave it alone. Saving only X and Y lost the main display's pending wrap. The phantom column past the last cell is a real cursor position and the one the print path reads to decide whether to wrap, and SetCursor clamps it onto the last cell and clears the flag -- so a line about to wrap came back as one that overwrote its own last character. Restored raw, clamped to Cols rather than Cols - 1, which lets SetCursorRaw derive the flag itself. DECSET 1049 while the status line was selected reassigned the write target, sending the program's next characters to the alternate screen while DECRQSS still reported the status line. The review suggested keeping the status row as the target; that is not available, because the switch's own work -- 1049's erase among it -- runs through the input handler's buffer, and a full-screen erase against a one-row buffer indexes off the end of it. Switching screens ends the status line's turn with the cursor instead, so one rule holds: the status line is selected, or the screen is. The row itself survives. DECRQSS read cached copies in the input handler that nothing reset, so after RIS it went on reporting the status line selected and host-writable when both had been undone -- the same lie this PR exists to remove, in a new place. It reads the terminal now, and the cached fields are gone rather than left unused. Restoring the cursor after Resize(0, 0) threw: the clamp is Clamp(x, 0, Cols - 1), whose maximum is negative at zero, and Resize explicitly allows zero because a host reports it before its control has been laid out. Eight tests added, one per finding plus the two halves of the screen switch. 2167 passed, 0 failed. Co-Authored-By: Claude Fable 5 --- src/XTerm.NET.Tests/StatusLineTests.cs | 140 ++++++++++++++++++++++ src/XTerm.NET/InputHandler.Dcs.cs | 7 +- src/XTerm.NET/InputHandler.StoredModes.cs | 5 +- src/XTerm.NET/InputHandler.cs | 8 +- src/XTerm.NET/Terminal.cs | 97 +++++++++++++-- 5 files changed, 238 insertions(+), 19 deletions(-) diff --git a/src/XTerm.NET.Tests/StatusLineTests.cs b/src/XTerm.NET.Tests/StatusLineTests.cs index 7815299..e7d83a4 100644 --- a/src/XTerm.NET.Tests/StatusLineTests.cs +++ b/src/XTerm.NET.Tests/StatusLineTests.cs @@ -184,4 +184,144 @@ public void RIS_takes_the_status_line_with_everything_else() Assert.Equal(0, terminal.StatusDisplayType); Assert.False(terminal.StatusLineActive); } + + // ------------------------------------------------ from the review on the first version + + [Fact] + public void The_status_line_keeps_its_own_cursor_across_selections() + { + // "Each display has its own cursor" has to hold for the SECOND selection too. Homing on + // every DECSASD 1 meant a program writing half a message, stepping back to the screen and + // returning began again at column one and overwrote what it had written. + var terminal = Fresh(); + terminal.Write(Type(2)); + + terminal.Write(Select(1) + "abc" + Select(0)); + terminal.Write(Select(1) + "def" + Select(0)); + + Assert.Equal("abcdef", StatusText(terminal)); + } + + [Fact] + public void The_displays_pending_wrap_survives_a_trip_to_the_status_line() + { + // A cursor at the phantom column past the last cell is a different state from one clamped + // onto that cell. SetCursor clears the flag, so restoring through it turned a line that was + // about to wrap into one that overwrote its own last character. + var terminal = Fresh(); + var width = terminal.Cols; + + terminal.Write(new string('x', width)); + Assert.True(terminal.Buffer.PendingWrap, "sanity: the cursor is at the phantom column"); + + terminal.Write(Type(2) + Select(1) + "status" + Select(0)); + + terminal.Write("Z"); + + Assert.Equal(new string('x', width), Row(terminal, 0)); + Assert.Equal("Z", Row(terminal, 1)); + } + + [Fact] + public void Switching_screens_ends_the_status_lines_turn_with_the_cursor() + { + // The first version reassigned the write target here, so the program's next characters went + // to the alternate screen while DECRQSS still reported the status line selected. Holding + // the status row instead is not available: the switch's own work -- 1049's erase among it + // -- runs through the input handler's buffer, and a full-screen erase against a one-row + // buffer indexes off the end of it. So the selection ends, and one rule holds: the status + // line is selected, or the screen is. + var terminal = Fresh(); + terminal.Write(Type(2) + Select(1) + "message"); + + terminal.Write(Csi("?1049h")); + + Assert.False(terminal.StatusLineActive); + + terminal.Write("on the alternate screen"); + Assert.Equal("on the alternate screen", Row(terminal, 0)); + + // The ROW survives -- only the selection ended. + Assert.Equal("message", StatusText(terminal)); + } + + [Fact] + public void The_status_line_can_be_selected_again_from_the_alternate_screen() + { + var terminal = Fresh(); + terminal.Write(Type(2) + Select(1) + "message" + Select(0)); + terminal.Write(Csi("?1049h")); + + terminal.Write(Select(1) + "!" + Select(0)); + + Assert.Equal("message!", StatusText(terminal)); + + terminal.Write("screen"); + Assert.Equal("screen", Row(terminal, 0)); + } + + [Fact] + public void DECRQSS_stops_reporting_a_status_line_that_RIS_undid() + { + // The controls were cached for the report and nothing reset the cache, so after RIS the + // terminal said the status line was still selected and still host-writable. A program that + // asks is then told yes about a row that no longer exists -- the same lie in a new place. + var terminal = Fresh(); + var replies = new List(); + terminal.DataReceived += (_, e) => replies.Add(e.Data); + + terminal.Write(Type(2) + Select(1)); + terminal.Write("\u001bc"); + + replies.Clear(); + terminal.Write("\u001bP$q$}\u001b\\"); + terminal.Write("\u001bP$q$~\u001b\\"); + + Assert.All(replies, r => Assert.DoesNotContain("1$}", r)); + Assert.All(replies, r => Assert.DoesNotContain("2$~", r)); + } + + [Fact] + public void One_event_for_a_batch_that_selects_and_writes_together() + { + // The claim the first version made in a comment and did not keep: the two setters each + // raised synchronously and the flush raised again, so this emitted three. + var terminal = Fresh(); + + var changes = 0; + terminal.StatusLineChanged += (_, _) => changes++; + + terminal.Write(Type(2) + Select(1) + "hello"); + + Assert.Equal(1, changes); + } + + [Fact] + public void A_host_calling_the_api_directly_is_told_straight_away() + { + // Outside a batch there is no end to wait for. + var terminal = Fresh(); + + var changes = 0; + terminal.StatusLineChanged += (_, _) => changes++; + + terminal.SetStatusDisplayType(2); + + Assert.Equal(1, changes); + } + + [Fact] + public void A_zero_sized_terminal_can_still_leave_the_status_line() + { + // Resize explicitly allows zero -- a host reports it while its control exists but has not + // been laid out. SetCursor's clamp is Clamp(x, 0, Cols - 1), which throws when Cols is 0. + var terminal = Fresh(); + terminal.Write(Type(2) + Select(1)); + + terminal.Resize(0, 0); + + terminal.Write(Select(0)); + + Assert.False(terminal.StatusLineActive); + } } diff --git a/src/XTerm.NET/InputHandler.Dcs.cs b/src/XTerm.NET/InputHandler.Dcs.cs index 8a089ec..27d8052 100644 --- a/src/XTerm.NET/InputHandler.Dcs.cs +++ b/src/XTerm.NET/InputHandler.Dcs.cs @@ -155,8 +155,11 @@ private void HandleDecrqss(string setting) "s" => $"\x1bP1$r{_buffer.ScrollLeft + 1};{_buffer.ScrollRight + 1}s\x1b\\", "t" => $"\x1bP1$r{_terminal.Rows}t\x1b\\", "*x" => $"\x1bP1$r{_attributeChangeExtent}*x\x1b\\", - "$}" => $"\x1bP1$r{_activeStatusDisplay}$}}\x1b\\", - "$~" => $"\x1bP1$r{_statusDisplayType}$~\x1b\\", + // From the TERMINAL, not from the cached copies: those are set when the control is + // parsed and nothing resets them, so after RIS this reported the status line still + // selected and its type still host-writable when both had been undone. + "$}" => $"\x1bP1$r{(_terminal.StatusLineActive ? 1 : 0)}$}}\x1b\\", + "$~" => $"\x1bP1$r{_terminal.StatusDisplayType}$~\x1b\\", "*|" => $"\x1bP1$r{_terminal.Rows}*|\x1b\\", _ => Deny, }; diff --git a/src/XTerm.NET/InputHandler.StoredModes.cs b/src/XTerm.NET/InputHandler.StoredModes.cs index 5ee79cf..4dd0b47 100644 --- a/src/XTerm.NET/InputHandler.StoredModes.cs +++ b/src/XTerm.NET/InputHandler.StoredModes.cs @@ -56,8 +56,9 @@ public partial class InputHandler /// status line to point at and never will. /// private int _attributeChangeExtent; // DECSACE (* x) - private int _activeStatusDisplay; // DECSASD ($ }) - private int _statusDisplayType; // DECSSDT ($ ~) + // DECSASD ($ }) and DECSSDT ($ ~) were cached here for DECRQSS to report. They are the + // terminal's state now, and DECRQSS reads it there -- a second copy is what let RIS undo the + // status line while the report went on describing the one that had been undone. /// Sets or resets a stored mode; false when the mode is not one of the stored set. private bool TrySetStoredMode(int mode, bool isPrivate, bool value) diff --git a/src/XTerm.NET/InputHandler.cs b/src/XTerm.NET/InputHandler.cs index 43e1d08..103ccc7 100644 --- a/src/XTerm.NET/InputHandler.cs +++ b/src/XTerm.NET/InputHandler.cs @@ -761,15 +761,11 @@ public void HandleCsi(string identifier, Params parameters) // rather than the requested one is the point: DECSASD 1 is refused unless a // host-writable status line exists, and reporting back a selection that was // refused is what told a program its text had somewhere to go when it did not. - _activeStatusDisplay = parameters.GetParam(0, 0); - _terminal.SetActiveStatusDisplay(_activeStatusDisplay); - _activeStatusDisplay = _terminal.StatusLineActive ? 1 : 0; + _terminal.SetActiveStatusDisplay(parameters.GetParam(0, 0)); break; case CsiCommand.SelectStatusDisplayType: - _statusDisplayType = parameters.GetParam(0, 0); - _terminal.SetStatusDisplayType(_statusDisplayType); - _statusDisplayType = _terminal.StatusDisplayType; + _terminal.SetStatusDisplayType(parameters.GetParam(0, 0)); break; case CsiCommand.RequestTerminalParameters: diff --git a/src/XTerm.NET/Terminal.cs b/src/XTerm.NET/Terminal.cs index 2465c66..48b5a5f 100644 --- a/src/XTerm.NET/Terminal.cs +++ b/src/XTerm.NET/Terminal.cs @@ -103,9 +103,54 @@ public class Terminal : IDisposable // back exactly, or a program that writes a status message loses its place mid-screen. private int _cursorXBeforeStatus, _cursorYBeforeStatus; + private void EnsureStatusBuffer() + { + _statusBuffer ??= new Buffer.TerminalBuffer(Cols, 1, 0, hasScrollback: false); + _statusBuffer.Resize(Cols, 1); + } + + /// Puts the cursor back on the display the status line borrowed it from. + /// + /// Through the RAW move plus the flag rather than SetCursor, which clamps and clears pending + /// wrap. The guard is for the zero-size buffer Resize explicitly allows: a host reports zero + /// while its control exists but has not been laid out, and SetCursor's clamp is + /// Clamp(x, 0, Cols - 1), which throws when the maximum is negative. + /// + private void RestoreDisplayCursor() + { + if (Cols <= 0 || Rows <= 0) + return; + + // Clamped to Cols, not Cols - 1. The phantom column past the last cell is a real cursor + // position and the one the print path tests to decide whether to wrap; clamping it onto the + // last cell turns a line about to wrap into one that overwrites its own last character. + // SetCursorRaw derives the flag from that column, which is why it is not set separately. + _buffer.SetCursorRaw(Math.Clamp(_cursorXBeforeStatus, 0, Cols), + Math.Clamp(_cursorYBeforeStatus, 0, Rows - 1)); + } + internal void RaiseStatusLineChanged() => StatusLineChanged?.Invoke(this, EventArgs.Empty); private bool _statusLineDirty; + private bool _parsing; + + /// Records a status-line change, to be announced at the end of the batch. + /// + /// Outside a batch -- a host calling the API directly -- there is no end to wait for, so the + /// event goes out immediately. Inside one it is held, which is what makes the batching real + /// rather than documented: a single write carrying DECSSDT, DECSASD and a message used to emit + /// three events, one per setter plus one for the flush. + /// + private void NoteStatusLineChanged() + { + if (_parsing) + { + _statusLineDirty = true; + return; + } + + RaiseStatusLineChanged(); + } /// Announces a status-line change, at most once for the batch just parsed. /// @@ -161,7 +206,7 @@ public void SetStatusDisplayType(int type) _statusBuffer.Resize(Cols, 1); } - RaiseStatusLineChanged(); + NoteStatusLineChanged(); } /// @@ -188,25 +233,31 @@ public void SetActiveStatusDisplay(int display) if (wantStatus) { + // X is saved as it stands, phantom column included: that column is what the print path + // reads to decide whether the next character wraps, so rounding it off here would turn + // a line about to wrap into one that overwrites its own last character. _cursorXBeforeStatus = _buffer.X; _cursorYBeforeStatus = _buffer.Y; - _statusBuffer ??= new Buffer.TerminalBuffer(Cols, 1, 0, hasScrollback: false); - _statusBuffer.Resize(Cols, 1); + EnsureStatusBuffer(); _statusLineActive = true; - _buffer = _statusBuffer; + _buffer = _statusBuffer!; _inputHandler.SetBuffer(_buffer); - _buffer.SetCursor(0, 0); + + // NOT homed. Each display keeps its own cursor, and that has to mean across repeated + // selections too: a program that writes half a status message, goes back to the screen + // and returns would otherwise start again at column one and overwrite what it wrote. + // The buffer is created at (0, 0), which is where a first selection finds it. } else { _statusLineActive = false; _buffer = _usingAltBuffer ? _altBuffer! : _normalBuffer!; _inputHandler.SetBuffer(_buffer); - _buffer.SetCursor(_cursorXBeforeStatus, _cursorYBeforeStatus); + RestoreDisplayCursor(); } - RaiseStatusLineChanged(); + NoteStatusLineChanged(); } /// @@ -835,7 +886,10 @@ public void Write(string data) if (_disposed || string.IsNullOrEmpty(data)) return; - _parser.Parse(data); + _parsing = true; + try { _parser.Parse(data); } + finally { _parsing = false; } + FlushStatusLineChange(); } @@ -853,7 +907,10 @@ public void Write(ReadOnlySpan data) if (_disposed || data.IsEmpty) return; - _parser.Parse(data); + _parsing = true; + try { _parser.Parse(data); } + finally { _parsing = false; } + FlushStatusLineChange(); } @@ -1913,6 +1970,17 @@ public void SwitchToAltBuffer() var shapeBefore = PointerShape; // ONE cursor, two screens: xterm shares the cursor across the switch, which is what lets // DECSET 47 flip screens mid-drawing without teleporting the pen. + // Switching screens ends the status line's turn with the cursor. Holding the status row as + // the write target across the switch is not available: the switch's own work -- 1049's + // erase among it -- runs through the input handler's buffer, so leaving that pointed at a + // one-row status buffer sends a full-screen erase off the end of it. Handing the cursor + // back first keeps one rule: the status line is selected, or the screen is. + // + // The ROW is untouched. Only the selection ends, so a program that switches screens and + // comes back finds its status message still there. + if (_statusLineActive) + SetActiveStatusDisplay(0); + var x = _buffer.X; var y = _buffer.Y; _buffer = _altBuffer!; @@ -1938,6 +2006,17 @@ public void SwitchToNormalBuffer() return; var shapeBefore = PointerShape; + // Switching screens ends the status line's turn with the cursor. Holding the status row as + // the write target across the switch is not available: the switch's own work -- 1049's + // erase among it -- runs through the input handler's buffer, so leaving that pointed at a + // one-row status buffer sends a full-screen erase off the end of it. Handing the cursor + // back first keeps one rule: the status line is selected, or the screen is. + // + // The ROW is untouched. Only the selection ends, so a program that switches screens and + // comes back finds its status message still there. + if (_statusLineActive) + SetActiveStatusDisplay(0); + var x = _buffer.X; var y = _buffer.Y; _buffer = _normalBuffer!; From 7d1880aee9acbb0db8af60801a0dbd0327bed96a Mon Sep 17 00:00:00 2001 From: John Campion Jr Date: Mon, 31 Aug 2026 21:04:01 -0400 Subject: [PATCH 3/3] Clear the status-line event on dispose, and make the test say so StatusLineChanged was added without being added to Dispose, so a disposed terminal held its subscribers. The subscriber is usually the host, which holds a control, which holds a window -- a leak with the terminal on the far end of it. The test that should have caught it is called Dispose_ClearsAllEvents and checks two events. It has been right about those two since it was written, and it cannot be wrong about the others because it never looks at them, which is how this got in. So it is enumerated now rather than listed. The new test subscribes to every public event on Terminal through reflection -- so a field that happened to be null already cannot pass by accident -- disposes, and asserts every backing field came back null. An event added tomorrow is covered the day it is declared, and one that is forgotten is named in the failure. Confirmed it bites: with the new line removed the test fails and reports the retained event, which is the whole point of writing it this way rather than appending a third name to the old one. 2168 passed, 0 failed. Co-Authored-By: Claude Fable 5 --- src/XTerm.NET.Tests/TerminalTests.cs | 57 ++++++++++++++++++++++++++++ src/XTerm.NET/Terminal.cs | 1 + 2 files changed, 58 insertions(+) diff --git a/src/XTerm.NET.Tests/TerminalTests.cs b/src/XTerm.NET.Tests/TerminalTests.cs index 21746a0..e39c05a 100644 --- a/src/XTerm.NET.Tests/TerminalTests.cs +++ b/src/XTerm.NET.Tests/TerminalTests.cs @@ -470,6 +470,63 @@ public void Dispose_ClearsAllEvents() Assert.Equal(0, count); // Events should not fire after dispose } + [Fact] + public void Dispose_ClearsEveryEvent_NotJustTheOnesSomeoneRemembered() + { + // Dispose_ClearsAllEvents above is named for the contract and checks two events, which is + // how StatusLineChanged was added without cleanup and nothing noticed. Enumerated instead, + // so the test cannot fall behind the class: every field-like event on Terminal must be null + // after Dispose, and a new one is covered the moment it is declared. + // + // A retained subscriber is a leak with the terminal on the far end of it -- the host is + // usually the subscriber, and it holds a control, which holds a window. + var terminal = new Terminal(); + + // Subscribe to everything, so a field that was already null cannot pass by accident. + var events = typeof(Terminal) + .GetEvents(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance) + .ToList(); + + Assert.NotEmpty(events); + + var subscribed = new List(); + foreach (var e in events) + { + var handlerType = e.EventHandlerType!; + var invoke = handlerType.GetMethod("Invoke")!; + var parameters = invoke.GetParameters() + .Select(p => System.Linq.Expressions.Expression.Parameter(p.ParameterType)) + .ToArray(); + var handler = System.Linq.Expressions.Expression.Lambda( + handlerType, + System.Linq.Expressions.Expression.Empty(), + parameters).Compile(); + + e.AddEventHandler(terminal, handler); + subscribed.Add(e); + } + + terminal.Dispose(); + + var retained = new List(); + foreach (var e in subscribed) + { + // The backing field of a field-like event carries the invocation list. An event with a + // hand-written add/remove has none, and cannot be checked this way. + var field = typeof(Terminal).GetField( + e.Name, + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + + if (field is null) + continue; + + if (field.GetValue(terminal) is not null) + retained.Add(e.Name); + } + + Assert.Empty(retained); + } + [Fact] public void Write_WithBackspace_MovesBack() { diff --git a/src/XTerm.NET/Terminal.cs b/src/XTerm.NET/Terminal.cs index 48b5a5f..37a3758 100644 --- a/src/XTerm.NET/Terminal.cs +++ b/src/XTerm.NET/Terminal.cs @@ -2170,6 +2170,7 @@ public void Dispose() SynchronizedOutputChanged = null; BufferChanged = null; TitleChanged = null; + StatusLineChanged = null; BellRang = null; Resized = null; Scrolled = null;