--- author: Mike Griese @zadjii-msft created on: 2019-06-19 last updated: 2019-07-14 issue id: 1142 --- # Arbitrary Keybindings Arguments ## Abstract The goal of this change is to both simplify the keybindings, and also enable far more flexibility when editing a user's keybindings. Currently, we have many actions that are very similar in implementation - for example, `newTabProfile0`, `newTabProfile1`, `newTabProfile2`, etc. All these actions are _fundamentally_ the same function. However, we've needed to define 9 different actions to enable the user to provide different values to the `newTab` function. With this change, we'll be able to remove these _essentially_ duplicated events, and allow the user to specify arbitrary arguments to these functions. ## Inspiration Largely inspired by the keybindings in VsCode and Sublime Text. Additionally, much of the content regarding keybinding events being "handled" was designed as a solution for [#2285]. ## Solution Design We'll need to introduce args to some actions that we already have defined. These are the actions I'm thinking about when writing this spec: ```csharp // These events already exist like this: delegate void NewTabWithProfileEventArgs(Int32 profileIndex); delegate void SwitchToTabEventArgs(Int32 profileIndex); delegate void ResizePaneEventArgs(Direction direction); delegate void MoveFocusEventArgs(Direction direction); // These events either exist in another form or don't exist. delegate void CopyTextEventArgs(Boolean copyWhitespace); delegate void ScrollEventArgs(Int32 numLines); delegate void SplitProfileEventArgs(Orientation splitOrientation, Int32 profileIndex); ``` Ideally, after this change, the bindings for these actions would look something like the following: ```js { "keys": ["ctrl+shift+1"], "command": "newTabProfile", "args": { "profileIndex":0 } }, { "keys": ["ctrl+shift+2"], "command": "newTabProfile", "args": { "profileIndex":1 } }, // etc... { "keys": ["alt+1"], "command": "switchToTab", "args": { "index":0 } }, { "keys": ["alt+2"], "command": "switchToTab", "args": { "index":1 } }, // etc... { "keys": ["alt+shift+down"], "command": "resizePane", "args": { "direction":"down" } }, { "keys": ["alt+shift+up"], "command": "resizePane", "args": { "direction":"up" } }, // etc... { "keys": ["alt+down"], "command": "moveFocus", "args": { "direction":"down" } }, { "keys": ["alt+up"], "command": "moveFocus", "args": { "direction":"up" } }, // etc... { "keys": ["ctrl+c"], "command": "copy", "args": { "copyWhitespace":true } }, { "keys": ["ctrl+shift+c"], "command": "copy", "args": { "copyWhitespace":false } }, { "keys": ["ctrl+shift+down"], "command": "scroll", "args": { "numLines":1 } }, { "keys": ["ctrl+shift+up"], "command": "scroll", "args": { "numLines":-1 } }, { "keys": ["ctrl+alt+1"], "command": "splitProfile", "args": { "orientation":"vertical", "profileIndex": 0 } }, { "keys": ["ctrl+alt+shift+1"], "command": "splitProfile", "args": { "orientation":"horizontal", "profileIndex": 0 } }, { "keys": ["ctrl+alt+2"], "command": "splitProfile", "args": { "orientation":"vertical", "profileIndex": 1 } }, { "keys": ["ctrl+alt+shift+2"], "command": "splitProfile", "args": { "orientation":"horizontal", "profileIndex": 1 } }, // etc... ``` Note that instead of having 9 different `newTabProfile` actions, we have a singular `newTabProfile` action, and that action requires a `profileIndex` in the `args` object. Also, pay attention to the last set of keybindings, the `splitProfile` ones. This is a function that requires two arguments, both an `orientation` and a `profileIndex`. Before this change we would have needed to create 20 separate actions (10 profile indices * 2 directions) to handle these cases. Now it can be done with a single action that can be much more flexible in its implementation. ### Parsing KeyBinding Arguments We'll add two new interfaces: `IActionArgs` and `IActionEventArgs`. Classes that implement `IActionArgs` will contain all the per-action args, like `CopyWhitespace` or `ProfileIndex`. `IActionArgs` by itself will be an empty interface, but all other arguments will derive from it. `IActionEventArgs` will have a single property `Handled`, which will be used for indicating if a particular event was processed or not. When parsing args, we'll build `IActionArgs` to contain all the parameters. When dispatching events, we'll build `IActionEventArgs` using the `IActionArgs` to set all the parameter values. All current keybinding events will be changed from their current types to `TypedEventHandler`s. These `TypedEventHandler`s second param will always be an instance of `IActionEventArgs`. So, for example: ```csharp delegate void CopyTextEventArgs(); delegate void NewTabEventArgs(); delegate void NewTabWithProfileEventArgs(Int32 profileIndex); // ... [default_interface] runtimeclass AppKeyBindings : Microsoft.Terminal.Settings.IKeyBindings { event CopyTextEventArgs CopyText; event NewTabEventArgs NewTab; event NewTabWithProfileEventArgs NewTabWithProfile; ``` Becomes: ```csharp interface IActionArgs { /* Empty */ } runtimeclass ActionEventArgs { Boolean Handled; ActionArgs Args; } runtimeclass CopyTextArgs : IActionArgs { Boolean CopyWhitespace; } runtimeclass NewTabWithProfileArgs : IActionArgs { Int32 ProfileIndex; } runtimeclass NewTabWithProfileEventArgs : NewTabWithProfileArgs, IActionArgs { } [default_interface] runtimeclass AppKeyBindings : Microsoft.Terminal.Settings.IKeyBindings { event Windows.Foundation.TypedEventHandler CopyText; event Windows.Foundation.TypedEventHandler NewTab; event Windows.Foundation.TypedEventHandler NewTabWithProfile; ``` In this above example, the `CopyTextArgs` class actually contains all the potential arguments to the Copy action. `ActionEventArgs` is the class that holds any `ActionArgs`. When we parse the arguments, we'll build a `CopyTextArgs`, and when we're dispatching the event, we'll build a `ActionEventArgs` that holds a `CopyTextArgs` as its `Args` value, and dispatch the `ActionEventArgs` object. We'll also change our existing map in the `AppKeyBindings` implementation. Currently, it's a `std::unordered_map`, which uses the `KeyChord` to lookup the `ShortcutAction`. We'll need to introduce a new type `ActionAndArgs`: ```csharp runtimeclass ActionAndArgs { ShortcutAction Action; IActionArgs Args; } ``` and we'll change the map in `AppKeyBindings` to a `std::unordered_map`. When we're parsing keybindings, we'll need to construct args for each of the events to go with each binding. When we find some key chord bound to a given Action, we'll construct the `IActionArgs` for that action. For many actions, these args will be an empty class. However, when we do find an action that needs additional parsing, `AppKeyBindingsSerialization` will do the extra work to parse the args for that action. We'll keep a collection of functions that can be used for quickly determining how to parse the args for an action if necessary. This map will be a `std::unordered_map>`. For most actions which don't require args, the function in this map will be set to nullptr, and we'll know that the action doesn't need to parse any more args. However, for actions that _do_ require args, we'll set up a global function that can be used to parse a json blob into an `IActionArgs`. Once the `IActionArgs` is built for the keybinding, we'll set it in `AppKeyBindings` with a updated `AppKeyBindings::SetKeyBinding` call. `SetKeyBinding`'s signature will be updated to take a `ActionAndArgs` instead. Should an action not need arguments, the `Args` member can be left `null` in the `ActionAndArgs`. ### Executing KeyBinding Actions with Arguments When we're handling a keybinding in `AppKeyBindings::_DoAction`, we'll trigger the event handlers with the `IActionArgs` we've stored in the map with the `ShortcutAction`. Then, in `App`, we'll handle each of these events. We set up lambdas as event handlers for each event in `App::_HookupKeyBindings`. In each of those functions, we'll inspect the `IActionArgs` parameter, and use args from its implementation to call callbacks in the `App` class. We will update `App` to have methods defined with the actual keybinding function signatures. Instead of: ```c++ void App::_HookupKeyBindings(TerminalApp::AppKeyBindings bindings) noexcept { // ... bindings.NewTabWithProfile([this](const auto index) { _OpenNewTab({ index }); }); } ``` The code will look like: ```c++ void App::_HookupKeyBindings(TerminalApp::AppKeyBindings bindings) noexcept { // ... bindings.NewTabWithProfile({ this, &App::_OpenNewTab }); } // ... void App::_OpenNewTab(const TerminalApp::AppKeyBindings& sender, const NewTabEventArgs& args) { auto profileIndex = args.ProfileIndex(); args.Handled(true); // ... } ``` ### Handling Keybinding Events Common to all implementations of `IActionArgs` is the `Handled` property. This will let the app indicate if it was able to actually process a keybinding event or not. While in the large majority of cases, the events will all be marked handled, there are some scenarios where the Terminal will need to know if the event could not be performed. For example, in the case of the `copy` event, the Terminal is only capable of copying text if there's actually a selection active. If there isn't a selection active, the `App` should make sure to not mark the event as not handled (it will leave `args.Handled(false)`). The App should only mark an event handled if it has actually dispatched the event. When an event is handled, we'll make sure to return `true` from `AppKeyBindings::TryKeyChord`, so that the terminal does not actually process that keypress. For events that were not handled by the application, the terminal will get another chance to dispatch the keypress. ### Serializing KeyBinding Arguments Similar to how we parse arguments from the json, we'll need to update the `AppKeyBindingsSerialization` code to be able to serialize the arguments from a particular `IActionArgs`. ## UI/UX Design ### Keybindings in the New Tab Dropdown Small modifications will need to be made to the code responsible for the new tab dropdown. The new tab dropdown currently also displays the keybindings for each profile in the new tab dropdown. It does this by querying for the keybinding associated with each action. As we'll be removing the old `ShortcutAction`s that this dropdown uses, we'll need a new way to find which key chord corresponds to opening a given profile. We'll need to be able to not only lookup a keybinding by `ShortcutAction`, but also by a `ShortcutAction` and `IActionArgs`. We'll need to update the `AppKeyBindings::GetKeyBinding` method to also accept a `IActionArgs`. We'll also probably want each `IActionArgs` implementation to define an `Equals(IActionArgs)` method, so that we can easily check if two different `IActionArgs` are the same in this method. ## Capabilities ### Accessibility N/A ### Security This should not introduce any _new_ security concerns. We're relying on the security of jsoncpp for parsing json. Adding new keys to the settings file will rely on jsoncpp's ability to securely parse those json values. ### Reliability We'll need to make sure that invalid keybindings are ignored. Currently, we already gracefully ignore keybindings that have invalid `keys` or invalid `commands`. We'll need to add additional validation on invalid sets of `args`. When we're parsing the args from a Json blob, we'll make sure to only ever look for keys we're expecting and ignore everything else. If a keybinding requires certain args, but those args are not provided, we'll need to make sure those args each have reasonable default values to use. If for any reason a reasonable default can't be used for a keybinding argument, then we'll need to make sure to display an error dialog to the user for that scenario. When we're re-serializing settings, we'll only know about the keybinding arg keys that were successfully parsed. Other keys will be lost on re-serialization. ### Compatibility This change will need to carefully be crafted to enable upgrading the legacy keybindings seamlessly. For most actions, the upgrade should be seamless. Since they already don't have args, their serializations will remain exactly the same. However, for the following actions that we'll be removing in favor of actions with arguments, we'll need to leave legacy deserialization in place to be able to find these old actions, and automatically build the correct `IActionArgs` for them: * `newTabProfile` - We'll need to make sure to build args with the right `profileIndex` corresponding to the old action. * `switchToTab` - We'll need to make sure to build args with the right `index` corresponding to the old action. * `resizePane` and `moveFocus` - We'll need to make sure to build args with the right `direction` corresponding to the old action. * `scroll` - We'll need to make sure to build args with the right `amount` value corresponding to the old action. `Up` will be -1, and `Down` will be 1. ### Performance, Power, and Efficiency N/A ## Potential Issues N/A ## Future considerations * Should we support some sort of conversion from num keys to an automatic arg? For example, by default, Alt+<N> to focuses the Nth tab. Currently, those are 8 separate entries in the keybindings. Should we enable some way for them be combined into a single binding entry, where the binding automatically receives the number pressed as an arg? I couldn't find any prior art of this, so it doesn't seem worth it to try and invent currently. This might be something that we want to loop back on, but for the time being, it remains out of scope of this PR. * When we inevitable support extensions, we'll need to allow extensions to also be able to support their own custom keybindings and args. We'll probably want to pass the settings to the extension to have the extension parse its own settings. We'll want to be able to ask the extension for its own set of `ActionAndArgs`[1] that it builds from the `keybindings`. Once we have that set of actions, we'll be able to store them locally, and dispatch them quickly. - [1] We probably won't be able to use the `ActionAndArgs` class directly, since that class is specific to the actions we define. We'll need another way for extensions to be able to uniquely identify their own actions. ## Resources N/A [#2285]: https://github.com/microsoft/terminal/issues/2285