FIXES #4367 - BREAKING CHANGE - Remove legacy ConfigurationManager after MEC migration#5416
Draft
tig wants to merge 13 commits into
Draft
FIXES #4367 - BREAKING CHANGE - Remove legacy ConfigurationManager after MEC migration#5416tig wants to merge 13 commits into
tig wants to merge 13 commits into
Conversation
tig
changed the title
tig
changed the title
tig
changed the title
tig
changed the title
tig
added a commit
that referenced
this pull request
May 26, 2026
- 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>
tig
added a commit
that referenced
this pull request
May 26, 2026
…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>
tig
force-pushed
the
tig/remove-cm-followup
branch
from
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>
tig
added a commit
that referenced
this pull request
May 27, 2026
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>
tig
force-pushed
the
tig/remove-cm-followup
branch
from
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>
tig
added a commit
that referenced
this pull request
Jun 8, 2026
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>
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
1 participant
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.
This is a stacked follow-up PR on top of
copilot/replace-cm-with-mec, (#5411) focused on removing legacy ConfigurationManager (CM) and reducing NativeAOT size.