Skip to content

Commit

Permalink
Remove a heap of legacy settings deserialization (#5190)
Browse files Browse the repository at this point in the history 
This commit removes support for:

* legacy keybindings of all types
* `colorScheme.colors`, as an array
* A `globals` object in the root of the settings file
* `profile.colorTable` and `profile.colorscheme` (the rare v0.1 all-lowercase variety)

Fixes #4091.
Fixes #1069.
  • Loading branch information
@DHowett
DHowett committed Mar 31, 2020
1 parent ca6b54e commit cd9e854
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 632 deletions.
3 changes: 1 addition & 2 deletions doc/cascadia/SettingsSchema.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,13 @@ For commands with arguments:

| Command | Command Description | Action (*=required) | Action Arguments | Argument Descriptions |
| ------- | ------------------- | ------ | ---------------- | ----------------- |
| `adjustFontSize` | Change the text size by a specified point amount. | `delta` | integer | Amount of size change per command invocation. |
| `closePane` | Close the active pane. | | | |
| `closeTab` | Close the current tab. | | | |
| `closeWindow` | Close the current window and all tabs within it. | | | |
| `copy` | Copy the selected terminal content to your Windows Clipboard. | `trimWhitespace` | boolean | When `true`, newlines persist from the selected text. When `false`, copied content will paste on one line. |
| `decreaseFontSize` | Make the text smaller by one delta. | `delta` | integer | Amount of size decrease per command invocation. |
| `duplicateTab` | Make a copy and open the current tab. | | | |
| `find` | Open the search dialog box. | | | |
| `increaseFontSize` | Make the text larger by one delta. | `delta` | integer | Amount of size increase per command invocation. |
| `moveFocus` | Focus on a different pane depending on direction. | `direction`* | `left`, `right`, `up`, `down` | Direction in which the focus will move. |
| `newTab` | Create a new tab. Without any arguments, this will open the default profile in a new tab. | 1. `commandLine`<br>2. `startingDirectory`<br>3. `tabTitle`<br>4. `index`<br>5. `profile` | 1. string<br>2. string<br>3. string<br>4. integer<br>5. string | 1. Executable run within the tab.<br>2. Directory in which the tab will open.<br>3. Title of the new tab.<br>4. Profile that will open based on its position in the dropdown (starting at 0).<br>5. Profile that will open based on its GUID or name. |
| `nextTab` | Open the tab to the right of the current one. | | | |
Expand Down
49 changes: 19 additions & 30 deletions doc/cascadia/profiles.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,57 +25,28 @@
},
"ShortcutActionName": {
"enum": [
"adjustFontSize",
"closePane",
"closeTab",
"closeWindow",
"copy",
"copyTextWithoutNewlines",
"decreaseFontSize",
"duplicateTab",
"increaseFontSize",
"moveFocus",
"moveFocusDown",
"moveFocusLeft",
"moveFocusRight",
"moveFocusUp",
"newTab",
"newTabProfile0",
"newTabProfile1",
"newTabProfile2",
"newTabProfile3",
"newTabProfile4",
"newTabProfile5",
"newTabProfile6",
"newTabProfile7",
"newTabProfile8",
"nextTab",
"openNewTabDropdown",
"openSettings",
"paste",
"prevTab",
"resetFontSize",
"resizePane",
"resizePaneDown",
"resizePaneLeft",
"resizePaneRight",
"resizePaneUp",
"scrollDown",
"scrollDownPage",
"scrollUp",
"scrollUpPage",
"splitHorizontal",
"splitVertical",
"splitPane",
"switchToTab",
"switchToTab0",
"switchToTab1",
"switchToTab2",
"switchToTab3",
"switchToTab4",
"switchToTab5",
"switchToTab6",
"switchToTab7",
"switchToTab8",
"toggleFullscreen",
"find"
],
Expand Down Expand Up @@ -135,6 +106,23 @@
],
"type": "object"
},
"AdjustFontSizeAction": {
"description": "Arguments corresponding to an Adjust Font Size Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "adjustFontSize" },
"delta": {
"type": "integer",
"default": 0,
"description": "How much to change the current font point size"
}
}
}
],
"required": [ "delta" ]
},
"CopyAction": {
"description": "Arguments corresponding to a Copy Text Action",
"allOf": [
Expand Down Expand Up @@ -238,6 +226,7 @@
"command": {
"description": "The action executed when the associated key bindings are pressed.",
"oneOf": [
{ "$ref": "#/definitions/AdjustFontSizeAction" },
{ "$ref": "#/definitions/CopyAction" },
{ "$ref": "#/definitions/ShortcutActionName" },
{ "$ref": "#/definitions/NewTabAction" },
Expand Down
2 changes: 1 addition & 1 deletion doc/user-docs/UsingJsonSettings.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ following objects into your `globals.keybindings` array:
{ "command": "paste", "keys": ["ctrl+shift+v"] }
```

> 👉 **Note**: you can also add a keybinding for the `copyTextWithoutNewlines` command. This removes newlines as the text is copied to your clipboard.
> 👉 **Note**: you can also add a keybinding for the `copy` command with the argument `"trimWhitespace": true`. This removes newlines as the text is copied to your clipboard.
This will add copy and paste on <kbd>ctrl+shift+c</kbd>
and <kbd>ctrl+shift+v</kbd> respectively.
Expand Down
76 changes: 8 additions & 68 deletions src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,18 @@ namespace TerminalAppLocalTests
{
const std::string bindings0String{ R"([
{ "command": "copy", "keys": ["ctrl+c"] },
{ "command": "copyTextWithoutNewlines", "keys": ["alt+c"] },
{ "command": { "action": "copy", "trimWhitespace": false }, "keys": ["ctrl+shift+c"] },
{ "command": { "action": "copy", "trimWhitespace": true }, "keys": ["alt+shift+c"] },
{ "command": "newTab", "keys": ["ctrl+t"] },
{ "command": { "action": "newTab", "index": 0 }, "keys": ["ctrl+shift+t"] },
{ "command": "newTabProfile0", "keys": ["alt+shift+t"] },
{ "command": { "action": "newTab", "index": 11 }, "keys": ["ctrl+shift+y"] },
{ "command": "newTabProfile8", "keys": ["alt+shift+y"] },
{ "command": { "action": "copy", "madeUpBool": true }, "keys": ["ctrl+b"] },
{ "command": { "action": "copy" }, "keys": ["ctrl+shift+b"] },
{ "command": "increaseFontSize", "keys": ["ctrl+f"] },
{ "command": "decreaseFontSize", "keys": ["ctrl+g"] }
{ "command": { "action": "adjustFontSize", "delta": 1 }, "keys": ["ctrl+f"] },
{ "command": { "action": "adjustFontSize", "delta": -1 }, "keys": ["ctrl+g"] }
])" };

Expand All @@ -194,7 +191,7 @@ namespace TerminalAppLocalTests
VERIFY_IS_NOT_NULL(appKeyBindings);
VERIFY_ARE_EQUAL(0u, appKeyBindings->_keyShortcuts.size());
appKeyBindings->LayerJson(bindings0Json);
VERIFY_ARE_EQUAL(13u, appKeyBindings->_keyShortcuts.size());
VERIFY_ARE_EQUAL(10u, appKeyBindings->_keyShortcuts.size());

{
Log::Comment(NoThrowString().Format(
Expand All @@ -207,17 +204,6 @@ namespace TerminalAppLocalTests
VERIFY_IS_TRUE(realArgs.TrimWhitespace());
}

{
Log::Comment(NoThrowString().Format(
L"Verify that `copyTextWithoutNewlines` parses as Copy(TrimWhitespace=false)"));
KeyChord kc{ false, true, false, static_cast<int32_t>('C') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
const auto& realArgs = actionAndArgs.Args().try_as<CopyTextArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_IS_FALSE(realArgs.TrimWhitespace());
}

{
Log::Comment(NoThrowString().Format(
L"Verify that `copy` with args parses them correctly"));
Expand Down Expand Up @@ -265,19 +251,6 @@ namespace TerminalAppLocalTests
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
VERIFY_ARE_EQUAL(0, realArgs.TerminalArgs().ProfileIndex().Value());
}
{
Log::Comment(NoThrowString().Format(
L"Verify that `newTabProfile0` parses as NewTab(Index=0)"));
KeyChord kc{ false, true, true, static_cast<int32_t>('T') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTabProfile0, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
VERIFY_ARE_EQUAL(0, realArgs.TerminalArgs().ProfileIndex().Value());
}
{
Log::Comment(NoThrowString().Format(
L"Verify that `newTab` with an index greater than the legacy "
Expand All @@ -292,19 +265,6 @@ namespace TerminalAppLocalTests
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
VERIFY_ARE_EQUAL(11, realArgs.TerminalArgs().ProfileIndex().Value());
}
{
Log::Comment(NoThrowString().Format(
L"Verify that `newTabProfile8` parses as NewTab(Index=8)"));
KeyChord kc{ false, true, true, static_cast<int32_t>('Y') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::NewTabProfile8, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
VERIFY_ARE_EQUAL(8, realArgs.TerminalArgs().ProfileIndex().Value());
}

{
Log::Comment(NoThrowString().Format(
Expand Down Expand Up @@ -332,10 +292,10 @@ namespace TerminalAppLocalTests

{
Log::Comment(NoThrowString().Format(
L"Verify that `increaseFontSize` without args parses as AdjustFontSize(Delta=1)"));
L"Verify that `adjustFontSize` with a positive delta parses args correctly"));
KeyChord kc{ true, false, false, static_cast<int32_t>('F') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::IncreaseFontSize, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::AdjustFontSize, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<AdjustFontSizeArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
Expand All @@ -344,10 +304,10 @@ namespace TerminalAppLocalTests

{
Log::Comment(NoThrowString().Format(
L"Verify that `decreaseFontSize` without args parses as AdjustFontSize(Delta=-1)"));
L"Verify that `adjustFontSize` with a negative delta parses args correctly"));
KeyChord kc{ true, false, false, static_cast<int32_t>('G') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::DecreaseFontSize, actionAndArgs.Action());
VERIFY_ARE_EQUAL(ShortcutAction::AdjustFontSize, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<AdjustFontSizeArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
Expand All @@ -358,8 +318,6 @@ namespace TerminalAppLocalTests
void KeyBindingsTests::TestSplitPaneArgs()
{
const std::string bindings0String{ R"([
{ "keys": ["ctrl+a"], "command": "splitVertical" },
{ "keys": ["ctrl+b"], "command": "splitHorizontal" },
{ "keys": ["ctrl+c"], "command": { "action": "splitPane", "split": null } },
{ "keys": ["ctrl+d"], "command": { "action": "splitPane", "split": "vertical" } },
{ "keys": ["ctrl+e"], "command": { "action": "splitPane", "split": "horizontal" } },
Expand All @@ -375,26 +333,8 @@ namespace TerminalAppLocalTests
VERIFY_IS_NOT_NULL(appKeyBindings);
VERIFY_ARE_EQUAL(0u, appKeyBindings->_keyShortcuts.size());
appKeyBindings->LayerJson(bindings0Json);
VERIFY_ARE_EQUAL(9u, appKeyBindings->_keyShortcuts.size());
VERIFY_ARE_EQUAL(7u, appKeyBindings->_keyShortcuts.size());

{
KeyChord kc{ true, false, false, static_cast<int32_t>('A') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitVertical, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Vertical, realArgs.SplitStyle());
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('B') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitHorizontal, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Horizontal, realArgs.SplitStyle());
}
{
KeyChord kc{ true, false, false, static_cast<int32_t>('C') };
auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc);
Expand Down
Loading

0 comments on commit cd9e854

Please sign in to comment.