maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Add Warnings during settings load (#2422)

Browse source 
* Warn the user when their settings are bad

  The start of work on #1348

* Display an error dialog for errors during validation

* Polish for PR

  * Add a ton of tests
  * Polish the _GetMessageText bits
  * Add code to check for duplicate profiles
  * Verify that many warnings work at the same time
  * comments y'all

* Apply fixes for dustin's thoughts from PR

* Add a proper exception type, use an array instead of a map

* PR Fixes

  * Fix x86 build break
  * Add a bit on "using the defaults" when we encountering an exception
  * remove a redundant variable

* guid->GUID

* Address Michael's PR comments

* Clean up this error text, and catch exceptions better

* Update src/cascadia/TerminalApp/Resources/en-US/Resources.resw
This commit is contained in:
Mike Griese 2019-08-16 16:21:43 -05:00 committed by msftbot[bot]
parent 24ea0866d3
commit d7d96f723a
11 changed files with 803 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

324
src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file

@ -4,11 +4,13 @@
#include "precomp.h"
#include "../TerminalApp/ColorScheme.h"
#include "../TerminalApp/CascadiaSettings.h"
using namespace Microsoft::Console;
using namespace TerminalApp;
using namespace WEX::Logging;
using namespace WEX::TestExecution;
using namespace WEX::Common;
namespace TerminalAppLocalTests
{
@ -29,8 +31,31 @@ namespace TerminalAppLocalTests
END_TEST_CLASS()
TEST_METHOD(TryCreateWinRTType);
TEST_METHOD(ValidateProfilesExist);
TEST_METHOD(ValidateDefaultProfileExists);
TEST_METHOD(ValidateDuplicateProfiles);
TEST_METHOD(ValidateManyWarnings);
TEST_CLASS_SETUP(ClassSetup)
{
reader = std::unique_ptr<Json::CharReader>(Json::CharReaderBuilder::CharReaderBuilder().newCharReader());
return true;
}
Json::Value VerifyParseSucceeded(std::string content);
private:
std::unique_ptr<Json::CharReader> reader;
};
Json::Value SettingsTests::VerifyParseSucceeded(std::string content)
{
Json::Value root;
std::string errs;
const bool parseResult = reader->parse(content.c_str(), content.c_str() + content.size(), &root, &errs);
VERIFY_IS_TRUE(parseResult, winrt::to_hstring(errs).c_str());
return root;
}
void SettingsTests::TryCreateWinRTType()
{
winrt::Microsoft::Terminal::Settings::TerminalSettings settings{};
@ -41,4 +66,303 @@ namespace TerminalAppLocalTests
VERIFY_ARE_NOT_EQUAL(oldFontSize, newFontSize);
}
void SettingsTests::ValidateProfilesExist()
{
const std::string settingsWithProfiles{ R"(
{
"profiles": [
{
"name" : "profile0"
}
]
})" };
const std::string settingsWithoutProfiles{ R"(
{
"defaultProfile": "{6239a42c-1de4-49a3-80bd-e8fdd045185c}"
})" };
const std::string settingsWithEmptyProfiles{ R"(
{
"profiles": []
})" };
{
// Case 1: Good settings
const auto settingsObject = VerifyParseSucceeded(settingsWithProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateProfilesExist();
}
{
// Case 2: Bad settings
const auto settingsObject = VerifyParseSucceeded(settingsWithoutProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
bool caughtExpectedException = false;
try
{
settings->_ValidateProfilesExist();
}
catch (const ::TerminalApp::SettingsException& ex)
{
VERIFY_IS_TRUE(ex.Error() == ::TerminalApp::SettingsLoadErrors::NoProfiles);
caughtExpectedException = true;
}
VERIFY_IS_TRUE(caughtExpectedException);
}
{
// Case 3: Bad settings
const auto settingsObject = VerifyParseSucceeded(settingsWithEmptyProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
bool caughtExpectedException = false;
try
{
settings->_ValidateProfilesExist();
}
catch (const ::TerminalApp::SettingsException& ex)
{
VERIFY_IS_TRUE(ex.Error() == ::TerminalApp::SettingsLoadErrors::NoProfiles);
caughtExpectedException = true;
}
VERIFY_IS_TRUE(caughtExpectedException);
}
}
void SettingsTests::ValidateDefaultProfileExists()
{
const std::string goodProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile0",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
}
]
})" };
const std::string badProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
}
]
})" };
const std::string noDefaultAtAll{ R"(
{
"globals": {
"alwaysShowTabs": true
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-5555-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-6666-49a3-80bd-e8fdd045185c}"
}
]
})" };
{
// Case 1: Good settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids, and the defaultProfile is one of those guids"));
const auto settingsObject = VerifyParseSucceeded(goodProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateDefaultProfileExists();
VERIFY_ARE_EQUAL(static_cast<size_t>(0), settings->_warnings.size());
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}
{
// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids, but the defaultProfile is NOT one of those guids"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateDefaultProfileExists();
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile, settings->_warnings.at(0));
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}
{
// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids, and no defaultProfile at all"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateDefaultProfileExists();
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile, settings->_warnings.at(0));
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}
}
void SettingsTests::ValidateDuplicateProfiles()
{
const std::string goodProfiles{ R"(
{
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile0",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
}
]
})" };
const std::string badProfiles{ R"(
{
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
}
]
})" };
const std::string veryBadProfiles{ R"(
{
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-5555-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile2",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile3",
"guid": "{6239a42c-4444-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile4",
"guid": "{6239a42c-6666-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile5",
"guid": "{6239a42c-5555-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile6",
"guid": "{6239a42c-7777-49a3-80bd-e8fdd045185c}"
}
]
})" };
{
// Case 1: Good settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with unique guids"));
const auto settingsObject = VerifyParseSucceeded(goodProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateNoDuplicateProfiles();
VERIFY_ARE_EQUAL(static_cast<size_t>(0), settings->_warnings.size());
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
}
{
// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with the same guid"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateNoDuplicateProfiles();
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::DuplicateProfile, settings->_warnings.at(0));
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_profiles.size());
VERIFY_ARE_EQUAL(L"profile0", settings->_profiles.at(0).GetName());
}
{
// Case 3: Very bad settings
Log::Comment(NoThrowString().Format(
L"Testing a set of profiles, many of which with duplicated guids"));
const auto settingsObject = VerifyParseSucceeded(veryBadProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateNoDuplicateProfiles();
VERIFY_ARE_EQUAL(static_cast<size_t>(1), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::DuplicateProfile, settings->_warnings.at(0));
VERIFY_ARE_EQUAL(static_cast<size_t>(4), settings->_profiles.size());
VERIFY_ARE_EQUAL(L"profile0", settings->_profiles.at(0).GetName());
VERIFY_ARE_EQUAL(L"profile1", settings->_profiles.at(1).GetName());
VERIFY_ARE_EQUAL(L"profile4", settings->_profiles.at(2).GetName());
VERIFY_ARE_EQUAL(L"profile6", settings->_profiles.at(3).GetName());
}
}
void SettingsTests::ValidateManyWarnings()
{
const std::string badProfiles{ R"(
{
"globals": {
"defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}"
},
"profiles": [
{
"name" : "profile0",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile1",
"guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
},
{
"name" : "profile2",
"guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
}
]
})" };
// Case 2: Bad settings
Log::Comment(NoThrowString().Format(
L"Testing a pair of profiles with the same guid"));
const auto settingsObject = VerifyParseSucceeded(badProfiles);
auto settings = CascadiaSettings::FromJson(settingsObject);
settings->_ValidateSettings();
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_warnings.size());
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::DuplicateProfile, settings->_warnings.at(0));
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile, settings->_warnings.at(1));
VERIFY_ARE_EQUAL(static_cast<size_t>(2), settings->_profiles.size());
VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid());
}
}

209
src/cascadia/TerminalApp/App.cpp
View file

@ -26,6 +26,94 @@ namespace winrt
using IInspectable = Windows::Foundation::IInspectable;
}
// clang-format off
// !!! IMPORTANT !!!
// Make sure that these keys are in the same order as the
// SettingsLoadWarnings/Errors enum is!
static const std::array<std::wstring_view, 2> settingsLoadWarningsLabels {
L"MissingDefaultProfileText",
L"DuplicateProfileText"
};
static const std::array<std::wstring_view, 1> settingsLoadErrorsLabels {
L"NoProfilesText"
};
// clang-format on
// Function Description:
// - General-purpose helper for looking up a localized string for a
// warning/error. First will look for the given key in the provided map of
// keys->strings, where the values in the map are ResourceKeys. If it finds
// one, it will lookup the localized string from that ResourceKey.
// - If it does not find a key, it'll return an empty string
// Arguments:
// - key: the value to use to look for a resource key in the given map
// - map: A map of keys->Resource keys.
// - loader: the ScopedResourceLoader to use to look up the localized string.
// Return Value:
// - the localized string for the given type, if it exists.
template<std::size_t N>
static winrt::hstring _GetMessageText(uint32_t index, std::array<std::wstring_view, N> keys, ScopedResourceLoader loader)
{
if (index < keys.size())
{
return loader.GetLocalizedString(keys.at(index));
}
return {};
}
// Function Description:
// - Gets the text from our ResourceDictionary for the given
// SettingsLoadWarning. If there is no such text, we'll return nullptr.
// - The warning should have an entry in settingsLoadWarningsLabels.
// Arguments:
// - warning: the SettingsLoadWarnings value to get the localized text for.
// - loader: the ScopedResourceLoader to use to look up the localized string.
// Return Value:
// - localized text for the given warning
static winrt::hstring _GetWarningText(::TerminalApp::SettingsLoadWarnings warning, ScopedResourceLoader loader)
{
return _GetMessageText(static_cast<uint32_t>(warning), settingsLoadWarningsLabels, loader);
}
// Function Description:
// - Gets the text from our ResourceDictionary for the given
// SettingsLoadError. If there is no such text, we'll return nullptr.
// - The warning should have an entry in settingsLoadErrorsLabels.
// Arguments:
// - error: the SettingsLoadErrors value to get the localized text for.
// - loader: the ScopedResourceLoader to use to look up the localized string.
// Return Value:
// - localized text for the given error
static winrt::hstring _GetErrorText(::TerminalApp::SettingsLoadErrors error, ScopedResourceLoader loader)
{
return _GetMessageText(static_cast<uint32_t>(error), settingsLoadErrorsLabels, loader);
}
// Function Description:
// - Creates a Run of text to display an error message. The text is yellow or
// red for dark/light theme, respectively.
// Arguments:
// - text: The text of the error message.
// - resources: The application's resource loader.
// Return Value:
// - The fully styled text run.
static Documents::Run _BuildErrorRun(const winrt::hstring& text, const ResourceDictionary& resources)
{
Documents::Run textRun;
textRun.Text(text);
// Color the text red (light theme) or yellow (dark theme) based on the system theme
winrt::IInspectable key = winrt::box_value(L"ErrorTextBrush");
if (resources.HasKey(key))
{
winrt::IInspectable g = resources.Lookup(key);
auto brush = g.try_as<winrt::Windows::UI::Xaml::Media::Brush>();
textRun.Foreground(brush);
}
return textRun;
}
namespace winrt::TerminalApp::implementation
{
App::App() :
@ -177,6 +265,81 @@ namespace winrt::TerminalApp::implementation
_ShowDialog(winrt::box_value(title), winrt::box_value(message), buttonText);
}
// Method Description:
// - Displays a dialog for errors found while loading or validating the
// settings. Uses the resources under the provided title and content keys
// as the title and first content of the dialog, then also displays a
// message for whatever exception was found while validating the settings.
// - Only one dialog can be visible at a time. If another dialog is visible
// when this is called, nothing happens. See _ShowDialog for details
// Arguments:
// - titleKey: The key to use to lookup the title text from our resources.
// - contentKey: The key to use to lookup the content text from our resources.
void App::_ShowLoadErrorsDialog(const winrt::hstring& titleKey,
const winrt::hstring& contentKey)
{
auto title = _resourceLoader.GetLocalizedString(titleKey);
auto buttonText = _resourceLoader.GetLocalizedString(L"Ok");
Controls::TextBlock warningsTextBlock;
// Make sure you can copy-paste
warningsTextBlock.IsTextSelectionEnabled(true);
// Make sure the lines of text wrap
warningsTextBlock.TextWrapping(TextWrapping::Wrap);
winrt::Windows::UI::Xaml::Documents::Run errorRun;
const auto errorLabel = _resourceLoader.GetLocalizedString(contentKey);
errorRun.Text(errorLabel);
warningsTextBlock.Inlines().Append(errorRun);
if (FAILED(_settingsLoadedResult))
{
if (!_settingsLoadExceptionText.empty())
{
warningsTextBlock.Inlines().Append(_BuildErrorRun(_settingsLoadExceptionText, Resources()));
}
}
// Add a note that we're using the default settings in this case.
winrt::Windows::UI::Xaml::Documents::Run usingDefaultsRun;
const auto usingDefaultsText = _resourceLoader.GetLocalizedString(L"UsingDefaultSettingsText");
usingDefaultsRun.Text(usingDefaultsText);
warningsTextBlock.Inlines().Append(usingDefaultsRun);
_ShowDialog(winrt::box_value(title), warningsTextBlock, buttonText);
}
// Method Description:
// - Displays a dialog for warnings found while loading or validating the
// settings. Displays messages for whatever warnings were found while
// validating the settings.
// - Only one dialog can be visible at a time. If another dialog is visible
// when this is called, nothing happens. See _ShowDialog for details
void App::_ShowLoadWarningsDialog()
{
auto title = _resourceLoader.GetLocalizedString(L"SettingsValidateErrorTitle");
auto buttonText = _resourceLoader.GetLocalizedString(L"Ok");
Controls::TextBlock warningsTextBlock;
// Make sure you can copy-paste
warningsTextBlock.IsTextSelectionEnabled(true);
// Make sure the lines of text wrap
warningsTextBlock.TextWrapping(TextWrapping::Wrap);
const auto& warnings = _settings->GetWarnings();
for (const auto& warning : warnings)
{
// Try looking up the warning message key for each warning.
const auto warningText = _GetWarningText(warning, _resourceLoader);
if (!warningText.empty())
{
warningsTextBlock.Inlines().Append(_BuildErrorRun(warningText, Resources()));
}
}
_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
@ -261,7 +424,11 @@ namespace winrt::TerminalApp::implementation
{
const winrt::hstring titleKey = L"InitialJsonParseErrorTitle";
const winrt::hstring textKey = L"InitialJsonParseErrorText";
_ShowOkDialog(titleKey, textKey);
_ShowLoadErrorsDialog(titleKey, textKey);
}
else if (_settingsLoadedResult == S_FALSE)
{
_ShowLoadWarningsDialog();
}
}
@ -314,7 +481,8 @@ namespace winrt::TerminalApp::implementation
auto keyBindings = _settings->GetKeybindings();
const GUID defaultProfileGuid = _settings->GlobalSettings().GetDefaultProfile();
auto const profileCount = gsl::narrow_cast<int>(_settings->GetProfiles().size()); // the number of profiles should not change in the loop for this to work
// the number of profiles should not change in the loop for this to work
auto const profileCount = gsl::narrow_cast<int>(_settings->GetProfiles().size());
for (int profileIndex = 0; profileIndex < profileCount; profileIndex++)
{
const auto& profile = _settings->GetProfiles()[profileIndex];
@ -324,7 +492,8 @@ namespace winrt::TerminalApp::implementation
if (profileIndex < 9)
{
// enum value for ShortcutAction::NewTabProfileX; 0==NewTabProfile0
auto profileKeyChord = keyBindings.GetKeyBinding(static_cast<ShortcutAction>(profileIndex + static_cast<int>(ShortcutAction::NewTabProfile0)));
const auto action = static_cast<ShortcutAction>(profileIndex + static_cast<int>(ShortcutAction::NewTabProfile0));
auto profileKeyChord = keyBindings.GetKeyBinding(action);
// make sure we find one to display
if (profileKeyChord)
@ -512,13 +681,20 @@ namespace winrt::TerminalApp::implementation
{
auto newSettings = CascadiaSettings::LoadAll(saveOnLoad);
_settings = std::move(newSettings);
hr = S_OK;
const auto& warnings = _settings->GetWarnings();
hr = warnings.size() == 0 ? S_OK : S_FALSE;
}
catch (const winrt::hresult_error& e)
{
hr = e.code();
_settingsLoadExceptionText = e.message();
LOG_HR(hr);
}
catch (const ::TerminalApp::SettingsException& ex)
{
hr = E_INVALIDARG;
_settingsLoadExceptionText = _GetErrorText(ex.Error(), _resourceLoader);
}
catch (...)
{
hr = wil::ResultFromCaughtException();
@ -631,11 +807,17 @@ namespace winrt::TerminalApp::implementation
_root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() {
const winrt::hstring titleKey = L"ReloadJsonParseErrorTitle";
const winrt::hstring textKey = L"ReloadJsonParseErrorText";
_ShowOkDialog(titleKey, textKey);
_ShowLoadErrorsDialog(titleKey, textKey);
});
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.
@ -1385,7 +1567,8 @@ namespace winrt::TerminalApp::implementation
// Takes into account a special case for an error condition for a comma
// Arguments:
// - MenuFlyoutItem that will be displayed, and a KeyChord to map an accelerator
void App::_SetAcceleratorForMenuItem(Windows::UI::Xaml::Controls::MenuFlyoutItem& menuItem, const winrt::Microsoft::Terminal::Settings::KeyChord& keyChord)
void App::_SetAcceleratorForMenuItem(Controls::MenuFlyoutItem& menuItem,
const winrt::Microsoft::Terminal::Settings::KeyChord& keyChord)
{
#ifdef DEP_MICROSOFT_UI_XAML_708_FIXED
// work around https://github.com/microsoft/microsoft-ui-xaml/issues/708 in case of VK_OEM_COMMA
@ -1426,7 +1609,8 @@ namespace winrt::TerminalApp::implementation
// - the terminal settings
// Return value:
// - the desired connection
TerminalConnection::ITerminalConnection App::_CreateConnectionFromSettings(GUID profileGuid, winrt::Microsoft::Terminal::Settings::TerminalSettings settings)
TerminalConnection::ITerminalConnection App::_CreateConnectionFromSettings(GUID profileGuid,
winrt::Microsoft::Terminal::Settings::TerminalSettings settings)
{
const auto* const profile = _settings->FindProfile(profileGuid);
TerminalConnection::ITerminalConnection connection{ nullptr };
@ -1437,9 +1621,12 @@ namespace winrt::TerminalApp::implementation
connectionType = profile->GetConnectionType();
}
if (profile->HasConnectionType() && profile->GetConnectionType() == AzureConnectionType && TerminalConnection::AzureConnection::IsAzureConnectionAvailable())
if (profile->HasConnectionType() &&
profile->GetConnectionType() == AzureConnectionType &&
TerminalConnection::AzureConnection::IsAzureConnectionAvailable())
{
connection = TerminalConnection::AzureConnection(settings.InitialRows(), settings.InitialCols());
connection = TerminalConnection::AzureConnection(settings.InitialRows(),
settings.InitialCols());
}
else
{
@ -1484,6 +1671,6 @@ namespace winrt::TerminalApp::implementation
// These macros will define them both for you.
DEFINE_EVENT(App, TitleChanged, _titleChangeHandlers, TerminalControl::TitleChangedEventArgs);
DEFINE_EVENT(App, LastTabClosed, _lastTabClosedHandlers, winrt::TerminalApp::LastTabClosedEventArgs);
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(App, SetTitleBarContent, _setTitleBarContentHandlers, TerminalApp::App, winrt::Windows::UI::Xaml::UIElement);
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(App, RequestedThemeChanged, _requestedThemeChangedHandlers, TerminalApp::App, winrt::Windows::UI::Xaml::ElementTheme);
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(App, SetTitleBarContent, _setTitleBarContentHandlers, TerminalApp::App, UIElement);
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(App, RequestedThemeChanged, _requestedThemeChangedHandlers, TerminalApp::App, ElementTheme);
}

3
src/cascadia/TerminalApp/App.h
View file

@ -63,6 +63,7 @@ namespace winrt::TerminalApp::implementation
std::unique_ptr<::TerminalApp::CascadiaSettings> _settings;
HRESULT _settingsLoadedResult;
winrt::hstring _settingsLoadExceptionText{};
bool _loadedInitialSettings;
std::shared_mutex _dialogLock;
@ -80,6 +81,8 @@ namespace winrt::TerminalApp::implementation
const winrt::hstring& closeButtonText);
void _ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey);
void _ShowAboutDialog();
void _ShowLoadWarningsDialog();
void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey);
[[nodiscard]] HRESULT _TryLoadSettings(const bool saveOnLoad) noexcept;
void _LoadSettings();

5
src/cascadia/TerminalApp/App.xaml
View file

@ -44,6 +44,11 @@ the MIT License. See LICENSE in the project root for license information. -->
<Setter Target="CloseButtonStyle" Value="{StaticResource AccentButtonStyle}" />
</Style>
<!-- We need to manually create the error text brush as a
theme-dependent brush. SystemControlErrorTextForegroundBrush
is unfortunately static. -->
<SolidColorBrush x:Name="ErrorTextBrush" Color="{ThemeResource SystemErrorTextColor}" />
<ResourceDictionary.ThemeDictionaries>
<ResourceDictionary x:Key="Dark">
<!-- Define resources for Dark mode here -->

129
src/cascadia/TerminalApp/CascadiaSettings.cpp
View file

@ -643,3 +643,132 @@ Profile CascadiaSettings::_CreateDefaultProfile(const std::wstring_view name)
return newProfile;
}
// Method Description:
// - Gets our list of warnings we found during loading. These are things that we
// knew were bad when we called `_ValidateSettings` last.
// Return Value:
// - a reference to our list of warnings.
std::vector<TerminalApp::SettingsLoadWarnings>& CascadiaSettings::GetWarnings()
{
return _warnings;
}
// Method Description:
// - Attempts to validate this settings structure. If there are critical errors
// found, they'll be thrown as a SettingsLoadError. Non-critical errors, such
// as not finding the default profile, will only result in an error. We'll add
// all these warnings to our list of warnings, and the application can chose
// to display these to the user.
// Arguments:
// - <none>
// Return Value:
// - <none>
void CascadiaSettings::_ValidateSettings()
{
_warnings.clear();
// Make sure to check that profiles exists at all first and foremost:
_ValidateProfilesExist();
// Then do some validation on the profiles. The order of these does not
// terribly matter.
_ValidateNoDuplicateProfiles();
_ValidateDefaultProfileExists();
}
// Method Description:
// - Checks if the settings contain profiles at all. As we'll need to have some
// profiles at all, we'll throw an error if there aren't any profiles.
void CascadiaSettings::_ValidateProfilesExist()
{
const bool hasProfiles = !_profiles.empty();
if (!hasProfiles)
{
// Throw an exception. This is an invalid state, and we want the app to
// be able to gracefully use the default settings.
// We can't add the warning to the list of warnings here, because this
// object is not going to be returned at any point.
throw ::TerminalApp::SettingsException(::TerminalApp::SettingsLoadErrors::NoProfiles);
}
}
// Method Description:
// - Checks if the "globals.defaultProfile" is set to one of the profiles we
// actually have. If the value is unset, or the value is set to something that
// doesn't exist in the list of profiles, we'll arbitrarily pick the first
// profile to use temporarily as the default.
// - Appends a SettingsLoadWarnings::MissingDefaultProfile to our list of
// warnings if we failed to find the default.
void CascadiaSettings::_ValidateDefaultProfileExists()
{
const auto defaultProfileGuid = GlobalSettings().GetDefaultProfile();
const bool nullDefaultProfile = defaultProfileGuid == GUID{};
bool defaultProfileNotInProfiles = true;
for (const auto& profile : _profiles)
{
if (profile.GetGuid() == defaultProfileGuid)
{
defaultProfileNotInProfiles = false;
break;
}
}
if (nullDefaultProfile || defaultProfileNotInProfiles)
{
_warnings.push_back(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile);
// Use the first profile as the new default
// _temporarily_ set the default profile to the first profile. Because
// we're adding a warning, this settings change won't be re-serialized.
GlobalSettings().SetDefaultProfile(_profiles[0].GetGuid());
}
}
// Method Description:
// - Checks to make sure there aren't any duplicate profiles in the list of
// profiles. If so, we'll remove the subsequent entries (temporarily), as they
// won't be accessible anyways.
// - Appends a SettingsLoadWarnings::DuplicateProfile to our list of warnings if
// we find any such duplicate.
void CascadiaSettings::_ValidateNoDuplicateProfiles()
{
bool foundDupe = false;
std::vector<size_t> indiciesToDelete{};
// Helper to establish an ordering on guids
struct GuidEquality
{
bool operator()(const GUID& lhs, const GUID& rhs) const
{
return memcmp(&lhs, &rhs, sizeof(rhs)) < 0;
}
};
std::set<GUID, GuidEquality> uniqueGuids{};
// Try collecting all the unique guids. If we ever encounter a guid that's
// already in the set, then we need to delete that profile.
for (int i = 0; i < _profiles.size(); i++)
{
if (!uniqueGuids.insert(_profiles.at(i).GetGuid()).second)
{
foundDupe = true;
indiciesToDelete.push_back(i);
}
}
// Remove all the duplicates we've marked
// Walk backwards, so we don't accidentally shift any of the elements
for (auto iter = indiciesToDelete.rbegin(); iter != indiciesToDelete.rend(); iter++)
{
_profiles.erase(_profiles.begin() + *iter);
}
if (foundDupe)
{
_warnings.push_back(::TerminalApp::SettingsLoadWarnings::DuplicateProfile);
}
}

19
src/cascadia/TerminalApp/CascadiaSettings.h
View file

@ -3,7 +3,7 @@ Copyright (c) Microsoft Corporation
Licensed under the MIT license.
Module Name:
- CascadiaSettings.hpp
- CascadiaSettings.h
Abstract:
- This class acts as the container for all app settings. It's composed of two
@ -18,10 +18,17 @@ Author(s):
#pragma once
#include <winrt/Microsoft.Terminal.TerminalControl.h>
#include "GlobalAppSettings.h"
#include "TerminalWarnings.h"
#include "Profile.h"
static constexpr GUID AzureConnectionType = { 0xd9fcfdfa, 0xa479, 0x412c, { 0x83, 0xb7, 0xc5, 0x64, 0xe, 0x61, 0xcd, 0x62 } };
// fwdecl unittest classes
namespace TerminalAppLocalTests
{
class SettingsTests;
}
namespace TerminalApp
{
class CascadiaSettings;
@ -53,9 +60,12 @@ public:
void CreateDefaults();
std::vector<TerminalApp::SettingsLoadWarnings>& GetWarnings();
private:
GlobalAppSettings _globals;
std::vector<Profile> _profiles;
std::vector<TerminalApp::SettingsLoadWarnings> _warnings{};
void _CreateDefaultKeybindings();
void _CreateDefaultSchemes();
@ -65,8 +75,15 @@ private:
static void _WriteSettings(const std::string_view content);
static std::optional<std::string> _ReadSettings();
void _ValidateSettings();
void _ValidateProfilesExist();
void _ValidateDefaultProfileExists();
void _ValidateNoDuplicateProfiles();
static bool _isPowerShellCoreInstalledInPath(const std::wstring_view programFileEnv, std::filesystem::path& cmdline);
static bool _isPowerShellCoreInstalled(std::filesystem::path& cmdline);
static void _AppendWslProfiles(std::vector<TerminalApp::Profile>& profileStorage);
static Profile _CreateDefaultProfile(const std::wstring_view name);
friend class TerminalAppLocalTests::SettingsTests;
};

21
src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp
View file

@ -41,7 +41,10 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll(const bool saveOnLoa
std::optional<std::string> 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().empty();
if (foundFile && fileHasData)
{
const auto actualData = fileData.value();
@ -59,18 +62,20 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll(const bool saveOnLoa
// `parse` will return false if it fails.
if (!reader->parse(actualDataStart, actualData.c_str() + actualData.size(), &root, &errs))
{
// TODO:GH#990 display this exception text to the user, in a
// copy-pasteable way.
// This will be caught by App::_TryLoadSettings, who will display
// the text to the user.
throw winrt::hresult_error(WEB_E_INVALID_JSON_STRING, winrt::to_hstring(errs));
}
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 (saveOnLoad)
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

86
src/cascadia/TerminalApp/Resources/en-US/Resources.resw
View file

@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema
Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.
Example:
... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.
mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
@ -118,17 +118,39 @@
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="InitialJsonParseErrorText" xml:space="preserve">
<value>Settings could not be loaded from file - temporarily using the default settings. Check for syntax errors, including trailing commas.</value>
<value>Settings could not be loaded from file. Check for syntax errors, including trailing commas.
</value>
</data>
<data name="MissingDefaultProfileText" xml:space="preserve">
<value>Could not find your default profile in your list of profiles - using the first profile. Check to make sure the defaultProfile matches the GUID of one of your profiles.
</value>
</data>
<data name="DuplicateProfileText" xml:space="preserve">
<value>Found multiple profiles with the same GUID in your settings file - ignoring duplicates. Make sure each profile's GUID is unique.
</value>
</data>
<data name="NoProfilesText" xml:space="preserve">
<value>No profiles were found in your settings.
</value>
</data>
<data name="ReloadJsonParseErrorText" xml:space="preserve">
<value>Settings could not be reloaded from file. Check for syntax errors, including trailing commas.
</value>
</data>
<data name="UsingDefaultSettingsText" xml:space="preserve">
<value>
Temporarily using the Windows Terminal default settings.
</value>
</data>
<data name="InitialJsonParseErrorTitle" xml:space="preserve">
<value>Failed to load settings</value>
</data>
<data name="SettingsValidateErrorTitle" xml:space="preserve">
<value>Encountered errors while loading user settings</value>
</data>
<data name="Ok" xml:space="preserve">
<value>Ok</value>
</data>
<data name="ReloadJsonParseErrorText" xml:space="preserve">
<value>Settings could not be reloaded from file. Check for syntax errors, including trailing commas.</value>
</data>
<data name="ReloadJsonParseErrorTitle" xml:space="preserve">
<value>Failed to reload settings</value>
</data>
@ -171,4 +193,4 @@
<data name="SettingsMenuItem" xml:space="preserve">
<value>Settings</value>
</data>
</root>
</root>

57
src/cascadia/TerminalApp/TerminalWarnings.h Normal file
View file

@ -0,0 +1,57 @@
/*++
Copyright (c) Microsoft Corporation
Licensed under the MIT license.
Module Name:
- TerminalWarnings.h
Abstract:
- This file contains definitions for warnings, errors and exceptions used by the
Windows Terminal
Author(s):
- Mike Griese - August 2019
--*/
#pragma once
namespace TerminalApp
{
// SettingsLoadWarnings are scenarios where the settings contained
// information we knew was invalid, but we could recover from.
enum class SettingsLoadWarnings : uint32_t
{
MissingDefaultProfile = 0,
DuplicateProfile = 1
};
// SettingsLoadWarnings are scenarios where the settings had invalid state
// that we could not recover from.
enum class SettingsLoadErrors : uint32_t
{
NoProfiles = 0
};
// This is a helper class to wrap up a SettingsLoadErrors into a proper
// exception type.
class SettingsException : public std::runtime_error
{
public:
SettingsException(const SettingsLoadErrors& error) :
std::runtime_error{ nullptr },
_error{ error } {};
// We don't use the what() method - we want to be able to display
// localizable error messages. Catchers of this exception should use
// _GetErrorText (in App.cpp) to get the localized exception string.
const char* what() const override
{
return "Exception while loading or validating Terminal settings";
};
SettingsLoadErrors Error() const noexcept { return _error; };
private:
const SettingsLoadErrors _error;
};
};

1
src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj
View file

@ -68,6 +68,7 @@
<ClInclude Include="../AppKeyBindingsSerialization.h" />
<ClInclude Include="../KeyChordSerialization.h" />
<ClInclude Include="../Utils.h" />
<ClInclude Include="../TerminalWarnings.h" />
<ClInclude Include="pch.h" />
<ClInclude Include="../AppKeyBindings.h" />
<ClInclude Include="../App.h" >

1
src/inc/LibraryIncludes.h
View file

@ -43,6 +43,7 @@
#include <iomanip>
#include <filesystem>
#include <functional>
#include <set>
// WIL
#include <wil/Common.h>