maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Bugfix: update color scheme references properly (#9103)

Browse source 
`CascadiaSettings::UpdateColorSchemeReferences` had two bugs in it:
1. we would never check/update the base layer
2. we would explicitly set the color scheme on a profile referencing the
   old name

This PR fixes both of those issues by checking/updating the base layer,
and ensuring that we check if a profile has an explicit reference before
updating it.

Since the affected code is in TSM, I also created an automated local
test.

## Validation Steps Performed
Bug repro steps.
Specifically tested [DHowett's scenario] too.
Test added.

Closes #9094 

[DHowett's scenario]: https://github.com/microsoft/terminal/issues/9094#issuecomment-776412781
This commit is contained in:
Carlos Zamora 2021-02-10 14:11:56 -08:00 committed by GitHub
parent ed19301ad3
commit 42511265e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 87 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

78
src/cascadia/LocalTests_SettingsModel/ColorSchemeTests.cpp
View file

@ -35,6 +35,7 @@ namespace SettingsModelLocalTests
TEST_METHOD(CanLayerColorScheme);
TEST_METHOD(LayerColorSchemeProperties);
TEST_METHOD(LayerColorSchemesOnArray);
TEST_METHOD(UpdateSchemeReferences);
TEST_CLASS_SETUP(ClassSetup)
{
@ -290,4 +291,81 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(ARGB(0, 7, 7, 7), scheme2->_Background);
}
}
void ColorSchemeTests::UpdateSchemeReferences()
{
const std::string settingsString{ R"json({
"defaultProfile": "Inherited reference",
"profiles": {
"defaults": {
"colorScheme": "Scheme 1"
},
"list": [
{
"name": "Explicit scheme reference",
"colorScheme": "Scheme 1"
},
{
"name": "Explicit reference; hidden",
"colorScheme": "Scheme 1",
"hidden": true
},
{
"name": "Inherited reference"
},
{
"name": "Different reference",
"colorScheme": "Scheme 2"
}
]
},
"schemes": [
{ "name": "Scheme 1" },
{ "name": "Scheme 2" },
{ "name": "Scheme 1 (renamed)" }
]
})json" };
auto settings{ winrt::make_self<CascadiaSettings>(false) };
settings->_ParseJsonString(settingsString, false);
settings->_ApplyDefaultsFromUserSettings();
settings->LayerJson(settings->_userSettings);
settings->_ValidateSettings();
// update all references to "Scheme 1"
const auto newName{ L"Scheme 1 (renamed)" };
settings->UpdateColorSchemeReferences(L"Scheme 1", newName);
// verify profile defaults
Log::Comment(L"Profile Defaults");
VERIFY_ARE_EQUAL(newName, settings->ProfileDefaults().ColorSchemeName());
VERIFY_IS_TRUE(settings->ProfileDefaults().HasColorSchemeName());
// verify all other profiles
const auto& profiles{ settings->AllProfiles() };
{
const auto& prof{ profiles.GetAt(0) };
Log::Comment(prof.Name().c_str());
VERIFY_ARE_EQUAL(newName, prof.ColorSchemeName());
VERIFY_IS_TRUE(prof.HasColorSchemeName());
}
{
const auto& prof{ profiles.GetAt(1) };
Log::Comment(prof.Name().c_str());
VERIFY_ARE_EQUAL(newName, prof.ColorSchemeName());
VERIFY_IS_TRUE(prof.HasColorSchemeName());
}
{
const auto& prof{ profiles.GetAt(2) };
Log::Comment(prof.Name().c_str());
VERIFY_ARE_EQUAL(newName, prof.ColorSchemeName());
VERIFY_IS_FALSE(prof.HasColorSchemeName());
}
{
const auto& prof{ profiles.GetAt(3) };
Log::Comment(prof.Name().c_str());
VERIFY_ARE_EQUAL(L"Scheme 2", prof.ColorSchemeName());
VERIFY_IS_TRUE(prof.HasColorSchemeName());
}
}
}

10
src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
View file

@ -874,10 +874,18 @@ winrt::Microsoft::Terminal::Settings::Model::ColorScheme CascadiaSettings::GetCo
// - <none>
void CascadiaSettings::UpdateColorSchemeReferences(const hstring oldName, const hstring newName)
{
// update profiles.defaults, if necessary
if (_userDefaultProfileSettings &&
_userDefaultProfileSettings->HasColorSchemeName() &&
_userDefaultProfileSettings->ColorSchemeName() == oldName)
{
_userDefaultProfileSettings->ColorSchemeName(newName);
}
// update all profiles referencing this color scheme
for (const auto& profile : _allProfiles)
{
if (profile.ColorSchemeName() == oldName)
if (profile.HasColorSchemeName() && profile.ColorSchemeName() == oldName)
{
profile.ColorSchemeName(newName);
}