Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a spec for the Command Palette #2193

Merged
merged 14 commits into from
Apr 13, 2020
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
269 changes: 269 additions & 0 deletions doc/cascadia/Command-Palette-spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
---
author: Mike Griese @zadjii-msft
created on: 2019-08-01
last updated: 2019-08-01
issue id: 2046
---

# Spec Title

## Abstract

This spec covers the addition of a "command palette" to the Windows Terminal.
The Command Palette is a GUI that the user can activate to search for and
execute commands. Beneficially, the command palette allows the user to execute
commands _even if they aren't bound to a keybinding_.

## Inspiration

This feature is largely inspired by the "Command Palette" in text editors like
VsCode, Sublime Text and others.

This spec was initially drafted in [a
comment](https://github.com/microsoft/terminal/issues/2046#issuecomment-514219791)
in [#2046]. That was authored during the annual Microsoft Hackathon, where I
proceeded to prototype the solution. This spec is influenced by things I learned
prototyping.

## Solution Design

First off, for the sake of clarity, we'll rename the `command` of a keybinding
to `action`. This will help keep the mental model between commands and actions
clearer. When deserializing keybindings, we'll include a check for the old
`command` key to migrate it.

We'll introduce a new top-level array to the user settings, under the key
`commands`. `commands` will contain an array of commands, each with the
following schema:

```js
{
"name": string,
"action": string,
"iconPath": string
}
```

Command names should be human-friendly names of actions, though they don't need
to necessarily be related to the action that it fires. For example, a command
with `newTab` as the action could have `"Open New Tab"` as the name.

The command will be parsed into a new class, `Command`:
```c++
class Command
{
winrt::hstring Name();
winrt::TerminalApp::ShortcutAction Action();
winrt::hstring IconPath();
}
```
We'll add another structure in GlobalAppSettings to hold all these actions. It
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
will just be a `std::vector<Command>` in `GlobalAppSettings`.

We'll need app to be able to turn this vector into a `ListView`, or similar, so
that we can display this list of actions. Each element in the view will be
intrinsically associated with the `Command` object it's associated with. In
order to support this, we'll make `Command` a winrt type that implements
`Windows.UI.Xaml.Data.INotifyPropertyChanged`. This will let us bind the XAML
element to the winrt type.

When an element is clicked on in the list of commands, we'll raise the event
corresponding to that `ShortcutAction`. `AppKeyBindings` already does a great
job of dispatching `ShortcutActions`, so we'll re-use that. We'll pull the basic
parts of dispatching `ShortcutAction` callbacks into another class,
`ShortcutActionDispatch`, with a single `PerformAction(ShortcutAction)` method
(and events for each action). `AppKeybindings` will be initialized with a
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
reference to the `ShortcutActionDispatch` object, so that it can call
`PerformAction` on it. Additionally, by having a singular
`ShortcutActionDispatch` instance, we won't need to re-hook up the
ShortcutAction keybindings each time we re-load the settings.

In `App`, when someone clicks on an item in the list, we'll get the
`ShortcutAction` associated with that list element, and call PerformAction on
the app's `ShortcutActionDispatch`. This will trigger the event handler just the
same as pressing the keybinding.

## UI/UX Design

We'll add another action that can be used to toggle the visibility of the
command palette. Pressing that keybinding will bring up the command palette.

**OPEN QUESTION**: Do we want the palette to appear over a single pane, or over
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
the entire tab content? In [#Future-considerations], I mention having
profile-scoped commands. Maybe it makes more sense in that scenario to have the
palette appear in a particular pane, to help identify that the palette has
commands for _that_ terminal. It could however clutter the UI quickly, and might
not be as usable for smaller pane sizes. We could just have the per-profile
commands added when the palette is opened. The way I have it now is as an
overlay over the entire tab.

The palette will consist of two main UI elements: a text box for searching for
commands, and a list of commands.

The list of commands will be populated with all the commands by default. Each
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
command will appear like a `MenuFlyoutItem`, with an icon at the left (if it has
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
one) and the name visible. When opened, the palette will automatically highlight
the first entry in the list.

The user can navigate the list of entries with the arrow keys. Hitting enter
will close the palette and execute the action that's highlighted. Hitting escape
will dismiss the pallete, returning control to the terminal. When the palette is
closed for any reason (executing a command, dismissing with either escape or the
`toggleCommandPalette` keybinding), we'll clear out any search text from the
palette, so the user can start fresh again.

We'll also want to enable the command palette to be filterable, so that the user
can type the name of a command, and the command palette will automatically
filter the list of commands. This should be more powerful then just a simple
string compare - the user should be able to type a search string, and get all
the commands that match a "fuzzy search" for that string. This will allow users
to find the command they're looking for without needing to type the entire
command.

For example, consider the following list of commands:

```json
"commands": [
{ "icon": null, "name": "New Tab", "action": "newTab" },
{ "icon": null, "name": "Close Tab", "action": "closeTab" },
{ "icon": null, "name": "Close Pane", "action": "closePane" },
{ "icon": null, "name": "[-] Split Horizontal", "action": "splitHorizontal" },
{ "icon": null, "name": "[ | ] Split Vertical", "action": "splitVertical" },
{ "icon": null, "name": "Next Tab", "action": "nextTab" },
{ "icon": null, "name": "Prev Tab", "action": "prevTab" },
{ "icon": null, "name": "Open Settings", "action": "openSettings" },
{ "icon": null, "name": "Open Media Controls", "action": "openTestPane" }
],
```

* "open" should return both "**Open** Settings" and "**Open** Media Controls".
* "Tab" would return "New **Tab**", "Close **Tab**", "Next **Tab**" and "Prev
**Tab**".
* "P" would return "Close **P**ane", "[-] S**p**lit Horizontal", "[ | ]
S**p**lit Vertical", "**P**rev Tab", "O**p**en Settings" and "O**p**en Media
Controls".
* Even more powerfully, "sv" would return "[ | ] **S**plit **V**ertical". This
is a great example of how a user could execute a command with very few
keystrokes.

As the user types, we should **bold** each matching character in the command
name, to show how their input correlates to the results on screen.

## Capabilities

### Accessibility

As the entire command palette will be a native XAML element, it'll automatically
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
be hooked up to the UIA tree, allowing for screen readers to naturally find it.

### 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 commands are ignored. A command could be
invalid because:
* it has a null `name`, or a name with the empty string for a value.
* it has a null `action`, or an action specified that's not an actual
`ShortcutAction`.

We'll ignore invalid commands from the user's settings, instead of hard
crashing. I don't believe this is a scenario that warrants an error dialog to
indicate to the user that there's a problem with the json.

### Compatibility

We will need to define default _commands_ for all the existing keybinding
commands. With #754, we could add all the actions (that make sense) as commands
to the commands list, so that everyone wouldn't need to define them manually.

### Performance, Power, and Efficiency

We'll be adding a few extra XAML elements to our tree which will certainly
increase our runtime memory footprint while the palette is open.

We'll aditionally be introducing a few extra json values to parse, so that could
increase our load times (though this will likely be negligible).

zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
## Potential Issues

This will first require the work in [#1205] to work properly. Right now we
heavily lean on the "focused" element to determine which terminal is "active".
However, when the command plaette is opened, focus will move out of the terminal
control into the command palette, which leads to some hard to debug crashes.

## Future considerations

* Once [#1142] is also complete, we'll also add a `executeCommand` action. It
will take a single parameter `command`, which will be the name of a command
from the list of commands. This will enable a user to bind a command to a
keybinding. This could allow the user to bind a specific pairing of `action`
and `args` that they've set up in a command as a keybinding, without having to
repeat the entry in both the `commands` and `keybindings`.
* Commands will provide an easy point for allowing an extension to add it's
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
actions to the UI, without forcing the user to bind the extension's actions to
a keybinding
* Also discussed in [#2046] was the potential for adding a command that inputs a
certain commandline to be run by the shell. I felt that was out of scope for
this spec, so I'm not including it in detail. I believe that would be
accomplished by adding a `inputCommand` action, with two args: `commandline`,
a string, and `suppressNewline`, an optional bool, defaulted to false. The
`inputCommand` action would deliver the given `commandline` as input to the
connection, followed by a newline (as to execute the command).
`suppressNewline` would prevent the newline from being added. This would work
relatively well, so long as you're sitting at a shell prompt. If you were in
an application like `vim`, this might be handy for executing a sequence of
vim-specific keybindings. Otherwise, you're just going to end up writing a
commadline to the buffer of vim. It would be weird, but not unexpected.
* Additionally mentioned in [#2046] was the potential for profile-scoped
commands. While that's a great idea, I believe it's out of scope for this
spec.
* Once [#754] lands, we'll need to make sure to include commands for each action
manually in the default settings. This will add some overhead that the
developer will need to do whenever they add an action. That's unfortunate, but
will be largely beneficial to the end user.
* We could theoretically also display the keybinding for a certain command in
the `ListViewItem` for the command. We'd need some way to correlate a
command's action to a keybinding's action. This could be done in a follow-up
task.
* We might want to alter the fuzzy-search algorithim, to give higher precedence
in the results list to commands with more consecutive matching characters.
Alternatively we could give more weight to commands where the search matched
the initial character of words in the command.
- For example: `ot` would give more weight to "**O**pen **T**ab" than
"**O**pen Se**t**tings").
* Another idea for a future spec is the concept of "nested commands", where a
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
single command has many sub-commands. This would hide the children commands
from the entire list of commands, allowing for much more succinct top-level
list of commands, and allowing related commands to be grouped together.
- For example, I have a text editor plugin that enables rendering markdown to
a number of different styles. To use that command in my text editor, first I
hit enter on the "Render Markdown..." command, then I select which style I
want to render to, in another list of options. This way, I don't need to
have three options for "Render Markdown to github", "Render Markdown to
gitlab", all in the top-level list.
* We may want to add a button to the New Tab Button's dropdown to "Show Command
Palette". I'm hesitant to keep adding new buttons to that UI, but the command
palette is otherwise not highly discoverable.
- We could add another button to the UI to toggle the visibility of the
command palette. This was the idea initially proposed in [#2046].
- For both these options, we may want a global setting to hide that button, to
keep the UI as minimal as possible.

## Resources
Initial post that inspired this spec: #[2046](https://github.com/microsoft/terminal/issues/2046)

Keybindings args: #[1349](https://github.com/microsoft/terminal/pull/1349)

Cascading User & Default Settings: #[754](https://github.com/microsoft/terminal/issues/754)

Untie "active control" from "currently XAML-focused control" #[1205](https://github.com/microsoft/terminal/issues/1205)

<!-- Footnotes -->
[#754]: https://github.com/microsoft/terminal/issues/754
[#1205]: https://github.com/microsoft/terminal/issues/1205
[#1142]: https://github.com/microsoft/terminal/pull/1349
[#2046]: https://github.com/microsoft/terminal/issues/2046