From 49ed479c39b113cb248df32f7b372898566c1a7d Mon Sep 17 00:00:00 2001 From: Jeroen Bastenhof Date: Sun, 11 Sep 2022 11:17:21 +0200 Subject: [PATCH] Fix duplication issue for unfocused tabs --- .../LocalTests_TerminalApp/TabTests.cpp | 10 +++--- .../TerminalApp/AppActionHandlers.cpp | 3 +- src/cascadia/TerminalApp/TabManagement.cpp | 6 ++-- src/cascadia/TerminalApp/TerminalPage.cpp | 32 ++++++++----------- src/cascadia/TerminalApp/TerminalPage.h | 2 +- 5 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index 6fe34c2eb68..53013f93721 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp @@ -503,7 +503,7 @@ namespace TerminalAppLocalTests Log::Comment(NoThrowString().Format(L"Duplicate the first pane")); result = RunOnUIThread([&page]() { - page->_SplitPane(SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, true, nullptr)); + page->_SplitPane(SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); VERIFY_ARE_EQUAL(1u, page->_tabs.Size()); auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0)); @@ -521,7 +521,7 @@ namespace TerminalAppLocalTests Log::Comment(NoThrowString().Format(L"Duplicate the pane, and don't crash")); result = RunOnUIThread([&page]() { - page->_SplitPane(SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, true, nullptr)); + page->_SplitPane(SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); VERIFY_ARE_EQUAL(1u, page->_tabs.Size()); auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0)); @@ -843,7 +843,7 @@ namespace TerminalAppLocalTests // | 1 | 2 | // | | | // ------------------- - page->_SplitPane(SplitDirection::Right, 0.5f, page->_MakePane(nullptr, true, nullptr)); + page->_SplitPane(SplitDirection::Right, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); secondId = tab->_activePane->Id().value(); }); Sleep(250); @@ -861,7 +861,7 @@ namespace TerminalAppLocalTests // | 3 | | // | | | // ------------------- - page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, true, nullptr)); + page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0)); // Split again to make the 3rd tab thirdId = tab->_activePane->Id().value(); @@ -881,7 +881,7 @@ namespace TerminalAppLocalTests // | 3 | 4 | // | | | // ------------------- - page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, true, nullptr)); + page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0)); fourthId = tab->_activePane->Id().value(); }); diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 18cae0d5262..650650ae285 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -235,10 +235,11 @@ namespace winrt::TerminalApp::implementation } } + const auto duplicateFromTab{ realArgs.SplitMode() == SplitType::Duplicate ? _GetFocusedTab() : nullptr }; _SplitPane(realArgs.SplitDirection(), // This is safe, we're already filtering so the value is (0, 1) ::base::saturated_cast(realArgs.SplitSize()), - _MakePane(realArgs.TerminalArgs(), realArgs.SplitMode() == SplitType::Duplicate)); + _MakePane(realArgs.TerminalArgs(), duplicateFromTab)); args.Handled(true); } } diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index ab9a6a318d6..4ec1a9a3b47 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -86,7 +86,7 @@ namespace winrt::TerminalApp::implementation // // This call to _MakePane won't return nullptr, we already checked that // case above with the _maybeElevate call. - _CreateNewTabFromPane(_MakePane(newTerminalArgs, false, existingConnection)); + _CreateNewTabFromPane(_MakePane(newTerminalArgs, nullptr, existingConnection)); const auto tabCount = _tabs.Size(); const auto usedManualProfile = (newTerminalArgs != nullptr) && @@ -364,7 +364,7 @@ namespace winrt::TerminalApp::implementation // In the future, it may be preferable to just duplicate the // current control's live settings (which will include changes // made through VT). - _CreateNewTabFromPane(_MakePane(nullptr, true, nullptr)); + _CreateNewTabFromPane(_MakePane(nullptr, tab, nullptr)); const auto runtimeTabText{ tab.GetTabText() }; if (!runtimeTabText.empty()) @@ -387,7 +387,7 @@ namespace winrt::TerminalApp::implementation try { _SetFocusedTab(tab); - _SplitPane(tab, SplitDirection::Automatic, 0.5f, _MakePane(nullptr, true)); + _SplitPane(tab, SplitDirection::Automatic, 0.5f, _MakePane(nullptr, tab)); } CATCH_LOG(); } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 873fd151ad2..3093b2585ea 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -2484,8 +2484,8 @@ namespace winrt::TerminalApp::implementation // - newTerminalArgs: an object that may contain a blob of parameters to // control which profile is created and with possible other // configurations. See CascadiaSettings::BuildSettings for more details. - // - duplicate: a boolean to indicate whether the pane we create should be - // a duplicate of the currently focused pane + // - sourceTab: an optional tab reference that indicates that the created + // pane should be a duplicate of the tab's focused pane // - existingConnection: optionally receives a connection from the outside // world instead of attempting to create one // Return Value: @@ -2493,29 +2493,25 @@ namespace winrt::TerminalApp::implementation // connection, then we'll return nullptr. Otherwise, we'll return a new // Pane for this connection. std::shared_ptr TerminalPage::_MakePane(const NewTerminalArgs& newTerminalArgs, - const bool duplicate, + const winrt::TerminalApp::TabBase& sourceTab, TerminalConnection::ITerminalConnection existingConnection) { TerminalSettingsCreateResult controlSettings{ nullptr }; Profile profile{ nullptr }; - if (duplicate) + if (const auto& terminalTab{ _GetTerminalTabImpl(sourceTab) }) { - const auto focusedTab{ _GetFocusedTabImpl() }; - if (focusedTab) + profile = terminalTab->GetFocusedProfile(); + if (profile) { - profile = focusedTab->GetFocusedProfile(); - if (profile) + // TODO GH#5047 If we cache the NewTerminalArgs, we no longer need to do this. + profile = GetClosestProfileForDuplicationOfProfile(profile); + controlSettings = TerminalSettings::CreateWithProfile(_settings, profile, *_bindings); + const auto workingDirectory = terminalTab->GetActiveTerminalControl().WorkingDirectory(); + const auto validWorkingDirectory = !workingDirectory.empty(); + if (validWorkingDirectory) { - // TODO GH#5047 If we cache the NewTerminalArgs, we no longer need to do this. - profile = GetClosestProfileForDuplicationOfProfile(profile); - controlSettings = TerminalSettings::CreateWithProfile(_settings, profile, *_bindings); - const auto workingDirectory = focusedTab->GetActiveTerminalControl().WorkingDirectory(); - const auto validWorkingDirectory = !workingDirectory.empty(); - if (validWorkingDirectory) - { - controlSettings.DefaultSettings().StartingDirectory(workingDirectory); - } + controlSettings.DefaultSettings().StartingDirectory(workingDirectory); } } } @@ -3292,7 +3288,7 @@ 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); - const auto newPane = _MakePane(newTerminalArgs, false, connection); + const auto newPane = _MakePane(newTerminalArgs, nullptr, connection); newPane->WalkTree([](auto pane) { pane->FinalizeConfigurationGivenDefault(); }); diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 8b0ca266b05..e2291270b1e 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -388,7 +388,7 @@ namespace winrt::TerminalApp::implementation const winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection& connection); std::shared_ptr _MakePane(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr, - const bool duplicate = false, + const winrt::TerminalApp::TabBase& sourceTab = nullptr, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr); void _RefreshUIForSettingsReload();