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

(cherry picked from commit 16b8ea1)
  • Loading branch information
PankajBhojwani authored and DHowett committed Oct 27, 2020
1 parent fe4f0f3 commit 7ca3e35
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 5 deletions.
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ static const std::array<std::wstring_view, static_cast<uint32_t>(winrt::Terminal
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>(winrt::TerminalApp::SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels {
USES_RESOURCE(L"NoProfilesText"),
Expand Down
12 changes: 10 additions & 2 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@ IVectorView<winrt::TerminalApp::SettingsLoadWarnings> CascadiaSettings::Warnings
return _warnings.GetView();
}

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

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

winrt::Windows::Foundation::IReference<winrt::TerminalApp::SettingsLoadErrors> CascadiaSettings::GetLoadingError()
{
return _loadError;
Expand All @@ -148,8 +158,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/TerminalApp/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ namespace winrt::TerminalApp::implementation
TerminalApp::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
10 changes: 9 additions & 1 deletion src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ winrt::TerminalApp::CascadiaSettings CascadiaSettings::LoadAll()
{
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::TerminalApp::CascadiaSettings CascadiaSettings::LoadAll()
// 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
5 changes: 4 additions & 1 deletion src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -664,4 +664,7 @@
<data name="CouldNotOpenUriDialog.PrimaryButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
</root>
<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>
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalWarnings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace TerminalApp
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 7ca3e35

Please sign in to comment.