Add resize reflow for the normal buffer (supersedes #12, with an empty-group crash fix)

[JohnCampionJr]

Supersedes #12. All of @buchmiet's work is preserved commit-for-commit; this branch is #12 merged up to current main, plus one crash fix and its regression tests on top.

I reviewed #12 by building and testing against it rather than reading the diff. It is a careful, faithful port of xterm.js BufferReflow.ts and it passes 628 tests as submitted. I found one crash.

The crash

ReflowSmallerGetNewLineLengths loops while cellsAvailable < cellsNeeded, so a wrapped group whose trimmed length is zero never enters the loop and returns an empty array. ReflowSmaller then reads [Length - 1] from it:

var destLineIndex = destLineLengths.Length - 1;   // -1
var destCol = destLineLengths[destLineIndex];     // IndexOutOfRangeException

Only a one-row group can be empty. GetWrappedLineTrimmedLength returns cols for every row of a group except the last, so anything with two rows already counts a full row of cells whatever it contains. A one-row group means a continuation row at index 0 with an unwrapped row beneath it — which is exactly what the scrollback leaves behind once the row it continued has been trimmed away, and it is blank whenever the wrap happened over whitespace.

That makes it reachable from ordinary output rather than only by hand:

var terminal = new Terminal(new TerminalOptions { Cols = 6, Rows = 2, Scrollback = 1 });
terminal.Write(new string(' ', 12));   // wraps: the tail row is blank AND wrapped
terminal.Write("\r\nx");
terminal.Write("\r\ny");               // pushes the head out; blank continuation lands at row 0
terminal.Resize(4, 2);                 // IndexOutOfRangeException

Any command emitting long runs of spaces sets this up, and the crash lands on window resize.

The fix

Skip the group. This is right rather than merely safe — there is no content to redistribute.

var destLineLengths = BufferReflow.ReflowSmallerGetNewLineLengths(wrappedLines, _cols, newCols);
if (destLineLengths.Length == 0)
{
    continue;
}

Three regression tests in ReflowEmptyGroupTests.cs: the terminal-level route above, a check that the rows which do have content still survive it, and the same layout built directly so the regression stays pinned if the terminal-level route ever stops producing it.

631 passed, 0 failed.

One correction to #12's description, in its favour

#12 says xterm.js "disables reflow under windowsMode / windowsPty because ConPTY re-wraps the viewport" and describes this port's unconditional behaviour as a deliberate divergence needing a follow-up toggle. The actual gate is:

private get _isReflowEnabled(): boolean {
  const windowsPty = this._optionsService.rawOptions.windowsPty;
  if (windowsPty && windowsPty.buildNumber) {
    return this._hasScrollback && windowsPty.backend === 'conpty' && windowsPty.buildNumber >= 21376;
  }
  return this._hasScrollback;
}

xterm.js enables reflow for ConPTY at build >= 21376, and its default path — no windowsPty configured — is just _hasScrollback, which is what this port already does. So it is not a divergence. The only gap is an opt-out for winpty and pre-21376 ConPTY, and every Windows 11 build is well past 21376. I would not treat the toggle as a blocker.

Two things I looked at and am deliberately NOT changing

A written space at the wrap column is dropped. With a wrapped row ending in a space and the next row starting with a wide char, "ab cd 汉e" round-trips to "ab cd汉e". I first took this for a port bug — xterm.js's hasContent is a content-mask test that a space satisfies — but making HasContent match breaks Reflow_WideCharactersWhenShrinking. The real cause is that BufferLine fills with BufferCell.Space, so an unwritten cell is a space and cannot be told from a written one. Given that model, this PR's HasContent is the only definition that works. Fixing it properly means moving the fill cell to Empty across the buffer, which is a separate change.

Smaller notes, left alone: CircularList.SetLength does not clear slots when growing, unlike xterm.js's length setter — latent, since its only call site shrinks. And ReflowSmaller has a wrappedLines[destLineIndex] == null check that breaks mid-copy, which would truncate silently rather than fail; not in xterm.js.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain in buffer reflow and resize handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds xterm.js-style column-resize reflow for the normal buffer, preserving wrapped content and adding regression coverage.

Changes:

• Adds resize reflow orchestration and helpers.
• Handles wide cells, capacity trimming, and empty groups.
• Adds integration and pure-function tests plus documentation.

File summaries

File	Summary	Review findings
src/XTerm.NET/Terminal.cs	Alternate-buffer reflow behavior	No final comments
src/XTerm.NET/Buffer/TerminalBuffer.cs	Resize reflow and capacity handling	4 critical and 1 moderate unresolved findings
src/XTerm.NET/Buffer/CircularList.cs	Batched logical-length updates	No final comments
src/XTerm.NET/Buffer/BufferReflow.cs	Reflow calculations and layout helpers	1 critical unresolved finding
src/XTerm.NET/Buffer/BufferLine.cs	Cell-width and content helpers	No final comments
src/XTerm.NET.Tests/Buffer/ReflowEmptyGroupTests.cs	Empty-group regression tests	No final comments
src/XTerm.NET.Tests/Buffer/BufferTests.cs	Reflow integration coverage	No final comments
src/XTerm.NET.Tests/Buffer/BufferReflowTests.cs	Pure reflow tests	No final comments
FIXES.md	Reflow documentation	No final comments

Review details

Suppressed comments (4)

src/XTerm.NET/Buffer/TerminalBuffer.cs:426

• countRemoved cannot be used to adjust _y without knowing where each removal occurred. If the cursor is on row 1 and a wrapped group on rows 2–3 is merged while growing, this branch decrements _y even though the rows before the cursor are unchanged (and then pads the bottom). The cursor should shift only for removals before its absolute position; retain the removal indexes or adjust the viewport from the layout.

                }
                if (_lines.Length < newRows)
                {
                    _lines.Push(new BufferLine(newCols, nullCell));

src/XTerm.NET/Buffer/TerminalBuffer.cs:116

• The MaxLength > _rows part disables reflow for a normal buffer whose configured scrollback is 0, even though this buffer is still constructed with hasScrollback: true and the PR's normal-vs-alternate contract makes that flag the reflow gate. A column-only resize of new TerminalBuffer(..., 0) therefore still truncates long normal-buffer lines, while only the alternate buffer should keep truncation behavior. Gate this on the buffer kind rather than the current capacity.

    private bool IsReflowEnabled => _hasScrollback && _lines.MaxLength > _rows;

src/XTerm.NET/Buffer/TerminalBuffer.cs:602

• Rebuilding the line array inserts wrapped rows but does not remap active selections. SelectionManager stores absolute line indexes and only adjusts them from Trimmed; when an insertion occurs before a selected line without capacity trimming, the range remains at its old y and GetSelectionText reads the wrong rows after a shrink. Apply the insertion mapping to selection coordinates (or expose an equivalent reflow notification).

            var originalLinesLength = originalLines.Count;
            RebuildWithInsertions(originalLines, toInsert, countToInsert);

src/XTerm.NET/Buffer/TerminalBuffer.cs:408

• Applying newLayout removes wrapped rows without remapping absolute selections. SelectionManager listens only to Trimmed, but this path never raises it; growing the terminal can therefore leave a selection's y pointing at the line after a removed continuation row. Update selection coordinates from the removed-row mapping when applying this layout.

            var newLayoutResult = BufferReflow.ReflowLargerCreateNewLayout(_lines, toRemove);
            BufferReflow.ReflowLargerApplyNewLayout(_lines, newLayoutResult.Layout);
            ReflowLargerAdjustViewport(newCols, newRows, newLayoutResult.CountRemoved);

• Files reviewed: 9/9 changed files
• Comments generated: 6
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[tomlm]

can you address the HIGH flagged issues copilot raises?

[JohnCampionJr]

can you address the HIGH flagged issues copilot raises?

on it - this is why we like code review :)

[Copilot]

🟡 Changes recommended

Zero-column resizing can hang, reflow can leave selections misaligned, and the documented test count is inconsistent.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (6)

FIXES.md:24

• The PR description says this branch is #12 plus only the empty-group crash fix, but this change also includes six independently fixed resize edge cases and their regression tests. Please update the description to reflect the actual scope, or split those fixes if they are not intended to ship together.

Six further defects, each reproduced before being fixed and each reachable from ordinary use:

- **One-column reflow hung, then threw `OutOfMemoryException`.** A wide glyph at the wrap boundary made the new line length zero, so `ReflowSmallerGetNewLineLengths` never advanced and appended rows until the list could not grow. A wide glyph cannot be shown in one column, so it is clipped.
- **The viewport adjustment popped rows the outer loop was still walking**, throwing `IndexOutOfRangeException`.
- **A line expanding past the remaining capacity indexed below zero** in the batched rebuild, throwing. Rows that do not fit are the oldest, which capacity trimming discards anyway.
- **`Math.Min` dropped the cursor's lower bound.** Moving to the new column count was the point of that change, but a negative cursor -- which `SetCursorRaw` exists to allow -- survived the resize and left the buffer reporting an out-of-bounds position.
- **The viewport was shifted by the trim amount rather than recomputed.** A 5-row buffer with 5 of scrollback resized to 3 rows showed rows 3..5 of 8, with the live bottom unseen at row 7 and later output landing outside the visible area.
- **A zero-row buffer could never be initialised by a later resize**, because the row-fill loop had moved inside a "has lines" guard. `Lines.Length` stayed 0 and the next write indexed an empty list.

src/XTerm.NET/Buffer/BufferReflow.cs:171

• This layout application removes wrapped rows and shifts later absolute row indexes, but it emits no deletion/selection update. SelectionManager only adjusts coordinates for Trimmed, which does not fire when reflow merely removes a continuation row, so a selection below a merged group moves onto the wrong line after growing the terminal. Emit deletion marker updates or otherwise adjust absolute selections for each removed range.

        lines.SetLength(newLayout.Length);

src/XTerm.NET/Buffer/TerminalBuffer.cs:455

• ReflowLargerGetLinesToRemove can remove a continuation at any buffer index, but this adjustment only decrements _yDisp when it is already at _yBase. If a user is scrolled up and a removed row lies before _yDisp, the compacted lines shift left while _yDisp stays unchanged, so the viewport jumps to the next line. Adjust it based on removed positions, not only the total count.

                {
                    _yDisp--;
                }
                _yBase--;
            }

src/XTerm.NET/Buffer/TerminalBuffer.cs:608

• When shrinking while the user is scrolled up, inserted rows before _yDisp must move _yDisp down. This branch increments it only when _yBase == _yDisp, so a group above a scrolled-up viewport makes the viewport show different content. Track insertion positions and adjust for insertions before the viewport.

                        if (_yBase == _yDisp)
                        {
                            _yDisp++;
                        }
                        _yBase++;

src/XTerm.NET/Buffer/TerminalBuffer.cs:116

• This property is evaluated before Resize assigns the new _rows, after the method may already have grown _lines.MaxLength to newMaxLength. For a normal buffer with Scrollback = 0 (documented as disabling scrollback), resizing from 10x5 to 5x10 makes this condition true (MaxLength == 10, old _rows == 5) and runs reflow on a no-scrollback buffer instead of preserving its truncation behavior. Base the decision on the new capacity (newMaxLength > newRows) or derive it from the configured scrollback rather than the transient old dimensions.

    private bool IsReflowEnabled => _hasScrollback && _lines.MaxLength > _rows;

src/XTerm.NET/Buffer/TerminalBuffer.cs:632

• This overflow path drops rows from the start after reflow, but shifts _yBase and _yDisp instead of recomputing them from the retained buffer. For a 10x5 buffer with one scrollback row, expanding a line near row 3 to five 2-column rows leaves six retained lines; this produces YBase == 0 even though the viewport bottom is Lines.Length - newRows == 1, so the cursor/view is one row above the retained live bottom. Preserve bottom-following state and recompute the viewport as the outer resize-trim path does.

                _yBase = Math.Max(_yBase - amountToTrim, 0);
                _yDisp = Math.Max(_yDisp - amountToTrim, 0);
                SavedCursorState.Y = Math.Max(SavedCursorState.Y - amountToTrim, 0);

• Files reviewed: 10/10 changed files
• Comments generated: 3
• Review effort level: Lite