Skip to content

Commit

Permalink
Refactor ActionMap and Command to use ActionIDs (#17162)
Browse files Browse the repository at this point in the history
As outlined in #16816, refactor `ActionMap` to use the new action IDs
added in #16904

## Validation steps performed

- [x] Legacy style commands are parsed correctly (and rewritten to the
new format)
- [x] Actions are still layered correctly and their IDs can be used to
'overwrite' actions in earlier layers
- [x] Keybindings that refer to an ID defined in another layer work
correctly
- [x] User-defined actions without an ID have one generated for them
(and their settings file is edited with it)
- [x] Schema updated

Refs #16816 
Closes #17133
  • Loading branch information
PankajBhojwani authored Jun 4, 2024
1 parent babd344 commit ece0c04
Show file tree
Hide file tree
Showing 17 changed files with 1,089 additions and 726 deletions.
63 changes: 43 additions & 20 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2058,7 +2058,7 @@
]
}
},
"Keybinding": {
"FullCommand": {
"additionalProperties": false,
"properties": {
"command": {
Expand Down Expand Up @@ -2186,21 +2186,6 @@
}
]
},
"keys": {
"description": "Defines the key combinations used to call the command. It must be composed of...\n -any number of modifiers (ctrl/alt/shift)\n -a non-modifier key",
"oneOf": [
{
"$ref": "#/$defs/KeyChordSegment"
},
{
"items": {
"$ref": "#/$defs/KeyChordSegment"
},
"minItems": 1,
"type": "array"
}
]
},
"icon": {
"$ref": "#/$defs/Icon"
},
Expand Down Expand Up @@ -2235,10 +2220,10 @@
"type": "object",
"properties": {
"command": {
"$ref": "#/$defs/Keybinding/properties/command"
"$ref": "#/$defs/FullCommand/properties/command"
},
"name": {
"$ref": "#/$defs/Keybinding/properties/name"
"$ref": "#/$defs/FullCommand/properties/name"
}
}
},
Expand All @@ -2261,6 +2246,44 @@
],
"type": "object"
},
"Keybinding": {
"additionalProperties": false,
"properties": {
"id": {
"description": "The ID of the command this keybinding should execute.",
"type": "string"
},
"keys": {
"description": "Defines the key combinations used to call the command. It must be composed of...\n -any number of modifiers (ctrl/alt/shift)\n -a non-modifier key",
"oneOf": [
{
"$ref": "#/$defs/KeyChordSegment"
},
{
"items": {
"$ref": "#/$defs/KeyChordSegment"
},
"minItems": 1,
"type": "array"
}
]
}
},
"anyOf": [
{
"required": [
"keys",
"id"
]
},
{
"required": [
"keys"
]
}
],
"type": "object"
},
"Globals": {
"additionalProperties": true,
"description": "Properties that affect the entire window, regardless of the profile settings.",
Expand Down Expand Up @@ -2464,12 +2487,12 @@
"actions": {
"description": "Properties are specific to each custom action.",
"items": {
"$ref": "#/$defs/Keybinding"
"$ref": "#/$defs/FullCommand"
},
"type": "array"
},
"keybindings": {
"description": "[deprecated] Use actions instead.",
"description": "A list of keychords bound to action IDs",
"deprecated": true,
"items": {
"$ref": "#/$defs/Keybinding"
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/TabBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TabBase::_UpdateSwitchToTabKeyChord()
{
const auto keyChord = _actionMap ? _actionMap.GetKeyBindingForAction(ShortcutAction::SwitchToTab, SwitchToTabArgs{ _TabViewIndex }) : nullptr;
const auto id = fmt::format(FMT_COMPILE(L"Terminal.SwitchToTab{}"), _TabViewIndex);
const auto keyChord{ _actionMap.GetKeyBindingForAction(id) };
const auto keyChordText = keyChord ? KeyChordSerialization::ToString(keyChord) : L"";

if (_keyChord == keyChordText)
Expand Down
7 changes: 4 additions & 3 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ namespace winrt::TerminalApp::implementation
newTabFlyout.Items().Append(settingsItem);

auto actionMap = _settings.ActionMap();
const auto settingsKeyChord{ actionMap.GetKeyBindingForAction(ShortcutAction::OpenSettings, OpenSettingsArgs{ SettingsTarget::SettingsUI }) };
const auto settingsKeyChord{ actionMap.GetKeyBindingForAction(L"Terminal.OpenSettingsUI") };
if (settingsKeyChord)
{
_SetAcceleratorForMenuItem(settingsItem, settingsKeyChord);
Expand All @@ -848,7 +848,7 @@ namespace winrt::TerminalApp::implementation
commandPaletteFlyout.Click({ this, &TerminalPage::_CommandPaletteButtonOnClick });
newTabFlyout.Items().Append(commandPaletteFlyout);

const auto commandPaletteKeyChord{ actionMap.GetKeyBindingForAction(ShortcutAction::ToggleCommandPalette) };
const auto commandPaletteKeyChord{ actionMap.GetKeyBindingForAction(L"Terminal.ToggleCommandPalette") };
if (commandPaletteKeyChord)
{
_SetAcceleratorForMenuItem(commandPaletteFlyout, commandPaletteKeyChord);
Expand Down Expand Up @@ -1023,7 +1023,8 @@ namespace winrt::TerminalApp::implementation
// NewTab(ProfileIndex=N) action
NewTerminalArgs newTerminalArgs{ profileIndex };
NewTabArgs newTabArgs{ newTerminalArgs };
auto profileKeyChord{ _settings.ActionMap().GetKeyBindingForAction(ShortcutAction::NewTab, newTabArgs) };
const auto id = fmt::format(FMT_COMPILE(L"Terminal.OpenNewTabProfile{}"), profileIndex);
const auto profileKeyChord{ _settings.ActionMap().GetKeyBindingForAction(id) };

// make sure we find one to display
if (profileKeyChord)
Expand Down
Loading

0 comments on commit ece0c04

Please sign in to comment.