Skip to content

Commit

Permalink
Spec for global action IDs (#7175)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

⚠️ This spec has been moved from #6902. That version was branched off the new tab menu customization, and had a terribly convoluted git history. After discussion with the team, we've decided that it's best that this spec is merged atomically _first_, and used as the basis for #5888, as opposed to the other way around.

> This document is intended to serve as an addition to the [Command Palette Spec],
> as well as the [New Tab Menu Customization Spec].
> 
> As we come to rely more on actions being a mechanism by which the user defines
> "do something in the Terminal", we'll want to make it even easier for users to
> re-use the actions that they've already defined, as to reduce duplicated json as
> much as possible. This spec proposes a mechanism by which actions could be
> uniquely identifiable, so that the user could refer to bindings in other
> contexts without needing to replicate an entire json blob.
> 

## PR Checklist
* [x] Specs: #6899
* [x] References: #1571, #1912, #3337, #5025, #5524, #5633
* [x] I work here

## Detailed Description of the Pull Request / Additional comments
_\*<sup>\*</sup><sub>\*</sub> read the spec  <sub>\*</sub><sup>\*</sup>\*_



[Command Palette Spec]: https://github.com/microsoft/terminal/blob/master/doc/specs/%232046%20-%20Command%20Palette.md
[New Tab Menu Customization Spec]: https://github.com/microsoft/terminal/blob/master/doc/specs/%231571%20-%20New%20Tab%20Menu%20Customization.md
  • Loading branch information
zadjii-msft authored Aug 12, 2020
1 parent 849243a commit c241f83
Show file tree
Hide file tree
Showing 6 changed files with 228 additions and 0 deletions.
228 changes: 228 additions & 0 deletions doc/specs/#6899 - Action IDs/#6899 - Action IDs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
---
author: Mike Griese @zadjii-msft
created on: 2020-07-13
last updated: 2020-07-22
issue id: 6899
---

# Action IDs

## Abstract

This document is intended to serve as an addition to the [Command Palette Spec],
as well as the [New Tab Menu Customization Spec].

As we come to rely more on actions being a mechanism by which the user defines
"do something in the Terminal", we'll want to make it even easier for users to
re-use the actions that they've already defined, as to reduce duplicated json as
much as possible. This spec proposes a mechanism by which actions could be
uniquely identifiable, so that the user could refer to bindings in other
contexts without needing to replicate an entire json blob.

## Solution Design

This spec was largely inspired by the following diagram from @DHowett:

![figure 1](data-mockup.png)

The goal is to introduce an `id` parameter by which actions could be uniquely
refered to. If we'd ever like to use an action outside the list of `actions`, we
can simply refer to the action's ID, allowing the user to only define the action
_once_.

We'll start by renaming `bindings` to `actions`. `bindings` was suggested as a
rename for `keybindings` in [#6532], as a way to make the name more generic.
Discussion with the team lead to the understanding that the name `actions` would
be even better, as a way of making the meaning of the "list of actions" more
obvious.

When we're parsing `actions`, we'll make three passes:
* The first pass will scan the list for objects with an `id` property. We'll
attempt to parse those entries into `ActionAndArgs` which we'll store in the
global `id->ActionAndArgs` map. If any entry doesn't have an `id` set, we'll
skip it in this phase. If an entry doesn't have a `command` set, we'll ignore
it in this pass.
* The second pass will scan for _keybindings_. Any entries with `keys` set will
create a `KeyChord->ActionAndArgs` entry in the keybindings map. If the entry
has an `id` set, then we'll simply re-use the action we've already parsed for
the `id`, from the action map. If there isn't an `id`, then we'll parse the
action manually at this time. Entries without a `keys` set will be ignored in
this pass.
* The final pass will be to generate _commands_. Similar to the keybindings
pass, we'll attempt to lookup actions for entries with an `id` set. If there
isn't an `id`, then we'll parse the action manually at this time. We'll then
get the name for the entry, either from the `name` property if it's set, or
the action's `GenerateName` method.

For a visual representation, let's assume the user has the following in their
`actions`:

![figure 2](data-mockup-actions.png)

We'll first parse the `actions` to generate the mapping of `id`->`Actions`:

![figure 3](data-mockup-actions-and-ids.png)

Then, we'll parse the `actions` to generate the mapping of keys to actions, with
some actions already being defined in the map of `id`->`Actions`:

![figure 4](data-mockup-actions-and-ids-and-keys.png)


When layering `actions`, if a later settings file contains an action with the
same `id`, it will replace the current value. In this way, users can redefine
actions, or remove default ones (with something like `{ "id":
"Terminal.OpenTab", "command":null }`

We'd maintain a large list of default actions, each with unique `id`s set. These
are all given `id`'s with a `Terminal.` prefix, to easily identify them as
built-in, default actions. Not all of these actions will be given keys, but they
will all be given `id`s.

> 👉 NOTE: The IDs for the default actions will need to be manually created, not
> autogenerated. These `id`s are not strings displayed in the user interface, so
> localization is not a concern.
As we add additional menus to the Terminal, like the customization for the new
tab dropdown, or the tab context menu, or the `TermControl` context menu, they
could all refer to these actions by `id`, rather than duplicating the same json.


### Existing Scenarios

Keybindings will still be stored as a `keys->Action` mapping, so the user will
still be able to override default keybindings exactly the same as before.

Similarly, commands in the Command Palette will continue using their existing
`name->Action` mapping they're currently using. For a binding like

```json
{ "keys": "ctrl+alt+x", "id": "Terminal.OpenDefaultSettings" },
```
* We'll bind whatever action is defined as `Terminal.OpenDefaultSettings` to
<kbd>ctrl+alt+x</kbd>.
* We'll use whatever action is defined as `Terminal.OpenDefaultSettings` to
generate a name for the command palette.

### Future Context Menus

In [New Tab Menu Customization Spec], we discuss allowing the user to bind
actions to the new tab menu. In that spec, they can do so with something like
the following:

```json
{
"newTabMenu": [
{ "type":"action", "command": { "action": "adjustFontSize", "delta": 1 }, }
{ "type":"action", "command": { "action": "adjustFontSize", "delta": -1 }, }
{ "type":"action", "command": "resetFontSize", }
{ "type":"profile", "profile": "cmd" },
{ "type":"profile", "profile": "Windows PowerShell" },
{ "type":"separator" },
{
"type":"folder",
"name": "Settings...",
"icon": "C:\\path\\to\\icon.png",
"entries":[
{ "type":"action", "command": "openSettings" },
{ "type":"action", "command": { "action": "openSettings", "target": "defaultsFile" } },
]
}
]
}
```

In this example, the user has also exposed the "Increase font size", "Decrease
font size", and "Reset font size" actions, as well as the settings files in a
submenu. With this proposal, the above could instead be re-written as:

```json
{
"newTabMenu": [
{ "type":"action", "id": "Terminal.IncreaseFontSize" },
{ "type":"action", "id": "Terminal.DecreaseFontSize" },
{ "type":"action", "id": "Terminal.ResetFontSize" },
{ "type":"profile", "profile": "cmd" },
{ "type":"profile", "profile": "Windows PowerShell" },
{ "type":"separator" },
{
"type":"folder",
"name": "Settings...",
"icon": "C:\\path\\to\\icon.png",
"entries":[
{ "type":"action", "id": "Terminal.OpenDefaultSettings" },
{ "type":"action", "id": "Terminal.OpenSettings" },
]
}
]
}
```

In this example, the actions are looked up from the global map using the `id`
provided, enabling the user to re-use their existing definitions. If the user
re-defined the `Terminal.IncreaseFontSize` action to mean something else, then
the action in the new tab menu will also be automatically updated.

Furthermore, when additional menus are added (such as the tab context menu, or
the `TermControl` context menu), these could also leverage a similar syntax to
the above to allow re-use of the `id` parameter.

Discussion with the team also suggested that users shouldn't be able to define
actions in these menus _at all_. The actions should exclusively be defined in
`actions`, and other menus should only be able to refer to these actions by
`id`.

## UI/UX Design

There's not a whole lot of UI for this feature specifically. This is largely
behind-the-scenes refactoring of how actions can be defined.

## Capabilities

### Accessibility

_(not applicable)_

### Security

_(no change expected)_

### Reliability

_(no change expected)_

### Compatibility

_(no change expected)_

### Performance, Power, and Efficiency

_(no change expected)_

## Potential Issues

This won't necessarily play well with iterable commands in the Command Palette,
but that's okay. For iterable commands, users will still need to define the
actions manually.

## Future considerations

* See the following issues for other places where this might be useful:
- [#1912] - Context Menu for Tabs
* See also [#5524], [#5025], [#5633]
- [#3337] - Right-click menu inside TerminalControl (w/ Copy & Paste?)
* See also [#5633] and [#5025], both those actions seem reasonable in either
the tab context menu or the control context menu.

<!-- Footnotes -->
[Command Palette Spec]: https://github.com/microsoft/terminal/blob/master/doc/specs/%232046%20-%20Command%20Palette.md
[New Tab Menu Customization Spec]: https://github.com/microsoft/terminal/blob/master/doc/specs/%231571%20-%20New%20Tab%20Menu%20Customization.md

[#1571]: https://github.com/microsoft/terminal/issues/1571
[#1912]: https://github.com/microsoft/terminal/issues/1912
[#3337]: https://github.com/microsoft/terminal/issues/3337
[#5025]: https://github.com/microsoft/terminal/issues/5025
[#5524]: https://github.com/microsoft/terminal/issues/5524
[#5633]: https://github.com/microsoft/terminal/issues/5633
[#6532]: https://github.com/microsoft/terminal/issues/6532
[#6899]: https://github.com/microsoft/terminal/issues/6899
Binary file added doc/specs/#6899 - Action IDs/data-mockup-002.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added doc/specs/#6899 - Action IDs/data-mockup.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit c241f83

Please sign in to comment.