Skip to content

Commit

Permalink
Resolve the default profile during defaults load, don't crash on laun…
Browse files Browse the repository at this point in the history
…ch (#7237)

The "default profile as name" feature in 1.1 broke the loading of
default settings, as we would never get to the validation phase where
the default profile string was transformed into a guid.

I moved knowledge of the "unparsed default profile" optional to the
consumer so that we could make sure we only attempted to deserialize it
once (and only if it was present.)

Fixes #7236.

* [x] Closes #7236

(cherry picked from commit c03677b)
  • Loading branch information
DHowett committed Aug 11, 2020
1 parent b023fff commit e354313
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 9 deletions.
9 changes: 6 additions & 3 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,12 @@ void CascadiaSettings::_ValidateProfilesHaveGuid()
void CascadiaSettings::_ResolveDefaultProfile()
{
const auto unparsedDefaultProfile{ GlobalSettings().UnparsedDefaultProfile() };
auto maybeParsedDefaultProfile{ _GetProfileGuidByName(unparsedDefaultProfile) };
auto defaultProfileGuid{ Utils::CoalesceOptionals(maybeParsedDefaultProfile, GUID{}) };
GlobalSettings().DefaultProfile(defaultProfileGuid);
if (unparsedDefaultProfile)
{
auto maybeParsedDefaultProfile{ _GetProfileGuidByName(unparsedDefaultProfile) };
auto defaultProfileGuid{ Utils::CoalesceOptionals(maybeParsedDefaultProfile, GUID{}) };
GlobalSettings().DefaultProfile(defaultProfileGuid);
}
}

// Method Description:
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll()
// GH 3588, we need this below to know if the user chose something that wasn't our default.
// Collect it up here in case it gets modified by any of the other layers between now and when
// the user's preferences are loaded and layered.
const auto hardcodedDefaultGuid = resultPtr->GlobalSettings().UnparsedDefaultProfile();
const auto hardcodedDefaultGuid = resultPtr->GlobalSettings().DefaultProfile();

std::optional<std::string> fileData = _ReadUserSettings();
const bool foundFile = fileData.has_value();
Expand Down Expand Up @@ -141,12 +141,11 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll()
// is a lot of computation we can skip if no one cares.
if (TraceLoggingProviderEnabled(g_hTerminalAppProvider, 0, MICROSOFT_KEYWORD_MEASURES))
{
const auto hardcodedDefaultGuidAsGuid = Utils::GuidFromString(hardcodedDefaultGuid);
const auto guid = resultPtr->GlobalSettings().DefaultProfile();

// Compare to the defaults.json one that we set on install.
// If it's different, log what the user chose.
if (hardcodedDefaultGuidAsGuid != guid)
if (hardcodedDefaultGuid != guid)
{
TraceLoggingWrite(
g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider
Expand Down Expand Up @@ -229,6 +228,7 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadDefaults()
// them from a file (and the potential that could fail)
resultPtr->_ParseJsonString(DefaultJson, true);
resultPtr->LayerJson(resultPtr->_defaultSettings);
resultPtr->_ResolveDefaultProfile();

return resultPtr;
}
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/GlobalAppSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ GUID GlobalAppSettings::DefaultProfile() const
return _defaultProfile;
}

std::wstring GlobalAppSettings::UnparsedDefaultProfile() const
std::optional<std::wstring> GlobalAppSettings::UnparsedDefaultProfile() const
{
return _unparsedDefaultProfile.value();
return _unparsedDefaultProfile;
}

AppKeyBindings GlobalAppSettings::GetKeybindings() const noexcept
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/GlobalAppSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class TerminalApp::GlobalAppSettings final
// by higher layers in the app.
void DefaultProfile(const GUID defaultProfile) noexcept;
GUID DefaultProfile() const;
std::wstring UnparsedDefaultProfile() const;
std::optional<std::wstring> UnparsedDefaultProfile() const;

GETSET_PROPERTY(int32_t, InitialRows); // default value set in constructor
GETSET_PROPERTY(int32_t, InitialCols); // default value set in constructor
Expand Down

0 comments on commit e354313

Please sign in to comment.