Design: Enable overriding default Key Bindings with ConfigurationManager #4266
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.
Overview
This PR provides a comprehensive design document for addressing issue #3089, which requests the ability to configure default key bindings through ConfigurationManager. Currently, all default key bindings in Terminal.Gui are hard-coded in View constructors, making them non-configurable by users.
Problem Statement
Terminal.Gui views like
TextField
have key bindings hard-coded in their constructors:This creates several issues:
Proposed Design
1. Configuration Structure
Introduce a new
DefaultKeyBindings
section inconfig.json
:2. Implementation Approach
New Classes:
KeyBindingConfig
: Represents a configurable key binding with command, keys, and platform filtersDefaultKeyBindingsScope
: Static scope containing all default bindings configurationKeyBindingConfigManager
: Helper class that applies platform-filtered bindings to ViewsIntegration:
Views would call a helper method during initialization that automatically applies the appropriate platform-specific bindings from configuration:
Platform filtering happens automatically based on
RuntimeInformation.IsOSPlatform()
.3. Key Design Decisions
Challenge: Static vs Instance Properties
static
properties (enforced by reflection)Challenge: Platform-Specific Bindings
Challenge: Backward Compatibility
KeyBindings.Add()
Migration Path
Status
This PR contains the design document only - no implementation code has been written yet. This design review is intended to gather feedback before proceeding with implementation.
Open Questions
Related Issues
Closes #3089
Original prompt
This section details on the original issue you should resolve
<issue_title>Enable overriding default Key Bindings with
ConfigurationManager
</issue_title><issue_description>Terminal.Gui provides default key bindings, but those defaults are not configurable by the user.
ConfigurationManager
should allow users to redefine key bindings for the system, a user, or an application.All built-in view subclasses should use ConfigurationManger to specify the default keybindings.
For example,
TextField
currently has code like this in it's constructor:This should be replaced with configuration in
.\Terminal.Gui\Resources\config.json
like this:For this to work,
View
and any subclass that defines default keybindings should have a member like this:(This requires more thought - because CM requires config properties to be
static
it's not possible to inherit defualt keybindings!)Default KeyBinding mappings should be platform specific
The above config.json example includes both the Windows (DeleteChar) and Linux (Ctrl-D) idioms. When a user is on Linux, only the Linux binding should work and vice versa.
We need to figure out a way of enabling this. Current best ideas:
config.json
, but have a flag that indicates platform. Like this (roughly):ConsoleDrivers
to have mappings within them. This may not be a good idea given some drivers (esp Netdriver) run on all platforms.The codebase should be scoured for cases where code is looking at
Key
s and not using KeyBindings.</issue_description>
<agent_instructions>Review this Issue and the current Configuration Manager codebase. Provide a design that will address this Issue. Don't code it yet.</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@tig > I think one issue that will crop up in use, and in more places than just trying to use Microsoft.Extensions.Configuration (though that's a big one),I really wanted to be able to annotate members to determine what was a setting and what wasn't. I couldn't figure out how to get M.E.C to do that. I also wanted a more Unix like config model and M.E.C seemed in the way of that. But in all honesty I didn't look at it that hard when I wrote CM.
I wanted the same fmt as ToString/TryParse. It may not be needed
once there's a constructor that takes a string per your prod.
I think "Key+modifiers"
is simple and easy to remember. The old format was clumsy and brittle.
Probably doesn't need to be.
I don't like making things public until there's a clear need. </comment_new>
<comment_new>@tig
Great input. See #3089
At this point I'm not eager/willing to rewrite CM to use M.E.C. I have other fish to fry. If someone wants to take it on, I would be supportive though. But CM does a lot of things I really like and those would need to be supported in the replacement.</comment_new>
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.