Skip to content

Commit

Permalink
Warn the user when their settings are bad
Browse files Browse the repository at this point in the history 
  The start of work on #1348
  • Loading branch information
@zadjii-msft
zadjii-msft committed Aug 6, 2019
1 parent dfb8536 commit 80c18fb
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 34 deletions.
50 changes: 49 additions & 1 deletion src/cascadia/TerminalApp/App.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() :
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -550,6 +588,10 @@ namespace winrt::TerminalApp::implementation
_settings = std::make_unique<CascadiaSettings>();
_settings->CreateDefaults();
}
// else if (_settingsLoadedResult == S_FALSE)
// {
// _DisplayLoadingWarnings
// }

_HookupKeyBindings(_settings->GetKeybindings());

Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/App.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
45 changes: 45 additions & 0 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -643,3 +643,48 @@ Profile CascadiaSettings::_CreateDefaultProfile(const std::wstring_view name)

return newProfile;
}

const std::vector<TerminalApp::SettingsLoadWarnings>& 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());
}
}
10 changes: 10 additions & 0 deletions src/cascadia/TerminalApp/CascadiaSettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,9 +58,12 @@ class TerminalApp::CascadiaSettings final

void CreateDefaults();

const std::vector<TerminalApp::SettingsLoadWarnings>& GetLoadWarnings();

private:
GlobalAppSettings _globals;
std::vector<Profile> _profiles;
std::vector<TerminalApp::SettingsLoadWarnings> _warnings{};

void _CreateDefaultKeybindings();
void _CreateDefaultSchemes();
Expand All @@ -65,6 +73,8 @@ class TerminalApp::CascadiaSettings final
static void _WriteSettings(const std::string_view content);
static std::optional<std::string> _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<TerminalApp::Profile>& profileStorage);
Expand Down
16 changes: 11 additions & 5 deletions src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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().size() > 0;
if (foundFile && fileHasData)
{
const auto actualData = fileData.value();

Expand All @@ -65,10 +68,13 @@ std::unique_ptr<CascadiaSettings> 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)
{
Expand Down
65 changes: 37 additions & 28 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
View file
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -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.
-->
Expand Down Expand Up @@ -123,6 +123,15 @@
<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="MissingDefaultProfileText" xml:space="preserve">
<value>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.</value>
</data>
<data name="NoProfilesText" xml:space="preserve">
<value>Failed to load settings - no profiles were found in your settings.</value>
</data>
<data name="Ok" xml:space="preserve">
<value>Ok</value>
</data>
Expand Down Expand Up @@ -171,4 +180,4 @@
<data name="SettingsMenuItem" xml:space="preserve">
<value>Settings</value>
</data>
</root>
</root>

0 comments on commit 80c18fb

Please sign in to comment.