From 16b8ea14d6e17e7382566e86a863a832c30b910f Mon Sep 17 00:00:00 2001 From: PankajBhojwani Date: Thu, 22 Oct 2020 20:21:07 -0400 Subject: [PATCH] Display a warning for when we fail to write to the settings file (#7950) We wrap the call to `_WriteSettings` in `CascadiaSettingsSerialization.cpp` in a try/catch block, and if we catch an error we append a warning telling the user to check the permissions on their settings file. Closes #7727 --- src/cascadia/TerminalApp/AppLogic.cpp | 3 ++- .../TerminalApp/Resources/en-US/Resources.resw | 3 +++ .../TerminalSettingsModel/CascadiaSettings.cpp | 12 ++++++++++-- .../TerminalSettingsModel/CascadiaSettings.h | 2 ++ .../CascadiaSettingsSerialization.cpp | 10 +++++++++- .../TerminalSettingsModel/TerminalWarnings.idl | 1 + 6 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 112387f8676..72d7dae9cf2 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -40,7 +40,8 @@ static const std::array(SettingsLoadWar USES_RESOURCE(L"TooManyKeysForChord"), USES_RESOURCE(L"MissingRequiredParameter"), USES_RESOURCE(L"LegacyGlobalsProperty"), - USES_RESOURCE(L"FailedToParseCommandJson") + USES_RESOURCE(L"FailedToParseCommandJson"), + USES_RESOURCE(L"FailedToWriteToSettings") }; static const std::array(SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels { USES_RESOURCE(L"NoProfilesText"), diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 446bc161976..c7b08834f56 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -469,4 +469,7 @@ Cancel + + We could not write to your settings file. Check the permissions on that file to ensure that the read-only flag is not set and that write access is granted. + diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp index 0110c4babdf..def525891de 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp @@ -157,6 +157,16 @@ IVectorView C return _warnings.GetView(); } +void CascadiaSettings::ClearWarnings() +{ + _warnings.Clear(); +} + +void CascadiaSettings::AppendWarning(SettingsLoadWarnings warning) +{ + _warnings.Append(warning); +} + winrt::Windows::Foundation::IReference CascadiaSettings::GetLoadingError() { return _loadError; @@ -179,8 +189,6 @@ winrt::hstring CascadiaSettings::GetSerializationErrorMessage() // - void CascadiaSettings::_ValidateSettings() { - _warnings.Clear(); - // Make sure to check that profiles exists at all first and foremost: _ValidateProfilesExist(); diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h index 65aed31eb32..37a7e70564a 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h @@ -83,6 +83,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation Model::ColorScheme GetColorSchemeForProfile(const guid profileGuid) const; Windows::Foundation::Collections::IVectorView Warnings(); + void ClearWarnings(); + void AppendWarning(SettingsLoadWarnings warning); Windows::Foundation::IReference GetLoadingError(); hstring GetSerializationErrorMessage(); diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index a3fa1c6fc3e..7f77a98123c 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -106,6 +106,7 @@ winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings CascadiaSettings:: { auto settings = LoadDefaults(); auto resultPtr = winrt::get_self(settings); + resultPtr->ClearWarnings(); // 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 @@ -185,7 +186,14 @@ winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings CascadiaSettings:: // We should re-parse, but not re-layer resultPtr->_ParseJsonString(resultPtr->_userSettingsString, false); - _WriteSettings(resultPtr->_userSettingsString); + try + { + _WriteSettings(resultPtr->_userSettingsString); + } + catch (...) + { + resultPtr->AppendWarning(SettingsLoadWarnings::FailedToWriteToSettings); + } } // If this throws, the app will catch it and use the default settings diff --git a/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl b/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl index f6010952124..aa5bf80f109 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl +++ b/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl @@ -17,6 +17,7 @@ namespace Microsoft.Terminal.Settings.Model MissingRequiredParameter = 7, LegacyGlobalsProperty = 8, FailedToParseCommandJson = 9, + FailedToWriteToSettings = 10, WARNINGS_SIZE // IMPORTANT: This MUST be the last value in this enum. It's an unused placeholder. };