From 1eab243f7fc6f7b339ede5657d18b18a6da8773e Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 24 Aug 2023 10:31:09 -0500 Subject: [PATCH] Raise ShortcutActions with the sender (tab, control) context (#15773) This PR's goal is to allow something like a `Tab` to raise a ShortcutAction, by saying "this action should be performed on ME". We've had a whole category of these issues in the past: * #15734 * #15760 * #13579 * #13942 * #13942 * Heck even dating back to #10832 So, this tries to remove a bit of that footgun. This probably isn't the _final_ form of what this refactor might look like, but the code is certainly better than before. Basically, there's a few bits: * `ShortcutActionDispatch.DoAction` now takes a `sender`, which can be _anything_. * Most actions that use a "Get the focused _thing_ then do something to it" are changed to "If there was a sender, let's use that - otherwise, we'll use the focused _thing_". * TerminalTab was largely refactored to use this, instead of making requests to the `TerminalPage` to just do a thing to it. I've got a few TODO!s left, but wanted to get initial feedback. * [x] `TerminalPage::_HandleTogglePaneZoom` * [x] `TerminalPage::_HandleFocusPane` * [x] `TerminalPage::_MoveTab` Closes #15734 (cherry picked from commit 30eb9eed49c53bb61b257192170dbf57aff0cac6) Service-Card-Id: 90438493 Service-Version: 1.18 --- .../LocalTests_TerminalApp/TabTests.cpp | 10 +- .../TerminalApp/AppActionHandlers.cpp | 131 +++++++++++------- src/cascadia/TerminalApp/AppLogic.cpp | 4 +- .../TerminalApp/ShortcutActionDispatch.cpp | 17 ++- .../TerminalApp/ShortcutActionDispatch.h | 4 +- .../TerminalApp/ShortcutActionDispatch.idl | 3 +- src/cascadia/TerminalApp/TabManagement.cpp | 77 +++------- src/cascadia/TerminalApp/TerminalPage.cpp | 80 ++++------- src/cascadia/TerminalApp/TerminalPage.h | 13 +- src/cascadia/TerminalApp/TerminalTab.cpp | 123 ++++++++-------- src/cascadia/TerminalApp/TerminalTab.h | 16 +-- src/cascadia/inc/WindowingBehavior.h | 7 + 12 files changed, 236 insertions(+), 249 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index 63f9d1ed67a..4a09828c680 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp @@ -517,7 +517,7 @@ namespace TerminalAppLocalTests Log::Comment(NoThrowString().Format(L"Duplicate the first pane")); result = RunOnUIThread([&page]() { - page->_SplitPane(SplitDirection::Automatic, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); + page->_SplitPane(nullptr, 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)); @@ -535,7 +535,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, page->_GetFocusedTab(), nullptr)); + page->_SplitPane(nullptr, 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)); @@ -857,7 +857,7 @@ namespace TerminalAppLocalTests // | 1 | 2 | // | | | // ------------------- - page->_SplitPane(SplitDirection::Right, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); + page->_SplitPane(nullptr, SplitDirection::Right, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); secondId = tab->_activePane->Id().value(); }); Sleep(250); @@ -875,7 +875,7 @@ namespace TerminalAppLocalTests // | 3 | | // | | | // ------------------- - page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); + page->_SplitPane(nullptr, 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(); @@ -895,7 +895,7 @@ namespace TerminalAppLocalTests // | 3 | 4 | // | | | // ------------------- - page->_SplitPane(SplitDirection::Down, 0.5f, page->_MakePane(nullptr, page->_GetFocusedTab(), nullptr)); + page->_SplitPane(nullptr, 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 cd548423b56..7f109a288d9 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -29,6 +29,29 @@ namespace winrt namespace winrt::TerminalApp::implementation { + TermControl TerminalPage::_senderOrActiveControl(const IInspectable& sender) + { + if (sender) + { + if (auto arg{ sender.try_as() }) + { + return arg; + } + } + return _GetActiveControl(); + } + winrt::com_ptr TerminalPage::_senderOrFocusedTab(const IInspectable& sender) + { + if (sender) + { + if (auto tab{ sender.try_as() }) + { + return _GetTerminalTabImpl(tab); + } + } + return _GetFocusedTabImpl(); + } + void TerminalPage::_HandleOpenNewTabDropdown(const IInspectable& /*sender*/, const ActionEventArgs& args) { @@ -149,7 +172,7 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleSendInput(const IInspectable& /*sender*/, + void TerminalPage::_HandleSendInput(const IInspectable& sender, const ActionEventArgs& args) { if (args == nullptr) @@ -158,7 +181,7 @@ namespace winrt::TerminalApp::implementation } else if (const auto& realArgs = args.ActionArgs().try_as()) { - if (const auto termControl{ _GetActiveControl() }) + if (const auto termControl{ _senderOrActiveControl(sender) }) { termControl.SendInput(realArgs.Input()); args.Handled(true); @@ -166,10 +189,10 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleCloseOtherPanes(const IInspectable& /*sender*/, + void TerminalPage::_HandleCloseOtherPanes(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto terminalTab{ _GetFocusedTabImpl() }) + if (const auto& terminalTab{ _senderOrFocusedTab(sender) }) { const auto activePane = terminalTab->GetActivePane(); if (terminalTab->GetRootPane() != activePane) @@ -214,7 +237,7 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleSplitPane(const IInspectable& /*sender*/, + void TerminalPage::_HandleSplitPane(const IInspectable& sender, const ActionEventArgs& args) { if (args == nullptr) @@ -236,7 +259,11 @@ namespace winrt::TerminalApp::implementation } const auto& duplicateFromTab{ realArgs.SplitMode() == SplitType::Duplicate ? _GetFocusedTab() : nullptr }; - _SplitPane(realArgs.SplitDirection(), + + const auto& terminalTab{ _senderOrFocusedTab(sender) }; + + _SplitPane(terminalTab, + realArgs.SplitDirection(), // This is safe, we're already filtering so the value is (0, 1) ::base::saturated_cast(realArgs.SplitSize()), _MakePane(realArgs.TerminalArgs(), duplicateFromTab)); @@ -251,33 +278,27 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } - void TerminalPage::_HandleTogglePaneZoom(const IInspectable& /*sender*/, + void TerminalPage::_HandleTogglePaneZoom(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto terminalTab{ _senderOrFocusedTab(sender) }) { // Don't do anything if there's only one pane. It's already zoomed. - if (activeTab->GetLeafPaneCount() > 1) + if (terminalTab->GetLeafPaneCount() > 1) { - // First thing's first, remove the current content from the UI - // tree. This is important, because we might be leaving zoom, and if - // a pane is zoomed, then it's currently in the UI tree, and should - // be removed before it's re-added in Pane::Restore - _tabContent.Children().Clear(); - // Togging the zoom on the tab will cause the tab to inform us of // the new root Content for this tab. - activeTab->ToggleZoom(); + terminalTab->ToggleZoom(); } } args.Handled(true); } - void TerminalPage::_HandleTogglePaneReadOnly(const IInspectable& /*sender*/, + void TerminalPage::_HandleTogglePaneReadOnly(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { activeTab->TogglePaneReadOnly(); } @@ -285,10 +306,10 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } - void TerminalPage::_HandleEnablePaneReadOnly(const IInspectable& /*sender*/, + void TerminalPage::_HandleEnablePaneReadOnly(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { activeTab->SetPaneReadOnly(true); } @@ -296,10 +317,10 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } - void TerminalPage::_HandleDisablePaneReadOnly(const IInspectable& /*sender*/, + void TerminalPage::_HandleDisablePaneReadOnly(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { activeTab->SetPaneReadOnly(false); } @@ -529,11 +550,12 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleFind(const IInspectable& /*sender*/, + void TerminalPage::_HandleFind(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { + _SetFocusedTab(*activeTab); _Find(*activeTab); } args.Handled(true); @@ -637,7 +659,7 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleSetTabColor(const IInspectable& /*sender*/, + void TerminalPage::_HandleSetTabColor(const IInspectable& sender, const ActionEventArgs& args) { Windows::Foundation::IReference tabColor; @@ -647,7 +669,7 @@ namespace winrt::TerminalApp::implementation tabColor = realArgs.TabColor(); } - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { if (tabColor) { @@ -661,17 +683,22 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } - void TerminalPage::_HandleOpenTabColorPicker(const IInspectable& /*sender*/, + void TerminalPage::_HandleOpenTabColorPicker(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { - activeTab->RequestColorPicker(); + if (!_tabColorPicker) + { + _tabColorPicker = winrt::make(); + } + + activeTab->AttachColorPicker(_tabColorPicker); } args.Handled(true); } - void TerminalPage::_HandleRenameTab(const IInspectable& /*sender*/, + void TerminalPage::_HandleRenameTab(const IInspectable& sender, const ActionEventArgs& args) { std::optional title; @@ -681,7 +708,7 @@ namespace winrt::TerminalApp::implementation title = realArgs.Title(); } - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { if (title.has_value()) { @@ -695,10 +722,10 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } - void TerminalPage::_HandleOpenTabRenamer(const IInspectable& /*sender*/, + void TerminalPage::_HandleOpenTabRenamer(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { activeTab->ActivateTabRenamer(); } @@ -807,12 +834,12 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } - void TerminalPage::_HandleMoveTab(const IInspectable& /*sender*/, + void TerminalPage::_HandleMoveTab(const IInspectable& sender, const ActionEventArgs& actionArgs) { if (const auto& realArgs = actionArgs.ActionArgs().try_as()) { - auto moved = _MoveTab(realArgs); + auto moved = _MoveTab(_senderOrFocusedTab(sender), realArgs); actionArgs.Handled(moved); } } @@ -1048,6 +1075,16 @@ namespace winrt::TerminalApp::implementation if (const auto& realArgs = args.ActionArgs().try_as()) { const auto paneId = realArgs.Id(); + + // This action handler is not enlightened for _senderOrFocusedTab. + // There's currently no way for an inactive tab to be the sender of a focusPane command. + // If that ever changes, then we'll need to consider how this handler should behave. + // Should it + // * focus the tab that sent the command AND activate the requested pane? + // * or should it just activate the pane in the sender, and leave the focused tab alone? + // + // For now, we'll just focus the pane in the focused tab. + if (const auto activeTab{ _GetFocusedTabImpl() }) { _UnZoomIfNeeded(); @@ -1064,10 +1101,10 @@ namespace winrt::TerminalApp::implementation args.Handled(true); } - void TerminalPage::_HandleExportBuffer(const IInspectable& /*sender*/, + void TerminalPage::_HandleExportBuffer(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { if (args) { @@ -1135,10 +1172,10 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleSelectAll(const IInspectable& /*sender*/, + void TerminalPage::_HandleSelectAll(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto& control{ _GetActiveControl() }) + if (const auto& control{ _senderOrActiveControl(sender) }) { control.SelectAll(); args.Handled(true); @@ -1174,30 +1211,30 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleMarkMode(const IInspectable& /*sender*/, + void TerminalPage::_HandleMarkMode(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto& control{ _GetActiveControl() }) + if (const auto& control{ _senderOrActiveControl(sender) }) { control.ToggleMarkMode(); args.Handled(true); } } - void TerminalPage::_HandleToggleBlockSelection(const IInspectable& /*sender*/, + void TerminalPage::_HandleToggleBlockSelection(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto& control{ _GetActiveControl() }) + if (const auto& control{ _senderOrActiveControl(sender) }) { const auto handled = control.ToggleBlockSelection(); args.Handled(handled); } } - void TerminalPage::_HandleSwitchSelectionEndpoint(const IInspectable& /*sender*/, + void TerminalPage::_HandleSwitchSelectionEndpoint(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto& control{ _GetActiveControl() }) + if (const auto& control{ _senderOrActiveControl(sender) }) { const auto handled = control.SwitchSelectionEndpoint(); args.Handled(handled); @@ -1229,10 +1266,10 @@ namespace winrt::TerminalApp::implementation } } - void TerminalPage::_HandleRestartConnection(const IInspectable& /*sender*/, + void TerminalPage::_HandleRestartConnection(const IInspectable& sender, const ActionEventArgs& args) { - if (const auto activeTab{ _GetFocusedTabImpl() }) + if (const auto activeTab{ _senderOrFocusedTab(sender) }) { if (const auto activePane{ activeTab->GetActivePane() }) { diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index a31246a031c..fb39ebf72fd 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -597,11 +597,11 @@ namespace winrt::TerminalApp::implementation // use as a title though! // // First, check the reserved keywords: - if (parsedTarget == "new") + if (parsedTarget == NewWindow) { return winrt::make(WindowingBehaviorUseNew); } - else if (parsedTarget == "last") + else if (parsedTarget == MostRecentlyUsedWindow) { return winrt::make(WindowingBehaviorUseExisting); } diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp b/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp index 8b7a15cd312..b44e44976e3 100644 --- a/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp +++ b/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp @@ -10,11 +10,11 @@ using namespace winrt::Microsoft::Terminal; using namespace winrt::Microsoft::Terminal::Settings::Model; using namespace winrt::TerminalApp; -#define ACTION_CASE(action) \ - case ShortcutAction::action: \ - { \ - _##action##Handlers(*this, eventArgs); \ - break; \ +#define ACTION_CASE(action) \ + case ShortcutAction::action: \ + { \ + action.raise(sender, eventArgs); \ + break; \ } namespace winrt::TerminalApp::implementation @@ -27,7 +27,8 @@ namespace winrt::TerminalApp::implementation // - actionAndArgs: the ShortcutAction and associated args to raise an event for. // Return Value: // - true if we handled the event was handled, else false. - bool ShortcutActionDispatch::DoAction(const ActionAndArgs& actionAndArgs) + bool ShortcutActionDispatch::DoAction(const winrt::Windows::Foundation::IInspectable& sender, + const ActionAndArgs& actionAndArgs) { if (!actionAndArgs) { @@ -50,4 +51,8 @@ namespace winrt::TerminalApp::implementation return eventArgs.Handled(); } + bool ShortcutActionDispatch::DoAction(const ActionAndArgs& actionAndArgs) + { + return DoAction(nullptr, actionAndArgs); + } } diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.h b/src/cascadia/TerminalApp/ShortcutActionDispatch.h index de4f0e3d9af..d43d7fc2421 100644 --- a/src/cascadia/TerminalApp/ShortcutActionDispatch.h +++ b/src/cascadia/TerminalApp/ShortcutActionDispatch.h @@ -13,7 +13,7 @@ namespace TerminalAppLocalTests class KeyBindingsTests; } -#define DECLARE_ACTION(action) TYPED_EVENT(action, TerminalApp::ShortcutActionDispatch, Microsoft::Terminal::Settings::Model::ActionEventArgs); +#define DECLARE_ACTION(action) til::typed_event action; namespace winrt::TerminalApp::implementation { @@ -22,6 +22,8 @@ namespace winrt::TerminalApp::implementation ShortcutActionDispatch() = default; bool DoAction(const Microsoft::Terminal::Settings::Model::ActionAndArgs& actionAndArgs); + bool DoAction(const winrt::Windows::Foundation::IInspectable& sender, + const Microsoft::Terminal::Settings::Model::ActionAndArgs& actionAndArgs); #define ON_ALL_ACTIONS(action) DECLARE_ACTION(action); ALL_SHORTCUT_ACTIONS diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.idl b/src/cascadia/TerminalApp/ShortcutActionDispatch.idl index 74d4bed4e42..2798df77ac3 100644 --- a/src/cascadia/TerminalApp/ShortcutActionDispatch.idl +++ b/src/cascadia/TerminalApp/ShortcutActionDispatch.idl @@ -2,7 +2,7 @@ // Licensed under the MIT license. #include "../TerminalSettingsModel/AllShortcutActions.h" -#define ACTION_EVENT(name) event Windows.Foundation.TypedEventHandler name +#define ACTION_EVENT(name) event Windows.Foundation.TypedEventHandler name namespace TerminalApp { @@ -10,6 +10,7 @@ namespace TerminalApp ShortcutActionDispatch(); Boolean DoAction(Microsoft.Terminal.Settings.Model.ActionAndArgs actionAndArgs); + Boolean DoAction(Object sender, Microsoft.Terminal.Settings.Model.ActionAndArgs actionAndArgs); // When adding a new action, add them to AllShortcutActions.h! #define ON_ALL_ACTIONS(action) ACTION_EVENT(action); diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index 58539b1fbcc..dffc666c18c 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -168,49 +168,6 @@ namespace winrt::TerminalApp::implementation } }); - newTabImpl->DuplicateRequested([weakTab, weakThis{ get_weak() }]() { - auto page{ weakThis.get() }; - auto tab{ weakTab.get() }; - - if (page && tab) - { - page->_DuplicateTab(*tab); - } - }); - - newTabImpl->SplitTabRequested([weakTab, weakThis{ get_weak() }]() { - auto page{ weakThis.get() }; - auto tab{ weakTab.get() }; - - if (page && tab) - { - page->_SplitTab(*tab); - } - }); - - newTabImpl->ExportTabRequested([weakTab, weakThis{ get_weak() }]() { - auto page{ weakThis.get() }; - auto tab{ weakTab.get() }; - - if (page && tab) - { - // Passing empty string as the path to export tab will make it - // prompt for the path - page->_ExportTab(*tab, L""); - } - }); - - newTabImpl->FindRequested([weakTab, weakThis{ get_weak() }]() { - auto page{ weakThis.get() }; - auto tab{ weakTab.get() }; - - if (page && tab) - { - page->_SetFocusedTab(*tab); - page->_Find(*tab); - } - }); - auto tabViewItem = newTabImpl->TabViewItem(); _tabView.TabItems().InsertAt(insertPosition, tabViewItem); @@ -355,20 +312,6 @@ namespace winrt::TerminalApp::implementation CATCH_LOG(); } - // Method Description: - // - Sets the specified tab as the focused tab and splits its active pane - // Arguments: - // - tab: tab to split - void TerminalPage::_SplitTab(TerminalTab& tab) - { - try - { - _SetFocusedTab(tab); - _SplitPane(tab, SplitDirection::Automatic, 0.5f, _MakePane(nullptr, tab)); - } - CATCH_LOG(); - } - // Method Description: // - Exports the content of the Terminal Buffer inside the tab // Arguments: @@ -676,6 +619,21 @@ namespace winrt::TerminalApp::implementation return std::nullopt; } + // Method Description: + // - Returns the index in our list of tabs of the currently focused tab. If + // no tab is currently selected, returns nullopt. + // Return Value: + // - the index of the currently focused tab if there is one, else nullopt + std::optional TerminalPage::_GetTabIndex(const TerminalApp::TabBase& tab) const noexcept + { + uint32_t i; + if (_tabs.IndexOf(tab, i)) + { + return i; + } + return std::nullopt; + } + // Method Description: // - returns the currently focused tab. This might return null, // so make sure to check the result! @@ -731,7 +689,10 @@ namespace winrt::TerminalApp::implementation // sometimes set focus to an incorrect tab after removing some tabs auto weakThis{ get_weak() }; - co_await wil::resume_foreground(_tabView.Dispatcher()); + if (!_tabView.Dispatcher().HasThreadAccess()) + { + co_await winrt::resume_foreground(_tabView.Dispatcher()); + } if (auto page{ weakThis.get() }) { diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 275ff3597c5..f132d174fc1 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1158,7 +1158,8 @@ namespace winrt::TerminalApp::implementation } if (altPressed && !debugTap) { - this->_SplitPane(SplitDirection::Automatic, + this->_SplitPane(_GetFocusedTabImpl(), + SplitDirection::Automatic, 0.5f, newPane); } @@ -1694,20 +1695,6 @@ namespace winrt::TerminalApp::implementation } }); - hostingTab.ColorPickerRequested([weakTab, weakThis]() { - auto page{ weakThis.get() }; - auto tab{ weakTab.get() }; - if (page && tab) - { - if (!page->_tabColorPicker) - { - page->_tabColorPicker = winrt::make(); - } - - tab->AttachColorPicker(page->_tabColorPicker); - } - }); - // Add an event handler for when the terminal or tab wants to set a // progress indicator on the taskbar hostingTab.TaskbarProgressChanged({ get_weak(), &TerminalPage::_SetTaskbarProgressHandler }); @@ -2093,8 +2080,13 @@ namespace winrt::TerminalApp::implementation _RequestMoveContentHandlers(*this, *request); } - bool TerminalPage::_MoveTab(MoveTabArgs args) + bool TerminalPage::_MoveTab(winrt::com_ptr tab, MoveTabArgs args) { + if (!tab) + { + return false; + } + // If there was a windowId in the action, try to move it to the // specified window instead of moving it in our tab row. const auto windowId{ args.Window() }; @@ -2107,12 +2099,12 @@ namespace winrt::TerminalApp::implementation return true; } - if (const auto terminalTab{ _GetFocusedTabImpl() }) + if (tab) { - auto startupActions = terminalTab->BuildStartupActions(true); - _DetachTabFromWindow(terminalTab); + auto startupActions = tab->BuildStartupActions(true); + _DetachTabFromWindow(tab); _MoveContent(std::move(startupActions), args.Window(), 0); - _RemoveTab(*terminalTab); + _RemoveTab(*tab); return true; } } @@ -2120,9 +2112,13 @@ namespace winrt::TerminalApp::implementation const auto direction = args.Direction(); if (direction != MoveTabDirection::None) { - if (auto focusedTabIndex = _GetFocusedTabIndex()) + // Use the requested tab, if provided. Otherwise, use the currently + // focused tab. + const auto tabIndex = til::coalesce(_GetTabIndex(*tab), + _GetFocusedTabIndex()); + if (tabIndex) { - const auto currentTabIndex = focusedTabIndex.value(); + const auto currentTabIndex = tabIndex.value(); const auto delta = direction == MoveTabDirection::Forward ? 1 : -1; _TryMoveTab(currentTabIndex, currentTabIndex + delta); } @@ -2199,19 +2195,20 @@ namespace winrt::TerminalApp::implementation } // Method Description: - // - Split the focused pane either horizontally or vertically, and place the - // given pane accordingly in the tree + // - Split the focused pane of the given tab, either horizontally or vertically, and place the + // given pane accordingly // Arguments: + // - tab: The tab that is going to be split. // - newPane: the pane to add to our tree of panes // - splitDirection: one value from the TerminalApp::SplitDirection enum, indicating how the // new pane should be split from its parent. // - splitSize: the size of the split - void TerminalPage::_SplitPane(const SplitDirection splitDirection, + void TerminalPage::_SplitPane(const winrt::com_ptr& tab, + const SplitDirection splitDirection, const float splitSize, std::shared_ptr newPane) { - const auto focusedTab{ _GetFocusedTabImpl() }; - + auto activeTab = tab; // Clever hack for a crash in startup, with multiple sub-commands. Say // you have the following commandline: // @@ -2227,38 +2224,19 @@ namespace winrt::TerminalApp::implementation // Instead, let's just promote this first split to be a tab instead. // Crash avoided, and we don't need to worry about inserting a new-tab // command in at the start. - if (!focusedTab) + if (!tab) { if (_tabs.Size() == 0) { _CreateNewTabFromPane(newPane); + return; } else { - // The focused tab isn't a terminal tab - return; + activeTab = _GetFocusedTabImpl(); } } - else - { - _SplitPane(*focusedTab, splitDirection, splitSize, newPane); - } - } - // Method Description: - // - Split the focused pane of the given tab, either horizontally or vertically, and place the - // given pane accordingly - // Arguments: - // - tab: The tab that is going to be split. - // - newPane: the pane to add to our tree of panes - // - splitDirection: one value from the TerminalApp::SplitDirection enum, indicating how the - // new pane should be split from its parent. - // - splitSize: the size of the split - void TerminalPage::_SplitPane(TerminalTab& tab, - const SplitDirection splitDirection, - const float splitSize, - std::shared_ptr newPane) - { // If the caller is calling us with the return value of _MakePane // directly, it's possible that nullptr was returned, if the connections // was supposed to be launched in an elevated window. In that case, do @@ -2271,14 +2249,14 @@ namespace winrt::TerminalApp::implementation const auto contentHeight = ::base::saturated_cast(_tabContent.ActualHeight()); const winrt::Windows::Foundation::Size availableSpace{ contentWidth, contentHeight }; - const auto realSplitType = tab.PreCalculateCanSplit(splitDirection, splitSize, availableSpace); + const auto realSplitType = activeTab->PreCalculateCanSplit(splitDirection, splitSize, availableSpace); if (!realSplitType) { return; } _UnZoomIfNeeded(); - tab.SplitPane(*realSplitType, splitSize, newPane); + activeTab->SplitPane(*realSplitType, splitSize, newPane); // After GH#6586, the control will no longer focus itself // automatically when it's finished being laid out. Manually focus diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 81626087aba..e7e753a8c9c 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -326,7 +326,6 @@ namespace winrt::TerminalApp::implementation void _DuplicateFocusedTab(); void _DuplicateTab(const TerminalTab& tab); - void _SplitTab(TerminalTab& tab); winrt::fire_and_forget _ExportTab(const TerminalTab& tab, winrt::hstring filepath); winrt::Windows::Foundation::IAsyncAction _HandleCloseTabRequested(winrt::TerminalApp::TabBase tab); @@ -348,7 +347,7 @@ namespace winrt::TerminalApp::implementation bool _MoveFocus(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); bool _SwapPane(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); bool _MovePane(const Microsoft::Terminal::Settings::Model::MovePaneArgs args); - bool _MoveTab(const Microsoft::Terminal::Settings::Model::MoveTabArgs args); + bool _MoveTab(winrt::com_ptr tab, const Microsoft::Terminal::Settings::Model::MoveTabArgs args); template bool _ApplyToActiveControls(F f) @@ -372,6 +371,7 @@ namespace winrt::TerminalApp::implementation winrt::Microsoft::Terminal::Control::TermControl _GetActiveControl(); std::optional _GetFocusedTabIndex() const noexcept; + std::optional _GetTabIndex(const TerminalApp::TabBase& tab) const noexcept; TerminalApp::TabBase _GetFocusedTab() const noexcept; winrt::com_ptr _GetFocusedTabImpl() const noexcept; TerminalApp::TabBase _GetTabByTabViewItem(const Microsoft::UI::Xaml::Controls::TabViewItem& tabViewItem) const noexcept; @@ -387,10 +387,7 @@ namespace winrt::TerminalApp::implementation void _Scroll(ScrollDirection scrollDirection, const Windows::Foundation::IReference& rowsToScroll); - void _SplitPane(const Microsoft::Terminal::Settings::Model::SplitDirection splitType, - const float splitSize, - std::shared_ptr newPane); - void _SplitPane(TerminalTab& tab, + void _SplitPane(const winrt::com_ptr& tab, const Microsoft::Terminal::Settings::Model::SplitDirection splitType, const float splitSize, std::shared_ptr newPane); @@ -530,6 +527,10 @@ namespace winrt::TerminalApp::implementation void _SelectionMenuOpened(const IInspectable& sender, const IInspectable& args); void _PopulateContextMenu(const IInspectable& sender, const bool withSelection); winrt::Windows::UI::Xaml::Controls::MenuFlyout _CreateRunAsAdminFlyout(int profileIndex); + + winrt::Microsoft::Terminal::Control::TermControl _senderOrActiveControl(const winrt::Windows::Foundation::IInspectable& sender); + winrt::com_ptr _senderOrFocusedTab(const IInspectable& sender); + #pragma region ActionHandlers // These are all defined in AppActionHandlers.cpp #define ON_ALL_ACTIONS(action) DECLARE_ACTION_HANDLER(action); diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 9af0826c356..19634ad17b9 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -9,6 +9,7 @@ #include "Utils.h" #include "ColorHelper.h" #include "AppLogic.h" +#include "../inc/WindowingBehavior.h" using namespace winrt; using namespace winrt::Windows::UI::Xaml; @@ -1252,23 +1253,20 @@ namespace winrt::TerminalApp::implementation // "Color..." Controls::MenuFlyoutItem chooseColorMenuItem; - Controls::FontIcon colorPickSymbol; - colorPickSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); - colorPickSymbol.Glyph(L"\xE790"); + { + Controls::FontIcon colorPickSymbol; + colorPickSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); + colorPickSymbol.Glyph(L"\xE790"); - chooseColorMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->RequestColorPicker(); - } - }); - chooseColorMenuItem.Text(RS_(L"TabColorChoose")); - chooseColorMenuItem.Icon(colorPickSymbol); + chooseColorMenuItem.Click({ get_weak(), &TerminalTab::_chooseColorClicked }); + chooseColorMenuItem.Text(RS_(L"TabColorChoose")); + chooseColorMenuItem.Icon(colorPickSymbol); - const auto chooseColorToolTip = RS_(L"ChooseColorToolTip"); + const auto chooseColorToolTip = RS_(L"ChooseColorToolTip"); - WUX::Controls::ToolTipService::SetToolTip(chooseColorMenuItem, box_value(chooseColorToolTip)); - Automation::AutomationProperties::SetHelpText(chooseColorMenuItem, chooseColorToolTip); + WUX::Controls::ToolTipService::SetToolTip(chooseColorMenuItem, box_value(chooseColorToolTip)); + Automation::AutomationProperties::SetHelpText(chooseColorMenuItem, chooseColorToolTip); + } Controls::MenuFlyoutItem renameTabMenuItem; { @@ -1277,12 +1275,7 @@ namespace winrt::TerminalApp::implementation renameTabSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); renameTabSymbol.Glyph(L"\xE8AC"); // Rename - renameTabMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->ActivateTabRenamer(); - } - }); + renameTabMenuItem.Click({ get_weak(), &TerminalTab::_renameTabClicked }); renameTabMenuItem.Text(RS_(L"RenameTabText")); renameTabMenuItem.Icon(renameTabSymbol); @@ -1299,12 +1292,7 @@ namespace winrt::TerminalApp::implementation duplicateTabSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); duplicateTabSymbol.Glyph(L"\xF5ED"); - duplicateTabMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->_DuplicateRequestedHandlers(); - } - }); + duplicateTabMenuItem.Click({ get_weak(), &TerminalTab::_duplicateTabClicked }); duplicateTabMenuItem.Text(RS_(L"DuplicateTabText")); duplicateTabMenuItem.Icon(duplicateTabSymbol); @@ -1321,12 +1309,7 @@ namespace winrt::TerminalApp::implementation splitTabSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); splitTabSymbol.Glyph(L"\xF246"); // ViewDashboard - splitTabMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->_SplitTabRequestedHandlers(); - } - }); + splitTabMenuItem.Click({ get_weak(), &TerminalTab::_splitTabClicked }); splitTabMenuItem.Text(RS_(L"SplitTabText")); splitTabMenuItem.Icon(splitTabSymbol); @@ -1339,12 +1322,7 @@ namespace winrt::TerminalApp::implementation Controls::MenuFlyoutItem closePaneMenuItem = _closePaneMenuItem; { // "Close Pane" - closePaneMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->ClosePane(); - } - }); + closePaneMenuItem.Click({ get_weak(), &TerminalTab::_closePaneClicked }); closePaneMenuItem.Text(RS_(L"ClosePaneText")); const auto closePaneToolTip = RS_(L"ClosePaneToolTip"); @@ -1360,12 +1338,7 @@ namespace winrt::TerminalApp::implementation exportTabSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); exportTabSymbol.Glyph(L"\xE74E"); // Save - exportTabMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->_ExportTabRequestedHandlers(); - } - }); + exportTabMenuItem.Click({ get_weak(), &TerminalTab::_exportTextClicked }); exportTabMenuItem.Text(RS_(L"ExportTabText")); exportTabMenuItem.Icon(exportTabSymbol); @@ -1382,12 +1355,7 @@ namespace winrt::TerminalApp::implementation findSymbol.FontFamily(Media::FontFamily{ L"Segoe Fluent Icons, Segoe MDL2 Assets" }); findSymbol.Glyph(L"\xF78B"); // SearchMedium - findMenuItem.Click([weakThis](auto&&, auto&&) { - if (auto tab{ weakThis.get() }) - { - tab->_FindRequestedHandlers(); - } - }); + findMenuItem.Click({ get_weak(), &TerminalTab::_findClicked }); findMenuItem.Text(RS_(L"FindText")); findMenuItem.Icon(findSymbol); @@ -1509,20 +1477,6 @@ namespace winrt::TerminalApp::implementation return terminalBrush; } - // Method Description: - // - Send an event to request for the color picker - // - The listener should attach the color picker via AttachColorPicker() - // Arguments: - // - - // Return Value: - // - - void TerminalTab::RequestColorPicker() - { - ASSERT_UI_THREAD(); - - _ColorPickerRequestedHandlers(); - } - // - Get the total number of leaf panes in this tab. This will be the number // of actual controls hosted by this tab. // Arguments: @@ -1741,4 +1695,45 @@ namespace winrt::TerminalApp::implementation return Title(); } + + void TerminalTab::_chooseColorClicked(const winrt::Windows::Foundation::IInspectable& /* sender */, + const winrt::Windows::UI::Xaml::RoutedEventArgs& /* args */) + { + _dispatch.DoAction(*this, { ShortcutAction::OpenTabColorPicker, nullptr }); + } + void TerminalTab::_renameTabClicked(const winrt::Windows::Foundation::IInspectable& /* sender */, + const winrt::Windows::UI::Xaml::RoutedEventArgs& /* args */) + { + ActivateTabRenamer(); + } + void TerminalTab::_duplicateTabClicked(const winrt::Windows::Foundation::IInspectable& /* sender */, + const winrt::Windows::UI::Xaml::RoutedEventArgs& /* args */) + { + ActionAndArgs actionAndArgs{ ShortcutAction::DuplicateTab, nullptr }; + _dispatch.DoAction(*this, actionAndArgs); + } + void TerminalTab::_splitTabClicked(const winrt::Windows::Foundation::IInspectable& /* sender */, + const winrt::Windows::UI::Xaml::RoutedEventArgs& /* args */) + { + ActionAndArgs actionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Duplicate } }; + _dispatch.DoAction(*this, actionAndArgs); + } + void TerminalTab::_closePaneClicked(const winrt::Windows::Foundation::IInspectable& /* sender */, + const winrt::Windows::UI::Xaml::RoutedEventArgs& /* args */) + { + ClosePane(); + } + void TerminalTab::_exportTextClicked(const winrt::Windows::Foundation::IInspectable& /* sender */, + const winrt::Windows::UI::Xaml::RoutedEventArgs& /* args */) + { + ActionAndArgs actionAndArgs{}; + actionAndArgs.Action(ShortcutAction::ExportBuffer); + _dispatch.DoAction(*this, actionAndArgs); + } + void TerminalTab::_findClicked(const winrt::Windows::Foundation::IInspectable& /* sender */, + const winrt::Windows::UI::Xaml::RoutedEventArgs& /* args */) + { + ActionAndArgs actionAndArgs{ ShortcutAction::Find, nullptr }; + _dispatch.DoAction(*this, actionAndArgs); + } } diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index a4cc9aa8eca..4665aef9b67 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -71,7 +71,6 @@ namespace winrt::TerminalApp::implementation virtual std::optional GetTabColor() override; void SetRuntimeTabColor(const winrt::Windows::UI::Color& color); void ResetRuntimeTabColor(); - void RequestColorPicker(); void UpdateZoom(std::shared_ptr newFocus); void ToggleZoom(); @@ -98,11 +97,6 @@ namespace winrt::TerminalApp::implementation WINRT_CALLBACK(ActivePaneChanged, winrt::delegate<>); WINRT_CALLBACK(TabRaiseVisualBell, winrt::delegate<>); - WINRT_CALLBACK(DuplicateRequested, winrt::delegate<>); - WINRT_CALLBACK(SplitTabRequested, winrt::delegate<>); - WINRT_CALLBACK(FindRequested, winrt::delegate<>); - WINRT_CALLBACK(ExportTabRequested, winrt::delegate<>); - WINRT_CALLBACK(ColorPickerRequested, winrt::delegate<>); TYPED_EVENT(TaskbarProgressChanged, IInspectable, IInspectable); private: @@ -148,8 +142,6 @@ namespace winrt::TerminalApp::implementation bool _inRename{ false }; winrt::Windows::UI::Xaml::Controls::TextBox::LayoutUpdated_revoker _tabRenameBoxLayoutUpdatedRevoker; - winrt::TerminalApp::ShortcutActionDispatch _dispatch; - void _Setup(); std::optional _bellIndicatorTimer; @@ -178,6 +170,14 @@ namespace winrt::TerminalApp::implementation virtual winrt::Windows::UI::Xaml::Media::Brush _BackgroundBrush() override; + void _chooseColorClicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + void _renameTabClicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + void _duplicateTabClicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + void _splitTabClicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + void _closePaneClicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + void _exportTextClicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + void _findClicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + friend class ::TerminalAppLocalTests::TabTests; }; } diff --git a/src/cascadia/inc/WindowingBehavior.h b/src/cascadia/inc/WindowingBehavior.h index 348b2b51ab6..c00a33e9ce0 100644 --- a/src/cascadia/inc/WindowingBehavior.h +++ b/src/cascadia/inc/WindowingBehavior.h @@ -12,3 +12,10 @@ inline constexpr int32_t WindowingBehaviorUseName{ -4 }; inline constexpr int32_t WindowingBehaviorUseNone{ -5 }; inline constexpr std::wstring_view QuakeWindowName{ L"_quake" }; + +// Magic names for magic windowing behaviors. These are reserved names, in place +// of window names. "new" can also be used in MoveTab / MovePane actions. +// * new: to use a new window, always +// * last: use the most recent window +inline constexpr std::string_view NewWindow{ "new" }; +inline constexpr std::string_view MostRecentlyUsedWindow{ "last" };