From 23114f89f947dd08a81bf90bd46624021122e7cd Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Thu, 21 Jul 2022 12:21:01 -0700 Subject: [PATCH 1/6] new enum value --- src/cascadia/TerminalApp/Pane.cpp | 29 +++++++++++++++++++ src/cascadia/TerminalApp/Pane.h | 4 +++ src/cascadia/TerminalApp/TerminalPage.cpp | 6 +++- .../Resources/en-US/Resources.resw | 4 +++ .../TerminalSettingsModel/MTSMSettings.h | 2 +- .../TerminalSettingsModel/Profile.idl | 3 +- .../TerminalSettingsSerializationHelpers.h | 3 +- 7 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index ec9cbd6d4c9..54b625faf0f 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1017,6 +1017,13 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio return; } + if (_overrideCloseOnExit) + { + // We have been told to close upon process termination regardless of the value of + // the profile's CloseOnExit, so go ahead and close + Close(); + } + if (_profile) { const auto mode = _profile.CloseOnExit(); @@ -3108,6 +3115,28 @@ int Pane::GetLeafPaneCount() const noexcept return _IsLeaf() ? 1 : (_firstChild->GetLeafPaneCount() + _secondChild->GetLeafPaneCount()); } +// Method Description: +// - Should be called when this pane is created via a default terminal handoff +// - Finalizes our configuration given the information that we have been +// created via default handoff +void Pane::FinalizeConfigurationGivenDefault() +{ + if (_IsLeaf()) + { + if (_profile) + { + if (_profile.CloseOnExit() == CloseOnExitMode::GracefulIfLaunchedByTerminal) + { + // We only want to close 'gracefully' if we were launched by Terminal + // Since we were launched via defterm, override this (i.e. it will + // be treated as 'always', see _ControlConnectionStateChangedHandler + // for where this boolean is used, and see GH #13325 for discussion) + _overrideCloseOnExit = true; + } + } + } +} + // Method Description: // - Returns true if the pane or one of its descendants is read-only bool Pane::ContainsReadOnly() const diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 2c1fec243d6..c13f510f80b 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -132,6 +132,8 @@ class Pane : public std::enable_shared_from_this bool FocusPane(const std::shared_ptr pane); std::shared_ptr FindPane(const uint32_t id); + void FinalizeConfigurationGivenDefault(); + bool ContainsReadOnly() const; // Method Description: @@ -241,6 +243,8 @@ class Pane : public std::enable_shared_from_this bool _zoomed{ false }; + bool _overrideCloseOnExit{ false }; + winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr }; winrt::Windows::Media::Playback::MediaPlayer::MediaEnded_revoker _mediaEndedRevoker; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 2032125d0e4..374e71dcc72 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -3202,7 +3202,11 @@ namespace winrt::TerminalApp::implementation // elevated version of the Terminal with that profile... that's a // recipe for disaster. We won't ever open up a tab in this window. newTerminalArgs.Elevate(false); - _CreateNewTabFromPane(_MakePane(newTerminalArgs, false, connection)); + const auto newPane = _MakePane(newTerminalArgs, false, connection); + newPane->WalkTree([](auto pane) { + pane->FinalizeConfigurationGivenDefault(); + }); + _CreateNewTabFromPane(newPane); // Request a summon of this window to the foreground _SummonWindowRequestedHandlers(*this, nullptr); diff --git a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw index fe4323391ed..1ce00024b7e 100644 --- a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw @@ -714,6 +714,10 @@ Never close automatically An option to choose from for the "profile termination behavior" (or "close on exit") setting. When selected, the terminal never closes, even if the process exits in a controlled or uncontrolled scenario. The user would have to manually close the terminal. + + Graceful if launched by Windows Terminal + An option to choose from for the "profile termination behavior" (or "close on exit") setting. When selected, the terminal closes if the process exits in a controlled scenario successfully and the process was launched by Windows Terminal. + Color scheme Header for a control to select the scheme (or set) of colors used in the session. This is selected from a list of options managed by the user. diff --git a/src/cascadia/TerminalSettingsModel/MTSMSettings.h b/src/cascadia/TerminalSettingsModel/MTSMSettings.h index 12e3d4ccabd..9a817a2f757 100644 --- a/src/cascadia/TerminalSettingsModel/MTSMSettings.h +++ b/src/cascadia/TerminalSettingsModel/MTSMSettings.h @@ -74,7 +74,7 @@ Author(s): X(bool, SuppressApplicationTitle, "suppressApplicationTitle", false) \ X(guid, ConnectionType, "connectionType") \ X(hstring, Icon, "icon", L"\uE756") \ - X(CloseOnExitMode, CloseOnExit, "closeOnExit", CloseOnExitMode::Graceful) \ + X(CloseOnExitMode, CloseOnExit, "closeOnExit", CloseOnExitMode::GracefulIfLaunchedByTerminal) \ X(hstring, TabTitle, "tabTitle") \ X(Model::BellStyle, BellStyle, "bellStyle", BellStyle::Audible) \ X(bool, UseAtlasEngine, "experimental.useAtlasEngine", false) \ diff --git a/src/cascadia/TerminalSettingsModel/Profile.idl b/src/cascadia/TerminalSettingsModel/Profile.idl index 76357b65898..8adeafe0151 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.idl +++ b/src/cascadia/TerminalSettingsModel/Profile.idl @@ -26,7 +26,8 @@ namespace Microsoft.Terminal.Settings.Model { Never = 0, Graceful, - Always + Always, + GracefulIfLaunchedByTerminal }; [flags] diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h index 292ffbbd190..1f1a4ceb753 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h +++ b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h @@ -108,10 +108,11 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Control::TextAntialiasingMode) // - Helper for converting a user-specified closeOnExit value to its corresponding enum JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::CloseOnExitMode) { - JSON_MAPPINGS(3) = { + JSON_MAPPINGS(4) = { pair_type{ "always", ValueType::Always }, pair_type{ "graceful", ValueType::Graceful }, pair_type{ "never", ValueType::Never }, + pair_type{ "gracefulIfLaunchedByTerminal", ValueType::GracefulIfLaunchedByTerminal }, }; // Override mapping parser to add boolean parsing From 2bcd8184ca3166c5ffad8c9d764433dd3817612d Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 22 Jul 2022 11:07:29 -0700 Subject: [PATCH 2/6] combine nests --- src/cascadia/TerminalApp/Pane.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 54b625faf0f..0dc9574948b 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -3121,18 +3121,14 @@ int Pane::GetLeafPaneCount() const noexcept // created via default handoff void Pane::FinalizeConfigurationGivenDefault() { - if (_IsLeaf()) + if (_IsLeaf() && _profile && _profile.CloseOnExit() == CloseOnExitMode::GracefulIfLaunchedByTerminal) { - if (_profile) { - if (_profile.CloseOnExit() == CloseOnExitMode::GracefulIfLaunchedByTerminal) - { - // We only want to close 'gracefully' if we were launched by Terminal - // Since we were launched via defterm, override this (i.e. it will - // be treated as 'always', see _ControlConnectionStateChangedHandler - // for where this boolean is used, and see GH #13325 for discussion) - _overrideCloseOnExit = true; - } + // We only want to close 'gracefully' if we were launched by Terminal + // Since we were launched via defterm, override this (i.e. it will + // be treated as 'always', see _ControlConnectionStateChangedHandler + // for where this boolean is used, and see GH #13325 for discussion) + _overrideCloseOnExit = true; } } } From 9ca624f376a2a37f29caa234d0d60c38f25d0f04 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 22 Jul 2022 11:11:42 -0700 Subject: [PATCH 3/6] Rename flag --- src/cascadia/TerminalApp/Pane.cpp | 4 ++-- src/cascadia/TerminalApp/Pane.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 0dc9574948b..5ad74d991d6 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1017,7 +1017,7 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio return; } - if (_overrideCloseOnExit) + if (_isDefTermSession) { // We have been told to close upon process termination regardless of the value of // the profile's CloseOnExit, so go ahead and close @@ -3128,7 +3128,7 @@ void Pane::FinalizeConfigurationGivenDefault() // Since we were launched via defterm, override this (i.e. it will // be treated as 'always', see _ControlConnectionStateChangedHandler // for where this boolean is used, and see GH #13325 for discussion) - _overrideCloseOnExit = true; + _isDefTermSession = true; } } } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index c13f510f80b..7094f3b3643 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -243,7 +243,7 @@ class Pane : public std::enable_shared_from_this bool _zoomed{ false }; - bool _overrideCloseOnExit{ false }; + bool _isDefTermSession{ false }; winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr }; winrt::Windows::Media::Playback::MediaPlayer::MediaEnded_revoker _mediaEndedRevoker; From 69e6a2b4df9fbfe2a98f703e98d548487d36f490 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Tue, 26 Jul 2022 14:39:53 -0700 Subject: [PATCH 4/6] automatic --- src/cascadia/TerminalApp/Pane.cpp | 2 +- .../TerminalSettingsEditor/Resources/en-US/Resources.resw | 4 ++-- src/cascadia/TerminalSettingsModel/MTSMSettings.h | 2 +- src/cascadia/TerminalSettingsModel/Profile.idl | 2 +- .../TerminalSettingsSerializationHelpers.h | 2 +- src/cascadia/TerminalSettingsModel/defaults.json | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 5ad74d991d6..877c064a7a7 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -3121,7 +3121,7 @@ int Pane::GetLeafPaneCount() const noexcept // created via default handoff void Pane::FinalizeConfigurationGivenDefault() { - if (_IsLeaf() && _profile && _profile.CloseOnExit() == CloseOnExitMode::GracefulIfLaunchedByTerminal) + if (_IsLeaf() && _profile && _profile.CloseOnExit() == CloseOnExitMode::Automatic) { { // We only want to close 'gracefully' if we were launched by Terminal diff --git a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw index 1ce00024b7e..0116d9b4a9d 100644 --- a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw @@ -714,8 +714,8 @@ Never close automatically An option to choose from for the "profile termination behavior" (or "close on exit") setting. When selected, the terminal never closes, even if the process exits in a controlled or uncontrolled scenario. The user would have to manually close the terminal. - - Graceful if launched by Windows Terminal + + Automatic An option to choose from for the "profile termination behavior" (or "close on exit") setting. When selected, the terminal closes if the process exits in a controlled scenario successfully and the process was launched by Windows Terminal. diff --git a/src/cascadia/TerminalSettingsModel/MTSMSettings.h b/src/cascadia/TerminalSettingsModel/MTSMSettings.h index 9a817a2f757..d70d3039966 100644 --- a/src/cascadia/TerminalSettingsModel/MTSMSettings.h +++ b/src/cascadia/TerminalSettingsModel/MTSMSettings.h @@ -74,7 +74,7 @@ Author(s): X(bool, SuppressApplicationTitle, "suppressApplicationTitle", false) \ X(guid, ConnectionType, "connectionType") \ X(hstring, Icon, "icon", L"\uE756") \ - X(CloseOnExitMode, CloseOnExit, "closeOnExit", CloseOnExitMode::GracefulIfLaunchedByTerminal) \ + X(CloseOnExitMode, CloseOnExit, "closeOnExit", CloseOnExitMode::Automatic) \ X(hstring, TabTitle, "tabTitle") \ X(Model::BellStyle, BellStyle, "bellStyle", BellStyle::Audible) \ X(bool, UseAtlasEngine, "experimental.useAtlasEngine", false) \ diff --git a/src/cascadia/TerminalSettingsModel/Profile.idl b/src/cascadia/TerminalSettingsModel/Profile.idl index 8adeafe0151..608dfc5f914 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.idl +++ b/src/cascadia/TerminalSettingsModel/Profile.idl @@ -27,7 +27,7 @@ namespace Microsoft.Terminal.Settings.Model Never = 0, Graceful, Always, - GracefulIfLaunchedByTerminal + Automatic }; [flags] diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h index 1f1a4ceb753..9e43aa59346 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h +++ b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h @@ -112,7 +112,7 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::CloseOnExitMode) pair_type{ "always", ValueType::Always }, pair_type{ "graceful", ValueType::Graceful }, pair_type{ "never", ValueType::Never }, - pair_type{ "gracefulIfLaunchedByTerminal", ValueType::GracefulIfLaunchedByTerminal }, + pair_type{ "automatic", ValueType::Automatic }, }; // Override mapping parser to add boolean parsing diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index 7a672723c33..76d5f4b3bd1 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -43,7 +43,7 @@ "icon": "ms-appx:///ProfileIcons/{61c54bbd-c2c6-5271-96e7-009a87ff44bf}.png", "colorScheme": "Campbell", "antialiasingMode": "grayscale", - "closeOnExit": "graceful", + "closeOnExit": "automatic", "cursorShape": "bar", "fontFace": "Cascadia Mono", "fontSize": 12, @@ -62,7 +62,7 @@ "icon": "ms-appx:///ProfileIcons/{0caa0dad-35be-5f56-a8ff-afceeeaa6101}.png", "colorScheme": "Campbell", "antialiasingMode": "grayscale", - "closeOnExit": "graceful", + "closeOnExit": "automatic", "cursorShape": "bar", "fontFace": "Cascadia Mono", "fontSize": 12, From f385cbb1ffd84befff0779529d7d1d351ebdf3ca Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Tue, 26 Jul 2022 14:45:01 -0700 Subject: [PATCH 5/6] update schema --- doc/cascadia/profiles.schema.json | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 1543b7b3986..d740324b310 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -2106,14 +2106,15 @@ "$ref": "#/$defs/BellSound" }, "closeOnExit": { - "default": "graceful", - "description": "Sets how the profile reacts to termination or failure to launch. Possible values:\n -\"graceful\" (close when exit is typed or the process exits normally)\n -\"always\" (always close)\n -\"never\" (never close).\ntrue and false are accepted as synonyms for \"graceful\" and \"never\" respectively.", + "default": "automatic", + "description": "Sets how the profile reacts to termination or failure to launch. Possible values:\n -\"graceful\" (close when exit is typed or the process exits normally)\n -\"always\" (always close)\n -\"automatic\" (behave as \"graceful\" only for processes launched by terminal, behave as \"always\" otherwise)\n -\"never\" (never close).\ntrue and false are accepted as synonyms for \"graceful\" and \"never\" respectively.", "oneOf": [ { "enum": [ "never", "graceful", - "always" + "always", + "automatic" ], "type": "string" }, From 126a48fcc37ec8ba2d3a8bdb5f4417a158cf5bc7 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 1 Aug 2022 13:18:12 -0700 Subject: [PATCH 6/6] update given new bool name, check automatic in non-defterm session too --- src/cascadia/TerminalApp/Pane.cpp | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index d51b5efb5de..19b09d5e9ce 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1017,18 +1017,19 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio return; } - if (_isDefTermSession) - { - // We have been told to close upon process termination regardless of the value of - // the profile's CloseOnExit, so go ahead and close - Close(); - } - if (_profile) { + if (_isDefTermSession && _profile.CloseOnExit() == CloseOnExitMode::Automatic) + { + // For 'automatic', we only care about the connection state if we were launched by Terminal + // Since we were launched via defterm, ignore the connection state (i.e. we treat the + // close on exit mode as 'always', see GH #13325 for discussion) + Close(); + } + const auto mode = _profile.CloseOnExit(); if ((mode == CloseOnExitMode::Always) || - (mode == CloseOnExitMode::Graceful && newConnectionState == ConnectionState::Closed)) + ((mode == CloseOnExitMode::Graceful || mode == CloseOnExitMode::Automatic) && newConnectionState == ConnectionState::Closed)) { Close(); } @@ -3121,16 +3122,7 @@ int Pane::GetLeafPaneCount() const noexcept // created via default handoff void Pane::FinalizeConfigurationGivenDefault() { - if (_IsLeaf() && _profile && _profile.CloseOnExit() == CloseOnExitMode::Automatic) - { - { - // We only want to close 'gracefully' if we were launched by Terminal - // Since we were launched via defterm, override this (i.e. it will - // be treated as 'always', see _ControlConnectionStateChangedHandler - // for where this boolean is used, and see GH #13325 for discussion) - _isDefTermSession = true; - } - } + _isDefTermSession = true; } // Method Description: