Skip to content

Commit a4dfcbc

Browse files
committed
wip
1 parent 00ee884 commit a4dfcbc

File tree

11 files changed

+78
-246
lines changed

11 files changed

+78
-246
lines changed

src/cascadia/TerminalApp/AppCommandlineArgs.cpp

Lines changed: 21 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -961,18 +961,6 @@ std::vector<ActionAndArgs>& AppCommandlineArgs::GetStartupActions()
961961
return _startupActions;
962962
}
963963

964-
// Method Description:
965-
// - Returns whether we should start listening for inbound PTY connections
966-
// coming from the operating system default application feature.
967-
// Arguments:
968-
// - <none>
969-
// Return Value:
970-
// - True if the listener should be started. False otherwise.
971-
bool AppCommandlineArgs::IsHandoffListener() const noexcept
972-
{
973-
return _isHandoffListener;
974-
}
975-
976964
// Method Description:
977965
// - Get the string of text that should be displayed to the user on exit. This
978966
// is usually helpful for cases where the user entered some sort of invalid
@@ -1015,34 +1003,28 @@ bool AppCommandlineArgs::ShouldExitEarly() const noexcept
10151003
// - <none>
10161004
void AppCommandlineArgs::ValidateStartupCommands()
10171005
{
1018-
// Only check over the actions list for the potential to add a new-tab
1019-
// command if we are not starting for the purposes of receiving an inbound
1020-
// handoff connection from the operating system.
1021-
if (!_isHandoffListener)
1006+
// If we only have a single x-save command, then set our target to the
1007+
// current terminal window. This will prevent us from spawning a new
1008+
// window just to save the commandline.
1009+
if (_startupActions.size() == 1 &&
1010+
_startupActions.front().Action() == ShortcutAction::SaveSnippet &&
1011+
_windowTarget.empty())
10221012
{
1023-
// If we only have a single x-save command, then set our target to the
1024-
// current terminal window. This will prevent us from spawning a new
1025-
// window just to save the commandline.
1026-
if (_startupActions.size() == 1 &&
1027-
_startupActions.front().Action() == ShortcutAction::SaveSnippet &&
1028-
_windowTarget.empty())
1029-
{
1030-
_windowTarget = "0";
1031-
}
1032-
// If we parsed no commands, or the first command we've parsed is not a new
1033-
// tab action, prepend a new-tab command to the front of the list.
1034-
// (also, we don't need to do this if the only action is a x-save)
1035-
else if (_startupActions.empty() ||
1036-
(_startupActions.front().Action() != ShortcutAction::NewTab &&
1037-
_startupActions.front().Action() != ShortcutAction::SaveSnippet))
1038-
{
1039-
// Build the NewTab action from the values we've parsed on the commandline.
1040-
NewTerminalArgs newTerminalArgs{};
1041-
NewTabArgs args{ newTerminalArgs };
1042-
ActionAndArgs newTabAction{ ShortcutAction::NewTab, args };
1043-
// push the arg onto the front
1044-
_startupActions.insert(_startupActions.begin(), 1, newTabAction);
1045-
}
1013+
_windowTarget = "0";
1014+
}
1015+
// If we parsed no commands, or the first command we've parsed is not a new
1016+
// tab action, prepend a new-tab command to the front of the list.
1017+
// (also, we don't need to do this if the only action is a x-save)
1018+
else if (_startupActions.empty() ||
1019+
(_startupActions.front().Action() != ShortcutAction::NewTab &&
1020+
_startupActions.front().Action() != ShortcutAction::SaveSnippet))
1021+
{
1022+
// Build the NewTab action from the values we've parsed on the commandline.
1023+
NewTerminalArgs newTerminalArgs{};
1024+
NewTabArgs args{ newTerminalArgs };
1025+
ActionAndArgs newTabAction{ ShortcutAction::NewTab, args };
1026+
// push the arg onto the front
1027+
_startupActions.insert(_startupActions.begin(), 1, newTabAction);
10461028
}
10471029
}
10481030
std::optional<uint32_t> AppCommandlineArgs::GetPersistedLayoutIdx() const noexcept
@@ -1082,15 +1064,6 @@ std::optional<til::size> AppCommandlineArgs::GetSize() const noexcept
10821064
// - 0 if the commandline was successfully parsed
10831065
int AppCommandlineArgs::ParseArgs(winrt::array_view<const winrt::hstring> args)
10841066
{
1085-
for (const auto& arg : args)
1086-
{
1087-
if (arg == L"-Embedding")
1088-
{
1089-
_isHandoffListener = true;
1090-
return 0;
1091-
}
1092-
}
1093-
10941067
auto commands = ::TerminalApp::AppCommandlineArgs::BuildCommands(args);
10951068

10961069
for (auto& cmdBlob : commands)
@@ -1195,7 +1168,6 @@ void AppCommandlineArgs::FullResetState()
11951168
_startupActions.clear();
11961169
_exitMessage = "";
11971170
_shouldExitEarly = false;
1198-
_isHandoffListener = false;
11991171

12001172
_windowTarget = {};
12011173
}

src/cascadia/TerminalApp/AppCommandlineArgs.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ class TerminalApp::AppCommandlineArgs final
3535

3636
void ValidateStartupCommands();
3737
std::vector<winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs>& GetStartupActions();
38-
bool IsHandoffListener() const noexcept;
3938
const std::string& GetExitMessage() const noexcept;
4039
bool ShouldExitEarly() const noexcept;
4140

@@ -132,7 +131,6 @@ class TerminalApp::AppCommandlineArgs final
132131
std::optional<winrt::Microsoft::Terminal::Settings::Model::LaunchMode> _launchMode{ std::nullopt };
133132
std::optional<winrt::Microsoft::Terminal::Settings::Model::LaunchPosition> _position{ std::nullopt };
134133
std::optional<til::size> _size{ std::nullopt };
135-
bool _isHandoffListener{ false };
136134
std::vector<winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs> _startupActions;
137135
std::string _exitMessage;
138136
bool _shouldExitEarly{ false };

src/cascadia/TerminalApp/Remoting.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ namespace winrt::TerminalApp::implementation
2020
winrt::com_array<winrt::hstring>& CommandlineRef() noexcept;
2121

2222
// These bits are exposed via WinRT:
23-
public:
23+
til::property<winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection> Connection;
24+
2425
int32_t ExitCode() const noexcept;
2526
winrt::hstring ExitMessage() const;
2627
winrt::hstring TargetWindow() const;

src/cascadia/TerminalApp/Remoting.idl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ namespace TerminalApp
88
CommandlineArgs();
99
CommandlineArgs(String[] args, String cwd, UInt32 showWindowCommand, String env);
1010

11+
Microsoft.Terminal.TerminalConnection.ITerminalConnection Connection;
12+
1113
Int32 ExitCode { get; };
1214
String ExitMessage { get; };
1315
String TargetWindow { get; };

src/cascadia/TerminalApp/TerminalPage.cpp

Lines changed: 9 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
#include <LibraryResources.h>
99
#include <TerminalCore/ControlKeyStates.hpp>
10-
#include <til/latch.h>
1110
#include <Utils.h>
1211

1312
#include "../../types/inc/utils.hpp"
@@ -293,12 +292,11 @@ namespace winrt::TerminalApp::implementation
293292
// - true if we're not elevated but all relevant pane-spawning actions are elevated
294293
bool TerminalPage::ShouldImmediatelyHandoffToElevated(const CascadiaSettings& settings) const
295294
{
296-
// GH#12267: Don't forget about defterm handoff here. If we're being
297-
// created for embedding, then _yea_, we don't need to handoff to an
298-
// elevated window.
299-
if (_startupActions.empty() || IsRunningElevated() || _shouldStartInboundListener)
295+
if (_startupActions.empty() || _startupConnection || IsRunningElevated())
300296
{
301-
// there aren't startup actions, or we're elevated. In that case, go for it.
297+
// No point in handing off if we got no startup actions, or we're already elevated.
298+
// Also, we shouldn't need to elevate handoff ConPTY connections.
299+
assert(!_startupConnection);
302300
return false;
303301
}
304302

@@ -488,45 +486,9 @@ namespace winrt::TerminalApp::implementation
488486
{
489487
_startupState = StartupState::InStartup;
490488

491-
ProcessStartupActions(std::move(_startupActions), true);
492-
493-
// If we were told that the COM server needs to be started to listen for incoming
494-
// default application connections, start it now.
495-
// This MUST be done after we've registered the event listener for the new connections
496-
// or the COM server might start receiving requests on another thread and dispatch
497-
// them to nowhere.
498-
_StartInboundListener();
499-
}
500-
}
501-
502-
// Routine Description:
503-
// - Will start the listener for inbound console handoffs if we have already determined
504-
// that we should do so.
505-
// NOTE: Must be after TerminalPage::_OnNewConnection has been connected up.
506-
// Arguments:
507-
// - <unused> - Looks at _shouldStartInboundListener
508-
// Return Value:
509-
// - <none> - May fail fast if setup fails as that would leave us in a weird state.
510-
void TerminalPage::_StartInboundListener()
511-
{
512-
if (_shouldStartInboundListener)
513-
{
514-
_shouldStartInboundListener = false;
515-
516-
// Hook up inbound connection event handler
517-
_newConnectionRevoker = ConptyConnection::NewConnection(winrt::auto_revoke, { this, &TerminalPage::_OnNewConnection });
518-
519-
try
520-
{
521-
winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::StartInboundListener();
522-
}
523-
// If we failed to start the listener, it will throw.
524-
// We don't want to fail fast here because if a peasant has some trouble with
525-
// starting the listener, we don't want it to crash and take all its tabs down
526-
// with it.
527-
catch (...)
489+
if (!_startupActions.empty())
528490
{
529-
LOG_CAUGHT_EXCEPTION();
491+
ProcessStartupActions(std::move(_startupActions), true);
530492
}
531493
}
532494
}
@@ -629,7 +591,7 @@ namespace winrt::TerminalApp::implementation
629591
// GH#12267: Make sure that we don't instantly close ourselves when
630592
// we're readying to accept a defterm connection. In that case, we don't
631593
// have a tab yet, but will once we're initialized.
632-
if (_tabs.Size() == 0 && !_shouldStartInboundListener && !_isEmbeddingInboundListener)
594+
if (_tabs.Size() == 0)
633595
{
634596
CloseWindowRequested.raise(*this, nullptr);
635597
co_return;
@@ -3653,25 +3615,9 @@ namespace winrt::TerminalApp::implementation
36533615
_startupActions = std::move(actions);
36543616
}
36553617

3656-
// Routine Description:
3657-
// - Notifies this Terminal Page that it should start the incoming connection
3658-
// listener for command-line tools attempting to join this Terminal
3659-
// through the default application channel.
3660-
// Arguments:
3661-
// - isEmbedding - True if COM started us to be a server. False if we're doing it of our own accord.
3662-
// Return Value:
3663-
// - <none>
3664-
void TerminalPage::SetInboundListener(bool isEmbedding)
3618+
void TerminalPage::SetStartupConnection(ITerminalConnection connection)
36653619
{
3666-
_shouldStartInboundListener = true;
3667-
_isEmbeddingInboundListener = isEmbedding;
3668-
3669-
// If the page has already passed the NotInitialized state,
3670-
// then it is ready-enough for us to just start this immediately.
3671-
if (_startupState != StartupState::NotInitialized)
3672-
{
3673-
_StartInboundListener();
3674-
}
3620+
_startupConnection = std::move(connection);
36753621
}
36763622

36773623
winrt::TerminalApp::IDialogPresenter TerminalPage::DialogPresenter() const
@@ -4073,68 +4019,6 @@ namespace winrt::TerminalApp::implementation
40734019
ChangeMaximizeRequested.raise(*this, nullptr);
40744020
}
40754021

4076-
HRESULT TerminalPage::_OnNewConnection(const ConptyConnection& connection)
4077-
{
4078-
_newConnectionRevoker.revoke();
4079-
4080-
// We need to be on the UI thread in order for _OpenNewTab to run successfully.
4081-
// HasThreadAccess will return true if we're currently on a UI thread and false otherwise.
4082-
// When we're on a COM thread, we'll need to dispatch the calls to the UI thread
4083-
// and wait on it hence the locking mechanism.
4084-
if (!Dispatcher().HasThreadAccess())
4085-
{
4086-
til::latch latch{ 1 };
4087-
auto finalVal = S_OK;
4088-
4089-
Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [&]() {
4090-
finalVal = _OnNewConnection(connection);
4091-
latch.count_down();
4092-
});
4093-
4094-
latch.wait();
4095-
return finalVal;
4096-
}
4097-
4098-
try
4099-
{
4100-
NewTerminalArgs newTerminalArgs;
4101-
newTerminalArgs.Commandline(connection.Commandline());
4102-
newTerminalArgs.TabTitle(connection.StartingTitle());
4103-
// GH #12370: We absolutely cannot allow a defterm connection to
4104-
// auto-elevate. Defterm doesn't work for elevated scenarios in the
4105-
// first place. If we try accepting the connection, the spawning an
4106-
// elevated version of the Terminal with that profile... that's a
4107-
// recipe for disaster. We won't ever open up a tab in this window.
4108-
newTerminalArgs.Elevate(false);
4109-
const auto newPane = _MakePane(newTerminalArgs, nullptr, connection);
4110-
newPane->WalkTree([](const auto& pane) {
4111-
pane->FinalizeConfigurationGivenDefault();
4112-
});
4113-
_CreateNewTabFromPane(newPane);
4114-
4115-
// Request a summon of this window to the foreground
4116-
SummonWindowRequested.raise(*this, nullptr);
4117-
4118-
// TEMPORARY SOLUTION
4119-
// If the connection has requested for the window to be maximized,
4120-
// manually maximize it here. Ideally, we should be _initializing_
4121-
// the session maximized, instead of manually maximizing it after initialization.
4122-
// However, because of the current way our defterm handoff works,
4123-
// we are unable to get the connection info before the terminal session
4124-
// has already started.
4125-
4126-
// Make sure that there were no other tabs already existing (in
4127-
// the case that we are in glomming mode), because we don't want
4128-
// to be maximizing other existing sessions that did not ask for it.
4129-
if (_tabs.Size() == 1 && connection.ShowWindow() == SW_SHOWMAXIMIZED)
4130-
{
4131-
RequestSetMaximized(true);
4132-
}
4133-
return S_OK;
4134-
}
4135-
CATCH_RETURN()
4136-
}
4137-
41384022
TerminalApp::IPaneContent TerminalPage::_makeSettingsContent()
41394023
{
41404024
if (auto app{ winrt::Windows::UI::Xaml::Application::Current().try_as<winrt::TerminalApp::App>() })

src/cascadia/TerminalApp/TerminalPage.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,8 @@ namespace winrt::TerminalApp::implementation
129129
void RequestSetMaximized(bool newMaximized);
130130

131131
void SetStartupActions(std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> actions);
132+
void SetStartupConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection);
132133

133-
void SetInboundListener(bool isEmbedding);
134134
static std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> ConvertExecuteCommandlineToActions(const Microsoft::Terminal::Settings::Model::ExecuteCommandlineArgs& args);
135135

136136
winrt::TerminalApp::IDialogPresenter DialogPresenter() const;
@@ -151,6 +151,7 @@ namespace winrt::TerminalApp::implementation
151151
const winrt::hstring cwd = winrt::hstring{},
152152
const winrt::hstring env = winrt::hstring{});
153153

154+
154155
TerminalApp::WindowProperties WindowProperties() const noexcept { return _WindowProperties; };
155156

156157
bool CanDragDrop() const noexcept;
@@ -256,8 +257,7 @@ namespace winrt::TerminalApp::implementation
256257
StartupState _startupState{ StartupState::NotInitialized };
257258

258259
std::vector<Microsoft::Terminal::Settings::Model::ActionAndArgs> _startupActions;
259-
bool _shouldStartInboundListener{ false };
260-
bool _isEmbeddingInboundListener{ false };
260+
winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _startupConnection{ nullptr };
261261

262262
std::shared_ptr<Toast> _windowIdToast{ nullptr };
263263
std::shared_ptr<Toast> _actionSavedToast{ nullptr };
@@ -281,8 +281,6 @@ namespace winrt::TerminalApp::implementation
281281
winrt::Windows::Foundation::Point dragOffset{ 0, 0 };
282282
} _stashed;
283283

284-
winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::NewConnection_revoker _newConnectionRevoker;
285-
286284
safe_void_coroutine _NewTerminalByDrop(const Windows::Foundation::IInspectable&, winrt::Windows::UI::Xaml::DragEventArgs e);
287285

288286
__declspec(noinline) CommandPalette _loadCommandPaletteSlowPath();
@@ -468,8 +466,6 @@ namespace winrt::TerminalApp::implementation
468466
void _SetNewTabButtonColor(const Windows::UI::Color& color, const Windows::UI::Color& accentColor);
469467
void _ClearNewTabButtonColor();
470468

471-
void _StartInboundListener();
472-
473469
safe_void_coroutine _CompleteInitialization();
474470

475471
void _FocusActiveControl(IInspectable sender, IInspectable eventArgs);

0 commit comments

Comments
 (0)