Skip to content

Commit

Permalink
Display a warning for when we fail to write to the settings file (#7950)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
PankajBhojwani authored Oct 23, 2020
1 parent 4f39e8e commit 16b8ea1
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 4 deletions.
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ static const std::array<std::wstring_view, static_cast<uint32_t>(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<std::wstring_view, static_cast<uint32_t>(SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels {
USES_RESOURCE(L"NoProfilesText"),
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -469,4 +469,7 @@
<data name="CouldNotOpenUriDialog.PrimaryButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="FailedToWriteToSettings" xml:space="preserve">
<value>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.</value>
</data>
</root>
12 changes: 10 additions & 2 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,16 @@ IVectorView<winrt::Microsoft::Terminal::Settings::Model::SettingsLoadWarnings> C
return _warnings.GetView();
}

void CascadiaSettings::ClearWarnings()
{
_warnings.Clear();
}

void CascadiaSettings::AppendWarning(SettingsLoadWarnings warning)
{
_warnings.Append(warning);
}

winrt::Windows::Foundation::IReference<winrt::Microsoft::Terminal::Settings::Model::SettingsLoadErrors> CascadiaSettings::GetLoadingError()
{
return _loadError;
Expand All @@ -179,8 +189,6 @@ winrt::hstring CascadiaSettings::GetSerializationErrorMessage()
// - <none>
void CascadiaSettings::_ValidateSettings()
{
_warnings.Clear();

// Make sure to check that profiles exists at all first and foremost:
_ValidateProfilesExist();

Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
Model::ColorScheme GetColorSchemeForProfile(const guid profileGuid) const;

Windows::Foundation::Collections::IVectorView<SettingsLoadWarnings> Warnings();
void ClearWarnings();
void AppendWarning(SettingsLoadWarnings warning);
Windows::Foundation::IReference<SettingsLoadErrors> GetLoadingError();
hstring GetSerializationErrorMessage();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings CascadiaSettings::
{
auto settings = LoadDefaults();
auto resultPtr = winrt::get_self<CascadiaSettings>(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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/TerminalWarnings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
};

Expand Down

0 comments on commit 16b8ea1

Please sign in to comment.