Skip to content

Commit

Permalink
[DxD] Add 'Automatic' as a mode for CloseOnExit (#13560)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Adds a new mode to `CloseOnExit`: `Automatic`. In this mode, if a process handed off by defterm terminates for whatever reason, we always close (i.e. we treat the mode as `Always`), but for processes launched by Terminal we terminate as with the `Graceful` behaviour.

## PR Checklist
* [x] Closes #13325
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments

- Adds a new enum value to `CloseOnExit`
- Adds a new function to `Pane`: `FinalizeConfigurationGivenDefault`: this is a function that should be called when the pane is created via default terminal handoff, and can contain any special configurations we should set given that the pane was created via handoff

## Validation Steps Performed

(cherry picked from commit 89d57e8)
Service-Card-Id: 84836029
Service-Version: 1.15
  • Loading branch information
PankajBhojwani authored and DHowett committed Aug 8, 2022
1 parent fff3372 commit b68de3e
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 10 deletions.
7 changes: 4 additions & 3 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2086,14 +2086,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"
},
Expand Down
19 changes: 18 additions & 1 deletion src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1019,9 +1019,17 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio

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();
}
Expand Down Expand Up @@ -3108,6 +3116,15 @@ 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()
{
_isDefTermSession = true;
}

// Method Description:
// - Returns true if the pane or one of its descendants is read-only
bool Pane::ContainsReadOnly() const
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ class Pane : public std::enable_shared_from_this<Pane>
bool FocusPane(const std::shared_ptr<Pane> pane);
std::shared_ptr<Pane> FindPane(const uint32_t id);

void FinalizeConfigurationGivenDefault();

bool ContainsReadOnly() const;

// Method Description:
Expand Down Expand Up @@ -241,6 +243,8 @@ class Pane : public std::enable_shared_from_this<Pane>

bool _zoomed{ false };

bool _isDefTermSession{ false };

winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr };
winrt::Windows::Media::Playback::MediaPlayer::MediaEnded_revoker _mediaEndedRevoker;

Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3270,7 +3270,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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,10 @@
<value>Never close automatically</value>
<comment>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.</comment>
</data>
<data name="Profile_CloseOnExitAutomatic.Content" xml:space="preserve">
<value>Automatic</value>
<comment>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.</comment>
</data>
<data name="Profile_ColorScheme.Header" xml:space="preserve">
<value>Color scheme</value>
<comment>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.</comment>
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/MTSMSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,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::Automatic) \
X(hstring, TabTitle, "tabTitle") \
X(Model::BellStyle, BellStyle, "bellStyle", BellStyle::Audible) \
X(bool, UseAtlasEngine, "experimental.useAtlasEngine", false) \
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettingsModel/Profile.idl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ namespace Microsoft.Terminal.Settings.Model
{
Never = 0,
Graceful,
Always
Always,
Automatic
};

[flags]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{ "automatic", ValueType::Automatic },
};

// Override mapping parser to add boolean parsing
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalSettingsModel/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit b68de3e

Please sign in to comment.