starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-07 21:25:41 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

PRE-MERGE #17358 Fix two panes being closed when just one is

Browse Source
This commit is contained in:
Mike Griese
2024-06-04 05:55:26 -05:00
parent 4485e50cbc 3606188eec
commit 274010f2f3
8 changed files with 63 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
src/cascadia/TerminalApp/Pane.cpp
View File

@@ -27,10 +27,10 @@ static const int CombinedPaneBorderSize = 2 * PaneBorderSize;
static const int AnimationDurationInMilliseconds = 200;
static const Duration AnimationDuration = DurationHelper::FromTimeSpan(winrt::Windows::Foundation::TimeSpan(std::chrono::milliseconds(AnimationDurationInMilliseconds)));
Pane::Pane(const IPaneContent& content, const bool lastFocused) :
_content{ content },
Pane::Pane(IPaneContent content, const bool lastFocused) :
_lastActive{ lastFocused }
{
_setPaneContent(std::move(content));
_root.Children().Append(_borderFirst);
const auto& control{ _content.GetRoot() };
@@ -1172,17 +1172,7 @@ void Pane::_ContentLostFocusHandler(const winrt::Windows::Foundation::IInspectab
// - <none>
void Pane::Close()
{
// Pane has two events, CloseRequested and Closed. CloseRequested is raised by the content asking to be closed,
// but also by the window who owns the tab when it's closing. The event is then caught by the TerminalTab which
// calls Close() which then raises the Closed event. Now, if this is the last pane in the window, this will result
// in the window raising CloseRequested again which leads to infinite recursion, so we need to guard against that.
// Ideally we would have just a single event in the future.
if (_closed)
{
return;
}
_closed = true;
_setPaneContent(nullptr);
// Fire our Closed event to tell our parent that we should be removed.
Closed.raise(nullptr, nullptr);
}
@@ -1194,7 +1184,7 @@ void Pane::Shutdown()
{
if (_IsLeaf())
{
_content.Close();
_setPaneContent(nullptr);
}
else
{
@@ -1598,7 +1588,7 @@ void Pane::_CloseChild(const bool closeFirst)
_borders = _GetCommonBorders();
// take the control, profile, id and isDefTermSession of the pane that _wasn't_ closed.
_content = remainingChild->_content;
_setPaneContent(remainingChild->_takePaneContent());
_id = remainingChild->Id();
// Revoke the old event handlers. Remove both the handlers for the panes
@@ -1903,6 +1893,34 @@ void Pane::_SetupChildCloseHandlers()
});
}
// With this method you take ownership of the control from this Pane.
// Assign it to another Pane with _setPaneContent() or Close() it.
IPaneContent Pane::_takePaneContent()
{
_closeRequestedRevoker.revoke();
return std::move(_content);
}
// This method safely sets the content of the Pane. It'll ensure to revoke and
// assign event handlers, and to Close() the existing content if there's any.
// The new content can be nullptr to remove any content.
void Pane::_setPaneContent(IPaneContent content)
{
// The IPaneContent::Close() implementation may be buggy and raise the CloseRequested event again.
// _takePaneContent() avoids this as it revokes the event handler.
if (const auto c = _takePaneContent())
{
c.Close();
}
_content = std::move(content);
if (_content)
{
_closeRequestedRevoker = _content.CloseRequested(winrt::auto_revoke, [this](auto&&, auto&&) { Close(); });
}
}
// Method Description:
// - Sets up row/column definitions for this pane. There are three total
// row/cols. The middle one is for the separator. The first and third are for
@@ -2470,8 +2488,7 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitDirect
else
{
// Move our control, guid, isDefTermSession into the first one.
_firstChild = std::make_shared<Pane>(_content);
_content = nullptr;
_firstChild = std::make_shared<Pane>(_takePaneContent());
_firstChild->_broadcastEnabled = _broadcastEnabled;
}
@@ -2669,6 +2686,11 @@ bool Pane::_HasChild(const std::shared_ptr<Pane> child)
});
}
winrt::TerminalApp::TerminalPaneContent Pane::_getTerminalContent() const
{
return _IsLeaf() ? _content.try_as<winrt::TerminalApp::TerminalPaneContent>() : nullptr;
}
// Method Description:
// - Recursive function that finds a pane with the given ID
// Arguments:

13
src/cascadia/TerminalApp/Pane.h
View File

@@ -62,7 +62,7 @@ struct PaneResources
class Pane : public std::enable_shared_from_this<Pane>
{
public:
Pane(const winrt::TerminalApp::IPaneContent& content,
Pane(winrt::TerminalApp::IPaneContent content,
const bool lastFocused = false);
Pane(std::shared_ptr<Pane> first,
@@ -250,7 +250,6 @@ private:
std::optional<uint32_t> _id;
std::weak_ptr<Pane> _parentChildPath{};
bool _closed{ false };
bool _lastActive{ false };
winrt::event_token _firstClosedToken{ 0 };
winrt::event_token _secondClosedToken{ 0 };
@@ -260,10 +259,13 @@ private:
winrt::Windows::UI::Xaml::UIElement::GotFocus_revoker _gotFocusRevoker;
winrt::Windows::UI::Xaml::UIElement::LostFocus_revoker _lostFocusRevoker;
winrt::Windows::UI::Xaml::UIElement::ManipulationDelta_revoker _manipulationDeltaRevoker;
winrt::Windows::UI::Xaml::UIElement::ManipulationStarted_revoker _manipulationStartedRevoker;
bool _shouldManipulate{ false };
winrt::TerminalApp::IPaneContent::CloseRequested_revoker _closeRequestedRevoker;
Borders _borders{ Borders::None };
bool _zoomed{ false };
@@ -272,11 +274,10 @@ private:
bool _IsLeaf() const noexcept;
bool _HasFocusedChild() const noexcept;
void _SetupChildCloseHandlers();
winrt::TerminalApp::IPaneContent _takePaneContent();
void _setPaneContent(winrt::TerminalApp::IPaneContent content);
bool _HasChild(const std::shared_ptr<Pane> child);
winrt::TerminalApp::TerminalPaneContent _getTerminalContent() const
{
return _IsLeaf() ? _content.try_as<winrt::TerminalApp::TerminalPaneContent>() : nullptr;
}
winrt::TerminalApp::TerminalPaneContent _getTerminalContent() const;
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> _Split(winrt::Microsoft::Terminal::Settings::Model::SplitDirection splitType,
const float splitSize,

1
src/cascadia/TerminalApp/ScratchpadContent.cpp
View File

@@ -45,7 +45,6 @@ namespace winrt::TerminalApp::implementation
}
void ScratchpadContent::Close()
{
CloseRequested.raise(*this, nullptr);
}
INewContentArgs ScratchpadContent::GetNewTerminalArgs(const BuildStartupKind /* kind */) const

1
src/cascadia/TerminalApp/SettingsPaneContent.cpp
View File

@@ -47,7 +47,6 @@ namespace winrt::TerminalApp::implementation
}
void SettingsPaneContent::Close()
{
CloseRequested.raise(*this, nullptr);
}
INewContentArgs SettingsPaneContent::GetNewTerminalArgs(const BuildStartupKind /*kind*/) const

27
src/cascadia/TerminalApp/TerminalPaneContent.cpp
View File

@@ -78,8 +78,6 @@ namespace winrt::TerminalApp::implementation
_bellPlayer = nullptr;
_bellPlayerCreated = false;
}
CloseRequested.raise(*this, nullptr);
}
winrt::hstring TerminalPaneContent::Icon() const
@@ -239,19 +237,20 @@ namespace winrt::TerminalApp::implementation
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))
if (
// This one is obvious: If the user asked for "always" we do just that.
(mode == CloseOnExitMode::Always) ||
// Otherwise, and unless the user asked for the opposite of "always",
// close the pane when the connection closed gracefully (not failed).
(mode != CloseOnExitMode::Never && newConnectionState == ConnectionState::Closed) ||
// However, defterm handoff can result in Windows Terminal randomly opening which may be annoying,
// so by default we should at least always close the pane, even if the command failed.
// See GH #13325 for discussion.
(mode == CloseOnExitMode::Automatic && _isDefTermSession))
{
Close();
CloseRequested.raise(nullptr, nullptr);
}
}
}
@@ -331,7 +330,7 @@ namespace winrt::TerminalApp::implementation
void TerminalPaneContent::_closeTerminalRequestedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const winrt::Windows::Foundation::IInspectable& /*args*/)
{
Close();
CloseRequested.raise(nullptr, nullptr);
}
void TerminalPaneContent::_restartTerminalRequestedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/,

20
src/cascadia/TerminalApp/TerminalTab.cpp
View File

@@ -947,26 +947,6 @@ namespace winrt::TerminalApp::implementation
auto dispatcher = TabViewItem().Dispatcher();
ContentEventTokens events{};
events.CloseRequested = content.CloseRequested(
winrt::auto_revoke,
[this](auto&& sender, auto&&) {
if (const auto content{ sender.try_as<TerminalApp::IPaneContent>() })
{
// Calling Close() while walking the tree is not safe, because Close() mutates the tree.
const auto pane = _rootPane->_FindPane([&](const auto& p) -> std::shared_ptr<Pane> {
if (p->GetContent() == content)
{
return p;
}
return {};
});
if (pane)
{
pane->Close();
}
}
});
events.TitleChanged = content.TitleChanged(
winrt::auto_revoke,
[dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget {

1
src/cascadia/TerminalApp/TerminalTab.h
View File

@@ -134,7 +134,6 @@ namespace winrt::TerminalApp::implementation
winrt::TerminalApp::IPaneContent::ConnectionStateChanged_revoker ConnectionStateChanged;
winrt::TerminalApp::IPaneContent::ReadOnlyChanged_revoker ReadOnlyChanged;
winrt::TerminalApp::IPaneContent::FocusRequested_revoker FocusRequested;
winrt::TerminalApp::IPaneContent::CloseRequested_revoker CloseRequested;
// These events literally only apply if the content is a TermControl.
winrt::Microsoft::Terminal::Control::TermControl::KeySent_revoker KeySent;

10
src/cascadia/TerminalConnection/ConptyConnection.cpp
View File

@@ -446,12 +446,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
try
{
// GH#11556 - make sure to format the error code to this string as an UNSIGNED int
winrt::hstring exitText{ fmt::format(std::wstring_view{ RS_(L"ProcessExited") }, fmt::format(_errorFormat, status)) };
TerminalOutput.raise(L"\r\n");
TerminalOutput.raise(exitText);
TerminalOutput.raise(L"\r\n");
TerminalOutput.raise(RS_(L"CtrlDToClose"));
TerminalOutput.raise(L"\r\n");
const auto msg1 = fmt::format(std::wstring_view{ RS_(L"ProcessExited") }, fmt::format(_errorFormat, status));
const auto msg2 = RS_(L"CtrlDToClose");
const auto msg = fmt::format(FMT_COMPILE(L"\r\n{}\r\n{}\r\n"), msg1, msg2);
TerminalOutput.raise(msg);
}
CATCH_LOG();
}