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

Always create and link profiles.defaults object (#8445)

Browse source 
The Settings UI exposes the `profiles.defaults` (PD) object. Today, we
remove PD if there's nothing in it. However, that causes problems with
the Settings UI, because we have no `Profile` object to bind to
(resulting in a crash). Rather than making the Settings UI create a PD,
and link it in the inheritance tree, it's much easier to just _always_
create and link the PD object.

## References
#1564 - Settings UI (fixes a crash for this)
#7923 - Introduces inheritance

## PR Checklist
* [X] Tests added/passed

## Validation Steps Performed
* [x] repro steps for crash in Settings UI (copied changes over to that
      branch for testing)
* [x] tests passed
This commit is contained in:
Carlos Zamora 2020-12-01 06:11:26 -08:00 committed by GitHub
parent 4060a18937
commit 62131720aa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp
View file

@ -2568,9 +2568,9 @@ namespace SettingsModelLocalTests
const auto copy{ settings->Copy() };
const auto copyImpl{ winrt::get_self<implementation::CascadiaSettings>(copy) };
// test optimization: if we don't have profiles.defaults, don't add it to the tree
VERIFY_IS_NULL(settings->_userDefaultProfileSettings);
VERIFY_ARE_EQUAL(settings->_userDefaultProfileSettings, copyImpl->_userDefaultProfileSettings);
// if we don't have profiles.defaults, it should still be in the tree
VERIFY_IS_NOT_NULL(settings->_userDefaultProfileSettings);
VERIFY_IS_NOT_NULL(copyImpl->_userDefaultProfileSettings);
VERIFY_ARE_EQUAL(settings->ActiveProfiles().Size(), 1u);
VERIFY_ARE_EQUAL(settings->ActiveProfiles().Size(), copyImpl->ActiveProfiles().Size());
@ -2578,7 +2578,7 @@ namespace SettingsModelLocalTests
// so we should only have one parent, instead of two
auto srcProfile{ winrt::get_self<implementation::Profile>(settings->ActiveProfiles().GetAt(0)) };
auto copyProfile{ winrt::get_self<implementation::Profile>(copyImpl->ActiveProfiles().GetAt(0)) };
VERIFY_ARE_EQUAL(srcProfile->Parents().size(), 0u);
VERIFY_ARE_EQUAL(srcProfile->Parents().size(), 1u);
VERIFY_ARE_EQUAL(srcProfile->Parents().size(), copyProfile->Parents().size());
};

42
src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp
View file

@ -732,7 +732,8 @@ void CascadiaSettings::_ApplyDefaultsFromUserSettings()
{
// If `profiles` was an object, then look for the `defaults` object
// underneath it for the default profile settings.
auto defaultSettings{ Json::Value::null };
// If there isn't one, we still want to add an empty "default" profile to the inheritance tree.
Json::Value defaultSettings{ Json::ValueType::objectValue };
if (const auto profiles{ _userSettings[JsonKey(ProfilesKey)] })
{
if (profiles.isObject() && !profiles[JsonKey(DefaultSettingsKey)].empty())
@ -741,31 +742,26 @@ void CascadiaSettings::_ApplyDefaultsFromUserSettings()
}
}
// cache and apply default profile settings
// from user settings file
if (defaultSettings)
// Remove the `guid` member from the default settings. That'll
// hyper-explode, so just don't let them do that.
defaultSettings.removeMember({ "guid" });
_userDefaultProfileSettings = winrt::make_self<Profile>();
_userDefaultProfileSettings->LayerJson(defaultSettings);
const auto numOfProfiles{ _allProfiles.Size() };
for (uint32_t profileIndex = 0; profileIndex < numOfProfiles; ++profileIndex)
{
// Remove the `guid` member from the default settings. That'll
// hyper-explode, so just don't let them do that.
defaultSettings.removeMember({ "guid" });
// create a child, so we inherit from the defaults.json layer
auto parentProj{ _allProfiles.GetAt(profileIndex) };
auto parentImpl{ winrt::get_self<Profile>(parentProj) };
auto childImpl{ parentImpl->CreateChild() };
_userDefaultProfileSettings = winrt::make_self<Profile>();
_userDefaultProfileSettings->LayerJson(defaultSettings);
// Add profile.defaults as the _first_ parent to the child
childImpl->InsertParent(0, _userDefaultProfileSettings);
const auto numOfProfiles{ _allProfiles.Size() };
for (uint32_t profileIndex = 0; profileIndex < numOfProfiles; ++profileIndex)
{
// create a child, so we inherit from the defaults.json layer
auto parentProj{ _allProfiles.GetAt(profileIndex) };
auto parentImpl{ winrt::get_self<Profile>(parentProj) };
auto childImpl{ parentImpl->CreateChild() };
// Add profile.defaults as the _first_ parent to the child
childImpl->InsertParent(0, _userDefaultProfileSettings);
// replace parent in _profiles with child
_allProfiles.SetAt(profileIndex, *childImpl);
}
// replace parent in _profiles with child
_allProfiles.SetAt(profileIndex, *childImpl);
}
}