|
| 1 | +--- |
| 2 | +author: Mike Griese @zadjii-msft |
| 3 | +created on: 2020-07-13 |
| 4 | +last updated: 2020-07-22 |
| 5 | +issue id: 6899 |
| 6 | +--- |
| 7 | + |
| 8 | +# Action IDs |
| 9 | + |
| 10 | +## Abstract |
| 11 | + |
| 12 | +This document is intended to serve as an addition to the [Command Palette Spec], |
| 13 | +as well as the [New Tab Menu Customization Spec]. |
| 14 | + |
| 15 | +As we come to rely more on actions being a mechanism by which the user defines |
| 16 | +"do something in the Terminal", we'll want to make it even easier for users to |
| 17 | +re-use the actions that they've already defined, as to reduce duplicated json as |
| 18 | +much as possible. This spec proposes a mechanism by which actions could be |
| 19 | +uniquely identifiable, so that the user could refer to bindings in other |
| 20 | +contexts without needing to replicate an entire json blob. |
| 21 | + |
| 22 | +## Solution Design |
| 23 | + |
| 24 | +This spec was largely inspired by the following diagram from @DHowett: |
| 25 | + |
| 26 | + |
| 27 | + |
| 28 | +The goal is to introduce an `id` parameter by which actions could be uniquely |
| 29 | +refered to. If we'd ever like to use an action outside the list of `actions`, we |
| 30 | +can simply refer to the action's ID, allowing the user to only define the action |
| 31 | +_once_. |
| 32 | + |
| 33 | +We'll start by renaming `bindings` to `actions`. `bindings` was suggested as a |
| 34 | +rename for `keybindings` in [#6532], as a way to make the name more generic. |
| 35 | +Discussion with the team lead to the understanding that the name `actions` would |
| 36 | +be even better, as a way of making the meaning of the "list of actions" more |
| 37 | +obvious. |
| 38 | + |
| 39 | +When we're parsing `actions`, we'll make three passes: |
| 40 | +* The first pass will scan the list for objects with an `id` property. We'll |
| 41 | + attempt to parse those entries into `ActionAndArgs` which we'll store in the |
| 42 | + global `id->ActionAndArgs` map. If any entry doesn't have an `id` set, we'll |
| 43 | + skip it in this phase. If an entry doesn't have a `command` set, we'll ignore |
| 44 | + it in this pass. |
| 45 | +* The second pass will scan for _keybindings_. Any entries with `keys` set will |
| 46 | + create a `KeyChord->ActionAndArgs` entry in the keybindings map. If the entry |
| 47 | + has an `id` set, then we'll simply re-use the action we've already parsed for |
| 48 | + the `id`, from the action map. If there isn't an `id`, then we'll parse the |
| 49 | + action manually at this time. Entries without a `keys` set will be ignored in |
| 50 | + this pass. |
| 51 | +* The final pass will be to generate _commands_. Similar to the keybindings |
| 52 | + pass, we'll attempt to lookup actions for entries with an `id` set. If there |
| 53 | + isn't an `id`, then we'll parse the action manually at this time. We'll then |
| 54 | + get the name for the entry, either from the `name` property if it's set, or |
| 55 | + the action's `GenerateName` method. |
| 56 | + |
| 57 | +For a visual representation, let's assume the user has the following in their |
| 58 | +`actions`: |
| 59 | + |
| 60 | + |
| 61 | + |
| 62 | +We'll first parse the `actions` to generate the mapping of `id`->`Actions`: |
| 63 | + |
| 64 | + |
| 65 | + |
| 66 | +Then, we'll parse the `actions` to generate the mapping of keys to actions, with |
| 67 | +some actions already being defined in the map of `id`->`Actions`: |
| 68 | + |
| 69 | + |
| 70 | + |
| 71 | + |
| 72 | +When layering `actions`, if a later settings file contains an action with the |
| 73 | +same `id`, it will replace the current value. In this way, users can redefine |
| 74 | +actions, or remove default ones (with something like `{ "id": |
| 75 | +"Terminal.OpenTab", "command":null }` |
| 76 | + |
| 77 | +We'd maintain a large list of default actions, each with unique `id`s set. These |
| 78 | +are all given `id`'s with a `Terminal.` prefix, to easily identify them as |
| 79 | +built-in, default actions. Not all of these actions will be given keys, but they |
| 80 | +will all be given `id`s. |
| 81 | + |
| 82 | +> 👉 NOTE: The IDs for the default actions will need to be manually created, not |
| 83 | +> autogenerated. These `id`s are not strings displayed in the user interface, so |
| 84 | +> localization is not a concern. |
| 85 | +
|
| 86 | +As we add additional menus to the Terminal, like the customization for the new |
| 87 | +tab dropdown, or the tab context menu, or the `TermControl` context menu, they |
| 88 | +could all refer to these actions by `id`, rather than duplicating the same json. |
| 89 | + |
| 90 | + |
| 91 | +### Existing Scenarios |
| 92 | + |
| 93 | +Keybindings will still be stored as a `keys->Action` mapping, so the user will |
| 94 | +still be able to override default keybindings exactly the same as before. |
| 95 | + |
| 96 | +Similarly, commands in the Command Palette will continue using their existing |
| 97 | +`name->Action` mapping they're currently using. For a binding like |
| 98 | + |
| 99 | +```json |
| 100 | +{ "keys": "ctrl+alt+x", "id": "Terminal.OpenDefaultSettings" }, |
| 101 | +``` |
| 102 | +* We'll bind whatever action is defined as `Terminal.OpenDefaultSettings` to |
| 103 | + <kbd>ctrl+alt+x</kbd>. |
| 104 | +* We'll use whatever action is defined as `Terminal.OpenDefaultSettings` to |
| 105 | + generate a name for the command palette. |
| 106 | + |
| 107 | +### Future Context Menus |
| 108 | + |
| 109 | +In [New Tab Menu Customization Spec], we discuss allowing the user to bind |
| 110 | +actions to the new tab menu. In that spec, they can do so with something like |
| 111 | +the following: |
| 112 | + |
| 113 | +```json |
| 114 | +{ |
| 115 | + "newTabMenu": [ |
| 116 | + { "type":"action", "command": { "action": "adjustFontSize", "delta": 1 }, } |
| 117 | + { "type":"action", "command": { "action": "adjustFontSize", "delta": -1 }, } |
| 118 | + { "type":"action", "command": "resetFontSize", } |
| 119 | + { "type":"profile", "profile": "cmd" }, |
| 120 | + { "type":"profile", "profile": "Windows PowerShell" }, |
| 121 | + { "type":"separator" }, |
| 122 | + { |
| 123 | + "type":"folder", |
| 124 | + "name": "Settings...", |
| 125 | + "icon": "C:\\path\\to\\icon.png", |
| 126 | + "entries":[ |
| 127 | + { "type":"action", "command": "openSettings" }, |
| 128 | + { "type":"action", "command": { "action": "openSettings", "target": "defaultsFile" } }, |
| 129 | + ] |
| 130 | + } |
| 131 | + ] |
| 132 | +} |
| 133 | +``` |
| 134 | + |
| 135 | +In this example, the user has also exposed the "Increase font size", "Decrease |
| 136 | +font size", and "Reset font size" actions, as well as the settings files in a |
| 137 | +submenu. With this proposal, the above could instead be re-written as: |
| 138 | + |
| 139 | +```json |
| 140 | +{ |
| 141 | + "newTabMenu": [ |
| 142 | + { "type":"action", "id": "Terminal.IncreaseFontSize" }, |
| 143 | + { "type":"action", "id": "Terminal.DecreaseFontSize" }, |
| 144 | + { "type":"action", "id": "Terminal.ResetFontSize" }, |
| 145 | + { "type":"profile", "profile": "cmd" }, |
| 146 | + { "type":"profile", "profile": "Windows PowerShell" }, |
| 147 | + { "type":"separator" }, |
| 148 | + { |
| 149 | + "type":"folder", |
| 150 | + "name": "Settings...", |
| 151 | + "icon": "C:\\path\\to\\icon.png", |
| 152 | + "entries":[ |
| 153 | + { "type":"action", "id": "Terminal.OpenDefaultSettings" }, |
| 154 | + { "type":"action", "id": "Terminal.OpenSettings" }, |
| 155 | + ] |
| 156 | + } |
| 157 | + ] |
| 158 | +} |
| 159 | +``` |
| 160 | + |
| 161 | +In this example, the actions are looked up from the global map using the `id` |
| 162 | +provided, enabling the user to re-use their existing definitions. If the user |
| 163 | +re-defined the `Terminal.IncreaseFontSize` action to mean something else, then |
| 164 | +the action in the new tab menu will also be automatically updated. |
| 165 | + |
| 166 | +Furthermore, when additional menus are added (such as the tab context menu, or |
| 167 | +the `TermControl` context menu), these could also leverage a similar syntax to |
| 168 | +the above to allow re-use of the `id` parameter. |
| 169 | + |
| 170 | +Discussion with the team also suggested that users shouldn't be able to define |
| 171 | +actions in these menus _at all_. The actions should exclusively be defined in |
| 172 | +`actions`, and other menus should only be able to refer to these actions by |
| 173 | +`id`. |
| 174 | + |
| 175 | +## UI/UX Design |
| 176 | + |
| 177 | +There's not a whole lot of UI for this feature specifically. This is largely |
| 178 | +behind-the-scenes refactoring of how actions can be defined. |
| 179 | + |
| 180 | +## Capabilities |
| 181 | + |
| 182 | +### Accessibility |
| 183 | + |
| 184 | +_(not applicable)_ |
| 185 | + |
| 186 | +### Security |
| 187 | + |
| 188 | +_(no change expected)_ |
| 189 | + |
| 190 | +### Reliability |
| 191 | + |
| 192 | +_(no change expected)_ |
| 193 | + |
| 194 | +### Compatibility |
| 195 | + |
| 196 | +_(no change expected)_ |
| 197 | + |
| 198 | +### Performance, Power, and Efficiency |
| 199 | + |
| 200 | +_(no change expected)_ |
| 201 | + |
| 202 | +## Potential Issues |
| 203 | + |
| 204 | +This won't necessarily play well with iterable commands in the Command Palette, |
| 205 | +but that's okay. For iterable commands, users will still need to define the |
| 206 | +actions manually. |
| 207 | + |
| 208 | +## Future considerations |
| 209 | + |
| 210 | +* See the following issues for other places where this might be useful: |
| 211 | + - [#1912] - Context Menu for Tabs |
| 212 | + * See also [#5524], [#5025], [#5633] |
| 213 | + - [#3337] - Right-click menu inside TerminalControl (w/ Copy & Paste?) |
| 214 | + * See also [#5633] and [#5025], both those actions seem reasonable in either |
| 215 | + the tab context menu or the control context menu. |
| 216 | + |
| 217 | +<!-- Footnotes --> |
| 218 | +[Command Palette Spec]: https://github.com/microsoft/terminal/blob/master/doc/specs/%232046%20-%20Command%20Palette.md |
| 219 | +[New Tab Menu Customization Spec]: https://github.com/microsoft/terminal/blob/master/doc/specs/%231571%20-%20New%20Tab%20Menu%20Customization.md |
| 220 | + |
| 221 | +[#1571]: https://github.com/microsoft/terminal/issues/1571 |
| 222 | +[#1912]: https://github.com/microsoft/terminal/issues/1912 |
| 223 | +[#3337]: https://github.com/microsoft/terminal/issues/3337 |
| 224 | +[#5025]: https://github.com/microsoft/terminal/issues/5025 |
| 225 | +[#5524]: https://github.com/microsoft/terminal/issues/5524 |
| 226 | +[#5633]: https://github.com/microsoft/terminal/issues/5633 |
| 227 | +[#6532]: https://github.com/microsoft/terminal/issues/6532 |
| 228 | +[#6899]: https://github.com/microsoft/terminal/issues/6899 |
0 commit comments