Release v2.4.12#5550
Merged
Merged
Conversation
… add layout-convergence guards Addresses the safe, verifiable portion of #4522 (`NeedsLayout` is buggy) and documents — with measurements — why the remaining "convergence redesign" is not warranted after the #4973-era fixes (#5357/#5358/#5359). Library: - View.Layout.cs: replace the "bogus" NeedsLayout region comment (called out in the issue) with an accurate description of the real invariant and the internal write sites; note why the setter is internal (test seam) pending full removal. - View.Layout.cs: LayoutSubViews now re-reads GetContentSize() after the SubViewLayout *event* too (it already did after the OnSubViewLayout virtual, #4863), so a handler on that event that calls SetContentSize is honored. Tests: - StaleContentSizeCaptureTests: +1 regression test for the SubViewLayout-event content-size case. - LayoutConvergenceTests (new): permanent guards locking in the deterministic properties established by #5357/#5358/#5359 — single-pass convergence, no spurious FrameChanged, bounded ancestor-chain fan-out (no subtree fan-out), Dim.Auto growth correctness, and Pos.Right sibling repositioning. - AllViewsRenderFingerprintTests (new): all-views smoke guard (every concrete View lays out + draws in design mode without throwing); also the harness used to prove the LayoutSubViews change is byte-identical in rendering across all 61 view types. Verification: UnitTestsParallelizable 17,382 passed; UnitTests.NonParallelizable 74 passed; all-views Driver.ToString() fingerprint identical with/without the library change. See plans/4522-needslayout-lifecycle.md for the full assessment and measured fan-out numbers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Back-merge v2.4.11 from main into develop
…otchas Fixes #5525. Document v2 agent gotchas
--- updated-dependencies: - dependency-name: Spectre.Console dependency-version: 0.57.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…-0.57.1 Bump Spectre.Console from 0.57.0 to 0.57.1
P1 (bug): AllViewsRenderFingerprintTests now separates filesystem environment exceptions (UnauthorizedAccessException / IOException from FileDialog EnableForDesign) from real layout/draw exceptions. ENV: exceptions are logged but do not count as test failures. P2a (weak test): StaleContentSizeCaptureTests regression for the SubViewLayout event reread is now a genuine guard. NewContentSize is nullable and armed AFTER EndInit; SetContentSize(20×10) establishes a concrete baseline. Without the production reread the test fails with Expected:50 / Actual:20 — confirmed by mutation testing. P2b (overclaim): The 'SetFrame NeedsLayout mark is load-bearing for Dim.Auto' comment and plan wording softened to reflect that synthetic tests don't exercise that path (child.Width= already marks ancestors); the ad-hoc guard is still not recommended for other reasons. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
AllViewsRenderFingerprintTests: - Fix resource leak: catch block now disposes view and driver before returning |EX:, matching all other exit paths - Track 'successful' (laid out + drawn without exception) separately from 'rendered' (total processed); assert successful > 30 so the smoke check is meaningful, not a type-count tautology StaleContentSizeCaptureTests: - Rename Test 1: LayoutSubViews_Uses_Stale_ContentSize_... -> LayoutSubViews_Honors_SetContentSize_From_OnSubViewLayout (method name and summary both claimed to prove a bug; assertions verify the fixed behavior) - Rename Test 3: Dialog_..._Diverges_From_Captured_Value -> Dialog_..._Does_Not_Diverge_From_Layout_Value; update summary to match (Assert.Equal asserts NO divergence, not divergence) plans/4522-needslayout-lifecycle.md: - Fix §18-26 mechanism description: SetNeedsLayout propagates upward only when SuperView.NeedsLayout is false; during a layout pass the SuperView is already marked so SetFrame's call is effectively a no-op. DimAuto resolves in-place via SetRelativeLayout(), not via a 'mark + re-layout next iteration' mechanism. Reconcile with §57. - Fix stale line references: :862/:864-865 -> :866/:868-869 for the NeedsLayout clear and SubViewsLaidOut fire sites Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
A view hosting a raster ImageView (UseRasterGraphics, Sixel/Kitty) left its out-of-band graphic on screen after it stopped being rendered — closing a modal Dialog, hiding the ImageView, or removing it (or its container) from the hierarchy. The cells were repainted, but the Sixel DCS / Kitty placement is not part of the cell grid, so it lingered until the next full-screen clear. Root cause: a RasterImageCommand lives in the driver-level output buffer keyed by the owning view, but nothing removed it when the owning view stopped drawing (only explicit Dispose / Image=null did). This is general to any overlapping ImageView, not specific to Dialog/Runnable. Fix: reconcile the buffer's raster images against the rendered view hierarchy on each draw. After View.Draw, the application collects the ids of raster images owned by views that are still rendered (CollectActiveRasterImageIds, overridden by ImageView and ProgressBar) and calls IOutputBuffer.RetainRasterImages to drop the rest. Dropping a command marks its covered cells dirty, so the next Refresh repaints them and emits the Kitty delete for vanished placements. - IOutputBuffer/OutputBufferImpl: add RetainRasterImages(activeIds) - View: add internal virtual CollectActiveRasterImageIds (skips invisible subtrees) - ImageView/ProgressBar: override to report their active raster id - ApplicationImpl.LayoutAndDraw: reconcile after drawing Tests (UnitTestsParallelizable): ImageView raster released when removed, hidden, its container removed, and when a modal runnable ends; plus direct RetainRasterImages buffer tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fd918d316
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| view?.CollectActiveRasterImageIds (activeRasterIds); | ||
| } | ||
|
|
||
| rasterBuffer.RetainRasterImages (activeRasterIds); |
There was a problem hiding this comment.
For a third-party view that calls the public IOutputBuffer.AddRasterImage API from OnDrawingContent, this cleanup removes the command immediately after View.Draw and before Driver.Refresh(): the only way to populate activeRasterIds is CollectActiveRasterImageIds, but that hook is internal virtual, so app assemblies cannot override it to report their ids. In that scenario custom Sixel/Kitty-rendering controls stop rendering entirely unless this retention mechanism is made accessible or newly added commands are retained.
Useful? React with 👍 / 👎.
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.
Release v2.4.12
Version:
2.4.12NuGet Package:
Terminal.Gui 2.4.12What happens when this PR is merged
v2.4.12main→developwill be openedChecklist
2.4.12