From 80c18fb0ceb00ad4e33ab97d492cc09d418420f8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 6 Aug 2019 17:47:17 -0500 Subject: [PATCH] Warn the user when their settings are bad The start of work on #1348 --- src/cascadia/TerminalApp/App.cpp | 50 +++++++++++++- src/cascadia/TerminalApp/App.h | 1 + src/cascadia/TerminalApp/CascadiaSettings.cpp | 45 +++++++++++++ src/cascadia/TerminalApp/CascadiaSettings.h | 10 +++ .../CascadiaSettingsSerialization.cpp | 16 +++-- .../Resources/en-US/Resources.resw | 65 +++++++++++-------- 6 files changed, 153 insertions(+), 34 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 6c0ab7b673b..7bae4c0f19c 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -25,6 +25,13 @@ namespace winrt using IInspectable = Windows::Foundation::IInspectable; } +// clang-format off +static const std::unordered_map<::TerminalApp::SettingsLoadWarnings, std::wstring_view> settingsLoadWarningsLabels { + { SettingsLoadWarnings::MissingDefaultProfile , L"MissingDefaultProfileText"}, + { SettingsLoadWarnings::NoProfiles , L"NoProfilesText"}, +}; +// clang-format on + namespace winrt::TerminalApp::implementation { App::App() : @@ -176,6 +183,32 @@ namespace winrt::TerminalApp::implementation _ShowDialog(winrt::box_value(title), winrt::box_value(message), buttonText); } + void App::_ShowLoadWarningsDialog() + { + // TODO + auto title = _resourceLoader.GetLocalizedString(L"SettingsValidateErrorTitle"); + auto buttonText = _resourceLoader.GetLocalizedString(L"Ok"); + + Controls::TextBlock warningsTextBlock; + warningsTextBlock.IsTextSelectionEnabled(true); + const auto& warnings = _settings->GetLoadWarnings(); + + for (const auto& warning : warnings) + { + const auto labelFound = settingsLoadWarningsLabels.find(warning); + if (labelFound != settingsLoadWarningsLabels.end()) + { + winrt::Windows::UI::Xaml::Documents::Run warningRun; + const auto warningLabel = _resourceLoader.GetLocalizedString(labelFound->second); + warningRun.Text(warningLabel); + // TODO: Wrap this line, it's too long for the dialog + warningsTextBlock.Inlines().Append(warningRun); + } + } + + _ShowDialog(winrt::box_value(title), warningsTextBlock, buttonText); + } + // Method Description: // - Show a dialog with "About" information. Displays the app's Display // Name, version, getting started link, documentation link, and release @@ -262,6 +295,10 @@ namespace winrt::TerminalApp::implementation const winrt::hstring textKey = L"InitialJsonParseErrorText"; _ShowOkDialog(titleKey, textKey); } + else if (_settingsLoadedResult == S_FALSE) + { + _ShowLoadWarningsDialog(); + } } // Method Description: @@ -511,7 +548,8 @@ namespace winrt::TerminalApp::implementation { auto newSettings = CascadiaSettings::LoadAll(saveOnLoad); _settings = std::move(newSettings); - hr = S_OK; + const auto& warnings = _settings->GetLoadWarnings(); + hr = warnings.size() == 0 ? S_OK : S_FALSE; } catch (const winrt::hresult_error& e) { @@ -550,6 +588,10 @@ namespace winrt::TerminalApp::implementation _settings = std::make_unique(); _settings->CreateDefaults(); } + // else if (_settingsLoadedResult == S_FALSE) + // { + // _DisplayLoadingWarnings + // } _HookupKeyBindings(_settings->GetKeybindings()); @@ -635,6 +677,12 @@ namespace winrt::TerminalApp::implementation return; } + else if (_settingsLoadedResult == S_FALSE) + { + _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { + _ShowLoadWarningsDialog(); + }); + } // Here, we successfully reloaded the settings, and created a new // TerminalSettings object. diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 473f220d12c..90b58553961 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -79,6 +79,7 @@ namespace winrt::TerminalApp::implementation const winrt::hstring& closeButtonText); void _ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey); void _ShowAboutDialog(); + void _ShowLoadWarningsDialog(); [[nodiscard]] HRESULT _TryLoadSettings(const bool saveOnLoad) noexcept; void _LoadSettings(); diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index 2f0602d3475..8cb834c1285 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -643,3 +643,48 @@ Profile CascadiaSettings::_CreateDefaultProfile(const std::wstring_view name) return newProfile; } + +const std::vector& CascadiaSettings::GetLoadWarnings() +{ + return _warnings; +} + +void CascadiaSettings::_ValidateSettings() +{ + _warnings.clear(); + + const bool hasProfiles = _profiles.size() != 0; + if (!hasProfiles) + { + _warnings.push_back(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile); + + // Throw an exception. This is an invalid state, and we want the app to + // be able to gracefully use the default settings. + + // TODO: Throw something more specific? + throw winrt::hresult_invalid_argument(); + } + + const auto defaultProfileGuid = GlobalSettings().GetDefaultProfile(); + const bool nullDefaultProfile = defaultProfileGuid == GUID{}; + bool foundDefaultProfile = false; + for (const auto& profile : _profiles) + { + if (profile.GetGuid() == defaultProfileGuid) + { + foundDefaultProfile = true; + break; + } + } + + const bool defaultProfileNotInProfiles = !foundDefaultProfile; + + if (nullDefaultProfile || defaultProfileNotInProfiles) + { + _warnings.push_back(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile); + // Use the first profile as the new default + + // TODO: Do this _temporarily_. We don't want to have this be a permanent change. + GlobalSettings().SetDefaultProfile(_profiles[0].GetGuid()); + } +} diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index 8124aa08913..302e7788704 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -25,6 +25,11 @@ static constexpr GUID AzureConnectionType = { 0xd9fcfdfa, 0xa479, 0x412c, { 0x83 namespace TerminalApp { class CascadiaSettings; + enum class SettingsLoadWarnings : uint32_t + { + MissingDefaultProfile = 0, + NoProfiles = 1 + }; }; class TerminalApp::CascadiaSettings final @@ -53,9 +58,12 @@ class TerminalApp::CascadiaSettings final void CreateDefaults(); + const std::vector& GetLoadWarnings(); + private: GlobalAppSettings _globals; std::vector _profiles; + std::vector _warnings{}; void _CreateDefaultKeybindings(); void _CreateDefaultSchemes(); @@ -65,6 +73,8 @@ class TerminalApp::CascadiaSettings final static void _WriteSettings(const std::string_view content); static std::optional _ReadSettings(); + void _ValidateSettings(); + static bool _isPowerShellCoreInstalledInPath(const std::wstring_view programFileEnv, std::filesystem::path& cmdline); static bool _isPowerShellCoreInstalled(std::filesystem::path& cmdline); static void _AppendWslProfiles(std::vector& profileStorage); diff --git a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp index ee1d18ab93c..5c7c1054a2c 100644 --- a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp @@ -41,7 +41,10 @@ std::unique_ptr CascadiaSettings::LoadAll(const bool saveOnLoa std::optional fileData = _ReadSettings(); const bool foundFile = fileData.has_value(); - if (foundFile) + // Make sure the file isn't totally empty. If it is, we'll treat the file + // like it doesn't exist at all. + const bool fileHasData = foundFile && fileData.value().size() > 0; + if (foundFile && fileHasData) { const auto actualData = fileData.value(); @@ -65,10 +68,13 @@ std::unique_ptr CascadiaSettings::LoadAll(const bool saveOnLoa } resultPtr = FromJson(root); - if (resultPtr->GlobalSettings().GetDefaultProfile() == GUID{}) - { - throw winrt::hresult_invalid_argument(); - } + // If this throws, the app will catch it and use the default settings (temporarily) + resultPtr->_ValidateSettings(); + + // if (resultPtr->GlobalSettings().GetDefaultProfile() == GUID{}) + // { + // throw winrt::hresult_invalid_argument(); + // } if (saveOnLoad) { diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 8173dcac8e8..08801c5a637 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -1,17 +1,17 @@  - @@ -123,6 +123,15 @@ Failed to load settings + + Encountered errors while loading user settings + + + Could not find your default profile in your list of profiles - temporarily using the first profile. Check to make sure the defaultProfile matches the guid of one of your profiles. + + + Failed to load settings - no profiles were found in your settings. + Ok @@ -171,4 +180,4 @@ Settings - \ No newline at end of file +