fix: re-check the terminal size on every render pass [patch] - #120
Merged
Merged
Conversation
Fixes #111 UIApplication.Render() took the terminal size once, on the first pass that found the root element unsized, and never looked at it again. Resizing the window left every layout pinned to the size at launch: content drawn past the edge on a shrink, dead space around the UI on a grow. Each pass now compares ConsoleProvider.Dimensions against the size the current layout was computed for. When it differs, the root takes the new size and the tree is re-arranged. The walk is needed because ArrangeChildren reaches one level only and assigning Dimensions merely invalidates, so arranging the root alone would relayout the top level and leave everything under it at the old size. The input loop also wakes on a timer, not only on a keypress. A resize delivers no input, so a loop parked in Console.ReadKey cannot observe one — re-checking in Render() would have been correct and never reached. Each tick compares the size and renders only when it changed, so an idle terminal still draws nothing. The pending read is carried across ticks rather than restarted, which would leave several reads racing for the next key. Between resizes a size the host assigned to the root is left alone, as before. A resize overrides it, since the old size no longer fits the window. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmfzCZGV3tASUEeUcsXMNN
MSTEST0037: Assert.Contains reports which messages were logged when the assertion fails, where Assert.Contains-inside-Assert.IsTrue only reports "expected true". Verified the rewritten assertion still fails when the line is absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmfzCZGV3tASUEeUcsXMNN
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #111
The problem
UIApplication.Render()took the terminal size once, on the first pass that found the root element unsized, and never looked at it again:Resizing the window left every layout pinned to the size at launch — content drawn past the edge on a shrink, dead space around the UI on a grow.
The fix
Three parts, and the first alone would have been correct and never reached.
1. Compare the size on every pass.
Render()now checksConsoleProvider.Dimensionsagainst the size the current layout was computed for, and adopts a new one when it differs. ConfirmedSpectreConsoleProvider.Dimensionsis live rather than cached: Spectre'sProfile.Widthhas a null_widthbacking field unless a host sets one, so it delegates toAnsiConsoleOutput.Widthon each read.2. Re-arrange the whole tree, not just the root.
IUIContainer.ArrangeChildren()sizes and positions its own children but does not reach theirs, and assigningDimensionsonly invalidates — it does not re-run layout. So arranging the root alone relayouts the top level and leaves everything beneath it at the old size.ArrangeTreewalks the tree parents-first, since a child container can only lay its own children out once it knows its new size. (UIApplicationTests.CreateLayoutalready works around the missing walk with a manualpanel.ArrangeChildren().)3. Wake the input loop on a timer. A resize delivers no input, so a loop parked in
Console.ReadKeynever reaches another render pass — following the triage note, this takes the portable polling option rather than SIGWINCH, which the console abstraction does not expose. Each tick compares the size and renders only when it changed, so an idle terminal still draws nothing;ResizePollIntervaldefaults to 100ms and isinternalso tests can shorten it. The pending read is carried across ticks rather than restarted, which would otherwise leave several reads racing for the next key, and cancellation still ends a run blocked on the keyboard — the behaviour #117 added.Between resizes, a size the host assigned to the root is left alone, as before. A resize overrides it, since the old size no longer fits the window.
Tests
TUI.Test/UIApplicationResizeTests.cs, six tests: a pass after a resize adopts the new size; it re-lays out a nested container and its leaf (a label wider than the resized terminal, so the width it lands at reports which size the layout used); a host-assigned root size survives a pass with no resize; a running application picks a resize up with no key pressed; polling does not redraw while the size is unchanged; and the resize is logged.BlockingConsoleProvider.Dimensionsis now lock-guarded and countsClear()calls, since a resize test writes the size from the test thread while the loop reads it.RecordingLoggermoved out ofUIApplicationLifecycleTestsinto its own file, matching how the other test doubles are laid out, so both classes can use it.Verified the tests fail without the fix, reverting each mechanism separately so each test is tied to the part it covers:
RenderAfterAResizeGivesTheRootTheNewTerminalSize,RenderAfterAResizeRelaysOutEveryContainerBeneathTheRoot,AResizeWhileWaitingForInputRedrawsWithoutAKeypressArrangeTreemade non-recursiveRenderAfterAResizeRelaysOutEveryContainerBeneathTheRoot,AResizeWhileWaitingForInputRedrawsWithoutAKeypressAResizeWhileWaitingForInputRedrawsWithoutAKeypressIn each case the remaining tests still passed, including
WaitingForInputWithoutAResizeDoesNotRedraw— correct, since none of the three reverts makes the loop draw on a timer.Full suite: 154/154 pass.
dotnet build TUI.slnis clean in Debug and Release acrossnet10.0;net9.0;net8.0, 0 warnings.One thing not verified here: driving
TUI.Appunder a real terminal and resizing it. As #117 found,SetCursorVisibilityissues a cursor-position query (ESC[6n) that nothing in a headless container answers, so the app never gets past hiding the cursor. The acceptance criterion is covered by the tests above instead.Sequencing
#111's triage put this behind #109, which #113 closed. This branch is cut from
mainatb08abc6, which carries both #113 and #117, and touchesRender()andProcessInputAsync()on top of them rather than around them.Docs
CLAUDE.md's rendering flow gains the resize pass and the note thatArrangeChildren()reaches one level only — the fact that made part 2 necessary.🤖 Generated with Claude Code
https://claude.ai/code/session_01KmfzCZGV3tASUEeUcsXMNN
Generated by Claude Code