Don't overwrite the settings file (#2475)
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_.
This commit is contained in:
parent
f9752148d0
commit
8096d7cf2f
|
@ -680,19 +680,15 @@ namespace winrt::TerminalApp::implementation
|
||||||
|
|
||||||
// Method Description:
|
// Method Description:
|
||||||
// - Attempt to load the settings. If we fail for any reason, returns an error.
|
// - 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:
|
// Return Value:
|
||||||
// - S_OK if we successfully parsed the settings, otherwise an appropriate HRESULT.
|
// - 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;
|
HRESULT hr = E_FAIL;
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
auto newSettings = CascadiaSettings::LoadAll(saveOnLoad);
|
auto newSettings = CascadiaSettings::LoadAll();
|
||||||
_settings = std::move(newSettings);
|
_settings = std::move(newSettings);
|
||||||
const auto& warnings = _settings->GetWarnings();
|
const auto& warnings = _settings->GetWarnings();
|
||||||
hr = warnings.size() == 0 ? S_OK : S_FALSE;
|
hr = warnings.size() == 0 ? S_OK : S_FALSE;
|
||||||
|
@ -733,7 +729,7 @@ namespace winrt::TerminalApp::implementation
|
||||||
// we should display the loading error.
|
// we should display the loading error.
|
||||||
// * We can't display the error now, because we might not have a
|
// * We can't display the error now, because we might not have a
|
||||||
// UI yet. We'll display the error in _OnLoaded.
|
// UI yet. We'll display the error in _OnLoaded.
|
||||||
_settingsLoadedResult = _TryLoadSettings(true);
|
_settingsLoadedResult = _TryLoadSettings();
|
||||||
|
|
||||||
if (FAILED(_settingsLoadedResult))
|
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 change the settings (and don't actually apply the new settings)
|
||||||
// - don't persist them.
|
// - don't persist them.
|
||||||
// - display a loading error
|
// - display a loading error
|
||||||
_settingsLoadedResult = _TryLoadSettings(false);
|
_settingsLoadedResult = _TryLoadSettings();
|
||||||
|
|
||||||
if (FAILED(_settingsLoadedResult))
|
if (FAILED(_settingsLoadedResult))
|
||||||
{
|
{
|
||||||
|
|
|
@ -85,7 +85,7 @@ namespace winrt::TerminalApp::implementation
|
||||||
void _ShowLoadWarningsDialog();
|
void _ShowLoadWarningsDialog();
|
||||||
void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey);
|
void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey);
|
||||||
|
|
||||||
[[nodiscard]] HRESULT _TryLoadSettings(const bool saveOnLoad) noexcept;
|
[[nodiscard]] HRESULT _TryLoadSettings() noexcept;
|
||||||
void _LoadSettings();
|
void _LoadSettings();
|
||||||
void _OpenSettings();
|
void _OpenSettings();
|
||||||
|
|
||||||
|
|
|
@ -40,7 +40,7 @@ public:
|
||||||
CascadiaSettings();
|
CascadiaSettings();
|
||||||
~CascadiaSettings();
|
~CascadiaSettings();
|
||||||
|
|
||||||
static std::unique_ptr<CascadiaSettings> LoadAll(const bool saveOnLoad = true);
|
static std::unique_ptr<CascadiaSettings> LoadAll();
|
||||||
void SaveAll() const;
|
void SaveAll() const;
|
||||||
|
|
||||||
winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional<GUID> profileGuid) const;
|
winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional<GUID> profileGuid) const;
|
||||||
|
|
|
@ -30,12 +30,9 @@ static constexpr std::string_view Utf8Bom{ u8"\uFEFF" };
|
||||||
// it will load the settings from our packaged localappdata. If we're
|
// it will load the settings from our packaged localappdata. If we're
|
||||||
// running as an unpackaged application, it will read it from the path
|
// running as an unpackaged application, it will read it from the path
|
||||||
// we've set under localappdata.
|
// 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:
|
// Return Value:
|
||||||
// - a unique_ptr containing a new CascadiaSettings object.
|
// - a unique_ptr containing a new CascadiaSettings object.
|
||||||
std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll(const bool saveOnLoad)
|
std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll()
|
||||||
{
|
{
|
||||||
std::unique_ptr<CascadiaSettings> resultPtr;
|
std::unique_ptr<CascadiaSettings> resultPtr;
|
||||||
std::optional<std::string> fileData = _ReadSettings();
|
std::optional<std::string> fileData = _ReadSettings();
|
||||||
|
@ -70,22 +67,6 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll(const bool saveOnLoa
|
||||||
|
|
||||||
// If this throws, the app will catch it and use the default settings (temporarily)
|
// If this throws, the app will catch it and use the default settings (temporarily)
|
||||||
resultPtr->_ValidateSettings();
|
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
|
else
|
||||||
{
|
{
|
||||||
|
|
Loading…
Reference in a new issue