From a92a3b7b3b8e4a39a138e82019b8c4845962ab7d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 19 Aug 2019 12:29:26 -0500 Subject: [PATCH] Just never save the settings This is more trouble than it's worth. We had code before to re-serialize settings when they changed, to try and gracefully migrate settings from old schemas to new ones. This is good in theory, but with #754 coming soon, this is going to become a minefield. In the future we'll just always be providing a base schema that's reasonable, so this won't matter so much. Keys that users have that aren't understood will just be ignored, and that's _fine_. --- src/cascadia/TerminalApp/App.cpp | 12 ++++------- src/cascadia/TerminalApp/App.h | 2 +- src/cascadia/TerminalApp/CascadiaSettings.h | 2 +- .../CascadiaSettingsSerialization.cpp | 21 +------------------ 4 files changed, 7 insertions(+), 30 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index da2fe34c8ff..5103405f727 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -680,19 +680,15 @@ namespace winrt::TerminalApp::implementation // Method Description: // - Attempt to load the settings. If we fail for any reason, returns an error. - // Arguments: - // - saveOnLoad: If true, after loading the settings, we should re-write - // them to the file, to make sure the schema is updated. See - // `CascadiaSettings::LoadAll` for details. // Return Value: // - S_OK if we successfully parsed the settings, otherwise an appropriate HRESULT. - [[nodiscard]] HRESULT App::_TryLoadSettings(const bool saveOnLoad) noexcept + [[nodiscard]] HRESULT App::_TryLoadSettings() noexcept { HRESULT hr = E_FAIL; try { - auto newSettings = CascadiaSettings::LoadAll(saveOnLoad); + auto newSettings = CascadiaSettings::LoadAll(); _settings = std::move(newSettings); const auto& warnings = _settings->GetWarnings(); hr = warnings.size() == 0 ? S_OK : S_FALSE; @@ -733,7 +729,7 @@ namespace winrt::TerminalApp::implementation // we should display the loading error. // * We can't display the error now, because we might not have a // UI yet. We'll display the error in _OnLoaded. - _settingsLoadedResult = _TryLoadSettings(true); + _settingsLoadedResult = _TryLoadSettings(); if (FAILED(_settingsLoadedResult)) { @@ -813,7 +809,7 @@ namespace winrt::TerminalApp::implementation // - don't change the settings (and don't actually apply the new settings) // - don't persist them. // - display a loading error - _settingsLoadedResult = _TryLoadSettings(false); + _settingsLoadedResult = _TryLoadSettings(); if (FAILED(_settingsLoadedResult)) { diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 5d993e3410b..14b2f86d71c 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -85,7 +85,7 @@ namespace winrt::TerminalApp::implementation void _ShowLoadWarningsDialog(); void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey); - [[nodiscard]] HRESULT _TryLoadSettings(const bool saveOnLoad) noexcept; + [[nodiscard]] HRESULT _TryLoadSettings() noexcept; void _LoadSettings(); void _OpenSettings(); diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index 6334705bdc4..703cdaa0990 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -40,7 +40,7 @@ class TerminalApp::CascadiaSettings final CascadiaSettings(); ~CascadiaSettings(); - static std::unique_ptr LoadAll(const bool saveOnLoad = true); + static std::unique_ptr LoadAll(); void SaveAll() const; winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional profileGuid) const; diff --git a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp index dfaf68aa05c..d8b35a0a675 100644 --- a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp @@ -30,12 +30,9 @@ static constexpr std::string_view Utf8Bom{ u8"\uFEFF" }; // it will load the settings from our packaged localappdata. If we're // running as an unpackaged application, it will read it from the path // we've set under localappdata. -// Arguments: -// - saveOnLoad: If true, we'll write the settings back out after we load them, -// to make sure the schema is updated. // Return Value: // - a unique_ptr containing a new CascadiaSettings object. -std::unique_ptr CascadiaSettings::LoadAll(const bool saveOnLoad) +std::unique_ptr CascadiaSettings::LoadAll() { std::unique_ptr resultPtr; std::optional fileData = _ReadSettings(); @@ -70,22 +67,6 @@ std::unique_ptr CascadiaSettings::LoadAll(const bool saveOnLoa // If this throws, the app will catch it and use the default settings (temporarily) resultPtr->_ValidateSettings(); - - const bool foundWarnings = resultPtr->_warnings.size() > 0; - - // Don't save on load if there were warnings - we tried to gracefully - // handle them. - if (saveOnLoad && !foundWarnings) - { - // Logically compare the json we've parsed from the file to what - // we'd serialize at runtime. If the values are different, then - // write the updated schema back out. - const Json::Value reserialized = resultPtr->ToJson(); - if (reserialized != root) - { - resultPtr->SaveAll(); - } - } } else {