starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-05-20 13:57:43 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Fix two panes being closed when just one is

Browse Source
This commit is contained in:
Leonard Hecker
2024-06-03 22:09:19 +02:00
parent 01e4df152e
commit e770c137dc
7 changed files with 48 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
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() };
@@ -985,17 +985,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);
}
@@ -1007,7 +997,7 @@ void Pane::Shutdown()
{
if (_IsLeaf())
{
_content.Close();
_setPaneContent(nullptr);
}
else
{
@@ -1411,7 +1401,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(std::move(remainingChild->_content));
_id = remainingChild->Id();
// Revoke the old event handlers. Remove both the handlers for the panes
@@ -1716,6 +1706,25 @@ void Pane::_SetupChildCloseHandlers()
});
}
void Pane::_setPaneContent(IPaneContent content)
{
// The IPaneContent::Close() implementation may be buggy and raise the CloseRequested event again.
// To avoid this we unbind the event handlers before calling Close().
_closeRequestedRevoker.revoke();
if (_content)
{
_content.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
@@ -2266,8 +2275,8 @@ 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>(std::move(_content));
_setPaneContent(nullptr);
_firstChild->_broadcastEnabled = _broadcastEnabled;
}
@@ -2462,6 +2471,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:

10
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,
@@ -248,13 +248,13 @@ 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 };
winrt::Windows::UI::Xaml::UIElement::GotFocus_revoker _gotFocusRevoker;
winrt::Windows::UI::Xaml::UIElement::LostFocus_revoker _lostFocusRevoker;
winrt::TerminalApp::IPaneContent::CloseRequested_revoker _closeRequestedRevoker;
Borders _borders{ Borders::None };
@@ -264,11 +264,9 @@ private:
bool _IsLeaf() const noexcept;
bool _HasFocusedChild() const noexcept;
void _SetupChildCloseHandlers();
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;