FIXES #4367 - BREAKING CHANGE - Remove legacy ConfigurationManager after MEC migration - #5416
FIXES #4367 - BREAKING CHANGE - Remove legacy ConfigurationManager after MEC migration#5416tig wants to merge 27 commits into
Conversation
- Mark spec status as implementation-in-progress with #5411 focus - Add Phase 3A detailing remaining complex-type migration scope - Define #5411 done criteria for themes, schemes, key bindings, Colors16 - Clarify #5416 as follow-up for CM cleanup/removal phases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…facade, TuiSerializerContext extraction Pulled forward from #5416 to make the MEC story in #5411 coherent without expanding scope to JSON-shape changes or CM deletion. A1: IThemeManager gains a ThemeChanged event. MecThemeManager subscribes to legacy ThemeManager.ThemeChanged in its ctor and forwards. Runtime theme/scheme data still lives in legacy CM; A2 (Mec owning theme data) remains in #5416 because it requires the config.json shape decision. B: New static facade Terminal.Gui.Configuration.ThemeChanges bridges both ConfigurationManager.Applied and ThemeManager.ThemeChanged into a single ThemeChanged event. Menu, MenuBar, StatusBar, and LineCanvas now subscribe to ThemeChanges.ThemeChanged instead of ConfigurationManager.Applied. ConfigurationManager.Applied stays public+obsolete for external consumers. C: Extracted the configured SourceGenerationContext instance to a non-obsolete internal class TuiSerializerContext (Instance). The obsolete ConfigurationManager.SerializerContext delegates to it for back-compat. Updated all 7 internal JSON-converter consumers to use TuiSerializerContext.Instance, removing CS0618 pragmas from AttributeJsonConverter, SchemeJsonConverter, DictionaryJsonConverter, and ConcurrentDictionaryJsonConverter. DeepCloner, ScopeJsonConverter, and SourcesManager retain their pragmas for other obsolete CM uses. Spec updated with a new Phase 3A.x section documenting what landed in #5411 and why A2/D/E/PrintJsonErrors are deferred to #5416. All 17283 UnitTestsParallelizable tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Companion to specs/replace-cm-with-mec.md. Defines the scope, design, and phased execution of fully removing CM after the MEC migration in #5411. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes from scope: ConfigurationManager.Applied subscriber rewiring (done via ThemeChanges in #5411), JSON converter SerializerContext sweep (done via TuiSerializerContext in #5411). Adds Phase A2 (Mec managers own runtime theme/scheme data) as the gating prerequisite for ScopeJsonConverter deletion. Sharpens D-02 decision (Option alpha: nested-only + TuiConfigMigrator; Option beta: custom legacy MEC source). Corrects PrintJsonErrors framing — behavior-preserving replacement is possible via JsonConfigurationSource.OnLoadException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- AOT baseline (post-#5411 @ 83ded73): NativeAot.exe = 22.77 MB, Terminal.Gui.dll = 1.77 MB, ConfigPropertyHostTypes roots 31 types. - Examples inventory: ~115 files but ~105 are the same one-line CM.Enable() call; identifies the 10 non-trivial scenarios that need per-site review (ConfigurationEditor, Runner, UICatalogRunnable, etc.). - Test inventory: ~13 files to delete, ~11 to keep, ~2 benchmarks to port. Flags glyph + apply-over-defaults behaviors that need MEC-side ports rather than straight deletion. - Records the reproduce command + threshold for the eventual size delta. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6e9395d to
a73f6a7
Compare
Per source session decision: - Spec section 5.4 rewritten: pure-nested MEC read path + ~20 LOC peek-and-warn in TuiConfigurationBuilder.AddTuiJsonFile. Legacy shapes are NOT parsed; affected settings fall through to defaults. - Phase D scope shrinks: no TuiConfigMigrator, no LegacyTuiConfigurationSource, no round-trip tests. Two tests only (flat-key warn, array-themes warn). - Standalone Tools/MigrateConfig/ console app (not shipped in Terminal.Gui.dll) as migration aid; deletable any time. - Records explicitly-rejected alternatives (keep-both, silent-translate, throw) so future reviewers don't re-litigate. - Updated section 7 JSON-breaking-change wording and section 8 risk row to reflect the warn-and-default contract. - Prep artifact section 5 updated to reflect the now-unblocked Phase D step list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Standalone .NET 10 console utility, ~190 LOC including comments. Not included in Terminal.slnx and not part of any shipping artifact - exists solely so users on the legacy flat-key config.json shape have a one-shot upgrade path when the library stops parsing it. Transforms: - Top-level dotted property names split into nested objects. - 'Themes' array-of-single-key-objects collapses to dict. - 'Schemes' inside a theme follows the same collapse. Verified end-to-end on Terminal.Gui/Resources/config.json: 52 KB flat input -> 49 KB nested output, structurally correct. Includes Tools/README.md establishing the folder convention (not Examples, not in Terminal.slnx, deletable any release) and Tools/MigrateConfig/README.md with usage + lifecycle notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion only) Step 1 of N in the Phase A2/D commit chain for #5416. Lands the bind target for the post-D-rewrite nested `Themes` section in config.json. No production code consumes ThemeDefinition yet; the consumer (rewired MecThemeManager reading via IOptionsMonitor<ThemeSettings>) lands in a subsequent commit. Binding correctness validated by tests against in-memory MEC providers. Shape (named explicitly so reviewers can object to specific subsections): public class ThemeDefinition { public Dictionary<string, Scheme>? Schemes; // 18 nullable per-component override POCOs, matching every ThemeScope- // flavored BindSection<T> call in TuiConfigurationBuilder.ApplyToStaticFacades: public ButtonSettings? Button; public CheckBoxSettings? CheckBox; public CharMapSettings? CharMap; public DialogSettings? Dialog; public FrameViewSettings? FrameView; public HexViewSettings? HexView; public LinearRangeSettings? LinearRange; public MenuBarSettings? MenuBar; public MenuSettings? Menu; public MessageBoxSettings? MessageBox; public NerdFontsSettings? NerdFonts; public PopoverMenuSettings? PopoverMenu; public SelectorBaseSettings? SelectorBase; public StatusBarSettings? StatusBar; public TextFieldSettings? TextField; public TextViewSettings? TextView; public WindowSettings? Window; public GlyphSettings? Glyphs; // JSON section name is "Glyphs", not "Glyph" } Design choice: null = no theme-level override. Considered alternatives, rejected: (a) "Missing entry in dictionary" - forces a stringly-typed lookup; loses the IDE/compiler awareness that a strongly-typed nullable property gives. (b) "Explicit empty object" - makes "I appear in JSON but override nothing" indistinguishable from "I appear in JSON to assert my-own-values-as-overrides". Nullability avoids that ambiguity at the cost of zero ergonomics. Whether a non-null subsection (i) wholesale-replaces the root *Settings or (ii) property-level merges with it is a manager-rewire concern and is NOT encoded in the POCO. Both consumption strategies are compatible with this shape. Surfaced as an open design question for the PR thread. Source-gen registrations added for ThemeDefinition and Dictionary<string, ThemeDefinition>. These are additive, AOT-safe, and meaningful even before any consumer exists (they unblock the next commit without further SourceGenerationContext edits). Tests (Tests/UnitTestsParallelizable/Configuration/ThemeDefinitionBindingTests.cs): - Bind_FullAndPartialThemes_PartialHasNullsInOmittedSubsections - binds a nested sample with one fully-populated theme and one partial-override theme, asserts the partial theme has nulls in every unmentioned subsection. - Bind_SchemesDictionaryInsideTheme_PopulatesSchemes - binds Schemes as a Dictionary<string, Scheme> inside a ThemeDefinition. Verifies the MEC reflection binder handles Scheme directly via its parameterless ctor + init-only Normal property (PASSES - no SchemeDefinition DTO wrapper needed). - Bind_EmptyThemesSection_ProducesEmptyDictionary - degenerate case. All tests use AddJsonStream against in-memory JSON, not Resources/config.json, so they remain valid while the embedded library config keeps its legacy flat shape through Commits A and B. Explicit deferrals (subsequent commits): - MecThemeManager/MecSchemeManager rewire to consume ThemeDefinition via IOptionsMonitor<ThemeSettings>. - SwitchTheme overlay/replace algorithm. - Adding `Themes: Dictionary<string, ThemeDefinition>` to ThemeSettings. - TuiConfigurationBuilder.ApplyToStaticFacades themes binding. - Removal of legacy `ThemeManager.Theme = ...` and `.GetThemeNames()` delegation. - Resources/config.json rewrite to nested. - Peek-and-warn for legacy shapes. - ScopeJsonConverter deletion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Before mass-converting 18 *Settings POCOs to records with { get; internal set; }
per the A2 design contract, validated the binder's behavior on the proposed
accessibility shapes via a focused spike.
Three Facts in MecBinderAccessibilitySpike, all passing, documenting observed
behavior against this MEC version:
1. TwoPassBind_InternalSet_SilentlyIgnoredByBinder
`{ get; internal set; }` is NOT written by Bind(existingInstance) under
default BinderOptions. Both passes complete with no exception and the
POCO stays at constructor defaults. Default BindingFlags = Public|Instance
exclude internal accessors.
2. TwoPassBind_InitOnly_OverlaysCorrectly
`{ get; init; }` IS written by Bind(existingInstance). Root-pass populates
property A; subsequent overlay pass writes property B without disturbing A.
Two-pass merge semantics are preserved end-to-end.
3. TwoPassBind_InternalSet_WorksWithBindNonPublicProperties
Opting into o.BindNonPublicProperties = true at each Bind() call site
rescues internal set. Trade-off: extra trim hint, non-default code path,
two-line invocation at every bind site.
Implication for the A2 manager-rewire commit: do NOT use internal set as the
sender's design contract proposed. Two viable replacements:
(A) Use { get; init; } on all 18 *Settings records. Works with MEC's default
binder. Preserves immutability to public consumers. No accessibility
escape hatch. Recommended.
(B) Keep { get; internal set; } and opt into BindNonPublicProperties at every
MecThemeManager bind call. Works, but adds a code-path divergence from the
idiomatic MEC pattern and requires manual maintenance to stay enabled.
Recommendation: (A). The sender's stated objection to init ("doesn't work via
all reflection binder paths") is falsified by Fact 2 in this MEC version. If
the sender has a specific reflection path in mind where init breaks (e.g. AOT
source-gen binder for top-level Configure<T>(), as distinct from Bind(instance)),
that should be named so we can validate with another spike.
No production code changed in this commit. Spike file documents the wall so the
sender can re-litigate the design choice before the 430+ call-site rollout.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rrent swap
Lands the heart of A2: replaces the mutable `*Settings.Defaults` pattern with
immutable `sealed record` POCOs and a `Volatile`-swapped `Current` property,
binding each ThemeScope POCO through MEC's two-pass overlay
(root section + `Themes:<active>:<section>`).
Pattern (per POCO):
public sealed record FooSettings
{
public T Prop { get; init; } = ...;
public static FooSettings Default { get; } = new ();
public static FooSettings Current
{
get => Volatile.Read (ref _current);
internal set => Volatile.Write (ref _current, value);
}
private static FooSettings _current = Default;
}
Why `init` (not `internal set`): the spike at
Tests/UnitTestsParallelizable/Configuration/MecBinderAccessibilitySpike.cs
(commit 84dbd6a) proves that `Bind(existingInstance)` silently ignores
`internal set` under default BindingFlags=Public|Instance, while `init`
accessors are written normally. `init` keeps the binder happy without an
opt-in `BindNonPublicProperties=true` at every call site.
Why a record (not a class) + `with`-swap setter on the view facade:
records give us a free `with` expression for atomic replacement. The static
View facades (e.g. `Button.DefaultShadow`) now read `Current.X` on get and do
`Current = Current with { X = value }` on set. That keeps the legacy CM
`ConfigProperty.Apply` reflection-write path functional during the transition
(it calls `PropertyInfo.SetValue (null, value)` against the static facade,
which still works), while MEC writes `Current` wholesale.
Two-pass overlay (`TuiConfigurationBuilder.BindThemeScope<T>`):
T next = new ();
config.GetSection (sectionName).Bind (next); // root
config.GetSection ($"Themes:{activeTheme}:{sectionName}").Bind (next); // overlay
apply (next); // atomic Volatile.Write to Current
MEC's `Bind(existing)` only writes properties present in JSON, so unmentioned
overlay properties survive — property-level merge for free, matching legacy
CM `Scope.Apply` semantics. No `Merge<T>` helper, no DeepCloner equivalent.
Converted (17 POCOs):
ButtonSettings, CheckBoxSettings, CharMapSettings, DialogSettings,
FrameViewSettings, HexViewSettings, LinearRangeSettings, MenuBarSettings,
MenuSettings, MessageBoxSettings, NerdFontsSettings, PopoverMenuSettings,
SelectorBaseSettings, StatusBarSettings, TextFieldSettings,
TextViewSettings, WindowSettings.
View facades updated to `with`-swap (16 files, ~28 setter pairs):
Button.cs, CheckBox.cs, CharMap.cs, Dialog.cs, FrameView.cs, HexView.cs,
LinearRangeDefaults.cs, Menu.cs, MenuBar.cs, PopoverMenu.cs, MessageBox.cs,
SelectorBase.cs, StatusBar.cs, TextField.cs, TextView.cs, Window.cs,
Text/NerdFonts.cs.
Deferred to A2.2: `Glyphs` facade redesign. `GlyphSettings` keeps its
mutable `Defaults` pattern in this commit; `TuiConfigurationBuilder` still
uses `BindSection<GlyphSettings>` for it. 288 call sites under
`Glyphs.X` get the dedicated commit there.
Deferred to A2.3: `NerdFonts` static facade redesign (the POCO is converted
here; the facade still has a setter that does `with`-swap).
Deferred to A2.4: removal of uncalled public static setters on View types.
Out of scope (kept mutable, SettingsScope not ThemeScope):
ApplicationSettings, DriverSettings, FileDialogSettings,
FileDialogStyleSettings, KeySettings, ThemeSettings, TraceSettings.
Tests:
- New `ThemeOverlayMergeTests` (3 Facts) validates two-pass merge:
overlay-overrides-root, no-overlay-uses-root, atomic-swap-does-not-mutate.
- Updated `MecSettingsTests.StaticFacade_CanBeOverridden` to use `Current`.
- Full suites green: 17,275 / 0 / 17 parallelizable; 72 / 0 / 2
non-parallelizable.
Cross-assembly `init`: MEC's source-gen binder emits direct assignments for
`Bind(existing)`, which would fail cross-assembly against `init`-only
properties. Since `MecThemeManager` and `TuiConfigurationBuilder` live in
`Terminal.Gui.dll` alongside the POCOs, intra-assembly calls compile fine.
End-users hypothetically calling `cfg.Bind(ButtonSettings.Current)` from
their app would hit a compile error — but no real consumer does this; they
read `Current.X`. Reflection binder fallback works for that case anyway.
Refs #4943 (CM-to-MEC replacement spec, A2).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Converts GlyphSettings to the immutable-record + atomic-swap Current
pattern (matching A2.1's 17 ThemeScope POCOs) and collapses the Glyphs
static facade from a CM-reflection target into a read-only projection
over GlyphSettings.Current.
Changes
=======
Terminal.Gui/Configuration/Settings/GlyphSettings.cs
- public class -> public sealed record.
- ~143 Rune properties: { get; set; } -> { get; init; }.
- Replaces static `Defaults` block with:
Default (compile-time truth, never reassigned)
Current (Volatile-read/write, internal set)
_current (private backing, init'd to Default).
- Bind target shape now matches all 17 A2.1 POCOs.
Terminal.Gui/Drawing/Glyphs.cs
- All 144 properties rewritten from
[ConfigurationProperty]
public static Rune NAME { get => Defaults.NAME; set => Defaults.NAME = value; }
to bare expression-bodied readers:
public static Rune NAME => GlyphSettings.Current.NAME;
- Setters and [ConfigurationProperty] attributes fully removed.
- File-level comment about "generates default config" dropped; the
SaveDefaults reflection mechanism that produced that text is dead
now that Glyphs has no [ConfigurationProperty] surface. The
"Resources/config.json is source of truth at runtime" half stays
accurate (theme overlay can still override compile-time defaults).
- Call sites in Terminal.Gui/, Tests/, Examples/ unchanged: every
Glyphs.NAME reader keeps working; only the host changed.
Terminal.Gui/Configuration/Settings/TuiConfigurationBuilder.cs
- Glyphs binding switched from BindSection<GlyphSettings> to
BindThemeScope<GlyphSettings> (root -> overlay -> atomic publish),
matching the other 17 theme-overlay POCOs.
- activeTheme snapshot now has a TODO(A2) marker for the future
ThemeSettings record conversion (review flag #4).
Terminal.Gui/Configuration/ConfigPropertyHostTypes.cs
- Drops `typeof (Glyphs)` from the `_types` list and removes the
matching `[DynamicDependency (PRESERVED_MEMBERS, typeof (Glyphs))]`.
Glyphs is no longer a CM reflection host.
Terminal.Gui/Configuration/SourceGenerationContext.cs
- Drops `[JsonSerializable (typeof (Glyphs))]`. Glyphs has no JSON
bind state of its own; `GlyphSettings` is the bind target and is
already covered elsewhere.
specs/remove-legacy-cm.md
- §4.2 row for `Settings/*Settings.cs` annotated with the SettingsScope
vs ThemeScope pattern divergence rationale (review flag #3).
Tests/UnitTestsParallelizable/Configuration/SourcesManagerTests.cs
- Three Skip markers added with the rationale
"A2.2: Glyphs lost [ConfigurationProperty]; Resources/config.json
Glyphs.X flat keys are now CM-unknown. Test removed with CM in
step D."
affecting:
Load_WithValidResource_UpdatesSettingsScope
Load_Runtime_Overrides
Load_AddsResourceSourceToCollection
These tests load `Terminal.Gui.Resources.config.json` through CM's
SourcesManager.Load, which deserializes flat `Glyphs.X` keys via
ScopeJsonConverter looking up ConfigProperty hosts. With Glyphs no
longer registered, that path throws a JsonException -> Load returns
false. The legacy CM contract these tests assert (flat-key resolve
against reflection-discovered hosts) is exactly what A2.2 removes
for Glyphs and what step D removes wholesale. Skipping is correct;
the tests die with CM.
Test results
============
Tests/UnitTestsParallelizable: total 17292 / 17272 passed / 0 failed
/ 20 skipped (baseline post-A2.1 was 17275/0/17; delta is exactly
the 3 new SourcesManagerTests skips).
Tests/UnitTests.NonParallelizable: total 74 / 72 passed / 0 failed /
2 skipped (unchanged from baseline).
Design context
==============
This is commit A2.2 of the A2 series (POCO ownership migration on
PR #5416, stacked on copilot/replace-cm-with-mec). A2.1 (commit
2f7c13a) landed the 17 ThemeScope POCOs and BindThemeScope<T>. A2.2
brings the 18th (GlyphSettings) and finishes the Glyphs facade. A2.3
will repeat the facade redesign for NerdFonts. A2.4 will remove dead
public static setters on Button.DefaultShadow etc.
Cross-session review (PR #5411 owner) signed off on:
- { get; init; } on the 18 records (concedes prior internal-set
recommendation; init works on intra-assembly Bind(existing))
- Glyphs file-comment cleanup (drop the SaveDefaults half)
- Three Skip markers on SourcesManagerTests as the resolution path
for the CM-vs-Glyphs schema mismatch
Deferred (per A2 contract)
==========================
- `with`-swap setters on the view facades have a non-atomic read-modify-
write window vs. MEC's Volatile.Write; zero practical impact (single-
threaded reflection apply path). Bridge code removes in A2.4 / CM
deletion, eliminating the only lost-write race on Current.
- BindThemeScope<T>'s [UnconditionalSuppressMessage] still cites
ConfigPropertyHostTypes. When CM deletes in step D, the citation
will be re-justified against TuiSerializerContext's JsonSerializable
entries (or wired through explicit [DynamicDependency] per POCO).
- ThemeSettings itself stays mutable in this commit (record conversion
is a future micro-commit; non-blocking for A2.2 review).
- Resources/config.json still ships Glyphs.X as flat keys. That JSON
shape is part of step B/D (nested-section rewrite), not A2.2.
Refs: A2.1 = 2f7c13a, stacked on copilot/replace-cm-with-mec.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Converts GlyphSettings to the immutable-record + atomic-swap Current
pattern (matching A2.1's 17 ThemeScope POCOs) and collapses the Glyphs
static facade from a CM-reflection target into a read-only projection
over GlyphSettings.Current.
Changes
=======
Terminal.Gui/Configuration/Settings/GlyphSettings.cs
- public class -> public sealed record.
- ~143 Rune properties: { get; set; } -> { get; init; }.
- Replaces static `Defaults` block with:
Default (compile-time truth, never reassigned)
Current (Volatile-read/write, internal set)
_current (private backing, init'd to Default).
- Bind target shape now matches all 17 A2.1 POCOs.
Terminal.Gui/Drawing/Glyphs.cs
- All 144 properties rewritten from
[ConfigurationProperty]
public static Rune NAME { get => Defaults.NAME; set => Defaults.NAME = value; }
to bare expression-bodied readers:
public static Rune NAME => GlyphSettings.Current.NAME;
- Setters and [ConfigurationProperty] attributes fully removed.
- File-level comment about "generates default config" dropped; the
SaveDefaults reflection mechanism that produced that text is dead
now that Glyphs has no [ConfigurationProperty] surface. The
"Resources/config.json is source of truth at runtime" half stays
accurate (theme overlay can still override compile-time defaults).
- Call sites in Terminal.Gui/, Tests/, Examples/ unchanged: every
Glyphs.NAME reader keeps working; only the host changed.
Terminal.Gui/Configuration/Settings/TuiConfigurationBuilder.cs
- Glyphs binding switched from BindSection<GlyphSettings> to
BindThemeScope<GlyphSettings> (root -> overlay -> atomic publish),
matching the other 17 theme-overlay POCOs.
- activeTheme snapshot now has a TODO(A2) marker for the future
ThemeSettings record conversion (review flag #4).
Terminal.Gui/Configuration/ConfigPropertyHostTypes.cs
- Drops `typeof (Glyphs)` from the `_types` list and removes the
matching `[DynamicDependency (PRESERVED_MEMBERS, typeof (Glyphs))]`.
Glyphs is no longer a CM reflection host.
Terminal.Gui/Configuration/SourceGenerationContext.cs
- Drops `[JsonSerializable (typeof (Glyphs))]`. Glyphs has no JSON
bind state of its own; `GlyphSettings` is the bind target and is
already covered elsewhere.
specs/remove-legacy-cm.md
- §4.2 row for `Settings/*Settings.cs` annotated with the SettingsScope
vs ThemeScope pattern divergence rationale (review flag #3).
Tests/UnitTestsParallelizable/Configuration/SourcesManagerTests.cs
- Three Skip markers added with the rationale
"A2.2: Glyphs lost [ConfigurationProperty]; Resources/config.json
Glyphs.X flat keys are now CM-unknown. Test removed with CM in
step D."
affecting:
Load_WithValidResource_UpdatesSettingsScope
Load_Runtime_Overrides
Load_AddsResourceSourceToCollection
These tests load `Terminal.Gui.Resources.config.json` through CM's
SourcesManager.Load, which deserializes flat `Glyphs.X` keys via
ScopeJsonConverter looking up ConfigProperty hosts. With Glyphs no
longer registered, that path throws a JsonException -> Load returns
false. The legacy CM contract these tests assert (flat-key resolve
against reflection-discovered hosts) is exactly what A2.2 removes
for Glyphs and what step D removes wholesale. Skipping is correct;
the tests die with CM.
Non-Default theme Glyph dormancy
================================
The flat `Glyphs.X` keys inside non-Default theme subsections of
Resources/config.json (TurboPascal 5, Anders, Dark, Light, etc.) are
dormant from this commit through step D. Active theme at startup is
"Default", whose theme block in config.json is intentionally empty -
so default startup is structurally unchanged. Glyph values for the
Default theme come from GlyphSettings's `init` defaults, which by
spec are the canonical Default values.
Theme switching (MecThemeManager.SwitchTheme("TurboPascal 5") etc.)
does NOT apply Glyph overrides during this window:
- CM path: [ConfigurationProperty] removed from Glyphs in A2.2;
ScopeJsonConverter no-ops the keys.
- MEC path: BindThemeScope<GlyphSettings> reads section
Themes:<name>:Glyphs, but the flat key form
`"Glyphs.LeftBracket"` inside the theme block is treated by
MEC as a literal top-level-of-theme key, not a Glyphs:LeftBracket
nested path.
Step D's config.json rewrite to nested form reactivates Glyph
overrides for all theme-overlay POCOs uniformly. This dormancy is
the same window that applies to every other ThemeScope POCO's flat
keys in non-Default themes; Glyphs is not special.
Test results
============
Tests/UnitTestsParallelizable: total 17292 / 17272 passed / 0 failed
/ 20 skipped (baseline post-A2.1 was 17275/0/17; delta is exactly
the 3 new SourcesManagerTests skips).
Tests/UnitTests.NonParallelizable: total 74 / 72 passed / 0 failed /
2 skipped (unchanged from baseline).
Design context
==============
This is commit A2.2 of the A2 series (POCO ownership migration on
PR #5416, stacked on copilot/replace-cm-with-mec). A2.1 (commit
2f7c13a) landed the 17 ThemeScope POCOs and BindThemeScope<T>. A2.2
brings the 18th (GlyphSettings) and finishes the Glyphs facade. A2.3
will repeat the facade redesign for NerdFonts. A2.4 will remove dead
public static setters on Button.DefaultShadow etc.
Cross-session review (PR #5411 owner) signed off on:
- { get; init; } on the 18 records (concedes prior internal-set
recommendation; init works on intra-assembly Bind(existing))
- Glyphs file-comment cleanup (drop the SaveDefaults half)
- Three Skip markers on SourcesManagerTests as the resolution path
for the CM-vs-Glyphs schema mismatch
Deferred (per A2 contract)
==========================
- `with`-swap setters on the view facades have a non-atomic read-modify-
write window vs. MEC's Volatile.Write; zero practical impact (single-
threaded reflection apply path). Bridge code removes in A2.4 / CM
deletion, eliminating the only lost-write race on Current.
- BindThemeScope<T>'s [UnconditionalSuppressMessage] still cites
ConfigPropertyHostTypes. When CM deletes in step D, the citation
will be re-justified against TuiSerializerContext's JsonSerializable
entries (or wired through explicit [DynamicDependency] per POCO).
- ThemeSettings itself stays mutable in this commit (record conversion
is a future micro-commit; non-blocking for A2.2 review).
- Resources/config.json still ships Glyphs.X as flat keys. That JSON
shape is part of step B/D (nested-section rewrite), not A2.2.
Refs: A2.1 = 2f7c13a, stacked on copilot/replace-cm-with-mec.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
54ca1c3 to
441ef60
Compare
Mops up the small static facade for NerdFonts.Enable, matching the
A2.2 pattern used for the Glyphs facade. NerdFontsSettings was already
converted to a sealed record + Default/Current in A2.1; A2.3 just
removes the CM-reflection scaffolding on the consumer-facing static
facade.
Changes
=======
Terminal.Gui/Text/NerdFonts.cs
- NerdFonts.Enable rewritten from
[ConfigurationProperty (Scope = typeof (ThemeScope))]
public static bool Enable
{
get => NerdFontsSettings.Current.Enable;
set => NerdFontsSettings.Current = NerdFontsSettings.Current with { Enable = value };
}
to a bare expression-bodied reader:
public static bool Enable => NerdFontsSettings.Current.Enable;
- [ConfigurationProperty] attribute removed.
- `with`-swap setter removed; NerdFontsSettings.Current is now
exclusively written by MecThemeManager via BindThemeScope<T>.
- Caller surface unchanged: every NerdFonts.Enable reader keeps
working; only the host changed.
Terminal.Gui/Configuration/ConfigPropertyHostTypes.cs
- Drops `typeof (NerdFonts)` from the `_types` list and removes
the matching [DynamicDependency (PRESERVED_MEMBERS, typeof
(NerdFonts))]. NerdFonts is no longer a CM reflection host.
No Resources/config.json change needed
======================================
Grep against `Resources/config.json` for `NerdFonts` returns zero
matches; the file has never carried NerdFonts.X overrides. The A2.2
non-Default-theme dormancy footnote therefore does not apply here.
NerdFonts.Enable resolves to the C# init default
(NerdFontsSettings.Default.Enable = false) at startup and remains so
unless a consumer assigns NerdFontsSettings.Current via MEC binding,
which today happens only via TuiConfigurationBuilder's
BindThemeScope<NerdFontsSettings> against a (currently absent) MEC
section.
Test results
============
Tests/UnitTestsParallelizable: total 17292 / 17272 passed / 0 failed
/ 20 skipped (unchanged from A2.2; no new skips, no regressions).
Tests/UnitTests.NonParallelizable: total 74 / 72 passed / 0 failed /
2 skipped (unchanged).
Design context
==============
This is commit A2.3 of the A2 series (POCO ownership migration on
PR #5416, stacked on copilot/replace-cm-with-mec). A2.1 (2f7c13a)
landed the 17 ThemeScope POCOs and BindThemeScope<T>. A2.2 (441ef60
post-amend) landed the 18th (GlyphSettings) + Glyphs facade. A2.3
mops up NerdFonts. A2.4 will remove dead public static setters on
Button.DefaultShadow etc.
Cross-session review (PR #5411 owner) signed off on:
- One-line reader pattern for NerdFonts.Enable; drop the
`with`-swap setter that bridged CM's reflection write path.
- ConfigPropertyHostTypes row removal mirrors A2.2's Glyphs
treatment.
Deferred (per A2 contract)
==========================
- A2.4: removal of dead public static setters on view facades
(Button.DefaultShadow, etc.) -- next commit.
- Step D: CM deletion, config.json schema rewrite, BindThemeScope<T>
suppression re-justification.
Refs: A2.1 = 2f7c13a, A2.2 = 441ef60, stacked on
copilot/replace-cm-with-mec.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes [ConfigurationProperty (Scope = typeof (ThemeScope))] from
all view-class static facade properties (Button.DefaultShadow,
Dialog.DefaultBorderStyle, etc.) and collapses their bridge get/set
bodies to bare expression-bodied readers, matching the A2.2 Glyphs
and A2.3 NerdFonts patterns.
These properties existed solely as CM-reflection bind targets — the
`with`-swap setter (`Current = Current with { X = value }`) was the
only writer, called by ConfigProperty.Apply via PropertyInfo.SetValue
against the embedded Resources/config.json flat keys. Audit confirmed
zero external callers of the setters; tests that read the getters
(e.g. ButtonTests asserting `button.ShadowStyle == Button.DefaultShadow`)
continue to work — only the host plumbing changed.
Net effect at runtime
=====================
For every affected property:
- Default theme: value comes from `<X>Settings.Default.<Prop>`'s
C# init default. Unchanged.
- Non-Default themes (Dark, Light, TurboPascal 5, Anders, etc.):
flat-key overrides like `"Button.DefaultShadow": "Opaque"` in
Resources/config.json are dormant from this commit through step D.
Same scope as the A2.2 Glyphs dormancy: CM path no longer matches
these keys (the [ConfigurationProperty] hosts are gone) and MEC
path can't read them (the JSON is still flat-keyed, not nested).
Step D rewrites Resources/config.json to nested form, which
reactivates non-Default theme view-facade overrides for all
theme-overlay POCOs uniformly via the existing
BindThemeScope<ButtonSettings> / <DialogSettings> / etc. calls in
TuiConfigurationBuilder.ApplyToStaticFacades.
Files changed
=============
Terminal.Gui/Views/Button.cs (2 props)
Terminal.Gui/Views/CheckBox.cs (1 prop)
Terminal.Gui/Views/CharMap/CharMap.cs (1 prop)
Terminal.Gui/Views/Dialog.cs (4 props)
Terminal.Gui/Views/FrameView.cs (1 prop)
Terminal.Gui/Views/HexView.cs (1 prop)
Terminal.Gui/Views/LinearRange/LinearRangeDefaults.cs (1 prop)
Terminal.Gui/Views/Menu/Menu.cs (1 prop)
Terminal.Gui/Views/Menu/MenuBar.cs (1 prop)
Terminal.Gui/Views/MessageBox.cs (2 props)
Terminal.Gui/Views/Selectors/SelectorBase.cs (1 prop)
Terminal.Gui/Views/StatusBar.cs (1 prop)
Terminal.Gui/Views/TextInput/TextField/TextField.cs (1 prop)
Terminal.Gui/Views/TextInput/TextView/TextView.cs (1 prop)
Terminal.Gui/Views/Window.cs (2 props)
All ThemeScope-scoped view-facade props converted; total 21 properties
across 15 files. Pattern per property:
Before:
/// <summary>...</summary>
[ConfigurationProperty (Scope = typeof (ThemeScope))]
public static T Name
{
get => XSettings.Current.Name;
set => XSettings.Current = XSettings.Current with { Name = value };
}
After:
/// <summary>...</summary>
public static T Name => XSettings.Current.Name;
Out of scope
============
SettingsScope-scoped [ConfigurationProperty] on view classes
(FileDialog.MaxSearchResults, FileDialogStyle.DefaultUseColors,
MenuBar.DefaultKey, PopoverMenu.DefaultKey, View.DefaultMouseBindings,
View.ViewMouseBindings, BorderView.DefaultMouseBindings) are NOT
touched. SettingsScope follows the mutable-Defaults pattern (per
A2.1's divergence note in specs/remove-legacy-cm.md §4.2) and remains
CM-managed until step D.
Terminal.Gui/Configuration/ConfigPropertyHostTypes.cs
- Drops 14 entries (typeof + matching [DynamicDependency]) for
types whose only [ConfigurationProperty] attrs were ThemeScope
and are therefore now empty hosts:
Button, CharMap, CheckBox, Dialog, FrameView, HexView,
LinearRangeDefaults, Menu, MessageBox, SelectorBase,
StatusBar, TextField, TextView, Window.
- Keeps entries that still hold SettingsScope [ConfigurationProperty]:
FileDialog, FileDialogStyle, MenuBar, PopoverMenu, View,
BorderView, plus the unchanged Application / Color / Driver /
Key / Trace / ConfigurationManager / SchemeManager /
ThemeManager facades.
Tests/UnitTestsParallelizable/Configuration/ScopeJsonConverterTests.cs
- Drops the one InlineData row that exercised CM's
ScopeJsonConverter with `"Dialog.DefaultButtonAlignment": "End"`
(Dialog.DefaultButtonAlignment is one of the 21 properties this
commit removes [ConfigurationProperty] from; the converter now
rejects the key as Unknown). Comment notes the rationale and the
expected removal alongside CM in step D. Sibling InlineData rows
that don't reference dropped facade props continue to test the
converter.
Test results
============
Tests/UnitTestsParallelizable: total 17291 / 17271 passed / 0 failed
/ 20 skipped (one fewer test row vs. A2.3 baseline because the
ScopeJsonConverter InlineData row was removed by design; no
failures, no new skips, ConfigPropertyHostTypes drift-detector
still green because it tracks reflected hosts and we removed
matching list entries in lockstep).
Tests/UnitTests.NonParallelizable: total 74 / 72 passed / 0 failed /
2 skipped (unchanged).
Design context
==============
This is commit A2.4 of the A2 series (POCO ownership migration on
PR #5416, stacked on copilot/replace-cm-with-mec). A2.1 (2f7c13a)
landed the 17 ThemeScope POCOs and BindThemeScope<T>. A2.2 (441ef60)
landed GlyphSettings + Glyphs facade. A2.3 (f85e930) mopped up
NerdFonts. A2.4 finishes the series by removing the dead view-facade
setter scaffolding.
A2 status: complete. The cleared-out view-facade properties leave
ButtonSettings, CheckBoxSettings, DialogSettings, FrameViewSettings,
etc. as the sole owners of theme-overlayed state; MecThemeManager
mutates `<X>Settings.Current` exclusively via BindThemeScope<T>
intra-assembly; consumer reads go through the bare expression-bodied
getters on the view facades or directly through `<X>Settings.Current`.
Cross-session review (PR #5411 owner) signed off on:
- Removal of public static setters on view facades; "zero external
callers" audit accepted.
- Inheriting the A2.2 dormancy framing — Dark/Light/etc. theme
overrides for view facades are dormant from A2.4 through step D's
config.json rewrite. Same window as Glyphs.
Deferred to step D / later commits
==================================
- Resources/config.json rewrite to nested form (B/D).
- CM deletion (D).
- BindThemeScope<T> [UnconditionalSuppressMessage] re-justification
once ConfigPropertyHostTypes goes (D).
- ThemeSettings record + Current conversion (future micro-commit).
- Mec* prefix drops on the manager types (post-D).
Refs: A2.1 = 2f7c13a, A2.2 = 441ef60, A2.3 = f85e930,
stacked on copilot/replace-cm-with-mec.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds specs/breaking-changes-cm-mec.md analyzing the real consumer-facing impact of the CM->MEC migration: - #5411 removes/renames no public API; only marks 6 public CM types [Obsolete] (CS0618) and adds 4 Microsoft.Extensions.* dependencies. - In-repo builds hide CS0618 via .editorconfig; external NuGet consumers will see the deprecation warnings. - All hard, compile-breaking removals are deferred to #5416. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
|
@copilot resolve the merge conflicts in this pull request |
…name Mec* tests, NativeAOT size. Nested library Driver/Key/PopoverMenu sections skipped BindFlatDottedKeys, so RuntimeConfig dotted keys lost. Always overlay dotted keys after nested Bind. Phase F keeps MecThemeManager/MecSchemeManager because static ThemeManager/SchemeManager facades collide. Test files renamed. NativeAotSmoke win-arm64: 23.50 MB to 23.08 MB (-1.78%).
Phases D–I complete (
|
| bytes | MB | |
|---|---|---|
| develop | 24,640,512 | 23.50 |
| this PR | 24,201,728 | 23.08 |
| delta | −438,784 | −1.78% |
Schema follow-up remains #5631.
The hosted schema is this in-repo file (docs publish). It still described flat keys, array Themes, and ConfigurationManager.ThrowOnJsonErrors. Nested sections now match Settings POCOs; Themes/Schemes are dictionaries.
#5631 done in this PR (
|
… path. Binder no longer parses dotted keys or array-Themes (warn + MigrateConfig). ThemeCatalog owns names/Find. TrySwitchTheme publishes overlays then ThemeChanged. SchemeManager implements ISchemeManager; MecSchemeManager and unused ThemeDefinition are gone. Rune bind uses RuneJsonConverter.
Replace leftover ConfigurationProperty / dotted-key / ConfigLocations wording in docfx, lexicon, CONTRIBUTING, primers, and UICatalog help. Key bindings and ViewKeyBindings are documented as code-only. Specs are marked historical.
…, scheme merge. Fixes the 10 confirmed code-review findings on this branch, each with tests written first (25 new tests across 5 new files plus additions to 2): - Config sources can no longer crash the app at assembly load: inline and embedded JSON is validated before adding, user files get a load-exception handler, Build() falls back to library defaults, and the module initializer catches. TuiJsonErrors now has real callers, so Print() at shutdown works. - ./.tui/ config files resolve against the current directory (injectable via new currentDirectory parameters), not the app's install directory. - SchemeManager.ApplyFromConfiguration preserves runtime-added schemes and deep-merges root Schemes with the theme overlay (root-then-overlay contract); invalid scheme JSON is reported via TuiJsonErrors instead of swallowed. - ApplyToStaticFacades raises ThemeManager.ThemeChanged when the config's Theme key changes the active theme. - Legacy-shaped sources (files, TUI_CONFIG, RuntimeConfig, app-embedded) are actually skipped, making the existing warning truthful; unreadable files no longer throw; ThemeCatalog no longer filters numeric theme names. - UICatalog's config-file watcher reloads and re-applies (new TuiConfigurationBuilder.Reload) instead of only logging. - MenuBar.DefaultKey app-set values survive theme switches via an override. - MigrateConfig deep-merges mixed dotted and nested keys instead of dropping settings order-dependently. - Glyph strings parse glyph-first, so "6" is '6' while legacy numeric codepoints (e.g. 9733) still work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGq38GbuZPAEa5wZHkmPiZ
Key bindings were ConfigurationProperty in 2.4 and were silently dropped after the MEC migration because they are not Settings POCO properties. ApplyToStaticFacades now overlays Application.DefaultKeyBindings, View.DefaultKeyBindings, and View.ViewKeyBindings from nested JSON. ConfigurationSectionJson rebuilds MEC index children as JSON arrays so PlatformKeyBinding.All deserializes. Unmentioned commands keep hard-coded defaults.
Remove the global JsonStringEnumConverter from TuiSerializerContext; Command-name dictionary keys already deserialize with the default enum converter. Move STJ deserialize onto ConfigurationSectionJson and overlay ViewKeyBindings through the same command-dict merge. Treat MEC children as a JSON array only when keys are exactly 0..n-1.
Re-vendor Terminal.Gui preview to followup.2 built from 5416 head e8aba423 The followup.1 package was built from a stale commit (93c3b3d4) of tui-cs/Terminal.Gui#5416 that carried config-load defects fixed later on the branch. followup.2 is built from the branch head (e8aba423). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVbLXNzbus2MTXmiJDBErL @
…ba423 The followup.1 package was built from a stale commit (93c3b3d4) of tui-cs/Terminal.Gui#5416 that carried config-load defects fixed later on the branch. followup.2 is built from the branch head (e8aba423). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVbLXNzbus2MTXmiJDBErL
…, atomic arrays.
Addresses the 10 code-review findings on the reload/re-apply path:
- AddTuiInlineJson rejects non-object roots and duplicate-key JSON at add
time (single parse), so one bad TUI_CONFIG/RuntimeConfig source is skipped
per-source instead of throwing inside builder.Build() and discarding every
source. User files now flow through the same validated inline path,
replacing the file-load exception handler.
- TuiConfigurationBuilder builds a raw-JSON merged view of all sources
(JsonMerge: objects merge, arrays replace wholesale). Key bindings and
schemes bind from it, so a higher-priority source's shorter array
atomically replaces a lower-priority source's instead of inheriting MEC's
per-index tail merge.
- KeyBindingConfiguration records what each config overlay replaced and
reverts it before re-applying, so a binding removed from config regains
its default on reload ("unmentioned commands keep hard-coded defaults"
now holds across Reload). App-code mutations are preserved.
- ApplyToStaticFacades always raises ThemeManager.ThemeChanged - a hot
reload that edits the current theme's content now repaints instead of
updating facades invisibly.
- PopoverMenu.DefaultKey gets the same app-override treatment as
MenuBar.DefaultKey (the previous fix missed its only sibling).
- SchemeManager tracks runtime AddScheme entries explicitly, so updates to
existing schemes survive re-apply like added schemes do; scheme binding
and deep-merge now use the shared ConfigurationSectionJson/JsonMerge
helpers instead of hand-rolled clone/merge/deserialize copies.
- TuiJsonErrors clears at the start of each build, so a watcher re-applying
a persistently malformed source reports it once, not unboundedly.
- UICatalog's config watcher debounces (250ms), wraps the reload in
try/catch, and disposes its timer on stop - a bad config value can no
longer kill the process from the watcher thread.
- Config tests use a builder that skips user files and TUI_CONFIG
(internal includeUserSources ctor), so machine-local configuration cannot
change test outcomes.
Tests: 227 Configuration tests (10 new) plus full suites pass:
17,560 parallelizable, 32 non-parallelizable, 437 integration.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGq38GbuZPAEa5wZHkmPiZ
This is the follow-up to #5411 (MEC shims /
[Obsolete]). It removes legacy ConfigurationManager and ships nested-only MEC configuration.Fixes #4367. Also covers #5631 (nested
tui-config-schema.json). Gate for v2.5.0 (#5630).What landed
A2 — POCO ownership. Theme-scope settings are immutable records with atomic
Current. Glyphs and NerdFonts are read-only facades over those records. Dead CM[ConfigurationProperty]setters on view facades are gone.D/E — nested config + CM deletion.
Resources/config.jsonis nested. Legacy flat keys and array-Themesare not parsed (warn; convert withTools/MigrateConfig). These types are deleted:ConfigurationManager,ConfigProperty,ConfigPropertyHostTypesConfigurationPropertyAttribute,ConfigLocationsScope,ScopeJsonConverter,SourcesManager,DeepClonerAppSettingsScope,SettingsScope,ThemeScopeThemeManager/SchemeManagerare rewired to MEC. Dual-stack bind collapsed to nested-only; one theme-switch path (TrySwitchThemepublishes overlays, thenThemeChanged).F–I. Dotted
RuntimeConfigoverlay, test/file rename offMec*, docs and schema aligned to the nested shape.Hardening (ultrareview, test-first). Resilient config load (bad JSON cannot crash assembly init;
Build()falls back to library defaults)../.tui/paths resolve against CWD. Scheme merge preserves runtime-added schemes. Legacy-shaped sources are actually skipped. UICatalog watcher reloads viaTuiConfigurationBuilder.Reload.MigrateConfigdeep-merges mixed dotted + nested keys.Tools/MigrateConfigis a non-shipping aid for old flatconfig.json.Breaking changes
Apps that compiled against 2.4.17 (or any CM API) need source changes:
ConfigurationManageris gone. UseTuiConfigurationBuilderand Settings POCOs (ButtonSettings.Current,GlyphSettings.Current, etc.). Types marked[Obsolete]in Prepares for Fixing #4943 - RefactorsConfigurationManagerto be based onMEC#5411 are deleted.config.jsonis nested only. Convert flat-key / array-Themesfiles withTools/MigrateConfig.Glyphs.*and theme view facades are get-only. Writes go through the Settings records / MEC bind, notGlyphs.Apple = ….ConfigurationManager.Enable()/CM.Enable()calls in apps and examples must be removed or replaced.v1 users: see
docfx/docs/config.mdand the 2.5.0 notes in #5630.Size
Terminal.Gui/(library)Resources/config.jsonMost of the library − is real C# (CM stack + Glyphs facade), not JSON churn. The
config.jsonrewrite is +1,741 / −1,477 (shape change).Largest library deletions:
ConfigurationManager.cs(−844),ConfigProperty.cs(−654),DeepCloner.cs(−499),ScopeJsonConverter.cs(−440),SourcesManager.cs(−326),Glyphs.cs(−1,267 / +151). Tests that only existed to lock CM behavior were deleted with it (includingConfigurationMangerTests.cs−1,842).NativeAOT (win-arm64 smoke, recorded on this branch): 23.50 MB → 23.08 MB (−1.78%). A2 baseline after #5411 was 22.77 MB; the post-#5411 develop line grew, then this PR clawed some of that back. Size was a reason for #4367; this is a cut, not a collapse.
Out of scope / follow-ups
MecThemeManagerstays (staticThemeManagerfacade name collision).2.5.0-develop.N(tui-cs dependents + winprint) is Release v2.5.0 - breaking-change minor #5630 Workstream 4, not this PR.Test plan