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.

Sign up for GitHub

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

Jump to bottom

Remove a heap of legacy settings deserialization #5190

Merged
merged 5 commits into from
Mar 31, 2020
Merged

Remove a heap of legacy settings deserialization #5190

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bba1d65
Settings: Remove support for /globals
DHowett Jan 29, 2020
4a25554
Settings: remove legacy colorscheme key and colorTable
DHowett Jan 30, 2020
deef5e4
Settings: remove support for colorScheme.colors (array)
DHowett Jan 30, 2020
735c1f6
Settings: delete all legacy keybinding deserializers
DHowett Mar 30, 2020
14bd5c1
Update the docs for the deleted settings
DHowett Mar 31, 2020
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
76 changes: 8 additions & 68 deletions src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp
View file
Open in desktop
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
150 changes: 13 additions & 137 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ namespace TerminalAppLocalTests
TEST_METHOD(TestInvalidColorSchemeName);
TEST_METHOD(TestHelperFunctions);

TEST_METHOD(TestLayerGlobalsOnRoot);
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

TEST_METHOD(TestProfileIconWithEnvVar);
TEST_METHOD(TestProfileBackgroundImageWithEnvVar);

Expand Down Expand Up @@ -160,9 +158,7 @@ namespace TerminalAppLocalTests
{
const std::string goodProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name" : "profile0",
Expand All @@ -177,9 +173,7 @@ namespace TerminalAppLocalTests

const std::string badProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name" : "profile0",
Expand All @@ -194,9 +188,7 @@ namespace TerminalAppLocalTests

const std::string noDefaultAtAll{ R"(
{
"globals": {
"alwaysShowTabs": true
},
"alwaysShowTabs": true,
"profiles": [
{
"name" : "profile0",
Expand Down Expand Up @@ -388,9 +380,7 @@ namespace TerminalAppLocalTests
{
const std::string badProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name" : "profile0",
Expand Down Expand Up @@ -438,21 +428,17 @@ namespace TerminalAppLocalTests
{
const std::string settings0String{ R"(
{
"globals": {
"alwaysShowTabs": true,
"initialCols" : 120,
"initialRows" : 30,
"rowsToScroll" : 4
}
"alwaysShowTabs": true,
"initialCols" : 120,
"initialRows" : 30,
"rowsToScroll" : 4
})" };
const std::string settings1String{ R"(
{
"globals": {
"showTabsInTitlebar": false,
"initialCols" : 240,
"initialRows" : 60,
"rowsToScroll" : 8
}
"showTabsInTitlebar": false,
"initialCols" : 240,
"initialRows" : 60,
"rowsToScroll" : 8
})" };
const auto settings0Json = VerifyParseSucceeded(settings0String);
const auto settings1Json = VerifyParseSucceeded(settings1String);
Expand Down Expand Up @@ -1348,114 +1334,6 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(name2, prof2->GetName());
}

void SettingsTests::TestLayerGlobalsOnRoot()
{
// Test for microsoft/terminal#2906. We added the ability for the root
// to be used as the globals object in #2515. However, if you have a
// globals object, then the settings in the root would get ignored.
// This test ensures that settings from a child "globals" element
// _layer_ on top of root properties, and they don't cause the root
// properties to be totally ignored.

const std::string settings0String{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"initialRows": 123
}
})" };
const std::string settings1String{ R"(
{
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}",
"initialRows": 234
})" };
const std::string settings2String{ R"(
{
"defaultProfile": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
"initialRows": 345,
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
// initialRows should not be cleared here
}
})" };
const std::string settings3String{ R"(
{
"defaultProfile": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
"globals": {
"initialRows": 456
// defaultProfile should not be cleared here
}
})" };
const std::string settings4String{ R"(
{
"defaultProfile": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"defaultProfile": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
})" };
const std::string settings5String{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"defaultProfile": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
"globals": {
"defaultProfile": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
}
})" };

VerifyParseSucceeded(settings0String);
VerifyParseSucceeded(settings1String);
VerifyParseSucceeded(settings2String);
VerifyParseSucceeded(settings3String);
VerifyParseSucceeded(settings4String);
VerifyParseSucceeded(settings5String);
const auto guid1 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-1111-49a3-80bd-e8fdd045185c}");
const auto guid2 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-2222-49a3-80bd-e8fdd045185c}");
const auto guid3 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-3333-49a3-80bd-e8fdd045185c}");

{
CascadiaSettings settings;
settings._ParseJsonString(settings0String, false);
settings.LayerJson(settings._userSettings);
VERIFY_ARE_EQUAL(guid1, settings._globals._defaultProfile);
VERIFY_ARE_EQUAL(123, settings._globals._initialRows);
}
{
CascadiaSettings settings;
settings._ParseJsonString(settings1String, false);
settings.LayerJson(settings._userSettings);
VERIFY_ARE_EQUAL(guid1, settings._globals._defaultProfile);
VERIFY_ARE_EQUAL(234, settings._globals._initialRows);
}
{
CascadiaSettings settings;
settings._ParseJsonString(settings2String, false);
settings.LayerJson(settings._userSettings);
VERIFY_ARE_EQUAL(guid1, settings._globals._defaultProfile);
VERIFY_ARE_EQUAL(345, settings._globals._initialRows);
}
{
CascadiaSettings settings;
settings._ParseJsonString(settings3String, false);
settings.LayerJson(settings._userSettings);
VERIFY_ARE_EQUAL(guid2, settings._globals._defaultProfile);
VERIFY_ARE_EQUAL(456, settings._globals._initialRows);
}
{
CascadiaSettings settings;
settings._ParseJsonString(settings4String, false);
settings.LayerJson(settings._userSettings);
VERIFY_ARE_EQUAL(guid1, settings._globals._defaultProfile);
}
{
CascadiaSettings settings;
settings._ParseJsonString(settings5String, false);
settings.LayerJson(settings._userSettings);
VERIFY_ARE_EQUAL(guid3, settings._globals._defaultProfile);
}
}
void SettingsTests::TestProfileIconWithEnvVar()
{
const auto expectedPath = wil::ExpandEnvironmentStringsW<std::wstring>(L"%WINDIR%\\System32\\x_80.png");
Expand Down Expand Up @@ -2311,9 +2189,7 @@ namespace TerminalAppLocalTests
{
const std::string badSettings{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
},
"defaultProfile": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
"profiles": [
{
"name" : "profile0",
Expand Down
Loading