diff --git a/doc/cascadia/SettingsSchema.md b/doc/cascadia/SettingsSchema.md index 0a41c6964b3..827be7a2acc 100644 --- a/doc/cascadia/SettingsSchema.md +++ b/doc/cascadia/SettingsSchema.md @@ -28,7 +28,7 @@ Properties listed below are specific to each unique profile. | `backgroundImageAlignment` | Optional | String | `center` | Sets how the background image aligns to the boundaries of the window. Possible values: `"center"`, `"left"`, `"top"`, `"right"`, `"bottom"`, `"topLeft"`, `"topRight"`, `"bottomLeft"`, `"bottomRight"` | | `backgroundImageOpacity` | Optional | Number | `1.0` | Sets the transparency of the background image. Accepts floating point values from 0-1. | | `backgroundImageStretchMode` | Optional | String | `uniformToFill` | Sets how the background image is resized to fill the window. Possible values: `"none"`, `"fill"`, `"uniform"`, `"uniformToFill"` | -| `closeOnExit` | Optional | Boolean | `true` | When set to `true`, the selected tab closes when `exit` is typed. When set to `false`, the tab will remain open when `exit` is typed. | +| `closeOnExit` | Optional | String | `graceful` | Sets how the profile reacts to termination or failure to launch. Possible values: `"graceful"` (close when `exit` is typed or the process exits normally), `"always"` (always close) and `"never"` (never close). `true` and `false` are accepted as synonyms for `"graceful"` and `"never"` respectively. | | `colorScheme` | Optional | String | `Campbell` | Name of the terminal color scheme to use. Color schemes are defined under `schemes`. | | `colorTable` | Optional | Array[String] | | Array of colors used in the profile if `colorscheme` is not set. Array follows the format defined in `schemes`. | | `commandline` | Optional | String | | Executable used in the profile. | diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 8c7ab820e83..0edb05325a5 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -326,9 +326,21 @@ "type": "string" }, "closeOnExit": { - "default": true, - "description": "When set to true (default), the selected tab closes when the connected application exits. When set to false, the tab will remain open when the connected application exits.", - "type": "boolean" + "default": "graceful", + "description": "Sets how the profile reacts to termination or failure to launch. Possible values: \"graceful\" (close when exit is typed or the process exits normally), \"always\" (always close) and \"never\" (never close). true and false are accepted as synonyms for \"graceful\" and \"never\" respectively.", + "oneOf": [ + { + "enum": [ + "never", + "graceful", + "always" + ], + "type": "string" + }, + { + "type": "boolean" + } + ] }, "colorScheme": { "default": "Campbell", diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index a4446f984e3..21deac0a216 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -55,6 +55,9 @@ namespace TerminalAppLocalTests TEST_METHOD(TestProfileIconWithEnvVar); TEST_METHOD(TestProfileBackgroundImageWithEnvVar); + TEST_METHOD(TestCloseOnExitParsing); + TEST_METHOD(TestCloseOnExitCompatibilityShim); + TEST_CLASS_SETUP(ClassSetup) { InitializeJsonReader(); @@ -1414,4 +1417,67 @@ namespace TerminalAppLocalTests auto terminalSettings = settings._profiles[0].CreateTerminalSettings(globalSettings.GetColorSchemes()); VERIFY_ARE_EQUAL(expectedPath, terminalSettings.BackgroundImage()); } + void SettingsTests::TestCloseOnExitParsing() + { + const std::string settingsJson{ R"( + { + "profiles": [ + { + "name": "profile0", + "closeOnExit": "graceful" + }, + { + "name": "profile1", + "closeOnExit": "always" + }, + { + "name": "profile2", + "closeOnExit": "never" + }, + { + "name": "profile3", + "closeOnExit": null + }, + { + "name": "profile4", + "closeOnExit": { "clearly": "not a string" } + } + ] + })" }; + + VerifyParseSucceeded(settingsJson); + CascadiaSettings settings{}; + settings._ParseJsonString(settingsJson, false); + settings.LayerJson(settings._userSettings); + VERIFY_ARE_EQUAL(CloseOnExitMode::Graceful, settings._profiles[0].GetCloseOnExitMode()); + VERIFY_ARE_EQUAL(CloseOnExitMode::Always, settings._profiles[1].GetCloseOnExitMode()); + VERIFY_ARE_EQUAL(CloseOnExitMode::Never, settings._profiles[2].GetCloseOnExitMode()); + + // Unknown modes parse as "Graceful" + VERIFY_ARE_EQUAL(CloseOnExitMode::Graceful, settings._profiles[3].GetCloseOnExitMode()); + VERIFY_ARE_EQUAL(CloseOnExitMode::Graceful, settings._profiles[4].GetCloseOnExitMode()); + } + void SettingsTests::TestCloseOnExitCompatibilityShim() + { + const std::string settingsJson{ R"( + { + "profiles": [ + { + "name": "profile0", + "closeOnExit": true + }, + { + "name": "profile1", + "closeOnExit": false + } + ] + })" }; + + VerifyParseSucceeded(settingsJson); + CascadiaSettings settings{}; + settings._ParseJsonString(settingsJson, false); + settings.LayerJson(settings._userSettings); + VERIFY_ARE_EQUAL(CloseOnExitMode::Graceful, settings._profiles[0].GetCloseOnExitMode()); + VERIFY_ARE_EQUAL(CloseOnExitMode::Never, settings._profiles[1].GetCloseOnExitMode()); + } } diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index db739eefce8..b3e8c34b093 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -592,6 +592,13 @@ namespace winrt::TerminalApp::implementation }); } + // Method Description: + // - Returns a pointer to the global shared settings. + [[nodiscard]] std::shared_ptr<::TerminalApp::CascadiaSettings> AppLogic::GetSettings() const noexcept + { + return _settings; + } + // Method Description: // - Update the current theme of the application. This will trigger our // RequestedThemeChanged event, to have our host change the theme of the diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 6f90251ad05..06890380519 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -29,6 +29,7 @@ namespace winrt::TerminalApp::implementation void Create(); void LoadSettings(); + [[nodiscard]] std::shared_ptr<::TerminalApp::CascadiaSettings> GetSettings() const noexcept; Windows::Foundation::Point GetLaunchDimensions(uint32_t dpi); winrt::Windows::Foundation::Point GetLaunchInitialPositions(int32_t defaultInitialX, int32_t defaultInitialY); diff --git a/src/cascadia/TerminalApp/AzureCloudShellGenerator.cpp b/src/cascadia/TerminalApp/AzureCloudShellGenerator.cpp index 0414322ba06..b9e76dce7f0 100644 --- a/src/cascadia/TerminalApp/AzureCloudShellGenerator.cpp +++ b/src/cascadia/TerminalApp/AzureCloudShellGenerator.cpp @@ -39,7 +39,6 @@ std::vector AzureCloudShellGenerator::GenerateProfiles() azureCloudShellProfile.SetColorScheme({ L"Vintage" }); azureCloudShellProfile.SetAcrylicOpacity(0.6); azureCloudShellProfile.SetUseAcrylic(true); - azureCloudShellProfile.SetCloseOnExit(false); azureCloudShellProfile.SetConnectionType(AzureConnectionType); profiles.emplace_back(azureCloudShellProfile); } diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index d7046d9045f..ff861ab3e2b 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -9,6 +9,7 @@ #include "CascadiaSettings.h" #include "../../types/inc/utils.hpp" #include "../../inc/DefaultSettings.h" +#include "AppLogic.h" #include "Utils.h" #include "PowershellCoreProfileGenerator.h" @@ -26,6 +27,21 @@ static constexpr std::wstring_view PACKAGED_PROFILE_ICON_PATH{ L"ms-appx:///Prof static constexpr std::wstring_view PACKAGED_PROFILE_ICON_EXTENSION{ L".png" }; static constexpr std::wstring_view DEFAULT_LINUX_ICON_GUID{ L"{9acb9455-ca41-5af7-950f-6bca1bc9722f}" }; +// Method Description: +// - Returns the settings currently in use by the entire Terminal application. +// Throws: +// - HR E_INVALIDARG if the app isn't up and running. +const CascadiaSettings& CascadiaSettings::GetCurrentAppSettings() +{ + auto currentXamlApp{ winrt::Windows::UI::Xaml::Application::Current().as() }; + THROW_HR_IF_NULL(E_INVALIDARG, currentXamlApp); + + auto appLogic = winrt::get_self(currentXamlApp.Logic()); + THROW_HR_IF_NULL(E_INVALIDARG, appLogic); + + return *(appLogic->GetSettings()); +} + CascadiaSettings::CascadiaSettings() : CascadiaSettings(true) { diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index fde8177507c..a22a6df64e7 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -49,6 +49,8 @@ class TerminalApp::CascadiaSettings final static std::unique_ptr LoadDefaults(); static std::unique_ptr LoadAll(); + static const CascadiaSettings& GetCurrentAppSettings(); + winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional profileGuid) const; GlobalAppSettings& GlobalSettings(); diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index a0d3307898b..81a9c397707 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -3,6 +3,10 @@ #include "pch.h" #include "Pane.h" +#include "Profile.h" +#include "CascadiaSettings.h" + +#include "winrt/Microsoft.Terminal.TerminalConnection.h" using namespace winrt::Windows::Foundation; using namespace winrt::Windows::UI; @@ -11,7 +15,9 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Windows::UI::Xaml::Media; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; +using namespace winrt::Microsoft::Terminal::TerminalConnection; using namespace winrt::TerminalApp; +using namespace TerminalApp; static const int PaneBorderSize = 2; static const int CombinedPaneBorderSize = 2 * PaneBorderSize; @@ -28,7 +34,7 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus _root.Children().Append(_border); _border.Child(_control); - _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); + _connectionStateChangedToken = _control.ConnectionStateChanged({ this, &Pane::_ControlConnectionStateChangedHandler }); // On the first Pane's creation, lookup resources we'll use to theme the // Pane, including the brushed to use for the focused/unfocused border @@ -302,7 +308,7 @@ bool Pane::NavigateFocus(const Direction& direction) // - // Return Value: // - -void Pane::_ControlClosedHandler() +void Pane::_ControlConnectionStateChangedHandler(const TermControl& /*sender*/, const winrt::Windows::Foundation::IInspectable& /*args*/) { std::unique_lock lock{ _createCloseLock }; // It's possible that this event handler started being executed, then before @@ -317,10 +323,24 @@ void Pane::_ControlClosedHandler() return; } - if (_control.ShouldCloseOnExit()) + const auto newConnectionState = _control.ConnectionState(); + + if (newConnectionState < ConnectionState::Closed) + { + // Pane doesn't care if the connection isn't entering a terminal state. + return; + } + + const auto& settings = CascadiaSettings::GetCurrentAppSettings(); + auto paneProfile = settings.FindProfile(_profile.value()); + if (paneProfile) { - // Fire our Closed event to tell our parent that we should be removed. - _closedHandlers(); + auto mode = paneProfile->GetCloseOnExitMode(); + if ((mode == CloseOnExitMode::Always) || + (mode == CloseOnExitMode::Graceful && newConnectionState == ConnectionState::Closed)) + { + _ClosedHandlers(nullptr, nullptr); + } } } @@ -333,7 +353,7 @@ void Pane::_ControlClosedHandler() void Pane::Close() { // Fire our Closed event to tell our parent that we should be removed. - _closedHandlers(); + _ClosedHandlers(nullptr, nullptr); } // Method Description: @@ -570,7 +590,7 @@ void Pane::_CloseChild(const bool closeFirst) _profile = remainingChild->_profile; // Add our new event handler before revoking the old one. - _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); + _connectionStateChangedToken = _control.ConnectionStateChanged({ this, &Pane::_ControlConnectionStateChangedHandler }); // Revoke the old event handlers. Remove both the handlers for the panes // themselves closing, and remove their handlers for their controls @@ -578,8 +598,8 @@ void Pane::_CloseChild(const bool closeFirst) // they'll trigger only our event handler for the control's close. _firstChild->Closed(_firstClosedToken); _secondChild->Closed(_secondClosedToken); - closedChild->_control.ConnectionClosed(closedChild->_connectionClosedToken); - remainingChild->_control.ConnectionClosed(remainingChild->_connectionClosedToken); + closedChild->_control.ConnectionStateChanged(closedChild->_connectionStateChangedToken); + remainingChild->_control.ConnectionStateChanged(remainingChild->_connectionStateChangedToken); // If either of our children was focused, we want to take that focus from // them. @@ -659,7 +679,7 @@ void Pane::_CloseChild(const bool closeFirst) // Revoke event handlers on old panes and controls oldFirst->Closed(oldFirstToken); oldSecond->Closed(oldSecondToken); - closedChild->_control.ConnectionClosed(closedChild->_connectionClosedToken); + closedChild->_control.ConnectionStateChanged(closedChild->_connectionStateChangedToken); // Reset our UI: _root.Children().Clear(); @@ -720,13 +740,13 @@ void Pane::_CloseChild(const bool closeFirst) // - void Pane::_SetupChildCloseHandlers() { - _firstClosedToken = _firstChild->Closed([this]() { + _firstClosedToken = _firstChild->Closed([this](auto&& /*s*/, auto&& /*e*/) { _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=]() { _CloseChild(true); }); }); - _secondClosedToken = _secondChild->Closed([this]() { + _secondClosedToken = _secondChild->Closed([this](auto&& /*s*/, auto&& /*e*/) { _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=]() { _CloseChild(false); }); @@ -965,8 +985,8 @@ std::pair, std::shared_ptr> Pane::_Split(SplitState std::unique_lock lock{ _createCloseLock }; // revoke our handler - the child will take care of the control now. - _control.ConnectionClosed(_connectionClosedToken); - _connectionClosedToken.value = 0; + _control.ConnectionStateChanged(_connectionStateChangedToken); + _connectionStateChangedToken.value = 0; // Remove our old GotFocus handler from the control. We don't what the // control telling us that it's now focused, we want it telling its new @@ -1124,5 +1144,4 @@ void Pane::_SetupResources() } } -DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs); DEFINE_EVENT(Pane, GotFocus, _GotFocusHandlers, winrt::delegate>); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 233216e27f1..3eec4df00ac 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -71,7 +71,7 @@ class Pane : public std::enable_shared_from_this void Close(); - DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); + WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler); DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate>); private: @@ -89,7 +89,7 @@ class Pane : public std::enable_shared_from_this bool _lastActive{ false }; std::optional _profile{ std::nullopt }; - winrt::event_token _connectionClosedToken{ 0 }; + winrt::event_token _connectionStateChangedToken{ 0 }; winrt::event_token _firstClosedToken{ 0 }; winrt::event_token _secondClosedToken{ 0 }; @@ -119,7 +119,7 @@ class Pane : public std::enable_shared_from_this void _CloseChild(const bool closeFirst); void _FocusFirstChild(); - void _ControlClosedHandler(); + void _ControlConnectionStateChangedHandler(const winrt::Microsoft::Terminal::TerminalControl::TermControl& sender, const winrt::Windows::Foundation::IInspectable& /*args*/); std::pair _GetPaneSizes(const float& fullSize); diff --git a/src/cascadia/TerminalApp/Profile.cpp b/src/cascadia/TerminalApp/Profile.cpp index 1aee85c9183..d689bc6c7be 100644 --- a/src/cascadia/TerminalApp/Profile.cpp +++ b/src/cascadia/TerminalApp/Profile.cpp @@ -50,6 +50,11 @@ static constexpr std::string_view BackgroundImageOpacityKey{ "backgroundImageOpa static constexpr std::string_view BackgroundImageStretchModeKey{ "backgroundImageStretchMode" }; static constexpr std::string_view BackgroundImageAlignmentKey{ "backgroundImageAlignment" }; +// Possible values for closeOnExit +static constexpr std::string_view CloseOnExitAlways{ "always" }; +static constexpr std::string_view CloseOnExitGraceful{ "graceful" }; +static constexpr std::string_view CloseOnExitNever{ "never" }; + // Possible values for Scrollbar state static constexpr std::wstring_view AlwaysVisible{ L"visible" }; static constexpr std::wstring_view AlwaysHide{ L"hidden" }; @@ -109,7 +114,7 @@ Profile::Profile(const std::optional& guid) : _acrylicTransparency{ 0.5 }, _useAcrylic{ false }, _scrollbarState{}, - _closeOnExit{ true }, + _closeOnExitMode{ CloseOnExitMode::Graceful }, _padding{ DEFAULT_PADDING }, _icon{}, _backgroundImage{}, @@ -170,7 +175,6 @@ TerminalSettings Profile::CreateTerminalSettings(const std::unordered_map ParseImageAlignment(const std::string_view imageAlignment); static std::tuple _ConvertJsonToAlignment(const Json::Value& json); + static CloseOnExitMode ParseCloseOnExitMode(const Json::Value& json); + static std::string_view _SerializeCloseOnExitMode(const CloseOnExitMode closeOnExitMode); + static std::string_view SerializeImageAlignment(const std::tuple imageAlignment); static winrt::Microsoft::Terminal::Settings::CursorStyle _ParseCursorShape(const std::wstring& cursorShapeString); static std::wstring_view _SerializeCursorStyle(const winrt::Microsoft::Terminal::Settings::CursorStyle cursorShape); @@ -144,7 +154,7 @@ class TerminalApp::Profile final std::optional> _backgroundImageAlignment; std::optional _scrollbarState; - bool _closeOnExit; + CloseOnExitMode _closeOnExitMode; std::wstring _padding; std::optional _icon; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 93ce7a6a50f..6969c1c4fed 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -19,8 +19,8 @@ Tab::Tab(const GUID& profile, const TermControl& control) { _rootPane = std::make_shared(profile, control, true); - _rootPane->Closed([=]() { - _closedHandlers(); + _rootPane->Closed([=](auto&& /*s*/, auto&& /*e*/) { + _ClosedHandlers(nullptr, nullptr); }); _activePane = _rootPane; @@ -335,5 +335,4 @@ void Tab::_AttachEventHandlersToPane(std::shared_ptr pane) }); } -DEFINE_EVENT(Tab, Closed, _closedHandlers, ConnectionClosedEventArgs); DEFINE_EVENT(Tab, ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index fffb13926b8..336372a1d5f 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -35,7 +35,7 @@ class Tab void ClosePane(); - DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); + WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler); DECLARE_EVENT(ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>); private: diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 6e15d7d8d7c..ac594a6ec6c 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -464,7 +464,7 @@ namespace winrt::TerminalApp::implementation tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick }); // When the tab is closed, remove it from our list of tabs. - newTab->Closed([tabViewItem, this]() { + newTab->Closed([tabViewItem, this](auto&& /*s*/, auto&& /*e*/) { _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tabViewItem, this]() { _RemoveTabViewItem(tabViewItem); }); diff --git a/src/cascadia/TerminalApp/defaults.json b/src/cascadia/TerminalApp/defaults.json index d1ca9f8776f..1383df12779 100644 --- a/src/cascadia/TerminalApp/defaults.json +++ b/src/cascadia/TerminalApp/defaults.json @@ -17,7 +17,7 @@ "commandline": "powershell.exe", "hidden": false, "startingDirectory": "%USERPROFILE%", - "closeOnExit": true, + "closeOnExit": "graceful", "colorScheme": "Campbell Powershell", "cursorColor": "#FFFFFF", "cursorShape": "bar", @@ -35,7 +35,7 @@ "commandline": "cmd.exe", "hidden": false, "startingDirectory": "%USERPROFILE%", - "closeOnExit": true, + "closeOnExit": "graceful", "colorScheme": "Campbell", "cursorColor": "#FFFFFF", "cursorShape": "bar", diff --git a/src/cascadia/TerminalConnection/AzureConnection.cpp b/src/cascadia/TerminalConnection/AzureConnection.cpp index 2acda84576b..99b4c686c9b 100644 --- a/src/cascadia/TerminalConnection/AzureConnection.cpp +++ b/src/cascadia/TerminalConnection/AzureConnection.cpp @@ -49,51 +49,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation // - str: the string to write. void AzureConnection::_WriteStringWithNewline(const winrt::hstring& str) { - _outputHandlers(str + L"\r\n"); - } - - // Method description: - // - ascribes to the ITerminalConnection interface - // - registers an output event handler - // Arguments: - // - the handler - // Return value: - // - the event token for the handler - winrt::event_token AzureConnection::TerminalOutput(Microsoft::Terminal::TerminalConnection::TerminalOutputEventArgs const& handler) - { - return _outputHandlers.add(handler); - } - - // Method description: - // - ascribes to the ITerminalConnection interface - // - revokes an output event handler - // Arguments: - // - the event token for the handler - void AzureConnection::TerminalOutput(winrt::event_token const& token) noexcept - { - _outputHandlers.remove(token); - } - - // Method description: - // - ascribes to the ITerminalConnection interface - // - registers a terminal-disconnected event handler - // Arguments: - // - the handler - // Return value: - // - the event token for the handler - winrt::event_token AzureConnection::TerminalDisconnected(Microsoft::Terminal::TerminalConnection::TerminalDisconnectedEventArgs const& handler) - { - return _disconnectHandlers.add(handler); - } - - // Method description: - // - ascribes to the ITerminalConnection interface - // - revokes a terminal-disconnected event handler - // Arguments: - // - the event token for the handler - void AzureConnection::TerminalDisconnected(winrt::event_token const& token) noexcept - { - _disconnectHandlers.remove(token); + _TerminalOutputHandlers(str + L"\r\n"); } // Method description: @@ -112,7 +68,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation THROW_LAST_ERROR_IF_NULL(_hOutputThread); - _connected = true; + _transitionToState(ConnectionState::Connecting); } // Method description: @@ -122,7 +78,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation // the user's input void AzureConnection::WriteInput(hstring const& data) { - if (!_connected || _closing.load()) + // We read input while connected AND connecting. + if (!_isStateOneOf(ConnectionState::Connected, ConnectionState::Connecting)) { return; } @@ -131,7 +88,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation switch (_state) { // The user has stored connection settings, let them choose one of them, create a new one or remove all stored ones - case State::AccessStored: + case AzureState::AccessStored: { const auto s = winrt::to_string(data); int storeNum = -1; @@ -172,7 +129,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation return; } // The user has multiple tenants in their Azure account, let them choose one of them - case State::TenantChoice: + case AzureState::TenantChoice: { int tenantNum = -1; try @@ -195,7 +152,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation return; } // User has the option to save their connection settings for future logins - case State::StoreTokens: + case AzureState::StoreTokens: { std::lock_guard lg{ _commonMutex }; if (data == RS_(L"AzureUserEntry_Yes")) @@ -218,7 +175,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation return; } // We are connected, send user's input over the websocket - case State::TermConnected: + case AzureState::TermConnected: { websocket_outgoing_message msg; const auto str = winrt::to_string(data); @@ -238,12 +195,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation // - the new rows/cols values void AzureConnection::Resize(uint32_t rows, uint32_t columns) { - if (!_connected || !(_state == State::TermConnected)) + if (!_isConnected()) { _initialRows = rows; _initialCols = columns; } - else if (!_closing.load()) + else // We only transition to Connected when we've established the websocket. { // Initialize client http_client terminalClient(_cloudShellUri); @@ -264,24 +221,24 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation // - closes the websocket connection and the output thread void AzureConnection::Close() { - if (!_connected) - { - return; - } - - if (!_closing.exchange(true)) + if (_transitionToState(ConnectionState::Closing)) { _canProceed.notify_all(); - if (_state == State::TermConnected) + if (_state == AzureState::TermConnected) { // Close the websocket connection auto closedTask = _cloudShellSocket.close(); closedTask.wait(); } - // Tear down our output thread - WaitForSingleObject(_hOutputThread.get(), INFINITE); - _hOutputThread.reset(); + if (_hOutputThread) + { + // Tear down our output thread + WaitForSingleObject(_hOutputThread.get(), INFINITE); + _hOutputThread.reset(); + } + + _transitionToState(ConnectionState::Closed); } } @@ -320,45 +277,52 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { while (true) { + if (_isStateAtOrBeyond(ConnectionState::Closing)) + { + // If we enter a new state while closing, just bail. + return S_FALSE; + } + try { switch (_state) { // Initial state, check if the user has any stored connection settings and allow them to login with those // or allow them to login with a different account or allow them to remove the saved settings - case State::AccessStored: + case AzureState::AccessStored: { RETURN_IF_FAILED(_AccessHelper()); break; } // User has no saved connection settings or has opted to login with a different account // Azure authentication happens here - case State::DeviceFlow: + case AzureState::DeviceFlow: { RETURN_IF_FAILED(_DeviceFlowHelper()); break; } // User has multiple tenants in their Azure account, they need to choose which one to connect to - case State::TenantChoice: + case AzureState::TenantChoice: { RETURN_IF_FAILED(_TenantChoiceHelper()); break; } // Ask the user if they want to save these connection settings for future logins - case State::StoreTokens: + case AzureState::StoreTokens: { RETURN_IF_FAILED(_StoreHelper()); break; } // Connect to Azure, we only get here once we have everything we need (tenantID, accessToken, refreshToken) - case State::TermConnecting: + case AzureState::TermConnecting: { RETURN_IF_FAILED(_ConnectHelper()); break; } // We are connected, continuously read from the websocket until its closed - case State::TermConnected: + case AzureState::TermConnected: { + _transitionToState(ConnectionState::Connected); while (true) { // Read from websocket @@ -370,15 +334,15 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation } catch (...) { - // Websocket has been closed - if (!_closing.load()) + // Websocket has been closed; consider it a graceful exit? + // This should result in our termination. + if (_transitionToState(ConnectionState::Closed)) { - _state = State::NoConnect; - _disconnectHandlers(); + // End the output thread. return S_FALSE; } - break; } + auto msg = msgT.get(); auto msgStringTask = msg.extract_string(); auto msgString = msgStringTask.get(); @@ -387,21 +351,21 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation const auto hstr = winrt::to_hstring(msgString); // Pass the output to our registered event handlers - _outputHandlers(hstr); + _TerminalOutputHandlers(hstr); } return S_OK; } - case State::NoConnect: + case AzureState::NoConnect: { _WriteStringWithNewline(RS_(L"AzureInternetOrServerIssue")); - _disconnectHandlers(); + _transitionToState(ConnectionState::Failed); return E_FAIL; } } } catch (...) { - _state = State::NoConnect; + _state = AzureState::NoConnect; } } } @@ -425,7 +389,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation catch (...) { // No credentials are stored, so start the device flow - _state = State::DeviceFlow; + _state = AzureState::DeviceFlow; return S_FALSE; } _maxStored = 0; @@ -458,7 +422,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation _WriteStringWithNewline(RS_(L"AzureOldCredentialsFlushedMessage")); } // No valid up-to-date credentials were found, so start the device flow - _state = State::DeviceFlow; + _state = AzureState::DeviceFlow; return S_FALSE; } @@ -468,10 +432,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation std::unique_lock storedLock{ _commonMutex }; _canProceed.wait(storedLock, [=]() { - return (_storedNumber >= 0 && _storedNumber < _maxStored) || _removeOrNew.has_value() || _closing.load(); + return (_storedNumber >= 0 && _storedNumber < _maxStored) || _removeOrNew.has_value() || _isStateAtOrBeyond(ConnectionState::Closing); }); // User might have closed the tab while we waited for input - if (_closing.load()) + if (_isStateAtOrBeyond(ConnectionState::Closing)) { return E_FAIL; } @@ -479,13 +443,13 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { // User wants to remove the stored settings _RemoveCredentials(); - _state = State::DeviceFlow; + _state = AzureState::DeviceFlow; return S_OK; } else if (_removeOrNew.has_value() && !_removeOrNew.value()) { // User wants to login with a different account - _state = State::DeviceFlow; + _state = AzureState::DeviceFlow; return S_OK; } // User wants to login with one of the saved connection settings @@ -514,7 +478,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation } // We have everything we need, so go ahead and connect - _state = State::TermConnecting; + _state = AzureState::TermConnecting; return S_OK; } @@ -558,6 +522,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation if (tenantListAsArray.size() == 0) { _WriteStringWithNewline(RS_(L"AzureNoTenants")); + _transitionToState(ConnectionState::Failed); return E_FAIL; } else if (_tenantList.size() == 1) @@ -571,11 +536,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation _refreshToken = refreshResponse.at(L"refresh_token").as_string(); _expiry = std::stoi(refreshResponse.at(L"expires_on").as_string()); - _state = State::StoreTokens; + _state = AzureState::StoreTokens; } else { - _state = State::TenantChoice; + _state = AzureState::TenantChoice; } return S_OK; } @@ -602,10 +567,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation // Use a lock to wait for the user to input a valid number std::unique_lock tenantNumberLock{ _commonMutex }; _canProceed.wait(tenantNumberLock, [=]() { - return (_tenantNumber >= 0 && _tenantNumber < _maxSize) || _closing.load(); + return (_tenantNumber >= 0 && _tenantNumber < _maxSize) || _isStateAtOrBeyond(ConnectionState::Closing); }); // User might have closed the tab while we waited for input - if (_closing.load()) + if (_isStateAtOrBeyond(ConnectionState::Closing)) { return E_FAIL; } @@ -619,7 +584,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation _refreshToken = refreshResponse.at(L"refresh_token").as_string(); _expiry = std::stoi(refreshResponse.at(L"expires_on").as_string()); - _state = State::StoreTokens; + _state = AzureState::StoreTokens; return S_OK; } CATCH_RETURN(); @@ -636,10 +601,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation // Wait for user input std::unique_lock storeLock{ _commonMutex }; _canProceed.wait(storeLock, [=]() { - return _store.has_value() || _closing.load(); + return _store.has_value() || _isStateAtOrBeyond(ConnectionState::Closing); }); // User might have closed the tab while we waited for input - if (_closing.load()) + if (_isStateAtOrBeyond(ConnectionState::Closing)) { return E_FAIL; } @@ -651,7 +616,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation _WriteStringWithNewline(RS_(L"AzureTokensStored")); } - _state = State::TermConnecting; + _state = AzureState::TermConnecting; return S_OK; } @@ -667,6 +632,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation if (settingsResponse.has_field(L"error")) { _WriteStringWithNewline(RS_(L"AzureNoCloudAccount")); + _transitionToState(ConnectionState::Failed); return E_FAIL; } @@ -683,13 +649,13 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation const auto shellType = L"bash"; _WriteStringWithNewline(RS_(L"AzureRequestingTerminal")); const auto socketUri = _GetTerminal(shellType); - _outputHandlers(L"\r\n"); + _TerminalOutputHandlers(L"\r\n"); // Step 8: connecting to said terminal const auto connReqTask = _cloudShellSocket.connect(socketUri); connReqTask.wait(); - _state = State::TermConnected; + _state = AzureState::TermConnected; return S_OK; } @@ -759,7 +725,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation for (int count = 0; count < expiresIn / pollInterval; count++) { // User might close the tab while we wait for them to authenticate, this case handles that - if (_closing.load()) + if (_isStateAtOrBeyond(ConnectionState::Closing)) { throw "Tab closed."; } diff --git a/src/cascadia/TerminalConnection/AzureConnection.h b/src/cascadia/TerminalConnection/AzureConnection.h index 9467357fb42..f2a590c87cf 100644 --- a/src/cascadia/TerminalConnection/AzureConnection.h +++ b/src/cascadia/TerminalConnection/AzureConnection.h @@ -11,26 +11,24 @@ #include #include +#include "../cascadia/inc/cppwinrt_utils.h" +#include "ConnectionStateHolder.h" + namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { - struct AzureConnection : AzureConnectionT + struct AzureConnection : AzureConnectionT, ConnectionStateHolder { static bool IsAzureConnectionAvailable(); AzureConnection(const uint32_t rows, const uint32_t cols); - winrt::event_token TerminalOutput(TerminalConnection::TerminalOutputEventArgs const& handler); - void TerminalOutput(winrt::event_token const& token) noexcept; - winrt::event_token TerminalDisconnected(TerminalConnection::TerminalDisconnectedEventArgs const& handler); - void TerminalDisconnected(winrt::event_token const& token) noexcept; void Start(); void WriteInput(hstring const& data); void Resize(uint32_t rows, uint32_t columns); void Close(); - private: - winrt::event _outputHandlers; - winrt::event _disconnectHandlers; + WINRT_CALLBACK(TerminalOutput, TerminalOutputHandler); + private: uint32_t _initialRows{}; uint32_t _initialCols{}; int _storedNumber{ -1 }; @@ -40,7 +38,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation std::condition_variable _canProceed; std::mutex _commonMutex; - enum class State + enum class AzureState { AccessStored, DeviceFlow, @@ -51,14 +49,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation NoConnect }; - State _state{ State::AccessStored }; + AzureState _state{ AzureState::AccessStored }; std::optional _store; std::optional _removeOrNew; - bool _connected{}; - std::atomic _closing{ false }; - wil::unique_handle _hOutputThread; static DWORD WINAPI StaticOutputThreadProc(LPVOID lpParameter); diff --git a/src/cascadia/TerminalConnection/ConnectionStateHolder.h b/src/cascadia/TerminalConnection/ConnectionStateHolder.h new file mode 100644 index 00000000000..b38e0f13187 --- /dev/null +++ b/src/cascadia/TerminalConnection/ConnectionStateHolder.h @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "../inc/cppwinrt_utils.h" + +namespace winrt::Microsoft::Terminal::TerminalConnection::implementation +{ + template + struct ConnectionStateHolder + { + public: + ConnectionState State() const noexcept { return _connectionState; } + TYPED_EVENT(StateChanged, ITerminalConnection, winrt::Windows::Foundation::IInspectable); + + protected: + // Method Description: + // - Attempt to transition to and signal the specified connection state. + // The transition will only be effected if the state is "beyond" the current state. + // Arguments: + // - state: the new state + // Return Value: + // Whether we've successfully transitioned to the new state. + bool _transitionToState(const ConnectionState state) noexcept + { + { + std::lock_guard stateLock{ _stateMutex }; + // only allow movement up the state gradient + if (state < _connectionState) + { + return false; + } + _connectionState = state; + } + // Dispatch the event outside of lock. + _StateChangedHandlers(*static_cast(this), nullptr); + return true; + } + + // Method Description: + // - Returns whether the state is one of the N specified states. + // Arguments: + // - "...": the states + // Return Value: + // Whether we're in one of the states. + template + bool _isStateOneOf(Args&&... args) const noexcept + { + // Dark magic! This function uses C++17 fold expressions. These fold expressions roughly expand as follows: + // (... OP (expression_using_args)) + // into -> + // expression_using_args[0] OP expression_using_args[1] OP expression_using_args[2] OP (etc.) + // We use that to first check that All Args types are ConnectionState (type[0] == ConnectionState && type[1] == ConnectionState && etc.) + // and then later to check that the current state is one of the passed-in ones: + // (_state == args[0] || _state == args[1] || etc.) + static_assert((... && std::is_same::value), "all queried connection states must be from the ConnectionState enum"); + std::lock_guard stateLock{ _stateMutex }; + return (... || (_connectionState == args)); + } + + // Method Description: + // - Returns whether the state has reached or surpassed the specified state. + // Arguments: + // - state; the state to check against + // Return Value: + // Whether we're at or beyond the specified state + bool _isStateAtOrBeyond(const ConnectionState state) const noexcept + { + std::lock_guard stateLock{ _stateMutex }; + return _connectionState >= state; + } + + // Method Description: + // - (Convenience:) Returns whether we're "connected". + // Return Value: + // Whether we're "connected" + bool _isConnected() const noexcept + { + return _isStateOneOf(ConnectionState::Connected); + } + + private: + std::atomic _connectionState{ ConnectionState::NotConnected }; + mutable std::mutex _stateMutex; + }; +} diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index b1ffd4d7b81..06319381b67 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -11,9 +11,25 @@ #include "../../types/inc/Utils.hpp" #include "../../types/inc/Environment.hpp" #include "../../types/inc/UTF8OutPipeReader.hpp" +#include "LibraryResources.h" using namespace ::Microsoft::Console; +// Notes: +// There is a number of ways that the Conpty connection can be terminated (voluntarily or not): +// 1. The connection is Close()d +// 2. The pseudoconsole or process cannot be spawned during Start() +// 3. The client process exits with a code. +// (Successful (0) or any other code) +// 4. The read handle is terminated. +// (This usually happens when the pseudoconsole host crashes.) +// In each of these termination scenarios, we need to be mindful of tripping the others. +// Closing the pseudoconsole in response to the client exiting (3) can trigger (4). +// Close() (1) will cause the automatic triggering of (3) and (4). +// In a lot of cases, we use the connection state to stop "flapping." +// +// To figure out where we handle these, search for comments containing "EXIT POINT" + namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { // Function Description: @@ -150,26 +166,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation return _guid; } - winrt::event_token ConptyConnection::TerminalOutput(Microsoft::Terminal::TerminalConnection::TerminalOutputEventArgs const& handler) - { - return _outputHandlers.add(handler); - } - - void ConptyConnection::TerminalOutput(winrt::event_token const& token) noexcept - { - _outputHandlers.remove(token); - } - - winrt::event_token ConptyConnection::TerminalDisconnected(Microsoft::Terminal::TerminalConnection::TerminalDisconnectedEventArgs const& handler) - { - return _disconnectHandlers.add(handler); - } - - void ConptyConnection::TerminalDisconnected(winrt::event_token const& token) noexcept - { - _disconnectHandlers.remove(token); - } - void ConptyConnection::Start() try { @@ -205,32 +201,70 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation SetThreadpoolWait(_clientExitWait.get(), _piClient.hProcess, nullptr); - _connected = true; + _transitionToState(ConnectionState::Connected); } catch (...) { + // EXIT POINT const auto hr = wil::ResultFromCaughtException(); - // TODO GH#2563 - signal a transition into failed state here! - LOG_HR(hr); - _disconnectHandlers(); + winrt::hstring failureText{ wil::str_printf(RS_(L"ProcessFailedToLaunch").c_str(), static_cast(hr), _commandline.c_str()) }; + _TerminalOutputHandlers(failureText); + _transitionToState(ConnectionState::Failed); + + // Tear down any state we may have accumulated. + _hPC.reset(); } + // Method Description: + // - prints out the "process exited" message formatted with the exit code + // Arguments: + // - status: the exit code. + void ConptyConnection::_indicateExitWithStatus(unsigned int status) noexcept + { + try + { + winrt::hstring exitText{ wil::str_printf(RS_(L"ProcessExited").c_str(), (unsigned int)status) }; + _TerminalOutputHandlers(L"\r\n"); + _TerminalOutputHandlers(exitText); + } + CATCH_LOG(); + } + + // Method Description: + // - called when the client application (not necessarily its pty) exits for any reason void ConptyConnection::_ClientTerminated() noexcept { - if (_closing.load()) + if (_isStateAtOrBeyond(ConnectionState::Closing)) { - // This is okay, break out to kill the thread + // This termination was expected. return; } - // TODO GH#2563 - get the exit code from the process and see whether it was a failing one. - _disconnectHandlers(); + // EXIT POINT + DWORD exitCode{ 0 }; + GetExitCodeProcess(_piClient.hProcess, &exitCode); + + // Signal the closing or failure of the process. + // Load bearing. Terminating the pseudoconsole will make the output thread exit unexpectedly, + // so we need to signal entry into the correct closing state before we do that. + _transitionToState(exitCode == 0 ? ConnectionState::Closed : ConnectionState::Failed); + + // Close the pseudoconsole and wait for all output to drain. + _hPC.reset(); + if (auto localOutputThreadHandle = std::move(_hOutputThread)) + { + LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(localOutputThreadHandle.get(), INFINITE)); + } + + _indicateExitWithStatus(exitCode); + + _piClient.reset(); } void ConptyConnection::WriteInput(hstring const& data) { - if (!_connected || _closing.load()) + if (!_isConnected()) { return; } @@ -248,7 +282,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation _initialRows = rows; _initialCols = columns; } - else if (!_closing.load()) + else if (_isConnected()) { THROW_IF_FAILED(ConptyResizePseudoConsole(_hPC.get(), { Utils::ClampToShortMax(columns, 1), Utils::ClampToShortMax(rows, 1) })); } @@ -256,28 +290,32 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation void ConptyConnection::Close() { - if (!_connected) - { - return; - } - - if (!_closing.exchange(true)) + if (_transitionToState(ConnectionState::Closing)) { + // EXIT POINT _clientExitWait.reset(); // immediately stop waiting for the client to exit. - _hPC.reset(); + _hPC.reset(); // tear down the pseudoconsole (this is like clicking X on a console window) - _inPipe.reset(); + _inPipe.reset(); // break the pipes _outPipe.reset(); - // Tear down our output thread -- now that the output pipe was closed on the - // far side, we can run down our local reader. - LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(_hOutputThread.get(), INFINITE)); - _hOutputThread.reset(); + if (_hOutputThread) + { + // Tear down our output thread -- now that the output pipe was closed on the + // far side, we can run down our local reader. + LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(_hOutputThread.get(), INFINITE)); + _hOutputThread.reset(); + } + + if (_piClient.hProcess) + { + // Wait for the client to terminate (which it should do successfully) + LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(_piClient.hProcess, INFINITE)); + _piClient.reset(); + } - // Wait for the client to terminate. - LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(_piClient.hProcess, INFINITE)); - _piClient.reset(); + _transitionToState(ConnectionState::Closed); } } @@ -292,13 +330,15 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation HRESULT result = pipeReader.Read(strView); if (FAILED(result) || result == S_FALSE) { - if (_closing.load()) + if (_isStateAtOrBeyond(ConnectionState::Closing)) { - // This is okay, break out to kill the thread + // This termination was expected. return 0; } - _disconnectHandlers(); + // EXIT POINT + _indicateExitWithStatus(result); // print a message + _transitionToState(ConnectionState::Failed); return (DWORD)-1; } @@ -326,7 +366,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation auto hstr{ winrt::to_hstring(strView) }; // Pass the output to our registered event handlers - _outputHandlers(hstr); + _TerminalOutputHandlers(hstr); } return 0; diff --git a/src/cascadia/TerminalConnection/ConptyConnection.h b/src/cascadia/TerminalConnection/ConptyConnection.h index 7a0d5592758..165f2b4819e 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.h +++ b/src/cascadia/TerminalConnection/ConptyConnection.h @@ -1,9 +1,12 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT license. #pragma once #include "ConptyConnection.g.h" +#include "ConnectionStateHolder.h" +#include "../inc/cppwinrt_utils.h" + #include namespace wil @@ -14,14 +17,10 @@ namespace wil namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { - struct ConptyConnection : ConptyConnectionT + struct ConptyConnection : ConptyConnectionT, ConnectionStateHolder { ConptyConnection(const hstring& cmdline, const hstring& startingDirectory, const hstring& startingTitle, const uint32_t rows, const uint32_t cols, const guid& guid); - winrt::event_token TerminalOutput(TerminalConnection::TerminalOutputEventArgs const& handler); - void TerminalOutput(winrt::event_token const& token) noexcept; - winrt::event_token TerminalDisconnected(TerminalConnection::TerminalDisconnectedEventArgs const& handler); - void TerminalDisconnected(winrt::event_token const& token) noexcept; void Start(); void WriteInput(hstring const& data); void Resize(uint32_t rows, uint32_t columns); @@ -29,13 +28,13 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation winrt::guid Guid() const noexcept; + WINRT_CALLBACK(TerminalOutput, TerminalOutputHandler); + private: HRESULT _LaunchAttachedClient() noexcept; + void _indicateExitWithStatus(unsigned int status) noexcept; void _ClientTerminated() noexcept; - winrt::event _outputHandlers; - winrt::event _disconnectHandlers; - uint32_t _initialRows{}; uint32_t _initialCols{}; hstring _commandline; @@ -43,8 +42,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation hstring _startingTitle; guid _guid{}; // A unique session identifier for connected client - bool _connected{}; - std::atomic _closing{ false }; bool _recievedFirstByte{ false }; std::chrono::high_resolution_clock::time_point _startTime{}; diff --git a/src/cascadia/TerminalConnection/EchoConnection.cpp b/src/cascadia/TerminalConnection/EchoConnection.cpp index 1df63ad8187..1b1803c8581 100644 --- a/src/cascadia/TerminalConnection/EchoConnection.cpp +++ b/src/cascadia/TerminalConnection/EchoConnection.cpp @@ -13,27 +13,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { } - winrt::event_token EchoConnection::TerminalOutput(TerminalConnection::TerminalOutputEventArgs const& handler) - { - return _outputHandlers.add(handler); - } - - void EchoConnection::TerminalOutput(winrt::event_token const& token) noexcept - { - _outputHandlers.remove(token); - } - - winrt::event_token EchoConnection::TerminalDisconnected(TerminalConnection::TerminalDisconnectedEventArgs const& handler) - { - handler; - throw hresult_not_implemented(); - } - - void EchoConnection::TerminalDisconnected(winrt::event_token const& token) noexcept - { - token; - } - void EchoConnection::Start() { } @@ -56,7 +35,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation prettyPrint << wch; } } - _outputHandlers(prettyPrint.str()); + _TerminalOutputHandlers(prettyPrint.str()); } void EchoConnection::Resize(uint32_t rows, uint32_t columns) diff --git a/src/cascadia/TerminalConnection/EchoConnection.h b/src/cascadia/TerminalConnection/EchoConnection.h index 0f60a56331a..0a985224cf8 100644 --- a/src/cascadia/TerminalConnection/EchoConnection.h +++ b/src/cascadia/TerminalConnection/EchoConnection.h @@ -5,23 +5,23 @@ #include "EchoConnection.g.h" +#include "../cascadia/inc/cppwinrt_utils.h" + namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { struct EchoConnection : EchoConnectionT { EchoConnection(); - winrt::event_token TerminalOutput(TerminalConnection::TerminalOutputEventArgs const& handler); - void TerminalOutput(winrt::event_token const& token) noexcept; - winrt::event_token TerminalDisconnected(TerminalConnection::TerminalDisconnectedEventArgs const& handler); - void TerminalDisconnected(winrt::event_token const& token) noexcept; void Start(); void WriteInput(hstring const& data); void Resize(uint32_t rows, uint32_t columns); void Close(); - private: - winrt::event _outputHandlers; + ConnectionState State() const noexcept { return ConnectionState::Connected; } + + WINRT_CALLBACK(TerminalOutput, TerminalOutputHandler); + TYPED_EVENT(StateChanged, ITerminalConnection, IInspectable); }; } diff --git a/src/cascadia/TerminalConnection/ITerminalConnection.idl b/src/cascadia/TerminalConnection/ITerminalConnection.idl index 679623629fe..398724cd882 100644 --- a/src/cascadia/TerminalConnection/ITerminalConnection.idl +++ b/src/cascadia/TerminalConnection/ITerminalConnection.idl @@ -3,18 +3,28 @@ namespace Microsoft.Terminal.TerminalConnection { - delegate void TerminalOutputEventArgs(String output); - delegate void TerminalDisconnectedEventArgs(); + enum ConnectionState + { + NotConnected = 0, + Connecting, + Connected, + Closing, + Closed, + Failed + }; + + delegate void TerminalOutputHandler(String output); interface ITerminalConnection { - event TerminalOutputEventArgs TerminalOutput; - event TerminalDisconnectedEventArgs TerminalDisconnected; - void Start(); void WriteInput(String data); void Resize(UInt32 rows, UInt32 columns); void Close(); - }; + event TerminalOutputHandler TerminalOutput; + + event Windows.Foundation.TypedEventHandler StateChanged; + ConnectionState State { get; }; + }; } diff --git a/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw b/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw index 2a4ae126e64..43a0242a418 100644 --- a/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw @@ -205,4 +205,13 @@ n The user must enter this character to signify NO + + [process exited with code %u] + The first argument (%u) is the (positive) error code of the process. 0 is success. + + + [error 0x%8.08x when launching `%ls'] + The first argument is the hexadecimal error code. The second is the process name. +If this resource spans multiple lines, it will not be displayed properly. Yeah. + \ No newline at end of file diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 2d47dec277c..956c3367b81 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -156,8 +156,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // DON'T CALL _InitializeTerminal here - wait until the swap chain is loaded to do that. // Subscribe to the connection's disconnected event and call our connection closed handlers. - _connection.TerminalDisconnected([=]() { - _connectionClosedHandlers(); + _connectionStateChangedRevoker = _connection.StateChanged(winrt::auto_revoke, [weakThis = get_weak()](auto&& /*s*/, auto&& /*v*/) { + if (auto strongThis{ weakThis.get() }) + { + strongThis->_ConnectionStateChangedHandlers(*strongThis, nullptr); + } }); } @@ -421,6 +424,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation return _swapChainPanel.Margin(); } + TerminalConnection::ConnectionState TermControl::ConnectionState() const + { + return _connection.State(); + } + void TermControl::SwapChainChanged() { if (!_initializedTerminal) @@ -1545,8 +1553,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { if (!_closing.exchange(true)) { - // Stop accepting new output before we disconnect everything. + // Stop accepting new output and state changes before we disconnect everything. _connection.TerminalOutput(_connectionOutputEventToken); + _connectionStateChangedRevoker.revoke(); // Clear out the cursor timer, so it doesn't trigger again on us once we're destructed. if (auto localCursorTimer{ std::exchange(_cursorTimer, std::nullopt) }) @@ -1839,17 +1848,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation return flags; } - // Method Description: - // - Returns true if this control should close when its connection is closed. - // Arguments: - // - - // Return Value: - // - true iff the control should close when the connection is closed. - bool TermControl::ShouldCloseOnExit() const noexcept - { - return _settings.CloseOnExit(); - } - // Method Description: // - Gets the corresponding viewport terminal position for the cursor // by excluding the padding and normalizing with the font size. @@ -1961,7 +1959,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Winrt events need a method for adding a callback to the event and removing the callback. // These macros will define them both for you. DEFINE_EVENT(TermControl, TitleChanged, _titleChangedHandlers, TerminalControl::TitleChangedEventArgs); - DEFINE_EVENT(TermControl, ConnectionClosed, _connectionClosedHandlers, TerminalControl::ConnectionClosedEventArgs); DEFINE_EVENT(TermControl, ScrollPositionChanged, _scrollPositionChangedHandlers, TerminalControl::ScrollPositionChangedEventArgs); DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TermControl, PasteFromClipboard, _clipboardPasteHandlers, TerminalControl::TermControl, TerminalControl::PasteFromClipboardEventArgs); diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 614ad6ab7b4..8a735dd6db6 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -62,7 +62,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation bool CopySelectionToClipboard(bool trimTrailingWhitespace); void PasteTextFromClipboard(); void Close(); - bool ShouldCloseOnExit() const noexcept; Windows::Foundation::Size CharacterDimensions() const; Windows::Foundation::Size MinimumSize() const; @@ -81,16 +80,19 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const FontInfo GetActualFont() const; const Windows::UI::Xaml::Thickness GetPadding() const; + TerminalConnection::ConnectionState ConnectionState() const; + static Windows::Foundation::Point GetProposedDimensions(Microsoft::Terminal::Settings::IControlSettings const& settings, const uint32_t dpi); // clang-format off // -------------------------------- WinRT Events --------------------------------- DECLARE_EVENT(TitleChanged, _titleChangedHandlers, TerminalControl::TitleChangedEventArgs); - DECLARE_EVENT(ConnectionClosed, _connectionClosedHandlers, TerminalControl::ConnectionClosedEventArgs); DECLARE_EVENT(ScrollPositionChanged, _scrollPositionChangedHandlers, TerminalControl::ScrollPositionChangedEventArgs); DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(PasteFromClipboard, _clipboardPasteHandlers, TerminalControl::TermControl, TerminalControl::PasteFromClipboardEventArgs); DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(CopyToClipboard, _clipboardCopyHandlers, TerminalControl::TermControl, TerminalControl::CopyToClipboardEventArgs); + + TYPED_EVENT(ConnectionStateChanged, TerminalControl::TermControl, IInspectable); // clang-format on private: @@ -104,6 +106,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation TSFInputControl _tsfInputControl; event_token _connectionOutputEventToken; + TerminalConnection::ITerminalConnection::StateChanged_revoker _connectionStateChangedRevoker; std::unique_ptr<::Microsoft::Terminal::Core::Terminal> _terminal; diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl index 2fc56ed7e78..5eedc9a5458 100644 --- a/src/cascadia/TerminalControl/TermControl.idl +++ b/src/cascadia/TerminalControl/TermControl.idl @@ -4,7 +4,6 @@ namespace Microsoft.Terminal.TerminalControl { delegate void TitleChangedEventArgs(String newTitle); - delegate void ConnectionClosedEventArgs(); delegate void ScrollPositionChangedEventArgs(Int32 viewTop, Int32 viewHeight, Int32 bufferLength); runtimeclass CopyToClipboardEventArgs @@ -29,16 +28,19 @@ namespace Microsoft.Terminal.TerminalControl void UpdateSettings(Microsoft.Terminal.Settings.IControlSettings newSettings); event TitleChangedEventArgs TitleChanged; - event ConnectionClosedEventArgs ConnectionClosed; event Windows.Foundation.TypedEventHandler CopyToClipboard; event Windows.Foundation.TypedEventHandler PasteFromClipboard; + // This is an event handler forwarder for the underlying connection. + // We expose this and ConnectionState here so that it might eventually be data bound. + event Windows.Foundation.TypedEventHandler ConnectionStateChanged; + Microsoft.Terminal.TerminalConnection.ConnectionState ConnectionState { get; }; + String Title { get; }; Boolean CopySelectionToClipboard(Boolean trimTrailingWhitespace); void PasteTextFromClipboard(); void Close(); - Boolean ShouldCloseOnExit { get; }; Windows.Foundation.Size CharacterDimensions { get; }; Windows.Foundation.Size MinimumSize { get; }; diff --git a/src/cascadia/TerminalSettings/IControlSettings.idl b/src/cascadia/TerminalSettings/IControlSettings.idl index fad4f3435c2..7c417137d08 100644 --- a/src/cascadia/TerminalSettings/IControlSettings.idl +++ b/src/cascadia/TerminalSettings/IControlSettings.idl @@ -20,7 +20,6 @@ namespace Microsoft.Terminal.Settings interface IControlSettings requires Microsoft.Terminal.Settings.ICoreSettings { Boolean UseAcrylic; - Boolean CloseOnExit; Double TintOpacity; ScrollbarState ScrollState; diff --git a/src/cascadia/TerminalSettings/TerminalSettings.cpp b/src/cascadia/TerminalSettings/TerminalSettings.cpp index 06d3a778e6e..32889b8000f 100644 --- a/src/cascadia/TerminalSettings/TerminalSettings.cpp +++ b/src/cascadia/TerminalSettings/TerminalSettings.cpp @@ -24,7 +24,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation _wordDelimiters{ DEFAULT_WORD_DELIMITERS }, _copyOnSelect{ false }, _useAcrylic{ false }, - _closeOnExit{ true }, _tintOpacity{ 0.5 }, _padding{ DEFAULT_PADDING }, _fontFace{ DEFAULT_FONT_FACE }, @@ -181,16 +180,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation _useAcrylic = value; } - bool TerminalSettings::CloseOnExit() - { - return _closeOnExit; - } - - void TerminalSettings::CloseOnExit(bool value) - { - _closeOnExit = value; - } - double TerminalSettings::TintOpacity() { return _tintOpacity; diff --git a/src/cascadia/TerminalSettings/terminalsettings.h b/src/cascadia/TerminalSettings/terminalsettings.h index b672f6a9409..a6f792258ce 100644 --- a/src/cascadia/TerminalSettings/terminalsettings.h +++ b/src/cascadia/TerminalSettings/terminalsettings.h @@ -55,8 +55,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation bool UseAcrylic(); void UseAcrylic(bool value); - bool CloseOnExit(); - void CloseOnExit(bool value); double TintOpacity(); void TintOpacity(double value); hstring Padding(); @@ -114,7 +112,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation hstring _wordDelimiters; bool _useAcrylic; - bool _closeOnExit; double _tintOpacity; hstring _fontFace; int32_t _fontSize; diff --git a/src/cascadia/inc/cppwinrt_utils.h b/src/cascadia/inc/cppwinrt_utils.h index 9329c1cdef8..fc12881ae08 100644 --- a/src/cascadia/inc/cppwinrt_utils.h +++ b/src/cascadia/inc/cppwinrt_utils.h @@ -71,6 +71,20 @@ public: private: \ winrt::event> _##name##Handlers; +// This is a helper macro for both declaring the signature of a callback (nee event) and +// defining the body. Winrt callbacks need a method for adding a delegate to the +// callback and removing the delegate. This macro will both declare the method +// signatures and define them both for you, because they don't really vary from +// event to event. +// Use this in a class's header if you have a "delegate" type in your IDL. +#define WINRT_CALLBACK(name, args) \ +public: \ + winrt::event_token name(args const& handler) { return _##name##Handlers.add(handler); } \ + void name(winrt::event_token const& token) noexcept { _##name##Handlers.remove(token); } \ + \ +private: \ + winrt::event _##name##Handlers; + // This is a helper macro for both declaring the signature and body of an event // which is exposed by one class, but actually handled entirely by one of the // class's members. This type of event could be considered "forwarded" or