diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index f56deecd411..be076a177a9 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp @@ -93,6 +93,8 @@ namespace TerminalAppLocalTests TEST_METHOD(TestPreviewDismissScheme); TEST_METHOD(TestPreviewSchemeWhilePreviewing); + TEST_METHOD(TestClampSwitchToTab); + TEST_CLASS_SETUP(ClassSetup) { return true; @@ -1342,4 +1344,55 @@ namespace TerminalAppLocalTests }); } + void TabTests::TestClampSwitchToTab() + { + Log::Comment(L"Test that switching to a tab index higher than the number of tabs just clamps to the last tab."); + + auto page = _commonSetup(); + VERIFY_IS_NOT_NULL(page); + + Log::Comment(L"Create a second tab"); + TestOnUIThread([&page]() { + NewTerminalArgs newTerminalArgs{ 1 }; + page->_OpenNewTab(newTerminalArgs); + }); + VERIFY_ARE_EQUAL(2u, page->_tabs.Size()); + + Log::Comment(L"Create a third tab"); + TestOnUIThread([&page]() { + NewTerminalArgs newTerminalArgs{ 2 }; + page->_OpenNewTab(newTerminalArgs); + }); + VERIFY_ARE_EQUAL(3u, page->_tabs.Size()); + + TestOnUIThread([&page]() { + auto focusedTabIndexOpt{ page->_GetFocusedTabIndex() }; + VERIFY_IS_TRUE(focusedTabIndexOpt.has_value()); + VERIFY_ARE_EQUAL(2u, focusedTabIndexOpt.value()); + }); + + TestOnUIThread([&page]() { + Log::Comment(L"Switch to the first tab"); + page->_SelectTab(0); + }); + + TestOnUIThread([&page]() { + auto focusedTabIndexOpt{ page->_GetFocusedTabIndex() }; + + VERIFY_IS_TRUE(focusedTabIndexOpt.has_value()); + VERIFY_ARE_EQUAL(0u, focusedTabIndexOpt.value()); + }); + + TestOnUIThread([&page]() { + Log::Comment(L"Switch to the tab 6, which is greater than number of tabs. This should switch to the third tab"); + page->_SelectTab(6); + }); + + TestOnUIThread([&page]() { + auto focusedTabIndexOpt{ page->_GetFocusedTabIndex() }; + VERIFY_IS_TRUE(focusedTabIndexOpt.has_value()); + VERIFY_ARE_EQUAL(2u, focusedTabIndexOpt.value()); + }); + } + } diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index 718d8d6e2fe..1c753aa4388 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -497,24 +497,25 @@ namespace winrt::TerminalApp::implementation // TerminalPage::_OnTabSelectionChanged // Return Value: // true iff we were able to select that tab index, false otherwise - bool TerminalPage::_SelectTab(const uint32_t tabIndex) + bool TerminalPage::_SelectTab(uint32_t tabIndex) { - if (tabIndex >= 0 && tabIndex < _tabs.Size()) - { - auto tab{ _tabs.GetAt(tabIndex) }; - if (_startupState == StartupState::InStartup) - { - _tabView.SelectedItem(tab.TabViewItem()); - _UpdatedSelectedTab(tab); - } - else - { - _SetFocusedTab(tab); - } + // GH#9369 - if the argument is out of range, then clamp to the number + // of available tabs. Previously, we'd just silently do nothing if the + // value was greater than the number of tabs. + tabIndex = std::clamp(tabIndex, 0u, _tabs.Size() - 1); - return true; + auto tab{ _tabs.GetAt(tabIndex) }; + if (_startupState == StartupState::InStartup) + { + _tabView.SelectedItem(tab.TabViewItem()); + _UpdatedSelectedTab(tab); + } + else + { + _SetFocusedTab(tab); } - return false; + + return true; } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 6d7370b9acd..fa32f5e35e5 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -233,7 +233,7 @@ namespace winrt::TerminalApp::implementation void _ResizeTabContent(const winrt::Windows::Foundation::Size& newSize); void _SelectNextTab(const bool bMoveRight, const Windows::Foundation::IReference& customTabSwitcherMode); - bool _SelectTab(const uint32_t tabIndex); + bool _SelectTab(uint32_t tabIndex); void _MoveFocus(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); winrt::Microsoft::Terminal::Control::TermControl _GetActiveControl(); diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp index 6f253973b1b..a23d80d77e3 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp @@ -227,6 +227,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::hstring SwitchToTabArgs::GenerateName() const { + if (TabIndex() == UINT32_MAX) + { + return RS_(L"SwitchToLastTabCommandKey"); + } + return winrt::hstring{ fmt::format(L"{}, index:{}", RS_(L"SwitchToTabCommandKey"), TabIndex()) }; diff --git a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw index d44ecc5ddbf..e94d5066b46 100644 --- a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw @@ -355,6 +355,9 @@ Switch to tab + + Switch to the last tab + Search for tab... @@ -417,4 +420,4 @@ Windows Console Host Name describing the usage of the classic windows console as the terminal UI. (`conhost.exe`) - \ No newline at end of file + diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index 0003e0b72b2..9d7f0a706f9 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -329,7 +329,7 @@ { "command": { "action": "switchToTab", "index": 5 }, "keys": "ctrl+alt+6" }, { "command": { "action": "switchToTab", "index": 6 }, "keys": "ctrl+alt+7" }, { "command": { "action": "switchToTab", "index": 7 }, "keys": "ctrl+alt+8" }, - { "command": { "action": "switchToTab", "index": 8 }, "keys": "ctrl+alt+9" }, + { "command": { "action": "switchToTab", "index": 4294967295 }, "keys": "ctrl+alt+9" }, // Pane Management { "command": "closePane", "keys": "ctrl+shift+w" },