Port Editor and ted settings to MEC#240
Conversation
Add MEC-backed Editor settings and keybinding parsing, move ted startup and persistence to the new nested configuration shape, and keep CM compatibility for released Terminal.Gui builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7aeefb7fc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Ports the Editor’s default keybinding configuration and ted’s persisted settings from Terminal.Gui’s legacy ConfigurationManager (CM) shape to a Microsoft.Extensions.Configuration (MEC) shape, while keeping CM compatibility for released Terminal.Gui versions.
Changes:
- Introduces MEC-facing
Terminal.Gui.Editor.Configuration.EditorSettings+EditorConfigurationto applyEditor:DefaultKeyBindingsintoEditor.DefaultKeyBindings. - Updates ted startup and persistence to read/write a nested
"EditorSettings": { ... }section, while still accepting legacy flat (EditorSettings.*) and legacy CM (AppSettingsdotted keys) inputs. - Updates unit/integration/config tests and project references to cover MEC application and legacy compatibility paths.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Terminal.Gui.Editor.Tests/Terminal.Gui.Editor.Tests.csproj | Adds MEC configuration package dependency for new config-driven tests. |
| tests/Terminal.Gui.Editor.Tests/EditorKeyBindingConfigTests.cs | Switches from CM runtime config to MEC in-memory configuration for default keybinding overrides. |
| tests/Terminal.Gui.Editor.IntegrationTests/Terminal.Gui.Editor.IntegrationTests.csproj | Adds MEC configuration package dependency for integration tests. |
| tests/Terminal.Gui.Editor.IntegrationTests/TedSettingsPersistenceTests.cs | Updates persistence expectations to the nested MEC "EditorSettings" JSON shape and adds MEC/legacy load tests. |
| tests/Terminal.Gui.Editor.IntegrationTests/EditorKeyBindingIntegrationTests.cs | Switches integration test coverage from CM to MEC for keybinding overrides. |
| tests/Terminal.Gui.Editor.ConfigTests/TedConfigurationManagerTests.cs | Keeps CM end-to-end test, but resets via new EditorSettings.ResetDefaults(). |
| src/Terminal.Gui.Editor/Terminal.Gui.Editor.csproj | Adds Microsoft.Extensions.Configuration.Abstractions to support MEC-based configuration application. |
| src/Terminal.Gui.Editor/Editor.Commands.cs | Routes Editor.DefaultKeyBindings through the new EditorSettings.Defaults facade. |
| src/Terminal.Gui.Editor/Configuration/EditorSettings.cs | Adds MEC-friendly POCO + static Defaults holder for editor defaults. |
| src/Terminal.Gui.Editor/Configuration/EditorKeyBindingDefaults.cs | Extracts the editor keybinding default table into a dedicated factory. |
| src/Terminal.Gui.Editor/Configuration/EditorConfiguration.cs | Adds MEC reader/applicator to populate EditorSettings.Defaults.DefaultKeyBindings. |
| examples/ted/TerminalGuiConfigurationBootstrap.cs | Adds MEC-first startup bootstrap with reflection-based detection + CM fallback. |
| examples/ted/ted.csproj | Adds MEC Binder/Json packages needed for ted settings binding and JSON config loading. |
| examples/ted/Program.cs | Replaces direct CM enablement with the new MEC/CM bootstrapper. |
| examples/ted/EditorSettings.cs | Migrates ted settings read/write to nested "EditorSettings" section with legacy dotted-key migration support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 879a849b06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd2e01b83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08760144ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!Enum.TryParse (commandSection.Key, true, out Command command)) | ||
| { | ||
| throw new FormatException ($"Unknown editor command in configuration: '{commandSection.Key}'."); |
There was a problem hiding this comment.
EditorConfiguration.Apply throws FormatException for unknown command names (and similarly for invalid key strings), and Program calls this during startup without catching it. A single stale/typoed keybinding in user config (for example after command renames across versions) will therefore terminate app startup instead of ignoring/logging that entry and continuing with defaults, which is a user-facing regression from tolerant config loading.
Useful? React with 👍 / 👎.
| root[SectionName] = new JsonObject | ||
| { | ||
| [nameof (LineNumbers)] = LineNumbers, | ||
| [nameof (FoldIndicators)] = FoldIndicators, | ||
| [nameof (WordWrap)] = WordWrap, | ||
| [nameof (ShowTabs)] = ShowTabs, | ||
| [nameof (IndentSize)] = IndentSize, | ||
| [nameof (ConvertTabsToSpaces)] = ConvertTabsToSpaces, | ||
| [nameof (AutoIndent)] = AutoIndent, | ||
| [nameof (AutoComplete)] = AutoComplete, | ||
| [nameof (Scrollbars)] = Scrollbars | ||
| }; |
There was a problem hiding this comment.
Saving settings now replaces the entire EditorSettings object, so any unrecognized keys already present under that section (for example settings written by a newer ted version or another tool) are silently deleted on the next save. The previous save path updated known keys in place and preserved unrelated configuration, so this introduces avoidable config data loss during normal use.
Useful? React with 👍 / 👎.
Summary
Ports Editor/ted configuration to Terminal.Gui's MEC path and aligns with the split migration plan now documented upstream:
This PR updates Editor to consume the MEC-facing configuration flow (
TuiConfigurationBuilder) and keeps Editor-side compatibility behavior where needed (legacy key/value migration and load tolerance for ted settings files).What changed
TuiConfigurationBuilderdirectly in ted bootstrap (reflection fallback removed)Terminal.Gui.Editor.Editor.DefaultKeyBindingskey support during migrationTryParse+ error logging instead of startup exceptions)EditorSettingsJSON sectionDependency/rollout plan
CI note
Current Editor CI uses the published Terminal.Gui package line. Until a package containing #5411's
TuiConfigurationBuilderis available to CI, hosted checks fail with missing-type errors; local verification against a package built from latest #5411 succeeds.Local validation used for this branch state
MecCrFeedbackTests,MecDottedKeyTests) on latest #5411 head