Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ConnectionState and closeOnExit=graceful/always/never #3623

Merged
merged 18 commits into from
Nov 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/cascadia/SettingsSchema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
18 changes: 15 additions & 3 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,21 @@
"type": "string"
},
"closeOnExit": {
"default": true,
DHowett-MSFT marked this conversation as resolved.
Show resolved Hide resolved
"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",
Expand Down
66 changes: 66 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ namespace TerminalAppLocalTests
TEST_METHOD(TestProfileIconWithEnvVar);
TEST_METHOD(TestProfileBackgroundImageWithEnvVar);

TEST_METHOD(TestCloseOnExitParsing);
TEST_METHOD(TestCloseOnExitCompatibilityShim);

TEST_CLASS_SETUP(ClassSetup)
{
InitializeJsonReader();
Expand Down Expand Up @@ -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()
DHowett-MSFT marked this conversation as resolved.
Show resolved Hide resolved
{
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());
}
}
7 changes: 7 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/AzureCloudShellGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ std::vector<TerminalApp::Profile> AzureCloudShellGenerator::GenerateProfiles()
azureCloudShellProfile.SetColorScheme({ L"Vintage" });
azureCloudShellProfile.SetAcrylicOpacity(0.6);
azureCloudShellProfile.SetUseAcrylic(true);
azureCloudShellProfile.SetCloseOnExit(false);
azureCloudShellProfile.SetConnectionType(AzureConnectionType);
profiles.emplace_back(azureCloudShellProfile);
}
Expand Down
16 changes: 16 additions & 0 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<winrt::TerminalApp::App>() };
DHowett-MSFT marked this conversation as resolved.
Show resolved Hide resolved
THROW_HR_IF_NULL(E_INVALIDARG, currentXamlApp);

auto appLogic = winrt::get_self<winrt::TerminalApp::implementation::AppLogic>(currentXamlApp.Logic());
THROW_HR_IF_NULL(E_INVALIDARG, appLogic);

return *(appLogic->GetSettings());
}

CascadiaSettings::CascadiaSettings() :
CascadiaSettings(true)
{
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ class TerminalApp::CascadiaSettings final
static std::unique_ptr<CascadiaSettings> LoadDefaults();
static std::unique_ptr<CascadiaSettings> LoadAll();

static const CascadiaSettings& GetCurrentAppSettings();

winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional<GUID> profileGuid) const;

GlobalAppSettings& GlobalSettings();
Expand Down
49 changes: 34 additions & 15 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -302,7 +308,7 @@ bool Pane::NavigateFocus(const Direction& direction)
// - <none>
// Return Value:
// - <none>
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
Expand All @@ -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);
}
}
}

Expand All @@ -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:
Expand Down Expand Up @@ -570,16 +590,16 @@ 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
// closing. At this point, if the remaining child's control is closed,
// 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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -720,13 +740,13 @@ void Pane::_CloseChild(const bool closeFirst)
// - <none>
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);
});
Expand Down Expand Up @@ -965,8 +985,8 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> 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
Expand Down Expand Up @@ -1124,5 +1144,4 @@ void Pane::_SetupResources()
}
}

DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs);
DEFINE_EVENT(Pane, GotFocus, _GotFocusHandlers, winrt::delegate<std::shared_ptr<Pane>>);
6 changes: 3 additions & 3 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class Pane : public std::enable_shared_from_this<Pane>

void Close();

DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs);
WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate<std::shared_ptr<Pane>>);

private:
Expand All @@ -89,7 +89,7 @@ class Pane : public std::enable_shared_from_this<Pane>

bool _lastActive{ false };
std::optional<GUID> _profile{ std::nullopt };
winrt::event_token _connectionClosedToken{ 0 };
winrt::event_token _connectionStateChangedToken{ 0 };
winrt::event_token _firstClosedToken{ 0 };
winrt::event_token _secondClosedToken{ 0 };

Expand Down Expand Up @@ -119,7 +119,7 @@ class Pane : public std::enable_shared_from_this<Pane>
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<float, float> _GetPaneSizes(const float& fullSize);

Expand Down
Loading