Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 3, 2025

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:

// Current approach in TextField constructor
KeyBindings.Add(Key.Delete, Command.DeleteCharRight);
KeyBindings.Add(Key.D.WithCtrl, Command.DeleteCharRight);
KeyBindings.Add(Key.Backspace, Command.DeleteCharLeft);

This creates several issues:

  • Users cannot customize default key bindings without modifying source code
  • Platform-specific conventions (e.g., Delete on Windows vs Ctrl+D on Linux) cannot be configured
  • No way to override bindings at system, user, or application level

Proposed Design

1. Configuration Structure

Introduce a new DefaultKeyBindings section in config.json:

{
  "DefaultKeyBindings": {
    "TextField": [
      {
        "Command": "DeleteCharRight",
        "Keys": ["Delete"],
        "Platforms": ["Windows", "Linux", "macOS"]
      },
      {
        "Command": "DeleteCharRight", 
        "Keys": ["Ctrl+D"],
        "Platforms": ["Linux", "macOS"]
      }
    ]
  }
}

2. Implementation Approach

New Classes:

  • KeyBindingConfig: Represents a configurable key binding with command, keys, and platform filters
  • DefaultKeyBindingsScope: Static scope containing all default bindings configuration
  • KeyBindingConfigManager: Helper class that applies platform-filtered bindings to Views

Integration:
Views would call a helper method during initialization that automatically applies the appropriate platform-specific bindings from configuration:

// In TextField constructor
KeyBindingConfigManager.ApplyDefaultBindings(this, "TextField");

Platform filtering happens automatically based on RuntimeInformation.IsOSPlatform().

3. Key Design Decisions

Challenge: Static vs Instance Properties

  • ConfigurationManager requires static properties (enforced by reflection)
  • KeyBindings are instance properties on each View
  • Solution: Use a static configuration dictionary accessed by a helper manager class

Challenge: Platform-Specific Bindings

  • Different platforms need different key conventions
  • Solution: Include explicit platform filters in configuration; helper class filters at runtime

Challenge: Backward Compatibility

  • Existing code manually calls KeyBindings.Add()
  • Solution: Config-based bindings applied first; manual additions still work and can override

Migration Path

  1. Phase 1: Create infrastructure (classes, JSON support, manager)
  2. Phase 2: Migrate TextField as proof-of-concept
  3. Phase 3: Systematically migrate remaining views
  4. Phase 4: Update documentation

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

  1. Should we support platform wildcards like "Unix" (Linux + macOS)?
  2. How should view inheritance work? Should subclasses inherit parent bindings automatically?
  3. Should we validate Commands at config load time or silently skip invalid ones?
  4. Best format for "all platforms" - explicit list or special "All" value?

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:

KeyBindings.Add (KeyCode.DeleteChar, Command.DeleteCharRight);
KeyBindings.Add (KeyCode.D | KeyCode.CtrlMask, Command.DeleteCharRight);

KeyBindings.Add (KeyCode.Delete, Command.DeleteCharLeft);
KeyBindings.Add (KeyCode.Backspace, Command.DeleteCharLeft);

This should be replaced with configuration in .\Terminal.Gui\Resources\config.json like this:

"TextField.DefaultKeyBindings": {
  "DeleteCharRight" : {
    "Key" : "DeleteChar"
  },
  "DeleteCharRight" : {
    "Key" : "Ctrl+D"
  },
  "DeleteCharLeft" : {
     "Key" : "Delete"
  },
  "DeleteCharLeft" : {
     "Key" : "Backspace"
}
...

For this to work, View and any subclass that defines default keybindings should have a member like this:

	public partial class View : Responder, ISupportInitializeNotification {

		[SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
		public static Dictionary<Command, Key> DefaultKeyBindings { get; set; }

(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:

  • Have each View specify all possibilities in config.json, but have a flag that indicates platform. Like this (roughly):
"TextField.DefaultKeyBindings": {
  "DeleteCharRight" : {
    "Windows" : true,
    "Linux": false,
    "Key" : "DeleteChar",
    "Modifiers": [
    ]
  },
  "DeleteCharRight" : {
    "Windows" : false,
    "Linux": true,
    "Key" : "D",
    "Modifiers": [
      "Ctrl"
    ]
  }
}
...
  • Have some way for 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 Keys 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.

is the custom Json serializer that is currently forced by default by being placed on the Key class, rather than used if and when actually needed. It forces a non-standard representation of the object, and requires any deserializer to be aware of the attribute, which CM is not, unfortunately (seems a rather unfortunate shortcoming of CM, if you ask me).

So, four questions:

  • Why is the KeyJsonConverter there in the first place?

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.

  • Why does it serialize the specific way it does?

I think "Key+modifiers"
is simple and easy to remember. The old format was clumsy and brittle.

  • Why is it on the class, rather than referenced when needed?

Probably doesn't need to be.

  • Why is it internal?

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>

Fixes #3023

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@Copilot Copilot AI assigned Copilot and tig Oct 3, 2025
@Copilot Copilot AI changed the title [WIP] Enable overriding default Key Bindings with ConfigurationManager Design: Enable overriding default Key Bindings with ConfigurationManager Oct 3, 2025
@Copilot Copilot AI requested a review from tig October 3, 2025 19:08
Copilot finished work on behalf of tig October 3, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable overriding default Key Bindings with ConfigurationManager
2 participants