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.

Sign up for GitHub

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

Jump to bottom

[CmdLineArgs] "--keepOpen" to keep the window/tab/pane open after the process/profile has exited. #16497

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
+32 −0
Draft

[CmdLineArgs] "--keepOpen" to keep the window/tab/pane open after the process/profile has exited. #16497

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
aece58f
first changes
htcfreek Dec 25, 2023
8c2490e
revert small change
htcfreek Dec 25, 2023
ba5564b
try to fix problems
htcfreek Dec 25, 2023
fb530c5
comment not working code
htcfreek Dec 25, 2023
9d6078a
rename argument
htcfreek Dec 25, 2023
2621322
spell fix
htcfreek Dec 25, 2023
6fe6d21
fix small typo
htcfreek Dec 25, 2023
ee97500
fixes
htcfreek Dec 25, 2023
21d0fc3
fix TerminalSettings
htcfreek Dec 31, 2023
a25eef0
fix crash
htcfreek Dec 31, 2023
4241103
Merge branch 'microsoft:main' into Term_WindowParam
htcfreek Jun 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,8 @@ void AppCommandlineArgs::_addNewTerminalArgs(AppCommandlineArgs::NewTerminalSubc
_inheritEnvironment,
RS_A(L"CmdInheritEnvDesc"));

subcommand.keepOpenOption = subcommand.subcommand->add_flag("-ko,--keepOpen", _keepOpenOption, RS_A(L"CmdKeepOpenDesc"));
htcfreek marked this conversation as resolved.
Show resolved Hide resolved

// Using positionals_at_end allows us to support "wt new-tab -d wsl -d Ubuntu"
// without CLI11 thinking that we've specified -d twice.
// There's an alternate construction where we make all subcommands "prefix commands",
Expand Down Expand Up @@ -675,6 +677,11 @@ NewTerminalArgs AppCommandlineArgs::_getNewTerminalArgs(AppCommandlineArgs::NewT
}
args.ReloadEnvironmentVariables(!inheritEnv);

if (*subcommand.keepOpenOption)
{
args.KeepOpen(_keepOpenOption);
}

return args;
}

Expand Down Expand Up @@ -720,6 +727,7 @@ void AppCommandlineArgs::_resetStateToDefault()
_commandline.clear();
_suppressApplicationTitle = false;
_appendCommandLineOption = false;
_keepOpenOption = false;

_splitVertical = false;
_splitHorizontal = false;
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class TerminalApp::AppCommandlineArgs final
CLI::Option* colorSchemeOption;
CLI::Option* appendCommandLineOption;
CLI::Option* inheritEnvOption;
CLI::Option* keepOpenOption;
};

struct NewPaneSubcommand : public NewTerminalSubcommand
Expand Down Expand Up @@ -110,6 +111,8 @@ class TerminalApp::AppCommandlineArgs final
std::vector<std::string> _commandline;
bool _appendCommandLineOption{ false };

bool _keepOpenOption{ false };

bool _splitVertical{ false };
bool _splitHorizontal{ false };
bool _splitDuplicate{ false };
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,4 +905,7 @@
<data name="RestartConnectionToolTip" xml:space="preserve">
<value>Restart the active pane connection</value>
</data>
<data name="CmdKeepOpenDesc" xml:space="preserve">
<value>If set, the window/tab/pane will stay open after the command has exited. (It is he same as setting the profile termination behavior to never.)</value>
</data>
</root>
7 changes: 7 additions & 0 deletions src/cascadia/TerminalSettingsModel/ActionArgs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
ACTION_ARG(winrt::hstring, ColorScheme);
ACTION_ARG(Windows::Foundation::IReference<bool>, Elevate, nullptr);
ACTION_ARG(Windows::Foundation::IReference<bool>, ReloadEnvironmentVariables, nullptr);
ACTION_ARG(bool, KeepOpen, false)
ACTION_ARG(uint64_t, ContentId);

static constexpr std::string_view CommandlineKey{ "commandline" };
Expand All @@ -327,6 +328,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
static constexpr std::string_view ColorSchemeKey{ "colorScheme" };
static constexpr std::string_view ElevateKey{ "elevate" };
static constexpr std::string_view ReloadEnvironmentVariablesKey{ "reloadEnvironmentVariables" };
static constexpr std::string_view KeepOpenKey{ "keepOpen" };
static constexpr std::string_view ContentKey{ "__content" };

public:
Expand All @@ -349,6 +351,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
otherAsUs->_ColorScheme == _ColorScheme &&
otherAsUs->_Elevate == _Elevate &&
otherAsUs->_ReloadEnvironmentVariables == _ReloadEnvironmentVariables &&
otherAsUs->_KeepOpen == _KeepOpen &&
otherAsUs->_ContentId == _ContentId;
}
return false;
Expand All @@ -367,6 +370,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
JsonUtils::GetValueForKey(json, ColorSchemeKey, args->_ColorScheme);
JsonUtils::GetValueForKey(json, ElevateKey, args->_Elevate);
JsonUtils::GetValueForKey(json, ReloadEnvironmentVariablesKey, args->_ReloadEnvironmentVariables);
JsonUtils::GetValueForKey(json, KeepOpenKey, args->_KeepOpen);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct that this json code and the one in line 395 is used to read form/write to a WT arguments json file? Do I need to update a json schema? (I can't find one.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's pretty much it. The schema can be found at terminal/doc/cascadia /profiles.schema.json

JsonUtils::GetValueForKey(json, ContentKey, args->_ContentId);
return *args;
}
Expand All @@ -388,6 +392,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
JsonUtils::SetValueForKey(json, ColorSchemeKey, args->_ColorScheme);
JsonUtils::SetValueForKey(json, ElevateKey, args->_Elevate);
JsonUtils::SetValueForKey(json, ReloadEnvironmentVariablesKey, args->_ReloadEnvironmentVariables);
JsonUtils::SetValueForKey(json, KeepOpenKey, args->_KeepOpen);
JsonUtils::SetValueForKey(json, ContentKey, args->_ContentId);
return json;
}
Expand All @@ -404,6 +409,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
copy->_ColorScheme = _ColorScheme;
copy->_Elevate = _Elevate;
copy->_ReloadEnvironmentVariables = _ReloadEnvironmentVariables;
copy->_KeepOpen = _KeepOpen;
copy->_ContentId = _ContentId;
return *copy;
}
Expand All @@ -425,6 +431,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
h.write(ColorScheme());
h.write(Elevate());
h.write(ReloadEnvironmentVariables());
h.write(KeepOpen());
h.write(ContentId());
}
};
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/ActionArgs.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ namespace Microsoft.Terminal.Settings.Model
Windows.Foundation.IReference<Windows.UI.Color> TabColor;
String Profile; // Either a GUID or a profile's name if the GUID isn't a match
Boolean AppendCommandLine;
Boolean KeepOpen;

// We use IReference<> to treat some args as nullable where null means
// "use the inherited value". See ProfileIndex,
Expand Down
7 changes: 7 additions & 0 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
defaultSettings.ReloadEnvironmentVariables(newTerminalArgs.ReloadEnvironmentVariables().Value());
}

if (newTerminalArgs.KeepOpen())
{
// defaultSettings.CloseOnExit = CloseOnExitMode::Never;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
}
}

return settingsPair;
Expand Down Expand Up @@ -341,6 +346,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
_RepositionCursorWithMouse = profile.RepositionCursorWithMouse();

_ReloadEnvironmentVariables = profile.ReloadEnvironmentVariables();

_CloseOnExit = profile.CloseOnExit();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems... weird. This should already be plumbed through to the TermControl somewhere else...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. It's not actually used by the TermControl. The pane itself handles that, in this blob of code:

terminal/src/cascadia/TerminalApp/TerminalPaneContent.cpp

Lines 240 to 256 in 9c16c5c

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 || mode == CloseOnExitMode::Automatic) && newConnectionState == ConnectionState::Closed))
{
Close();
}
}

Looks like that's specifically looking up the CloseOnExit value out of the profile, not the TerminalSettings. I bet you could update that to use the value from the TerminalSettings too.

This code has an example in that file of getting settings from the cache:

terminal/src/cascadia/TerminalApp/TerminalPaneContent.cpp

Line 345 in 9c16c5c

if (const auto& settings{ _cache.TryLookup(_profile) })

That'll get you a TerminalSettingsCreateResult, who's .DefaultSettings() will have the value you're looking for

}

// Method Description:
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

INHERITABLE_SETTING(Model::TerminalSettings, bool, ReloadEnvironmentVariables, true);

INHERITABLE_SETTING(Model::TerminalSettings, CloseOnExitMode, CloseOnExit, CloseOnExitMode::Automatic);

private:
std::optional<std::array<Microsoft::Terminal::Core::Color, COLOR_TABLE_SIZE>> _ColorTable;
std::span<Microsoft::Terminal::Core::Color> _getColorTableImpl();
Expand Down