|
| 1 | +# Breaking-Change Analysis: CM → MEC Migration (#5411 vs #5416) |
| 2 | + |
| 3 | +> Companion to [replace-cm-with-mec.md](./replace-cm-with-mec.md). |
| 4 | +> Tracks the umbrella issue [#4943](https://github.com/gui-cs/Terminal.Gui/issues/4943). |
| 5 | +
|
| 6 | +This document records what is **actually** breaking in |
| 7 | +[#5411](https://github.com/gui-cs/Terminal.Gui/pull/5411) |
| 8 | +("Refactors `ConfigurationManager` to be based on `MEC`") versus what is deferred |
| 9 | +to its stacked follow-on |
| 10 | +[#5416](https://github.com/gui-cs/Terminal.Gui/pull/5416) |
| 11 | +("Remove legacy `ConfigurationManager` after MEC migration"). |
| 12 | + |
| 13 | +The two PRs are intentionally split so the **functional migration** (#5411) and the |
| 14 | +**legacy removal** (#5416) carry different risk profiles. |
| 15 | + |
| 16 | +## TL;DR |
| 17 | + |
| 18 | +| | #5411 | #5416 | |
| 19 | +|---|---|---| |
| 20 | +| Public types removed | **None** | Yes (CM machinery) | |
| 21 | +| Public members removed / renamed / re-signatured | **None** | Yes | |
| 22 | +| Public types newly `[Obsolete]` | **6** (see below) | n/a (they get deleted) | |
| 23 | +| New transitive package dependencies | **4** (Microsoft.Extensions.*) | — | |
| 24 | +| Behavioral change to runtime config | Intended **none** (shims preserved) | Yes (CM-backed paths removed) | |
| 25 | +| Net source-break for an external consumer | Only if they treat **CS0618** as an error | Hard compile breaks | |
| 26 | + |
| 27 | +**Bottom line:** Despite the `BREAKING CHANGE` label, #5411 is **source-compatible**. |
| 28 | +It removes nothing public and changes no public signature. Its only consumer-facing |
| 29 | +surface is **`[Obsolete]` (CS0618) deprecation warnings** plus four new package |
| 30 | +dependencies. All hard, compile-breaking removals live in **#5416**. |
| 31 | + |
| 32 | +--- |
| 33 | + |
| 34 | +## What #5411 actually does (verified against the merge with `develop`) |
| 35 | + |
| 36 | +### 1. No public API removed, renamed, or re-signatured |
| 37 | + |
| 38 | +Verified by diffing #5411's net contribution over `develop`: |
| 39 | + |
| 40 | +- **0 files deleted** (`--diff-filter=D`). |
| 41 | +- **0 files renamed** (`--diff-filter=R`). |
| 42 | +- **0 public/protected methods removed.** |
| 43 | +- The `-public ...` lines in the diff are all **auto-properties converted to |
| 44 | + delegating properties** — e.g. `Glyphs`, `Button.DefaultShadow`, |
| 45 | + `Driver.SizeDetection`. The signature (`public static Rune File { get; set; }`) |
| 46 | + is preserved; only the backing implementation moves to a Settings POCO |
| 47 | + (`GlyphSettings.Defaults.File`, etc.). Get/set behavior is unchanged. |
| 48 | + |
| 49 | +So nothing a consumer *calls* disappears or changes shape in #5411. |
| 50 | + |
| 51 | +### 2. Six public types marked `[Obsolete]` |
| 52 | + |
| 53 | +These remain present and functional, but emit **CS0618** when referenced: |
| 54 | + |
| 55 | +| Type | Replacement guidance in the attribute | |
| 56 | +|---|---| |
| 57 | +| `ConfigurationManager` | Use `TuiConfigurationBuilder` | |
| 58 | +| `ConfigurationPropertyAttribute` | Use Settings POCOs with `TuiConfigurationBuilder` | |
| 59 | +| `ConfigProperty` | Being replaced by MEC | |
| 60 | +| `SettingsScope` | Being replaced by MEC | |
| 61 | +| `ThemeScope` | Being replaced by MEC | |
| 62 | +| `AppSettingsScope` | Use MEC with `TuiConfigurationBuilder` | |
| 63 | + |
| 64 | +`ConfigPropertyHostTypes` is also `[Obsolete]` but is **`internal`** → no consumer impact. |
| 65 | + |
| 66 | +**Still public and NOT obsolete** (the common theming/scheme entry points keep working |
| 67 | +without warnings): |
| 68 | + |
| 69 | +- `ThemeManager` (incl. `Theme`, `Themes`, `ThemeChanged`) |
| 70 | +- `SchemeManager` |
| 71 | + |
| 72 | +#### Important: the in-repo build hides these warnings; external consumers will see them |
| 73 | + |
| 74 | +`.editorconfig` (line ~86) sets `dotnet_diagnostic.cs0618.severity = none` |
| 75 | +**repo-wide** (added Oct 2025 in #4362, *not* by #5411). That is why building |
| 76 | +`Terminal.Gui`, `UICatalog`, and the test projects produces **zero** CS0618 warnings |
| 77 | +even though `UICatalog`/`Runner.cs` calls `ConfigurationManager.Enable/Load/Apply` |
| 78 | +directly. |
| 79 | + |
| 80 | +That suppression only applies to projects **inside this repository**. A downstream |
| 81 | +app consuming the published NuGet package is **not** covered by it and **will** see |
| 82 | +CS0618 deprecation warnings for any use of the six types above. For consumers that |
| 83 | +build with `TreatWarningsAsErrors`, that is effectively a **source break** — the only |
| 84 | +one #5411 can produce. It is easily worked around (suppress CS0618, or migrate to |
| 85 | +`TuiConfigurationBuilder`). |
| 86 | + |
| 87 | +### 3. Four new transitive package dependencies |
| 88 | + |
| 89 | +Added to `Directory.Packages.props` and referenced by `Terminal.Gui`: |
| 90 | + |
| 91 | +- `Microsoft.Extensions.Configuration` (10.0.7) |
| 92 | +- `Microsoft.Extensions.Configuration.Binder` (10.0.7) |
| 93 | +- `Microsoft.Extensions.Configuration.Json` (10.0.7) |
| 94 | +- `Microsoft.Extensions.Options` (10.0.7) |
| 95 | + |
| 96 | +This is not a source break, but it **does** change the dependency closure of anyone |
| 97 | +referencing Terminal.Gui (relevant for size-sensitive / NativeAOT consumers; #5416 |
| 98 | +targets AOT-size reduction as follow-up). The MEC binder is reflection-based, so |
| 99 | +#5411 adds `IL2026`/`IL3050` to `NoWarn` and `[UnconditionalSuppressMessage]` on the |
| 100 | +binder paths to keep AOT/trim builds quiet. |
| 101 | + |
| 102 | +### 4. Behavior: intended to be preserved |
| 103 | + |
| 104 | +#5411 keeps the legacy CM paths alive as shims and routes effective configuration |
| 105 | +through MEC. Notable behavior-neutral plumbing: |
| 106 | + |
| 107 | +- `ConfigurationManager.SerializerContext` now delegates to the new non-obsolete |
| 108 | + `TuiSerializerContext.Instance` (literally the same readonly instance). |
| 109 | +- A new `Terminal.Gui.Configuration.ThemeChanges` facade bridges |
| 110 | + `ConfigurationManager.Applied` and `ThemeManager.ThemeChanged` into one event; |
| 111 | + `Menu`, `MenuBar`, `StatusBar`, and `LineCanvas` subscribe to it instead of |
| 112 | + `ConfigurationManager.Applied` (which stays public + obsolete for external use). |
| 113 | +- `IThemeManager`/`ISchemeManager` interfaces + `Mec*` implementations are added |
| 114 | + (additive). |
| 115 | + |
| 116 | +Validated locally: full solution build clean; the `Configuration` (230) and |
| 117 | +`CheckBox` (36) parallelizable suites pass, including develop's |
| 118 | +`Default_CheckState_Glyphs_Are_Distinct`. |
| 119 | + |
| 120 | +> ⚠️ One thing that *looks* like a #5411 behavior change but is **not**: the default |
| 121 | +> checkbox glyphs changed `☒→☑` and `□→⬛`. That came from `develop` commit |
| 122 | +> `bbd16ad1e` ("Update default checkbox state glyphs"), surfaced here only because the |
| 123 | +> merge had to reconcile it with #5411's POCO delegation. See the merge notes below. |
| 124 | +
|
| 125 | +--- |
| 126 | + |
| 127 | +## What is deferred to #5416 (the real removals) |
| 128 | + |
| 129 | +#5416 is stacked on #5411's branch and is where the hard breaks land: |
| 130 | + |
| 131 | +- **Deletes / strips the legacy CM machinery** — e.g. `Glyphs.cs` loses ~1260 lines |
| 132 | + (the `[ConfigurationProperty]` attribute surface), `ConfigPropertyHostTypes` is |
| 133 | + removed, and `[ConfigurationProperty]` attributes are stripped from the Settings |
| 134 | + POCOs and view defaults. |
| 135 | +- Reworks theme/scheme **data ownership** into MEC (`ThemeDefinition`, theme overlay |
| 136 | + merge), which requires the `config.json` shape decision that #5411 deliberately did |
| 137 | + not make. |
| 138 | +- Ships a `Tools/MigrateConfig` utility to convert existing `config.json` files — |
| 139 | + acknowledgement that #5416 is where the **on-disk config format** can break. |
| 140 | +- Targets NativeAOT size reduction (#4367). |
| 141 | + |
| 142 | +In short: **#5416 removes the obsolete types and the `[ConfigurationProperty]` |
| 143 | +contract** that #5411 only deprecated. Code that merely produced CS0618 warnings under |
| 144 | +#5411 will **fail to compile** under #5416, and custom `config.json` files may need the |
| 145 | +migration tool. |
| 146 | + |
| 147 | +--- |
| 148 | + |
| 149 | +## Recommended messaging for the #5411 changelog |
| 150 | + |
| 151 | +- "Marks `ConfigurationManager` and its scope/attribute types `[Obsolete]`; introduces |
| 152 | + `TuiConfigurationBuilder` and Settings POCOs as the replacement. **No public APIs are |
| 153 | + removed in this release** — existing code keeps compiling (you may see CS0618 |
| 154 | + deprecation warnings). Adds `Microsoft.Extensions.Configuration` dependencies. |
| 155 | + Legacy `ConfigurationManager` removal and `config.json` format changes follow in a |
| 156 | + later release (#5416)." |
| 157 | + |
| 158 | +--- |
| 159 | + |
| 160 | +## Appendix: merge-conflict resolution with `develop` |
| 161 | + |
| 162 | +Merging `develop` (101 commits ahead) into #5411 produced three conflicts, all |
| 163 | +resolved in favor of #5411's architecture while preserving develop's intent: |
| 164 | + |
| 165 | +1. **`App/Application.cs`** — kept develop's reordered global usings; re-wrapped the |
| 166 | + now-obsolete `global using CM = ConfigurationManager` alias in |
| 167 | + `#pragma warning disable/restore CS0618`. |
| 168 | +2. **`Configuration/ConcurrentDictionaryJsonConverter.cs`** — kept #5411's |
| 169 | + `TuiSerializerContext.Instance` (Phase C extraction) with develop's correct spacing |
| 170 | + (dropped the bot-introduced no-space formatting). |
| 171 | +3. **`Drawing/Glyphs.cs`** — kept #5411's POCO-delegation for `CheckStateChecked` / |
| 172 | + `CheckStateNone`, and **moved develop's new glyph values (`☑`, `⬛`) into |
| 173 | + `GlyphSettings.cs`** so the static defaults — and develop's |
| 174 | + `Default_CheckState_Glyphs_Are_Distinct` test — produce the new glyphs. |
| 175 | + `config.json` (the runtime source of truth) already carried `☑`/`⬛` from develop. |
0 commit comments