Release v2.4.16#5563
Merged
Merged
Conversation
…unchanged size The View.Text setter unconditionally called SetNeedsLayout(), which propagates layout work up the entire ancestor chain and forces a redraw on every text change - even when the recomputed Dim.Auto(DimAutoStyle.Text) size is identical to the current Frame (e.g. a once-per-second clock Label). When both Width and Height are exactly DimAutoStyle.Text and the view has been laid out, the setter now predicts the new Frame size by invoking the exact same Dim.Calculate path the layout engine uses (no reimplementation, so it cannot disagree with a real layout pass). If the size is unchanged, it skips SetNeedsLayout() and the ancestor propagation, marks TextFormatter.NeedsFormat and calls SetNeedsDraw() so the new content is still reformatted and redrawn. The optimization only ever avoids *setting* NeedsLayout; it never clears it, so a pre-existing pending layout request survives. DimAutoStyle.Auto, fixed dims, and not-yet-laid-out views all fall back to the original SetNeedsLayout() path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… fixed/one-axis Code review of #5549 found two correctness gaps and one scope limitation in the Text-change layout-skip optimization: 1. Position changes were ignored. The predictor compared only size to Frame, but a Pos can depend on Text (e.g. Pos.Func reading view.Text), so a same-size text change could still require a new Frame.X/Y and was wrongly skipped. 2. The "exactly Text auto" guard used dim.Has(...), which matches nested/composite dimensions (e.g. Dim.Auto(Text) + 2), letting location- or reference-sensitive composites into the simplified fast path. 3. The optimization was narrower than #5499 intended - it only handled both axes being DimAutoStyle.Text and forced layout for fixed-size and one-axis-auto text changes that are safe to skip. Fixes: - Extracted SetRelativeLayout's Frame computation into TryComputeRelativeFrame so the speculative check and the real layout share one source of truth. The predictor now resolves and compares the full Rectangle (position included), so a text-dependent Pos that moves the view correctly triggers layout. - The eligibility guard is now exact (`dim is DimAbsolute or DimAuto { Style: DimAutoStyle.Text }`). Composite/nested dims and DimAutoStyle.Content/.Auto (which lay out subviews while calculating) fall back to SetNeedsLayout. - Broadened to fixed dimensions and one-axis-auto, the common safe cases: a fixed or Text-only-auto view whose resolved Frame is unchanged now skips layout and only redraws. Tests updated/added: Pos-depends-on-Text moves view (regression for #1), composite dim bypasses fast path (#2), one-axis-auto same/different size, and fixed-size text redraw skips layout (#3). Full UnitTestsParallelizable: 17458 passed / 0 failed; UnitTests.NonParallelizable: 72 passed / 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A new review found the widened same-frame fast path could leave TextFormatter unconstrained. UpdateTextFormatterText() clears ConstrainToWidth/Height, and the fast path then skipped SetRelativeLayout() — including its post-resolve cleanup that restores constraints from GetContentWidth()/GetContentHeight(). For a fixed-size view with WordWrap, ConstrainToWidth stayed null (treated as int.MaxValue by GetLines()), so wrapped/clipped text could render incorrectly until some later layout restored the constraints. - Extracted that finalization into FinalizeTextFormatterConstraints(), shared by SetRelativeLayout() and the fast path, so the fast path leaves TextFormatter in exactly the state a full layout pass would. - Renamed the helper to TryRedrawWithoutLayout() to reflect that it now performs the redraw-only handling (finalize constraints, mark NeedsFormat, SetNeedsDraw) rather than just answering a question. Deep review (temporary probe harnesses, since removed) compared the isolated fast path against a full-layout control for Frame, both TextFormatter constraints, and rendered driver contents across a sweep of width/height dims (Text-auto, min/max, fixed) x WordWrap x a text-change sequence — all identical, and the sweep was confirmed to catch the regression when the finalization was removed. Also probed CheckBox (overrides UpdateTextFormatterText), Pos.Align sibling groups, fixed parents with subviews, reentrant TextChanged, and a 100-tick clock simulation — no unintended consequences. Permanent coverage added: fixed-size and one-axis-auto constraint survival with WordWrap, repeated-update frame-drift/ancestor-propagation, reentrancy, and render-equivalence tests (TextFastPathRenderTests) covering fixed-size wrap/clip and max-constrained auto wrapping. Full UnitTestsParallelizable: 17467 passed / 0 failed; UnitTests.NonParallelizable: 72 passed / 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Back-merge v2.4.14-beta.1 from main into develop
…-dim-auto-unchanged
release.yml ("Create Release") is broken and redundant:
- Broken: it pins GitVersion `versionSpec: '6.0.x'`, but
gittools/actions/gitversion/setup@v4.5.0 requires >=6.1.0, so the
"Install GitVersion" step fails immediately (every run since Jan 2026
has failed at this step). Its siblings publish.yml / prepare-release.yml
correctly use '6.x'.
- Redundant: releases are produced by the PR-based flow — Prepare Release
(develop → main release PR) + Finalize Release (tag, GitHub Release,
back-merge) — with publish.yml triggered by the `v*` tag.
Removes the workflow and updates all references:
- Terminal.slnx: drop the release.yml file entry
- Terminal.Gui/README.md: drop the legacy workflow table row
- .github/workflows/README.md: drop the "Create Release" section,
renumber, and fix the publish trigger description
- CONTRIBUTING.md: document the Prepare/Finalize release flow instead
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes 3 doc accuracy issues flagged in review: - .github/workflows/README.md: api-docs.yml triggers on push to `develop` + workflow_dispatch, not `main` - Terminal.Gui/README.md: publish.yml triggers on push to `develop` and `v*` tags, not `main` - CONTRIBUTING.md: clarify Prepare Release creates `release/<tag>` where the tag includes the pre-release suffix (e.g. release/v2.0.0-beta.1) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same trigger inaccuracies as the prior fixes, elsewhere in Terminal.Gui/README.md: - Prepare Release branch is `release/<tag>` including the pre-release suffix (e.g. release/v2.0.0-beta.1) - NuGet auto-publish is on push to `develop` + `v*` tag pushes, not on pushes to `main` - API docs deploy on push to `develop`, not `main` Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove legacy "Create Release" workflow (release.yml)
ConvertValue fell back to TypeDescriptor.GetConverter (RequiresUnreferencedCode / RequiresDynamicCode) for non-scalar settings, breaking NativeAOT/trimmed consumers (e.g. tui-cs/Editor's `ted`) with IL2026/IL3050. Regression from #5411. - ConvertValue: replace the TypeDescriptor.GetConverter fallback with an explicit Key fast path (Key.TryParse). Key was the only non-scalar/non-enum type bound via flat dotted keys (MenuBar.DefaultKey, PopoverMenu.DefaultKey); all other settings are string/bool/int/Rune/enum and already had fast paths. The public binding path now has no RequiresUnreferencedCode/RequiresDynamicCode dependency. - BindSection<T>/BindFlatDottedKeys<T>: annotate T with [DynamicallyAccessedMembers(PublicProperties)] so typeof(T).GetProperties() is trim-safe (fixes IL2090). Callers pass concrete settings types, so it is satisfiable; this preserves the properties the trimmer would otherwise drop. - Drop the now-unused System.ComponentModel using and the no-longer-needed IL2026 suppression on BindFlatDottedKeys. Verified with a trimmed publish of a consumer app: TuiConfigurationBuilder now produces zero IL trim/AOT warnings (previously IL2026 + IL2090). Adds a regression test that PopoverMenu.DefaultKey (a Key-typed flat dotted key) binds via the new fast path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes #5561 - Make TuiConfigurationBuilder flat-key binding trim/AOT-safe
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.16
Version:
2.4.16NuGet Package:
Terminal.Gui 2.4.16What happens when this PR is merged
v2.4.16main→developwill be openedChecklist
2.4.16