diff --git a/CLAUDE.md b/CLAUDE.md index 6763916..401d302 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,12 +41,20 @@ Provider Abstraction (SpectreConsoleProvider) **Rendering flow:** 1. `UIApplication` manages the main loop and input processing -2. Elements are arranged via `ArrangeChildren()` which sets child `Position` and `Dimensions` +2. Elements are arranged via `ArrangeChildren()` which sets child `Position` and `Dimensions`. + It reaches one level only — a container arranges its own children, not theirs — so anything + that resizes a subtree has to walk it 3. Each pass is a full clear followed by a full redraw: `UIApplication.Render()` clears the console and every visible element draws again. `Invalidate()` marks an element as changed and raises `Invalidated`, but it does not gate drawing — a full clear combined with a dirty-only redraw erases static elements rather than preserving them (ktsu-dev/TUI#109) -4. `UIContainerBase.Render()` renders itself then all visible children +4. Each pass also re-reads `ConsoleProvider.Dimensions`. When the terminal has changed size the + root takes the new size and the whole tree is re-arranged, so a resized window is laid out on + the next frame instead of staying pinned to the size at launch (ktsu-dev/TUI#111). The input + loop wakes on a timer (`UIApplication.ResizePollInterval`) as well as on input, because a + resize delivers no keypress to wake it — each tick compares the size and draws only when it + changed +5. `UIContainerBase.Render()` renders itself then all visible children ## Code Patterns diff --git a/TUI.Core/Services/UIApplication.cs b/TUI.Core/Services/UIApplication.cs index 7034486..7842282 100644 --- a/TUI.Core/Services/UIApplication.cs +++ b/TUI.Core/Services/UIApplication.cs @@ -70,6 +70,9 @@ public class UIApplication(IConsoleProvider consoleProvider, ILogger LogInterruptReceived = LoggerMessage.Define(LogLevel.Information, new EventId(17, nameof(LogInterruptReceived)), "Interrupt signal received, shutting down"); + private static readonly Action LogConsoleResized = + LoggerMessage.Define(LogLevel.Debug, new EventId(18, nameof(LogConsoleResized)), "Console resized to {Width}x{Height}, re-arranging the layout"); + /// /// Gets the source of process interrupt signals that shuts the application down /// @@ -78,6 +81,23 @@ public class UIApplication(IConsoleProvider consoleProvider, ILogger internal IInterruptSource InterruptSource { get; init; } = new ConsoleInterruptSource(); + /// + /// Gets how often the input loop wakes to re-check the terminal size while it is waiting for + /// a key + /// + /// + /// A resize delivers no input, so a loop that only wakes on a keypress cannot notice one. + /// Reading is cheap and nothing is drawn unless the + /// size actually changed, so this is a size comparison ten times a second rather than a + /// redraw. Tests shorten it so a resize is picked up without waiting out a real frame. + /// + internal TimeSpan ResizePollInterval { get; init; } = TimeSpan.FromMilliseconds(100); + + /// + /// The terminal size the current layout was computed for, or null before the first render + /// + private Models.Dimensions? _observedConsoleDimensions; + /// public IUIElement? RootElement { get; set; } @@ -181,6 +201,11 @@ public void Shutdown() /// Each pass clears the console and redraws every visible element. Dirty tracking is not used /// to skip elements — combining a full clear with a dirty-only redraw is what made static /// elements disappear on the frame after their first draw (ktsu-dev/TUI#109). + /// + /// Each pass also re-checks the terminal size, so a window the user resized mid-run is laid + /// out at its new size on the next frame rather than staying pinned to the size at launch + /// (ktsu-dev/TUI#111). + /// /// public void Render() { @@ -204,11 +229,9 @@ public void Render() // together: a clear without a full redraw erases whatever the last pass drew. ConsoleProvider.Clear(); - // Set root element dimensions to console dimensions if not set - if (RootElement.Dimensions.IsEmpty) - { - RootElement.Dimensions = ConsoleProvider.Dimensions; - } + // Size the layout to the terminal before drawing it, so a resize since the last pass + // is reflected in this one. + SyncRootToConsole(RootElement); // Render the root element RootElement.Render(ConsoleProvider); @@ -236,6 +259,73 @@ public void Render() } } + /// + /// Brings the root element's size in line with the terminal, re-arranging the tree when it changes + /// + /// The root element to size + /// + /// The size used to be taken once, on the first pass that found the root unsized, and never + /// looked at again — so resizing the window left every layout pinned to the size at launch + /// (ktsu-dev/TUI#111). + /// + private void SyncRootToConsole(IUIElement root) + { + Models.Dimensions console = ConsoleProvider.Dimensions; + bool resized = _observedConsoleDimensions is Models.Dimensions observed && observed != console; + _observedConsoleDimensions = console; + + // Adopt the terminal size when the root has none of its own, and again whenever the + // terminal is resized. In between, a size the host assigned to the root is left alone — a + // resize is the one thing that overrides it, since the old size no longer fits the window. + if (!resized && !root.Dimensions.IsEmpty) + { + return; + } + + if (resized && _logger != null) + { + LogConsoleResized(_logger, console.Width, console.Height, null); + } + + root.Dimensions = console; + + // Assigning Dimensions only invalidates; it does not re-run layout. Walk the tree so every + // container re-arranges inside its new size, not just the root. + ArrangeTree(root); + } + + /// + /// Re-arranges and every container beneath it, parents first + /// + /// The element to arrange + /// + /// A container's sizes and positions its own + /// children but does not reach theirs, so arranging only the root would relayout the top level + /// and leave everything under it at the old size. Parents are arranged first because a child + /// container can only lay its own children out once it knows its new size. + /// + private static void ArrangeTree(IUIElement element) + { + if (element is not IUIContainer container) + { + return; + } + + container.ArrangeChildren(); + + foreach (IUIElement child in container.Children) + { + ArrangeTree(child); + } + } + + /// + /// Gets whether the terminal has changed size since the last render pass + /// + /// True when a redraw is needed to pick the new size up + private bool HasConsoleResized() => + RootElement != null && ConsoleProvider.Dimensions != _observedConsoleDimensions; + /// public async Task ProcessInputAsync(CancellationToken cancellationToken = default) { @@ -244,16 +334,49 @@ public async Task ProcessInputAsync(CancellationToken cancellationToken = defaul LogStartingInputProcessing(_logger, null); } + // One read is carried across iterations. The resize poll below wakes the loop without a + // keypress, and starting a fresh read each time it woke would leave several reads racing + // for the next key. + Task? pendingRead = null; + while (!cancellationToken.IsCancellationRequested && IsRunning) { try { - // Abandon the read when cancellation is requested. A provider parked in - // Console.ReadKey does not observe the token, so awaiting it directly would keep - // the loop alive until the user pressed an unrelated key after asking to exit. - Models.InputResult input = await ConsoleProvider.ReadInputAsync() - .WaitAsync(cancellationToken) - .ConfigureAwait(false); + pendingRead ??= ConsoleProvider.ReadInputAsync(); + + if (!pendingRead.IsCompleted) + { + // Wake on a timer as well as on input. A resize produces no input at all, so a + // loop that only wakes for a key cannot notice one. Cancelling the delay is + // also what lets a shutdown request end a run blocked on the keyboard: a + // provider parked in Console.ReadKey does not observe the token itself. + Task idle = Task.Delay(ResizePollInterval, cancellationToken); + await Task.WhenAny(pendingRead, idle).ConfigureAwait(false); + + if (cancellationToken.IsCancellationRequested) + { + break; + } + + if (!pendingRead.IsCompleted) + { + // The timer won the race, so no key arrived. Redraw only if the terminal + // changed size while we waited, and go back to the same pending read. + if (HasConsoleResized()) + { + Render(); + } + + continue; + } + } + + // Cleared before the await so a read that failed is not retried forever by the + // recoverable-error branches below. + Task completedRead = pendingRead; + pendingRead = null; + Models.InputResult input = await completedRead.ConfigureAwait(false); if (_logger != null) { diff --git a/TUI.Test/BlockingConsoleProvider.cs b/TUI.Test/BlockingConsoleProvider.cs index 8c2086a..95aec5c 100644 --- a/TUI.Test/BlockingConsoleProvider.cs +++ b/TUI.Test/BlockingConsoleProvider.cs @@ -39,14 +39,40 @@ internal sealed class BlockingConsoleProvider : IConsoleProvider /// internal bool CursorVisible => cursorVisible; - /// - public Dimensions Dimensions { get; set; } = new(80, 24); + private readonly Lock dimensionsLock = new(); + private int clearCount; /// - public void Clear() + /// + /// Guarded because a resize test writes it from the test thread while the application reads it + /// from the thread running the loop — which is exactly the situation the resize poll exists for. + /// + public Dimensions Dimensions { - // Nothing to record: these tests assert on lifecycle, not on drawn output. - } + get + { + lock (dimensionsLock) + { + return field; + } + } + + set + { + lock (dimensionsLock) + { + field = value; + } + } + } = new(80, 24); + + /// + /// Gets the number of times was called, which is once per render pass. + /// + internal int ClearCount => Volatile.Read(ref clearCount); + + /// + public void Clear() => Interlocked.Increment(ref clearCount); /// public void Render(IUIElement element, Position position) => element?.Render(this); diff --git a/TUI.Test/RecordingLogger.cs b/TUI.Test/RecordingLogger.cs new file mode 100644 index 0000000..52f3669 --- /dev/null +++ b/TUI.Test/RecordingLogger.cs @@ -0,0 +1,45 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Services; +using Microsoft.Extensions.Logging; + +/// +/// An that records the messages written to it, so a test can +/// assert what the application reported rather than only that it did not throw. +/// +internal sealed class RecordingLogger : ILogger +{ + private readonly List messages = []; + + /// + /// Gets the messages logged so far, in call order. + /// + internal IEnumerable Messages + { + get + { + lock (messages) + { + return [.. messages]; + } + } + } + + /// + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + /// + public bool IsEnabled(LogLevel logLevel) => true; + + /// + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + string message = formatter is null ? string.Empty : formatter(state, exception); + lock (messages) + { + messages.Add(message); + } + } +} diff --git a/TUI.Test/UIApplicationLifecycleTests.cs b/TUI.Test/UIApplicationLifecycleTests.cs index dec4f89..22123b5 100644 --- a/TUI.Test/UIApplicationLifecycleTests.cs +++ b/TUI.Test/UIApplicationLifecycleTests.cs @@ -4,7 +4,6 @@ namespace ktsu.TUI.Test; using ktsu.TUI.Core.Models; using ktsu.TUI.Core.Services; -using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; /// @@ -262,43 +261,4 @@ private static async Task AssertCompletesAsync(Task task, string because) // Observed separately so a task that failed reports its own exception, not the timeout. await task.ConfigureAwait(false); } - - /// - /// An that records the messages written to it, so a test - /// can assert what the application reported rather than only that it did not throw. - /// - private sealed class RecordingLogger : ILogger - { - private readonly List messages = []; - - /// - /// Gets the messages logged so far, in call order. - /// - internal IEnumerable Messages - { - get - { - lock (messages) - { - return [.. messages]; - } - } - } - - /// - public IDisposable? BeginScope(TState state) where TState : notnull => null; - - /// - public bool IsEnabled(LogLevel logLevel) => true; - - /// - public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) - { - string message = formatter is null ? string.Empty : formatter(state, exception); - lock (messages) - { - messages.Add(message); - } - } - } } diff --git a/TUI.Test/UIApplicationResizeTests.cs b/TUI.Test/UIApplicationResizeTests.cs new file mode 100644 index 0000000..3e23878 --- /dev/null +++ b/TUI.Test/UIApplicationResizeTests.cs @@ -0,0 +1,287 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Contracts; +using ktsu.TUI.Core.Elements.Layouts; +using ktsu.TUI.Core.Elements.Primitives; +using ktsu.TUI.Core.Models; +using ktsu.TUI.Core.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests for 's handling of a terminal that changes size mid-run. +/// +/// +/// The size used to be read once, on the first pass that found the root element unsized, and never +/// looked at again — so resizing the window left every layout pinned to the size at launch +/// (ktsu-dev/TUI#111). Two halves are covered here: a render pass picks the new size up, and the +/// input loop produces a pass at all, since a resize delivers no keypress to wake it. +/// +[TestClass] +public sealed class UIApplicationResizeTests +{ + /// + /// How long any single step of a run is given before it is declared stuck. + /// + /// + /// Bounds every wait in this class. The regression under test is an application that never + /// notices a resize, so an unbounded wait would hang the suite instead of reporting it. + /// + private static readonly TimeSpan StepTimeout = TimeSpan.FromSeconds(10); + + /// + /// The size the terminal starts at in these tests. + /// + private static readonly Dimensions LaunchSize = new(80, 24); + + /// + /// The size the terminal is resized to. Narrower than so the clamping + /// in produces a visibly different layout rather than the same one. + /// + private static readonly Dimensions ResizedSize = new(40, 10); + + /// + /// Text wider than but narrower than , so + /// the width it is laid out at reports which of the two sizes the layout used. + /// + private static readonly string WideText = new('x', 60); + + /// + /// Gets or sets the test context MSTest injects, used for its cancellation token so a run + /// started by a test ends when the test run itself is cancelled. + /// + public TestContext TestContext { get; set; } = null!; + + /// + /// A render pass after the terminal changed size must lay the root out at the new size. It + /// used to keep the size from the first pass forever, so the UI stayed pinned to the size the + /// window happened to have at launch. + /// + [TestMethod] + public void RenderAfterAResizeGivesTheRootTheNewTerminalSize() + { + // Arrange + RecordingConsoleProvider provider = new() { Dimensions = LaunchSize }; + StackPanel root = []; + UIApplication app = new(provider) { RootElement = root }; + app.Render(); + Assert.AreEqual(LaunchSize, root.Dimensions, "The first pass should size the root to the terminal"); + + // Act + provider.Dimensions = ResizedSize; + app.Render(); + + // Assert + Assert.AreEqual(ResizedSize, root.Dimensions, "A render pass after a resize should adopt the new terminal size"); + } + + /// + /// The resize has to reach the whole tree, not just the root. A container arranges its own + /// children but not theirs, so re-arranging only the root would leave every grandchild at the + /// size it had before. + /// + [TestMethod] + public void RenderAfterAResizeRelaysOutEveryContainerBeneathTheRoot() + { + // Arrange + RecordingConsoleProvider provider = new() { Dimensions = LaunchSize }; + BorderElement root = CreateNestedLayout(out StackPanel panel, out TextElement leaf); + UIApplication app = new(provider) { RootElement = root }; + app.Render(); + Assert.AreEqual(LaunchSize.WithoutPadding(root.Padding), panel.Dimensions, "The panel should start out filling the terminal, inside the border"); + Assert.AreEqual(WideText.Length, leaf.Dimensions.Width, "The label should start out at its full width"); + + // Act + provider.Dimensions = ResizedSize; + app.Render(); + + // Assert + Dimensions resizedContentArea = ResizedSize.WithoutPadding(root.Padding); + Assert.AreEqual(resizedContentArea, panel.Dimensions, "The panel should be re-arranged into the resized root"); + Assert.AreEqual(resizedContentArea.Width, leaf.Dimensions.Width, "The label should be clamped to the narrower terminal, which only happens if the panel re-arranged its own children too"); + } + + /// + /// Between resizes, a size the host assigned to the root is its own business. Only an actual + /// change of terminal size overrides it — at which point keeping the old one would draw + /// outside the window. + /// + [TestMethod] + public void RenderWithoutAResizeLeavesAHostAssignedRootSizeAlone() + { + // Arrange + Dimensions chosenByHost = new(20, 5); + RecordingConsoleProvider provider = new() { Dimensions = LaunchSize }; + StackPanel root = new() { Dimensions = chosenByHost }; + UIApplication app = new(provider) { RootElement = root }; + + // Act + app.Render(); + app.Render(); + + // Assert + Assert.AreEqual(chosenByHost, root.Dimensions, "A root the host sized itself should keep that size while the terminal has not changed"); + } + + /// + /// A resize is worth a line in the log: it re-lays out the whole tree, and it is the one thing + /// that overrides a size the host chose, so someone reading the log should be able to see it + /// happen. + /// + [TestMethod] + public void AResizeIsReported() + { + // Arrange + RecordingLogger logger = new(); + RecordingConsoleProvider provider = new() { Dimensions = LaunchSize }; + StackPanel root = []; + UIApplication app = new(provider, logger) { RootElement = root }; + app.Render(); + + // Act + provider.Dimensions = ResizedSize; + app.Render(); + + // Assert + Assert.Contains( + m => m.Contains($"{ResizedSize.Width}x{ResizedSize.Height}", StringComparison.Ordinal), + logger.Messages, + $"The new terminal size should be reported, but the log held: {string.Join(" | ", logger.Messages)}"); + } + + /// + /// A resize arrives as no input at all, so a loop that only wakes for a keypress cannot see + /// one. The running application must notice it and redraw without the user pressing anything. + /// + [TestMethod] + public async Task AResizeWhileWaitingForInputRedrawsWithoutAKeypress() + { + // Arrange + BlockingConsoleProvider provider = new() { Dimensions = LaunchSize }; + BorderElement root = CreateNestedLayout(out StackPanel panel, out TextElement leaf); + UIApplication app = new(provider) + { + RootElement = root, + ResizePollInterval = TimeSpan.FromMilliseconds(10) + }; + + Task run = app.RunAsync(TestContext.CancellationToken); + await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); + + // Act + provider.Dimensions = ResizedSize; + + // Assert + Dimensions resizedContentArea = ResizedSize.WithoutPadding(root.Padding); + await AssertResizedAsync(leaf, resizedContentArea.Width, "A resize should be picked up while the application is blocked waiting for a key, with no key pressed").ConfigureAwait(false); + Assert.AreEqual(resizedContentArea, panel.Dimensions, "The whole tree should be re-arranged by the redraw the resize triggered"); + + app.Shutdown(); + await AssertCompletesAsync(run, "The run should still end when asked to shut down").ConfigureAwait(false); + } + + /// + /// Waking to check the size must not turn into redrawing on a timer: a full clear and redraw + /// ten times a second would flicker for no reason. Only a size that actually changed draws. + /// + [TestMethod] + public async Task WaitingForInputWithoutAResizeDoesNotRedraw() + { + // Arrange + TimeSpan pollInterval = TimeSpan.FromMilliseconds(10); + BlockingConsoleProvider provider = new() { Dimensions = LaunchSize }; + StackPanel root = []; + UIApplication app = new(provider) + { + RootElement = root, + ResizePollInterval = pollInterval + }; + + Task run = app.RunAsync(TestContext.CancellationToken); + await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); + int clearsAfterTheFirstRender = provider.ClearCount; + + // Act + await Task.Delay(pollInterval * 20, TestContext.CancellationToken).ConfigureAwait(false); + + // Assert + Assert.AreEqual(clearsAfterTheFirstRender, provider.ClearCount, "Polling for a resize should not redraw while the terminal size is unchanged"); + + app.Shutdown(); + await AssertCompletesAsync(run, "The run should still end when asked to shut down").ConfigureAwait(false); + } + + /// + /// Builds a root containing a container that in turn contains a leaf, so a test can tell a + /// relayout of the root apart from a relayout of the whole tree. + /// + /// The container between the root and the leaf. + /// The leaf whose width reports which size the layout used. + /// The root element. + private static BorderElement CreateNestedLayout(out StackPanel panel, out TextElement leaf) + { + leaf = new TextElement(WideText); + + panel = []; + panel.AddChild(leaf); + + BorderElement root = []; + root.AddChild(panel); + + return root; + } + + /// + /// Waits until has been laid out at . + /// + /// The element to watch. + /// The width the resize should produce. + /// The assertion message if it never happens. + /// + /// Driven by rather than by sleeping: assigning + /// raises it, and completing the task from the handler is + /// what establishes that the value the assertion reads was published by the application's + /// thread. + /// + private static async Task AssertResizedAsync(TextElement element, int expectedWidth, string because) + { + TaskCompletionSource resized = new(TaskCreationOptions.RunContinuationsAsynchronously); + + void OnInvalidated(object? sender, EventArgs e) + { + if (element.Dimensions.Width == expectedWidth) + { + resized.TrySetResult(); + } + } + + element.Invalidated += OnInvalidated; + + try + { + // Covers the resize having already landed between the act and this subscription, in + // which case no further event is coming. + OnInvalidated(element, EventArgs.Empty); + await AssertCompletesAsync(resized.Task, because).ConfigureAwait(false); + } + finally + { + element.Invalidated -= OnInvalidated; + } + } + + /// + /// Asserts that completes within . + /// + /// The task to wait for. + /// The assertion message if it does not complete in time. + private static async Task AssertCompletesAsync(Task task, string because) + { + Task finished = await Task.WhenAny(task, Task.Delay(StepTimeout)).ConfigureAwait(false); + Assert.AreSame(task, finished, because); + + // Observed separately so a task that failed reports its own exception, not the timeout. + await task.ConfigureAwait(false); + } +}