Enable overriding default Key Bindings with `ConfigurationManager`

[tig]

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.

[dodexahedron]

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), 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?
• Why does it serialize the specific way it does?
• Why is it on the class, rather than referenced when needed?
• Why is it internal?

[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.

[dodexahedron]

You know... I just realized you are talking about another internal type, not the M.E.C. ConfigurationManager. Just note that my previous comment was from that perspective.

Anyway, thanks for the answers. Getting to that:

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.

Yeah I was kinda figuring it might have been an earlier detail that has just been outgrown over time as things have fleshed out. My instinct is it's now more of a burden than benefit, at least as currently used.

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

I 100% agree and like that format very much, but I think other ways are more appropriate to achieve that goal. Parse methods and an overridden ToString are the typical ways that stuff is usually handled. As-is, I do think the redundancy of making an object in an object is a bit cumbersome and verbose. The whole converter can be condensed down to this if that's axed:

public class AlternateKeyJsonConverter : JsonConverter<Key>
{
    public override Key Read( ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options )
    {
        if ( Key.TryParse( reader.GetString( ), out Key key ) )
        {
            return key;
        }

        return Key.Empty;
    }

    public override void Write( Utf8JsonWriter writer, Key value, JsonSerializerOptions options )
    {
        writer.WriteStringValue( value.ToString( ) );
    }
}

I don't like making things public until there's a clear need.

Perfectly valid. But makes it not possible for use with other serialization mechanisms.

---

It's also important to realize JSON isn't the only possible way people might want to serialize a Key (or anything). For example, TerminalGuiDesigner uses YAML. Someone else might want to use XML. It's better to expose only that which is necessary for round-tripping in any format to a serializer.

Lots of ways to do that, of course.

Potentially the simplest might be to mark everything non-serializable in that class and add a new public property whose entire purpose is serialization. It would look like this:

public string StringSequence
{
  get => ToString();
  set
  {
    // Parse value and set the object up accordingly
  }
}

[dodexahedron]

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.

It's got its quirks, but it is the dotnet standard and handles any format anyone writes a handler for, and is the mechanism invoked when using the standard dotnet DI infrastructure.

As a consumer of any library, I resent when I am forced by that library to use separate configuration, especially if that configuration is non-standard and non-portable. M.E.C. solves that problem.

But, it does enforce some formality on you around configuration. Classes that aren't, themselves, explicitly configuration should not be exposed to configuration directly. The way you achieve the separation you desire is by having configuration classes that only serve configuration purposes. Then you also get the benefit of being able to make your real class be an implementation detail and thus internal.

That's all laid out in the design guides over at Microsoft Learn, so I won't ramble on about that, but I can give a real-world example relevant to the general case as well as specifically to Terminal.Gui:

Take the SnapsInAZfs project I created (which uses Terminal.Gui to provide a configuration interface). Configuration classes are just data. In fact, they're all record types. Because they are dead simple classes, there's no complexity there, and they coexist with Kestrel's configuration and with NLog's configuration, all in the same hierarchy, whether contained in one monolithic file, spread amongst several files, or even environment variables, command line parameters, or other format configuration files. And it requires no real effort on my part. And, if a consumer wanted to override the behavior, they trivially can, by hooking into the .Configure method in the DI host builder.

[tig]

Great input. See https://github.com/gui-cs/Terminal.Gui/pull/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.

[dodexahedron]

Well I kinda mean it all orthogonally to CM (which I wasn't even aware of til yesterday). While yes, unification there is probably a laudable goal at some point, there are of course more pressing work items right now.