Skip to content

Commit

Permalink
Settings: Remove support for /globals
Browse files Browse the repository at this point in the history 
Refs #1069.
  • Loading branch information
@DHowett
DHowett committed Mar 30, 2020
1 parent 1ae4252 commit bba1d65
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 155 deletions.
150 changes: 13 additions & 137 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file
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);

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
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/CascadiaSettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ void CascadiaSettings::_ValidateProfilesHaveGuid()
}

// Method Description:
// - Checks if the "globals.defaultProfile" is set to one of the profiles we
// - Checks if the "defaultProfile" is set to one of the profiles we
// actually have. If the value is unset, or the value is set to something that
// doesn't exist in the list of profiles, we'll arbitrarily pick the first
// profile to use temporarily as the default.
Expand Down
17 changes: 0 additions & 17 deletions src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ static constexpr std::string_view ProfilesKey{ "profiles" };
static constexpr std::string_view DefaultSettingsKey{ "defaults" };
static constexpr std::string_view ProfilesListKey{ "list" };
static constexpr std::string_view KeybindingsKey{ "keybindings" };
static constexpr std::string_view GlobalsKey{ "globals" };
static constexpr std::string_view SchemesKey{ "schemes" };

static constexpr std::string_view DisabledProfileSourcesKey{ "disabledProfileSources" };
Expand Down Expand Up @@ -501,19 +500,8 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::FromJson(const Json::Value&
// <none>
void CascadiaSettings::LayerJson(const Json::Value& json)
{
// microsoft/terminal#2906: First layer the root object as our globals. If
// there is also a `globals` object, layer that one on top of the settings
// from the root.
_globals.LayerJson(json);

if (auto globals{ json[GlobalsKey.data()] })
{
if (globals.isObject())
{
_globals.LayerJson(globals);
}
}

if (auto schemes{ json[SchemesKey.data()] })
{
for (auto schemeJson : schemes)
Expand Down Expand Up @@ -919,10 +907,5 @@ const Json::Value& CascadiaSettings::_GetProfilesJsonObject(const Json::Value& j
// given object
const Json::Value& CascadiaSettings::_GetDisabledProfileSourcesJsonObject(const Json::Value& json)
{
// Check the globals first, then look in the root.
if (json.isMember(JsonKey(GlobalsKey)))
{
return json[JsonKey(GlobalsKey)][JsonKey(DisabledProfileSourcesKey)];
}
return json[JsonKey(DisabledProfileSourcesKey)];
}

0 comments on commit bba1d65

Please sign in to comment.