From d0121a10f4b238db28df7c7d76ced8cf37accef8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 7 May 2019 09:39:08 -0500 Subject: [PATCH 1/8] start work on an error dialog --- .../Resources/en-US/Resources.resw | 9 ++ src/cascadia/TerminalApp/App.cpp | 88 ++++++++++++++++++- src/cascadia/TerminalApp/App.h | 2 + src/cascadia/TerminalApp/CascadiaSettings.cpp | 2 +- src/cascadia/TerminalApp/CascadiaSettings.h | 3 +- .../CascadiaSettingsSerialization.cpp | 9 +- src/inc/DefaultSettings.h | 2 +- 7 files changed, 107 insertions(+), 8 deletions(-) diff --git a/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw b/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw index f759e568acc..8370880efc6 100644 --- a/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw +++ b/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw @@ -123,4 +123,13 @@ Windows Terminal (Preview) + + Bar + + + Settings could not be loaded from file - temporarily using the default settings. Check for syntax errors, including trailing commas. + + + Failed to load settings + \ No newline at end of file diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 0d8f39b2754..1b12f0dd347 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -6,6 +6,7 @@ #include #include #include +#include using namespace winrt::Windows::ApplicationModel::DataTransfer; using namespace winrt::Windows::UI::Xaml; @@ -166,6 +167,47 @@ namespace winrt::TerminalApp::implementation _ApplyTheme(_settings->GlobalSettings().GetRequestedTheme()); _OpenNewTab(std::nullopt); + + // _root.Loaded([this](auto&&, auto&&) { + // if (FAILED(_settingsLoadedResult)) + // { + // Controls::ContentDialog dialog; + + // dialog.Title(winrt::box_value(L"Failed to parse settings")); + // dialog.Content(winrt::box_value(L"Failed to parse settings asdfa")); + // dialog.CloseButtonText(L"Ok"); + + // // IMPORTANT: Add the dialog to the _root UIElementw before you show it, so it knows how to attach to the XAML content. + // _root.Children().Append(dialog); + // dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); + + // } + // }); + + //auto l = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); + //l.GetString(L"InitialJsonParse.Text"); + + + _root.Loaded([this](auto&&, auto&&) { + if (FAILED(_settingsLoadedResult)) + { + + // Windows::ApplicationModel::Resources::ResourceLoader loader{}; + auto l = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); + auto s = l.GetString(L"InitialJsonParse.Text"); + auto bar = l.GetStringForUri(L"InitialJsonParse"); + auto foo = l.GetString(L"Foo"); + + Controls::ContentDialog dialog; + dialog.Title(winrt::box_value(L"Failed to parse settings")); + // dialog.Content(winrt::box_value(L"Failed to parse settings asdfa")); + dialog.Content(winrt::box_value(s)); + dialog.CloseButtonText(L"Ok"); + // IMPORTANT: Add the dialog to the _root UIElementw before you show it, so it knows how to attach to the XAML content. + _root.Children().Append(dialog); + dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); + } + }); } // Method Description: @@ -340,7 +382,42 @@ namespace winrt::TerminalApp::implementation // happening during startup, it'll need to happen on a background thread. void App::LoadSettings() { - _settings = CascadiaSettings::LoadAll(); + bool successfullyLoadedSettings = false; + HRESULT hr = E_FAIL; + // TODO: Try this. + try + { + auto newSettings = CascadiaSettings::LoadAll(); + _settings = std::move(newSettings); + successfullyLoadedSettings = true; + hr = S_OK; + } + catch (const winrt::hresult_error& e) + { + hr = e.code(); + LOG_HR(hr); + } + catch (...) + { + // TODO can this ever be hit? or will it always be a winrt::hresult_error? + LOG_HR(wil::ResultFromCaughtException()); + } + + _settingsLoadedResult = hr; + + if (!successfullyLoadedSettings) + { + // If it fails, + // - use Default settings, + // - don't persist them. + // - Set a flag saying that we should display the loading error. + // * Settings could not be loaded from file - temporarily using + // default settings. Check for syntax errors, including trailing + // commas. + _settings = std::make_unique(); + _settings->CreateDefaults(); + + } _HookupKeyBindings(_settings->GetKeybindings()); @@ -403,7 +480,16 @@ namespace winrt::TerminalApp::implementation // - void App::_ReloadSettings() { + // TODO: Try this. _settings = CascadiaSettings::LoadAll(); + // If it fails, + // - don't change the settings (and don't actually apply the new settings) + // - don't persist them. + // - display a loading error + // * Settings could not be reloaded from file. Check for syntax + // errors, including trailing commas. + + // Re-wire the keybindings to their handlers, as we'll have created a // new AppKeyBindings object. _HookupKeyBindings(_settings->GetKeybindings()); diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 87b3915748f..dd40d74afef 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -64,6 +64,8 @@ namespace winrt::TerminalApp::implementation std::unique_ptr<::TerminalApp::CascadiaSettings> _settings; + HRESULT _settingsLoadedResult = S_OK; + bool _loadedInitialSettings; wil::unique_folder_change_reader_nothrow _reader; diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index aa41e1602b6..b4dd5178f80 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -229,7 +229,7 @@ void CascadiaSettings::_CreateDefaultKeybindings() // - // Return Value: // - -void CascadiaSettings::_CreateDefaults() +void CascadiaSettings::CreateDefaults() { _CreateDefaultProfiles(); _CreateDefaultSchemes(); diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index 102d208830d..b874c6edfa3 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -50,6 +50,8 @@ class TerminalApp::CascadiaSettings final static winrt::hstring GetSettingsPath(); const Profile* FindProfile(GUID profileGuid) const noexcept; + + void CreateDefaults(); private: GlobalAppSettings _globals; std::vector _profiles; @@ -58,7 +60,6 @@ class TerminalApp::CascadiaSettings final void _CreateDefaultKeybindings(); void _CreateDefaultSchemes(); void _CreateDefaultProfiles(); - void _CreateDefaults(); static bool _IsPackaged(); static void _SaveAsPackagedApp(const winrt::hstring content); diff --git a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp index 5adc4b38d5a..7b1ce9bcf47 100644 --- a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp @@ -47,12 +47,13 @@ std::unique_ptr CascadiaSettings::LoadAll(const bool saveOnLoa { const auto actualData = fileData.value(); - JsonValue root{ nullptr }; - bool parsedSuccessfully = JsonValue::TryParse(actualData, root); + JsonObject root{ nullptr }; + // bool parsedSuccessfully = JsonValue::TryParse(actualData, root); + root = JsonObject::Parse(actualData); bool parsedSuccessfully = true; // TODO:MSFT:20737698 - Display an error if we failed to parse settings if (parsedSuccessfully) { - JsonObject obj = root.GetObjectW(); + JsonObject obj = root;//.GetObjectW(); resultPtr = FromJson(obj); // Update profile only if it has changed. @@ -77,7 +78,7 @@ std::unique_ptr CascadiaSettings::LoadAll(const bool saveOnLoa else { resultPtr = std::make_unique(); - resultPtr->_CreateDefaults(); + resultPtr->CreateDefaults(); // The settings file does not exist. Let's commit one. resultPtr->SaveAll(); diff --git a/src/inc/DefaultSettings.h b/src/inc/DefaultSettings.h index d959e41f7dc..421dc185976 100644 --- a/src/inc/DefaultSettings.h +++ b/src/inc/DefaultSettings.h @@ -24,7 +24,7 @@ constexpr COLORREF DEFAULT_BACKGROUND = COLOR_BLACK; constexpr COLORREF DEFAULT_BACKGROUND_WITH_ALPHA = OPACITY_OPAQUE | DEFAULT_BACKGROUND; constexpr short DEFAULT_HISTORY_SIZE = 9001; const std::wstring DEFAULT_FONT_FACE { L"Consolas" }; -constexpr int DEFAULT_FONT_SIZE = 12; +constexpr int DEFAULT_FONT_SIZE = 10; constexpr int DEFAULT_ROWS = 30; constexpr int DEFAULT_COLS = 120; From effefeb95918abfc5775fad46dbb183227a22bd9 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 8 May 2019 15:15:24 -0500 Subject: [PATCH 2/8] Continue work on an error dialog * Load messages from the Resources.resw file * Display a message when we fail to parse the settings on an initial parse, or on a reload. * Unfortunately, the dialog is styled incorrectly when the app theme is opposite the system theme. This can result in the button being invisible. --- .../Resources/en-US/Resources.resw | 16 ++- src/cascadia/TerminalApp/App.cpp | 129 +++++++++++------- src/cascadia/TerminalApp/App.h | 6 +- src/cascadia/TerminalApp/App.xaml | 3 + 4 files changed, 100 insertions(+), 54 deletions(-) diff --git a/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw b/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw index 8370880efc6..ef5a497b19e 100644 --- a/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw +++ b/src/cascadia/CascadiaPackage/Resources/en-US/Resources.resw @@ -123,13 +123,19 @@ Windows Terminal (Preview) - - Bar - - + Settings could not be loaded from file - temporarily using the default settings. Check for syntax errors, including trailing commas. - + Failed to load settings + + Ok + + + Settings could not be reloaded from file. Check for syntax errors, including trailing commas. + + + Failed to reload settings + \ No newline at end of file diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 1b12f0dd347..b5a996eb971 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -43,7 +43,9 @@ namespace winrt::TerminalApp::implementation base_type(parentProvider), _settings{ }, _tabs{ }, - _loadedInitialSettings{ false } + _loadedInitialSettings{ false }, + _settingsLoadedResult{ S_OK }, + _dialogLock{} { // For your own sanity, it's better to do setup outside the ctor. // If you do any setup in the ctor that ends up throwing an exception, @@ -168,46 +170,58 @@ namespace winrt::TerminalApp::implementation _OpenNewTab(std::nullopt); - // _root.Loaded([this](auto&&, auto&&) { - // if (FAILED(_settingsLoadedResult)) - // { - // Controls::ContentDialog dialog; + _root.Loaded({ this, &App::_OnLoaded }); + } + + fire_and_forget App::_ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& textKey) + { + // DON'T release this lock in a wil::scope_exit. The scope_exit will get + // called when we await, which is not what we want. + auto gotLock = _dialogLock.try_lock(); + if (!gotLock) + { + // Another dialog is visible. + return; + } - // dialog.Title(winrt::box_value(L"Failed to parse settings")); - // dialog.Content(winrt::box_value(L"Failed to parse settings asdfa")); - // dialog.CloseButtonText(L"Ok"); + auto l = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); + auto title = l.GetString(titleKey); + auto message = l.GetString(textKey); + auto buttonText = l.GetString(L"Ok"); - // // IMPORTANT: Add the dialog to the _root UIElementw before you show it, so it knows how to attach to the XAML content. - // _root.Children().Append(dialog); - // dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); + Controls::ContentDialog dialog; + dialog.Title(winrt::box_value(title)); + dialog.Content(winrt::box_value(message)); + dialog.CloseButtonText(buttonText); - // } - // }); + // This doesn't work. + // dialog.RequestedTheme(_settings->GlobalSettings().GetRequestedTheme()); - //auto l = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); - //l.GetString(L"InitialJsonParse.Text"); + // auto res = Resources(); + // IInspectable key = winrt::box_value(L"BackgroundContentDialogThemeStyle"); + // if (res.HasKey(key)) + // { + // IInspectable g = res.Lookup(key); + // winrt::Windows::UI::Xaml::Style style = g.try_as(); + // dialog.Style(style); + // } + // IMPORTANT: Add the dialog to the _root UIElementw before you show it, so it knows how to attach to the XAML content. + _root.Children().Append(dialog); + Controls::ContentDialogResult result = await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); - _root.Loaded([this](auto&&, auto&&) { - if (FAILED(_settingsLoadedResult)) - { - - // Windows::ApplicationModel::Resources::ResourceLoader loader{}; - auto l = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); - auto s = l.GetString(L"InitialJsonParse.Text"); - auto bar = l.GetStringForUri(L"InitialJsonParse"); - auto foo = l.GetString(L"Foo"); - - Controls::ContentDialog dialog; - dialog.Title(winrt::box_value(L"Failed to parse settings")); - // dialog.Content(winrt::box_value(L"Failed to parse settings asdfa")); - dialog.Content(winrt::box_value(s)); - dialog.CloseButtonText(L"Ok"); - // IMPORTANT: Add the dialog to the _root UIElementw before you show it, so it knows how to attach to the XAML content. - _root.Children().Append(dialog); - dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); - } - }); + // After the dialog is dismissed, release the dialog lock so another can be shown. + _dialogLock.unlock(); + } + + void App::_OnLoaded(const IInspectable& sender, const RoutedEventArgs& eventArgs) + { + if (FAILED(_settingsLoadedResult)) + { + const winrt::hstring titleKey = L"InitialJsonParseErrorTitle"; + const winrt::hstring textKey = L"InitialJsonParseErrorText"; + _ShowOkDialog(titleKey, textKey); + } } // Method Description: @@ -382,20 +396,18 @@ namespace winrt::TerminalApp::implementation // happening during startup, it'll need to happen on a background thread. void App::LoadSettings() { - bool successfullyLoadedSettings = false; - HRESULT hr = E_FAIL; + _settingsLoadedResult = E_FAIL; // TODO: Try this. try { auto newSettings = CascadiaSettings::LoadAll(); _settings = std::move(newSettings); - successfullyLoadedSettings = true; - hr = S_OK; + _settingsLoadedResult = S_OK; } catch (const winrt::hresult_error& e) { - hr = e.code(); - LOG_HR(hr); + _settingsLoadedResult = e.code(); + LOG_HR(_settingsLoadedResult); } catch (...) { @@ -403,9 +415,7 @@ namespace winrt::TerminalApp::implementation LOG_HR(wil::ResultFromCaughtException()); } - _settingsLoadedResult = hr; - - if (!successfullyLoadedSettings) + if (FAILED(_settingsLoadedResult)) { // If it fails, // - use Default settings, @@ -416,7 +426,6 @@ namespace winrt::TerminalApp::implementation // commas. _settings = std::make_unique(); _settings->CreateDefaults(); - } _HookupKeyBindings(_settings->GetKeybindings()); @@ -481,14 +490,38 @@ namespace winrt::TerminalApp::implementation void App::_ReloadSettings() { // TODO: Try this. - _settings = CascadiaSettings::LoadAll(); // If it fails, // - don't change the settings (and don't actually apply the new settings) // - don't persist them. // - display a loading error - // * Settings could not be reloaded from file. Check for syntax - // errors, including trailing commas. + _settingsLoadedResult = E_FAIL; + try + { + auto newSettings = CascadiaSettings::LoadAll(false); + _settings = std::move(newSettings); + _settingsLoadedResult = S_OK; + } + catch (const winrt::hresult_error& e) + { + _settingsLoadedResult = e.code(); + LOG_HR(_settingsLoadedResult); + } + catch (...) + { + // TODO can this ever be hit? or will it always be a winrt::hresult_error? + LOG_HR(wil::ResultFromCaughtException()); + } + if (FAILED(_settingsLoadedResult)) + { + _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { + const winrt::hstring titleKey = L"ReloadJsonParseErrorTitle"; + const winrt::hstring textKey = L"ReloadJsonParseErrorText"; + _ShowOkDialog(titleKey, textKey); + }); + + return; + } // Re-wire the keybindings to their handlers, as we'll have created a // new AppKeyBindings object. diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index dd40d74afef..9f349990a12 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -64,15 +64,18 @@ namespace winrt::TerminalApp::implementation std::unique_ptr<::TerminalApp::CascadiaSettings> _settings; - HRESULT _settingsLoadedResult = S_OK; + HRESULT _settingsLoadedResult; bool _loadedInitialSettings; + std::shared_mutex _dialogLock; wil::unique_folder_change_reader_nothrow _reader; void _Create(); void _CreateNewTabFlyout(); + fire_and_forget _ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& textKey); + void _LoadSettings(); void _HookupKeyBindings(TerminalApp::AppKeyBindings bindings) noexcept; @@ -98,6 +101,7 @@ namespace winrt::TerminalApp::implementation // Todo: add more event implementations here // MSFT:20641986: Add keybindings for New Window + void _OnLoaded(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs); void _OnTabSelectionChanged(const IInspectable& sender, const Windows::UI::Xaml::Controls::SelectionChangedEventArgs& eventArgs); void _OnTabClosing(const IInspectable& sender, const Microsoft::UI::Xaml::Controls::TabViewTabClosingEventArgs& eventArgs); void _OnTabItemsChanged(const IInspectable& sender, const Windows::Foundation::Collections::IVectorChangedEventArgs& eventArgs); diff --git a/src/cascadia/TerminalApp/App.xaml b/src/cascadia/TerminalApp/App.xaml index ce79d196b7e..c33c35d1913 100644 --- a/src/cascadia/TerminalApp/App.xaml +++ b/src/cascadia/TerminalApp/App.xaml @@ -28,6 +28,9 @@ + From 00a8da4045769f057ea32e64c06fc837e9070496 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 15 May 2019 17:56:26 -0500 Subject: [PATCH 3/8] This unfortunately didn't work --- src/cascadia/TerminalApp/App.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index b5a996eb971..5510e0c6903 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -162,6 +162,7 @@ namespace winrt::TerminalApp::implementation { IInspectable g = res.Lookup(key); winrt::Windows::UI::Xaml::Style style = g.try_as(); + _root.Style(style); _tabRow.Style(style); } From b51d03fa664d223db0ff76a23eba612324585977 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 May 2019 11:47:48 -0500 Subject: [PATCH 4/8] I'm just committing all of this, even though I really don't need most of it. --- src/cascadia/TerminalApp/App.cpp | 7 +- src/cascadia/TerminalApp/App.xaml | 18 ++- .../TerminalApp/CustomContentDialog.xaml | 139 ++++++++++++++++++ src/cascadia/TerminalApp/TerminalApp.vcxproj | 11 +- src/cascadia/WindowsTerminal/IslandWindow.cpp | 33 +++++ 5 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 src/cascadia/TerminalApp/CustomContentDialog.xaml diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 09e0f424cc6..a50bdc434ad 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -193,8 +193,8 @@ namespace winrt::TerminalApp::implementation Controls::ContentDialog dialog; dialog.Title(winrt::box_value(title)); dialog.Content(winrt::box_value(message)); - dialog.CloseButtonText(buttonText); - + // dialog.CloseButtonText(buttonText); + dialog.PrimaryButtonText(buttonText); // This doesn't work. // dialog.RequestedTheme(_settings->GlobalSettings().GetRequestedTheme()); @@ -206,8 +206,9 @@ namespace winrt::TerminalApp::implementation // winrt::Windows::UI::Xaml::Style style = g.try_as(); // dialog.Style(style); // } - + // IMPORTANT: Add the dialog to the _root UIElementw before you show it, so it knows how to attach to the XAML content. + _root.Children().Append(dialog); Controls::ContentDialogResult result = await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); diff --git a/src/cascadia/TerminalApp/App.xaml b/src/cascadia/TerminalApp/App.xaml index c33c35d1913..a66573d1dfa 100644 --- a/src/cascadia/TerminalApp/App.xaml +++ b/src/cascadia/TerminalApp/App.xaml @@ -1,3 +1,5 @@ + - + + + + + @@ -42,8 +53,11 @@ + + + diff --git a/src/cascadia/TerminalApp/CustomContentDialog.xaml b/src/cascadia/TerminalApp/CustomContentDialog.xaml new file mode 100644 index 00000000000..9965f31a7be --- /dev/null +++ b/src/cascadia/TerminalApp/CustomContentDialog.xaml @@ -0,0 +1,139 @@ + + + + + + + + + + + + + diff --git a/src/cascadia/TerminalApp/TerminalApp.vcxproj b/src/cascadia/TerminalApp/TerminalApp.vcxproj index 6a70d00dda9..948ad7bf6fc 100644 --- a/src/cascadia/TerminalApp/TerminalApp.vcxproj +++ b/src/cascadia/TerminalApp/TerminalApp.vcxproj @@ -28,12 +28,11 @@ - - + @@ -128,4 +127,4 @@ - \ No newline at end of file + diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index c7e00745512..bbd48306178 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -4,6 +4,9 @@ #include "pch.h" #include "IslandWindow.h" #include "../types/inc/Viewport.hpp" +#include +using namespace std::chrono_literals; +using namespace winrt; extern "C" IMAGE_DOS_HEADER __ImageBase; @@ -273,9 +276,39 @@ void IslandWindow::OnRestore() // TODO MSFT#21315817 Stop rendering island content when the app is minimized. } +winrt::fire_and_forget DoTheThing(winrt::Windows::UI::Xaml::Controls::Grid grid) { + winrt::Windows::UI::Xaml::Controls::ContentDialog dialog; + dialog.Title(winrt::box_value(L"Foo")); + dialog.Content(winrt::box_value(L"Bar")); + // dialog.CloseButtonText(buttonText); + // dialog.PrimaryButtonText(L"Primary"); + dialog.CloseButtonText(L"Close"); + // dialog.PrimaryButtonStyle() Do this thing here + grid.Children().Append(dialog); + // grid.RequestedTheme(ElementTheme::Dark); + co_await 3ms; + co_await winrt::resume_foreground(grid.Dispatcher()); + // grid.RequestedTheme(ElementTheme::Light); + Controls::ContentDialogResult result = await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); + +} + +winrt::fire_and_forget _ShowOkDialog(winrt::Windows::UI::Xaml::Controls::Grid grid) +{ + co_await 5s; + grid.Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [grid]() { + grid.RequestedTheme(ElementTheme::Light); + DoTheThing(grid); + }); + +} + void IslandWindow::SetRootContent(winrt::Windows::UI::Xaml::UIElement content) { _rootGrid.Children().Clear(); ApplyCorrection(_scale.ScaleX()); _rootGrid.Children().Append(content); + + _ShowOkDialog(_rootGrid); } + From c1d17f69103c9c022dcb32eb9097983f2501d068 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 May 2019 12:23:39 -0500 Subject: [PATCH 5/8] Cleanup for PR --- src/cascadia/TerminalApp/App.cpp | 122 +++++++++--------- src/cascadia/TerminalApp/App.h | 1 + src/cascadia/TerminalApp/App.xaml | 19 +-- .../CascadiaSettingsSerialization.cpp | 35 ++--- src/cascadia/TerminalApp/TerminalApp.vcxproj | 9 +- src/cascadia/WindowsTerminal/IslandWindow.cpp | 32 ----- 6 files changed, 92 insertions(+), 126 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index a50bdc434ad..d8369031aac 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -174,7 +174,16 @@ namespace winrt::TerminalApp::implementation _root.Loaded({ this, &App::_OnLoaded }); } - fire_and_forget App::_ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& textKey) + // Method Description: + // - Show a ContentDialog with a single "Ok" button to dismiss. Looks up the + // the title and text from our Resources using the provided keys. + // - Only one dialog can be visible at a time. If another dialog is visible + // when this is called, nothing happens. + // Arguments: + // - titleKey: The key to use to lookup the title text from our resources. + // - textKey: The key to use to lookup the content text from our resources. + fire_and_forget App::_ShowOkDialog(const winrt::hstring& titleKey, + const winrt::hstring& textKey) { // DON'T release this lock in a wil::scope_exit. The scope_exit will get // called when we await, which is not what we want. @@ -193,30 +202,29 @@ namespace winrt::TerminalApp::implementation Controls::ContentDialog dialog; dialog.Title(winrt::box_value(title)); dialog.Content(winrt::box_value(message)); - // dialog.CloseButtonText(buttonText); - dialog.PrimaryButtonText(buttonText); - // This doesn't work. - // dialog.RequestedTheme(_settings->GlobalSettings().GetRequestedTheme()); - - // auto res = Resources(); - // IInspectable key = winrt::box_value(L"BackgroundContentDialogThemeStyle"); - // if (res.HasKey(key)) - // { - // IInspectable g = res.Lookup(key); - // winrt::Windows::UI::Xaml::Style style = g.try_as(); - // dialog.Style(style); - // } - - // IMPORTANT: Add the dialog to the _root UIElementw before you show it, so it knows how to attach to the XAML content. - + dialog.CloseButtonText(buttonText); + + // IMPORTANT: Add the dialog to the _root UIElement before you show it, + // so it knows how to attach to the XAML content. _root.Children().Append(dialog); + + // Display the dialog. Controls::ContentDialogResult result = await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); // After the dialog is dismissed, release the dialog lock so another can be shown. _dialogLock.unlock(); } - void App::_OnLoaded(const IInspectable& sender, const RoutedEventArgs& eventArgs) + // Method Description: + // - Triggered when the application is fiished loading. If we failed to load + // the settings, then this will display the error dialog. This is done + // here instead of when loading the settings, because we need our UI to be + // visible to display the dialog, and when we're loading the settings, + // the UI might not be visible yet. + // Arguments: + // - + void App::_OnLoaded(const IInspectable& /*sender*/, + const RoutedEventArgs& /*eventArgs*/) { if (FAILED(_settingsLoadedResult)) { @@ -392,16 +400,18 @@ namespace winrt::TerminalApp::implementation } // Method Description: - // - Initialized our settings. See CascadiaSettings for more details. - // Additionally hooks up our callbacks for keybinding events to the - // keybindings object. - // NOTE: This must be called from a MTA if we're running as a packaged - // application. The Windows.Storage APIs require a MTA. If this isn't - // happening during startup, it'll need to happen on a background thread. - void App::LoadSettings() + // - Attempt to load the settings. If we fail for any reason, sets + // _settingsLoadedResult to the appropriate HRESULT. + // Arguments: + // - saveOnLoad: If true, after loading the settings, we should re-write + // them to the file, to make sure the schema is updated. See + // `CascadiaSettings::LoadAll` for details. + // Return Value: + // - + void App::_TryLoadSettings(const bool saveOnLoad) noexcept { _settingsLoadedResult = E_FAIL; - // TODO: Try this. + try { auto newSettings = CascadiaSettings::LoadAll(); @@ -415,19 +425,31 @@ namespace winrt::TerminalApp::implementation } catch (...) { - // TODO can this ever be hit? or will it always be a winrt::hresult_error? LOG_HR(wil::ResultFromCaughtException()); } + } + + // Method Description: + // - Initialized our settings. See CascadiaSettings for more details. + // Additionally hooks up our callbacks for keybinding events to the + // keybindings object. + // NOTE: This must be called from a MTA if we're running as a packaged + // application. The Windows.Storage APIs require a MTA. If this isn't + // happening during startup, it'll need to happen on a background thread. + void App::LoadSettings() + { + // Attempt to load the settings. + // If it fails, + // - use Default settings, + // - don't persist them (LoadAll won't save them in this case). + // - _settingsLoadedResult will be set to an error, indicating that + // we should display the loading error. + // * We can't display the error now, because we might not have a + // UI yet. We'll display the error in _OnLoaded. + _TryLoadSettings(true); if (FAILED(_settingsLoadedResult)) { - // If it fails, - // - use Default settings, - // - don't persist them. - // - Set a flag saying that we should display the loading error. - // * Settings could not be loaded from file - temporarily using - // default settings. Check for syntax errors, including trailing - // commas. _settings = std::make_unique(); _settings->CreateDefaults(); } @@ -487,34 +509,14 @@ namespace winrt::TerminalApp::implementation // Method Description: // - Reloads the settings from the profile.json. - // Arguments: - // - - // Return Value: - // - void App::_ReloadSettings() { - // TODO: Try this. + // Attempt to load our settings. // If it fails, // - don't change the settings (and don't actually apply the new settings) // - don't persist them. // - display a loading error - _settingsLoadedResult = E_FAIL; - try - { - auto newSettings = CascadiaSettings::LoadAll(false); - _settings = std::move(newSettings); - _settingsLoadedResult = S_OK; - } - catch (const winrt::hresult_error& e) - { - _settingsLoadedResult = e.code(); - LOG_HR(_settingsLoadedResult); - } - catch (...) - { - // TODO can this ever be hit? or will it always be a winrt::hresult_error? - LOG_HR(wil::ResultFromCaughtException()); - } + _TryLoadSettings(false); if (FAILED(_settingsLoadedResult)) { @@ -527,12 +529,16 @@ namespace winrt::TerminalApp::implementation return; } + // Here, we successfully reloaded the settings, and created a new + // TerminalSettings object. + // Re-wire the keybindings to their handlers, as we'll have created a // new AppKeyBindings object. _HookupKeyBindings(_settings->GetKeybindings()); - auto profiles = _settings->GetProfiles(); + // Refresh UI elements + auto profiles = _settings->GetProfiles(); for (auto &profile : profiles) { const GUID profileGuid = profile.GetGuid(); @@ -811,7 +817,7 @@ namespace winrt::TerminalApp::implementation // Negative values of `delta` will move the view up by one page, and positive values // will move the viewport down by one page. // Arguments: - // - delta: The direction to move the view relative to the current viewport(it + // - delta: The direction to move the view relative to the current viewport(it // is clamped between -1 and 1) void App::_ScrollPage(int delta) { diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 88ac440b6f7..d3aba75bd57 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -77,6 +77,7 @@ namespace winrt::TerminalApp::implementation fire_and_forget _ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& textKey); + void _TryLoadSettings(const bool saveOnLoad) noexcept; void _LoadSettings(); void _OpenSettings(); diff --git a/src/cascadia/TerminalApp/App.xaml b/src/cascadia/TerminalApp/App.xaml index a66573d1dfa..6b243b00b7c 100644 --- a/src/cascadia/TerminalApp/App.xaml +++ b/src/cascadia/TerminalApp/App.xaml @@ -18,7 +18,6 @@ the MIT License. See LICENSE in the project root for license information. --> - - - - + diff --git a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp index 7b1ce9bcf47..e80d8abf1dd 100644 --- a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp @@ -47,40 +47,29 @@ std::unique_ptr CascadiaSettings::LoadAll(const bool saveOnLoa { const auto actualData = fileData.value(); - JsonObject root{ nullptr }; - // bool parsedSuccessfully = JsonValue::TryParse(actualData, root); - root = JsonObject::Parse(actualData); bool parsedSuccessfully = true; - // TODO:MSFT:20737698 - Display an error if we failed to parse settings - if (parsedSuccessfully) + // If Parse fails, it'll throw a hresult_error + JsonObject root = JsonObject::Parse(actualData); + + resultPtr = FromJson(root); + + // Update profile only if it has changed. + if (saveOnLoad) { - JsonObject obj = root;//.GetObjectW(); - resultPtr = FromJson(obj); + const JsonObject json = resultPtr->ToJson(); + auto serializedSettings = json.Stringify(); - // Update profile only if it has changed. - if (saveOnLoad) + if (actualData != serializedSettings) { - const JsonObject json = resultPtr->ToJson(); - auto serializedSettings = json.Stringify(); - - if (actualData != serializedSettings) - { - resultPtr->SaveAll(); - } + resultPtr->SaveAll(); } } - else - { - // Until 20737698 is done, throw an error, so debugging can trace - // the exception here, instead of later on in unrelated code - THROW_HR(E_INVALIDARG); - } } else { resultPtr = std::make_unique(); resultPtr->CreateDefaults(); - // The settings file does not exist. Let's commit one. + // The settings file does not exist. Let's commit one. resultPtr->SaveAll(); } diff --git a/src/cascadia/TerminalApp/TerminalApp.vcxproj b/src/cascadia/TerminalApp/TerminalApp.vcxproj index 948ad7bf6fc..ad48839d926 100644 --- a/src/cascadia/TerminalApp/TerminalApp.vcxproj +++ b/src/cascadia/TerminalApp/TerminalApp.vcxproj @@ -28,11 +28,12 @@ - - - DefaultStyle + + diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index bbd48306178..ad106f23d15 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -4,9 +4,6 @@ #include "pch.h" #include "IslandWindow.h" #include "../types/inc/Viewport.hpp" -#include -using namespace std::chrono_literals; -using namespace winrt; extern "C" IMAGE_DOS_HEADER __ImageBase; @@ -276,39 +273,10 @@ void IslandWindow::OnRestore() // TODO MSFT#21315817 Stop rendering island content when the app is minimized. } -winrt::fire_and_forget DoTheThing(winrt::Windows::UI::Xaml::Controls::Grid grid) { - winrt::Windows::UI::Xaml::Controls::ContentDialog dialog; - dialog.Title(winrt::box_value(L"Foo")); - dialog.Content(winrt::box_value(L"Bar")); - // dialog.CloseButtonText(buttonText); - // dialog.PrimaryButtonText(L"Primary"); - dialog.CloseButtonText(L"Close"); - // dialog.PrimaryButtonStyle() Do this thing here - grid.Children().Append(dialog); - // grid.RequestedTheme(ElementTheme::Dark); - co_await 3ms; - co_await winrt::resume_foreground(grid.Dispatcher()); - // grid.RequestedTheme(ElementTheme::Light); - Controls::ContentDialogResult result = await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); - -} - -winrt::fire_and_forget _ShowOkDialog(winrt::Windows::UI::Xaml::Controls::Grid grid) -{ - co_await 5s; - grid.Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [grid]() { - grid.RequestedTheme(ElementTheme::Light); - DoTheThing(grid); - }); - -} - void IslandWindow::SetRootContent(winrt::Windows::UI::Xaml::UIElement content) { _rootGrid.Children().Clear(); ApplyCorrection(_scale.ScaleX()); _rootGrid.Children().Append(content); - - _ShowOkDialog(_rootGrid); } From 5547ca56efeaa74b0b71176dd39cf408b53831a2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 21 May 2019 12:13:58 -0500 Subject: [PATCH 6/8] PR feedback --- src/cascadia/TerminalApp/App.cpp | 34 ++--- src/cascadia/TerminalApp/App.h | 4 +- .../TerminalApp/CustomContentDialog.xaml | 139 ------------------ src/inc/DefaultSettings.h | 2 +- 4 files changed, 20 insertions(+), 159 deletions(-) delete mode 100644 src/cascadia/TerminalApp/CustomContentDialog.xaml diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index d8369031aac..67784a49933 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -180,22 +180,21 @@ namespace winrt::TerminalApp::implementation // - Only one dialog can be visible at a time. If another dialog is visible // when this is called, nothing happens. // Arguments: - // - titleKey: The key to use to lookup the title text from our resources. + // - contentKey: The key to use to lookup the title text from our resources. // - textKey: The key to use to lookup the content text from our resources. - fire_and_forget App::_ShowOkDialog(const winrt::hstring& titleKey, + fire_and_forget App::_ShowOkDialog(const winrt::hstring& contentKey, const winrt::hstring& textKey) { // DON'T release this lock in a wil::scope_exit. The scope_exit will get // called when we await, which is not what we want. - auto gotLock = _dialogLock.try_lock(); - if (!gotLock) + if (!_dialogLock.try_lock()) { // Another dialog is visible. return; } auto l = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); - auto title = l.GetString(titleKey); + auto title = l.GetString(contentKey); auto message = l.GetString(textKey); auto buttonText = l.GetString(L"Ok"); @@ -400,33 +399,34 @@ namespace winrt::TerminalApp::implementation } // Method Description: - // - Attempt to load the settings. If we fail for any reason, sets - // _settingsLoadedResult to the appropriate HRESULT. + // - Attempt to load the settings. If we fail for any reason, returns an error. // Arguments: // - saveOnLoad: If true, after loading the settings, we should re-write // them to the file, to make sure the schema is updated. See // `CascadiaSettings::LoadAll` for details. // Return Value: - // - - void App::_TryLoadSettings(const bool saveOnLoad) noexcept + // - S_OK if we successfully parsed the settings, otherwise an appropriate HRESULT. + HRESULT App::_TryLoadSettings(const bool saveOnLoad) noexcept { - _settingsLoadedResult = E_FAIL; + HRESULT hr = E_FAIL; try { - auto newSettings = CascadiaSettings::LoadAll(); + auto newSettings = CascadiaSettings::LoadAll(saveOnLoad); _settings = std::move(newSettings); - _settingsLoadedResult = S_OK; + hr = S_OK; } catch (const winrt::hresult_error& e) { - _settingsLoadedResult = e.code(); - LOG_HR(_settingsLoadedResult); + hr = e.code(); + LOG_HR(hr); } catch (...) { - LOG_HR(wil::ResultFromCaughtException()); + hr = wil::ResultFromCaughtException() + LOG_HR(hr); } + return hr; } // Method Description: @@ -446,7 +446,7 @@ namespace winrt::TerminalApp::implementation // we should display the loading error. // * We can't display the error now, because we might not have a // UI yet. We'll display the error in _OnLoaded. - _TryLoadSettings(true); + _settingsLoadedResult = _TryLoadSettings(true); if (FAILED(_settingsLoadedResult)) { @@ -516,7 +516,7 @@ namespace winrt::TerminalApp::implementation // - don't change the settings (and don't actually apply the new settings) // - don't persist them. // - display a loading error - _TryLoadSettings(false); + _settingsLoadedResult = _TryLoadSettings(false); if (FAILED(_settingsLoadedResult)) { diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index d3aba75bd57..db4840223be 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -75,9 +75,9 @@ namespace winrt::TerminalApp::implementation void _Create(); void _CreateNewTabFlyout(); - fire_and_forget _ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& textKey); + fire_and_forget _ShowOkDialog(const winrt::hstring& contentKey, const winrt::hstring& textKey); - void _TryLoadSettings(const bool saveOnLoad) noexcept; + HRESULT _TryLoadSettings(const bool saveOnLoad) noexcept; void _LoadSettings(); void _OpenSettings(); diff --git a/src/cascadia/TerminalApp/CustomContentDialog.xaml b/src/cascadia/TerminalApp/CustomContentDialog.xaml deleted file mode 100644 index 9965f31a7be..00000000000 --- a/src/cascadia/TerminalApp/CustomContentDialog.xaml +++ /dev/null @@ -1,139 +0,0 @@ - - - - - - - - - - - - - diff --git a/src/inc/DefaultSettings.h b/src/inc/DefaultSettings.h index 421dc185976..d959e41f7dc 100644 --- a/src/inc/DefaultSettings.h +++ b/src/inc/DefaultSettings.h @@ -24,7 +24,7 @@ constexpr COLORREF DEFAULT_BACKGROUND = COLOR_BLACK; constexpr COLORREF DEFAULT_BACKGROUND_WITH_ALPHA = OPACITY_OPAQUE | DEFAULT_BACKGROUND; constexpr short DEFAULT_HISTORY_SIZE = 9001; const std::wstring DEFAULT_FONT_FACE { L"Consolas" }; -constexpr int DEFAULT_FONT_SIZE = 10; +constexpr int DEFAULT_FONT_SIZE = 12; constexpr int DEFAULT_ROWS = 30; constexpr int DEFAULT_COLS = 120; From af455356490849ab4f0f4e2a36caf75af09ed8d0 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 22 May 2019 14:24:17 -0500 Subject: [PATCH 7/8] Minor PR nits --- src/cascadia/TerminalApp/App.cpp | 19 ++++++++++--------- src/cascadia/TerminalApp/App.h | 3 ++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 67784a49933..23d5f5cffbb 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -180,10 +180,10 @@ namespace winrt::TerminalApp::implementation // - Only one dialog can be visible at a time. If another dialog is visible // when this is called, nothing happens. // Arguments: - // - contentKey: The key to use to lookup the title text from our resources. - // - textKey: The key to use to lookup the content text from our resources. - fire_and_forget App::_ShowOkDialog(const winrt::hstring& contentKey, - const winrt::hstring& textKey) + // - 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. + fire_and_forget App::_ShowOkDialog(const winrt::hstring& titleKey, + const winrt::hstring& contentKey) { // DON'T release this lock in a wil::scope_exit. The scope_exit will get // called when we await, which is not what we want. @@ -193,10 +193,10 @@ namespace winrt::TerminalApp::implementation return; } - auto l = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); - auto title = l.GetString(contentKey); - auto message = l.GetString(textKey); - auto buttonText = l.GetString(L"Ok"); + auto resourceLoader = Windows::ApplicationModel::Resources::ResourceLoader::GetForCurrentView(); + auto title = resourceLoader.GetString(titleKey); + auto message = resourceLoader.GetString(contentKey); + auto buttonText = resourceLoader.GetString(L"Ok"); Controls::ContentDialog dialog; dialog.Title(winrt::box_value(title)); @@ -406,6 +406,7 @@ namespace winrt::TerminalApp::implementation // `CascadiaSettings::LoadAll` for details. // Return Value: // - S_OK if we successfully parsed the settings, otherwise an appropriate HRESULT. + [[nodiscard]] HRESULT App::_TryLoadSettings(const bool saveOnLoad) noexcept { HRESULT hr = E_FAIL; @@ -423,7 +424,7 @@ namespace winrt::TerminalApp::implementation } catch (...) { - hr = wil::ResultFromCaughtException() + hr = wil::ResultFromCaughtException(); LOG_HR(hr); } return hr; diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index db4840223be..0b455db5586 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -75,8 +75,9 @@ namespace winrt::TerminalApp::implementation void _Create(); void _CreateNewTabFlyout(); - fire_and_forget _ShowOkDialog(const winrt::hstring& contentKey, const winrt::hstring& textKey); + fire_and_forget _ShowOkDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey); + [[nodiscard]] HRESULT _TryLoadSettings(const bool saveOnLoad) noexcept; void _LoadSettings(); void _OpenSettings(); From 08dd6c5a9741132e503562e8090ea7f177c0413a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 23 May 2019 14:05:47 -0500 Subject: [PATCH 8/8] Fix minor nit from @dhowett-msft --- src/cascadia/TerminalApp/App.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 23d5f5cffbb..fc95ef637d3 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -187,7 +187,8 @@ namespace winrt::TerminalApp::implementation { // DON'T release this lock in a wil::scope_exit. The scope_exit will get // called when we await, which is not what we want. - if (!_dialogLock.try_lock()) + std::unique_lock lock{ _dialogLock, std::try_to_lock }; + if (!lock) { // Another dialog is visible. return; @@ -210,8 +211,8 @@ namespace winrt::TerminalApp::implementation // Display the dialog. Controls::ContentDialogResult result = await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup); - // After the dialog is dismissed, release the dialog lock so another can be shown. - _dialogLock.unlock(); + // After the dialog is dismissed, the dialog lock (held by `lock`) will + // be released so another can be shown. } // Method Description: