diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index 1fd53ebc7fc..65ba3b50608 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -70,6 +70,10 @@ namespace TerminalAppLocalTests TEST_METHOD(TestTerminalArgsForBinding); + TEST_METHOD(FindMissingProfile); + TEST_METHOD(MakeSettingsForProfileThatDoesntExist); + TEST_METHOD(MakeSettingsForDefaultProfileThatDoesntExist); + TEST_METHOD(TestLayerProfileOnColorScheme); TEST_METHOD(ValidateKeybindingsWarnings); @@ -2094,6 +2098,146 @@ namespace TerminalAppLocalTests } } + void SettingsTests::FindMissingProfile() + { + // Test that CascadiaSettings::FindProfile returns null for a GUID that + // doesn't exist + const std::string settingsString{ R"( + { + "defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name" : "profile0", + "guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}" + }, + { + "name" : "profile1", + "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}" + } + ] + })" }; + const auto settingsJsonObj = VerifyParseSucceeded(settingsString); + auto settings = CascadiaSettings::FromJson(settingsJsonObj); + + const auto guid1 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-1111-49a3-80bd-e8fdd045185c}"); + const auto guid2 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-2222-49a3-80bd-e8fdd045185c}"); + const auto guid3 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-3333-49a3-80bd-e8fdd045185c}"); + + const Profile* const profile1 = settings->FindProfile(guid1); + const Profile* const profile2 = settings->FindProfile(guid2); + const Profile* const profile3 = settings->FindProfile(guid3); + + VERIFY_IS_NOT_NULL(profile1); + VERIFY_IS_NOT_NULL(profile2); + VERIFY_IS_NULL(profile3); + + VERIFY_ARE_EQUAL(L"profile0", profile1->GetName()); + VERIFY_ARE_EQUAL(L"profile1", profile2->GetName()); + } + + void SettingsTests::MakeSettingsForProfileThatDoesntExist() + { + // Test that MakeSettings throws when the GUID doesn't exist + const std::string settingsString{ R"( + { + "defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name" : "profile0", + "guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "historySize": 1 + }, + { + "name" : "profile1", + "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}", + "historySize": 2 + } + ] + })" }; + const auto settingsJsonObj = VerifyParseSucceeded(settingsString); + auto settings = CascadiaSettings::FromJson(settingsJsonObj); + + const auto guid1 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-1111-49a3-80bd-e8fdd045185c}"); + const auto guid2 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-2222-49a3-80bd-e8fdd045185c}"); + const auto guid3 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-3333-49a3-80bd-e8fdd045185c}"); + + try + { + auto terminalSettings = settings->BuildSettings(guid1); + VERIFY_ARE_NOT_EQUAL(nullptr, terminalSettings); + VERIFY_ARE_EQUAL(1, terminalSettings.HistorySize()); + } + catch (...) + { + VERIFY_IS_TRUE(false, L"This call to BuildSettings should succeed"); + } + + try + { + auto terminalSettings = settings->BuildSettings(guid2); + VERIFY_ARE_NOT_EQUAL(nullptr, terminalSettings); + VERIFY_ARE_EQUAL(2, terminalSettings.HistorySize()); + } + catch (...) + { + VERIFY_IS_TRUE(false, L"This call to BuildSettings should succeed"); + } + + VERIFY_THROWS(auto terminalSettings = settings->BuildSettings(guid3), wil::ResultException, L"This call to BuildSettings should fail"); + + try + { + const auto [guid, termSettings] = settings->BuildSettings(nullptr); + VERIFY_ARE_NOT_EQUAL(nullptr, termSettings); + VERIFY_ARE_EQUAL(1, termSettings.HistorySize()); + } + catch (...) + { + VERIFY_IS_TRUE(false, L"This call to BuildSettings should succeed"); + } + } + + void SettingsTests::MakeSettingsForDefaultProfileThatDoesntExist() + { + // Test that MakeSettings _doesnt_ throw when we load settings with a + // defaultProfile that's not in the list, we validate the settings, and + // then call MakeSettings(nullopt). The validation should ensure that + // the default profile is something reasonable + const std::string settingsString{ R"( + { + "defaultProfile": "{6239a42c-3333-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name" : "profile0", + "guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "historySize": 1 + }, + { + "name" : "profile1", + "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}", + "historySize": 2 + } + ] + })" }; + const auto settingsJsonObj = VerifyParseSucceeded(settingsString); + auto settings = CascadiaSettings::FromJson(settingsJsonObj); + settings->_ValidateSettings(); + + VERIFY_ARE_EQUAL(2u, settings->_warnings.size()); + VERIFY_ARE_EQUAL(2u, settings->_profiles.size()); + VERIFY_ARE_EQUAL(settings->_globals.GetDefaultProfile(), settings->_profiles.at(0).GetGuid()); + try + { + const auto [guid, termSettings] = settings->BuildSettings(nullptr); + VERIFY_ARE_NOT_EQUAL(nullptr, termSettings); + VERIFY_ARE_EQUAL(1, termSettings.HistorySize()); + } + catch (...) + { + VERIFY_IS_TRUE(false, L"This call to BuildSettings should succeed"); + } + } + void SettingsTests::TestLayerProfileOnColorScheme() { Log::Comment(NoThrowString().Format( diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index 4d63c184136..e7886178ec5 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp @@ -13,8 +13,10 @@ using namespace Microsoft::Console; using namespace TerminalApp; +using namespace winrt::TerminalApp; using namespace WEX::Logging; using namespace WEX::TestExecution; +using namespace WEX::Common; namespace TerminalAppLocalTests { @@ -53,11 +55,20 @@ namespace TerminalAppLocalTests TEST_METHOD(CreateSimpleTerminalXamlType); TEST_METHOD(CreateTerminalMuxXamlType); + TEST_METHOD(CreateTerminalPage); + + TEST_METHOD(TryDuplicateBadTab); + TEST_METHOD(TryDuplicateBadPane); + TEST_CLASS_SETUP(ClassSetup) { InitializeJsonReader(); return true; } + + private: + void _initializeTerminalPage(winrt::com_ptr& page, + std::shared_ptr initialSettings); }; void TabTests::EnsureTestsActivate() @@ -164,4 +175,321 @@ namespace TerminalAppLocalTests VERIFY_SUCCEEDED(result); } + void TabTests::CreateTerminalPage() + { + winrt::com_ptr page{ nullptr }; + + auto result = RunOnUIThread([&page]() { + page = winrt::make_self(); + VERIFY_IS_NOT_NULL(page); + }); + VERIFY_SUCCEEDED(result); + } + + // Method Description: + // - This is a helper to set up a TerminalPage for a unittest. This method + // does a couple things: + // * Create()'s a TerminalPage with the given settings. Constructing a + // TerminalPage so that we can get at its implementation is wacky, so + // this helper will do it correctly for you, even if this doesn't make a + // ton of sense on the surface. This is also why you need to pass both a + // projection and a com_ptr to this method. + // * It will use the provided settings object to initialize the TerminalPage + // * It will add the TerminalPage to the test Application, so that we can + // get actual layout events. Much of the Terminal assumes there's a + // non-zero ActualSize to the Terminal window, and adding the Page to + // the Application will make it behave as expected. + // * It will wait for the TerminalPage to finish initialization before + // returning control to the caller. It does this by creating an event and + // only setting the event when the TerminalPage raises its Initialized + // event, to signal that startup is complete. At this point, there will + // be one tab with the default profile in the page. + // * It will also ensure that the first tab is focused, since that happens + // asynchronously in the application typically. + // Arguments: + // - page: a TerminalPage implementation ptr that will receive the new TerminalPage instance + // - initialSettings: a CascadiaSettings to initialize the TerminalPage with. + // Return Value: + // - + void TabTests::_initializeTerminalPage(winrt::com_ptr& page, + std::shared_ptr initialSettings) + { + // This is super wacky, but we can't just initialize the + // com_ptr in the lambda and assign it back out of + // the lambda. We'll crash trying to get a weak_ref to the TerminalPage + // during TerminalPage::Create() below. + // + // Instead, create the winrt object, then get a com_ptr to the + // implementation _from_ the winrt object. This seems to work, even if + // it's weird. + winrt::TerminalApp::TerminalPage projectedPage{ nullptr }; + + Log::Comment(NoThrowString().Format(L"Construct the TerminalPage")); + auto result = RunOnUIThread([&projectedPage, &page, initialSettings]() { + projectedPage = winrt::TerminalApp::TerminalPage(); + page.copy_from(winrt::get_self(projectedPage)); + page->_settings = initialSettings; + }); + VERIFY_SUCCEEDED(result); + + VERIFY_IS_NOT_NULL(page); + VERIFY_IS_NOT_NULL(page->_settings); + + ::details::Event waitForInitEvent; + if (!waitForInitEvent.IsValid()) + { + VERIFY_SUCCEEDED(HRESULT_FROM_WIN32(::GetLastError())); + } + page->Initialized([&waitForInitEvent](auto&&, auto&&) { + waitForInitEvent.Set(); + }); + + Log::Comment(L"Create() the TerminalPage"); + + result = RunOnUIThread([&page]() { + VERIFY_IS_NOT_NULL(page); + VERIFY_IS_NOT_NULL(page->_settings); + page->Create(); + Log::Comment(L"Create()'d the page successfully"); + + auto app = ::winrt::Windows::UI::Xaml::Application::Current(); + + winrt::TerminalApp::TerminalPage pp = *page; + winrt::Windows::UI::Xaml::Window::Current().Content(pp); + winrt::Windows::UI::Xaml::Window::Current().Activate(); + }); + VERIFY_SUCCEEDED(result); + + Log::Comment(L"Wait for the page to finish initializing..."); + VERIFY_SUCCEEDED(waitForInitEvent.Wait()); + Log::Comment(L"...Done"); + + result = RunOnUIThread([&page]() { + // In the real app, this isn't a problem, but doesn't happen + // reliably in the unit tests. + Log::Comment(L"Ensure we set the first tab as the selected one."); + auto tab{ page->_GetStrongTabImpl(0) }; + page->_tabView.SelectedItem(tab->GetTabViewItem()); + page->_UpdatedSelectedTab(0); + }); + VERIFY_SUCCEEDED(result); + } + + void TabTests::TryDuplicateBadTab() + { + // * Create a tab with a profile with GUID 1 + // * Reload the settings so that GUID 1 is no longer in the list of profiles + // * Try calling _DuplicateTabViewItem on tab 1 + // * No new tab should be created (and more importantly, the app should not crash) + // + // Created to test GH#2455 + + const std::string settingsJson0{ R"( + { + "defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name" : "profile0", + "guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "historySize": 1 + }, + { + "name" : "profile1", + "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}", + "historySize": 2 + } + ] + })" }; + + const std::string settingsJson1{ R"( + { + "defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name" : "profile1", + "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}", + "historySize": 2 + } + ] + })" }; + + VerifyParseSucceeded(settingsJson0); + auto settings0 = std::make_shared(false); + VERIFY_IS_NOT_NULL(settings0); + settings0->_ParseJsonString(settingsJson0, false); + settings0->LayerJson(settings0->_userSettings); + settings0->_ValidateSettings(); + + VerifyParseSucceeded(settingsJson1); + auto settings1 = std::make_shared(false); + VERIFY_IS_NOT_NULL(settings1); + settings1->_ParseJsonString(settingsJson1, false); + settings1->LayerJson(settings1->_userSettings); + settings1->_ValidateSettings(); + + const auto guid1 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-1111-49a3-80bd-e8fdd045185c}"); + const auto guid2 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-2222-49a3-80bd-e8fdd045185c}"); + const auto guid3 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-3333-49a3-80bd-e8fdd045185c}"); + + // This is super wacky, but we can't just initialize the + // com_ptr in the lambda and assign it back out of + // the lambda. We'll crash trying to get a weak_ref to the TerminalPage + // during TerminalPage::Create() below. + // + // Instead, create the winrt object, then get a com_ptr to the + // implementation _from_ the winrt object. This seems to work, even if + // it's weird. + winrt::com_ptr page{ nullptr }; + _initializeTerminalPage(page, settings0); + + auto result = RunOnUIThread([&page]() { + VERIFY_ARE_EQUAL(1u, page->_tabs.Size()); + }); + VERIFY_SUCCEEDED(result); + + Log::Comment(L"Duplicate the first tab"); + result = RunOnUIThread([&page]() { + page->_DuplicateTabViewItem(); + VERIFY_ARE_EQUAL(2u, page->_tabs.Size()); + }); + VERIFY_SUCCEEDED(result); + + Log::Comment(NoThrowString().Format( + L"Change the settings of the TerminalPage so the first profile is " + L"no longer in the list of profiles")); + result = RunOnUIThread([&page, settings1]() { + page->_settings = settings1; + }); + VERIFY_SUCCEEDED(result); + + Log::Comment(L"Duplicate the tab, and don't crash"); + result = RunOnUIThread([&page]() { + page->_DuplicateTabViewItem(); + VERIFY_ARE_EQUAL(2u, page->_tabs.Size(), L"We should gracefully do nothing here - the profile no longer exists."); + }); + VERIFY_SUCCEEDED(result); + } + + void TabTests::TryDuplicateBadPane() + { + // * Create a tab with a profile with GUID 1 + // * Reload the settings so that GUID 1 is no longer in the list of profiles + // * Try calling _SplitPane(Duplicate) on tab 1 + // * No new pane should be created (and more importantly, the app should not crash) + // + // Created to test GH#2455 + + const std::string settingsJson0{ R"( + { + "defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name" : "profile0", + "guid": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "historySize": 1 + }, + { + "name" : "profile1", + "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}", + "historySize": 2 + } + ] + })" }; + + const std::string settingsJson1{ R"( + { + "defaultProfile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}", + "profiles": [ + { + "name" : "profile1", + "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}", + "historySize": 2 + } + ] + })" }; + + VerifyParseSucceeded(settingsJson0); + auto settings0 = std::make_shared(false); + VERIFY_IS_NOT_NULL(settings0); + settings0->_ParseJsonString(settingsJson0, false); + settings0->LayerJson(settings0->_userSettings); + settings0->_ValidateSettings(); + + VerifyParseSucceeded(settingsJson1); + auto settings1 = std::make_shared(false); + VERIFY_IS_NOT_NULL(settings1); + settings1->_ParseJsonString(settingsJson1, false); + settings1->LayerJson(settings1->_userSettings); + settings1->_ValidateSettings(); + + const auto guid1 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-1111-49a3-80bd-e8fdd045185c}"); + const auto guid2 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-2222-49a3-80bd-e8fdd045185c}"); + const auto guid3 = Microsoft::Console::Utils::GuidFromString(L"{6239a42c-3333-49a3-80bd-e8fdd045185c}"); + + // This is super wacky, but we can't just initialize the + // com_ptr in the lambda and assign it back out of + // the lambda. We'll crash trying to get a weak_ref to the TerminalPage + // during TerminalPage::Create() below. + // + // Instead, create the winrt object, then get a com_ptr to the + // implementation _from_ the winrt object. This seems to work, even if + // it's weird. + winrt::com_ptr page{ nullptr }; + _initializeTerminalPage(page, settings0); + + auto result = RunOnUIThread([&page]() { + VERIFY_ARE_EQUAL(1u, page->_tabs.Size()); + }); + VERIFY_SUCCEEDED(result); + + result = RunOnUIThread([&page]() { + VERIFY_ARE_EQUAL(1u, page->_tabs.Size()); + auto tab = page->_GetStrongTabImpl(0); + VERIFY_ARE_EQUAL(1, tab->_GetLeafPaneCount()); + }); + VERIFY_SUCCEEDED(result); + + Log::Comment(NoThrowString().Format(L"Duplicate the first pane")); + result = RunOnUIThread([&page]() { + page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, nullptr); + + VERIFY_ARE_EQUAL(1u, page->_tabs.Size()); + auto tab = page->_GetStrongTabImpl(0); + VERIFY_ARE_EQUAL(2, tab->_GetLeafPaneCount()); + }); + VERIFY_SUCCEEDED(result); + + Log::Comment(NoThrowString().Format( + L"Change the settings of the TerminalPage so the first profile is " + L"no longer in the list of profiles")); + result = RunOnUIThread([&page, settings1]() { + page->_settings = settings1; + }); + VERIFY_SUCCEEDED(result); + + Log::Comment(NoThrowString().Format(L"Duplicate the pane, and don't crash")); + result = RunOnUIThread([&page]() { + page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, nullptr); + + VERIFY_ARE_EQUAL(1u, page->_tabs.Size()); + auto tab = page->_GetStrongTabImpl(0); + VERIFY_ARE_EQUAL(2, + tab->_GetLeafPaneCount(), + L"We should gracefully do nothing here - the profile no longer exists."); + }); + VERIFY_SUCCEEDED(result); + + auto cleanup = wil::scope_exit([] { + auto result = RunOnUIThread([]() { + // There's something causing us to crash north of + // TSFInputControl::NotifyEnter, or LayoutRequested. It's very + // unclear what that issue is. Since these tests don't run in + // CI, simply log a message so that the dev running these tests + // knows it's expected. + Log::Comment(L"This test often crashes on cleanup, even when it succeeds. If it succeeded, then crashes, that's okay."); + }); + VERIFY_SUCCEEDED(result); + }); + } + } diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 3b77385f72e..b931755c1e3 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -70,7 +70,7 @@ void Pane::ResizeContent(const Size& newSize) const auto width = newSize.Width; const auto height = newSize.Height; - _CreateRowColDefinitions(newSize); + _CreateRowColDefinitions(); if (_splitState == SplitState::Vertical) { @@ -790,23 +790,24 @@ void Pane::_SetupChildCloseHandlers() // which is stored in _desiredSplitPosition // - Does nothing if our split state is currently set to SplitState::None // Arguments: -// - rootSize: The dimensions in pixels that this pane (and its children should consume.) +// - // Return Value: // - -void Pane::_CreateRowColDefinitions(const Size& rootSize) +void Pane::_CreateRowColDefinitions() { + const auto first = _desiredSplitPosition * 100.0f; + const auto second = 100.0f - first; if (_splitState == SplitState::Vertical) { _root.ColumnDefinitions().Clear(); // Create two columns in this grid: one for each pane - const auto paneSizes = _CalcChildrenSizes(rootSize.Width); auto firstColDef = Controls::ColumnDefinition(); - firstColDef.Width(GridLengthHelper::FromValueAndType(paneSizes.first, GridUnitType::Star)); + firstColDef.Width(GridLengthHelper::FromValueAndType(first, GridUnitType::Star)); auto secondColDef = Controls::ColumnDefinition(); - secondColDef.Width(GridLengthHelper::FromValueAndType(paneSizes.second, GridUnitType::Star)); + secondColDef.Width(GridLengthHelper::FromValueAndType(second, GridUnitType::Star)); _root.ColumnDefinitions().Append(firstColDef); _root.ColumnDefinitions().Append(secondColDef); @@ -816,35 +817,18 @@ void Pane::_CreateRowColDefinitions(const Size& rootSize) _root.RowDefinitions().Clear(); // Create two rows in this grid: one for each pane - const auto paneSizes = _CalcChildrenSizes(rootSize.Height); auto firstRowDef = Controls::RowDefinition(); - firstRowDef.Height(GridLengthHelper::FromValueAndType(paneSizes.first, GridUnitType::Star)); + firstRowDef.Height(GridLengthHelper::FromValueAndType(first, GridUnitType::Star)); auto secondRowDef = Controls::RowDefinition(); - secondRowDef.Height(GridLengthHelper::FromValueAndType(paneSizes.second, GridUnitType::Star)); + secondRowDef.Height(GridLengthHelper::FromValueAndType(second, GridUnitType::Star)); _root.RowDefinitions().Append(firstRowDef); _root.RowDefinitions().Append(secondRowDef); } } -// Method Description: -// - Initializes our UI for a new split in this pane. Sets up row/column -// definitions, and initializes the separator grid. Does nothing if our split -// state is currently set to SplitState::None -// Arguments: -// - -// Return Value: -// - -void Pane::_CreateSplitContent() -{ - Size actualSize{ gsl::narrow_cast(_root.ActualWidth()), - gsl::narrow_cast(_root.ActualHeight()) }; - - _CreateRowColDefinitions(actualSize); -} - // Method Description: // - Sets the thickness of each side of our borders to match our _borders state. // Arguments: @@ -1077,7 +1061,7 @@ std::pair, std::shared_ptr> Pane::_Split(SplitState _control = { nullptr }; _secondChild = std::make_shared(profile, control); - _CreateSplitContent(); + _CreateRowColDefinitions(); _root.Children().Append(_firstChild->GetRootElement()); _root.Children().Append(_secondChild->GetRootElement()); @@ -1522,4 +1506,74 @@ void Pane::_SetupResources() } } +int Pane::GetLeafPaneCount() const noexcept +{ + return _IsLeaf() ? 1 : (_firstChild->GetLeafPaneCount() + _secondChild->GetLeafPaneCount()); +} + +// Method Description: +// - This is a helper to determine which direction an "Automatic" split should +// happen in for a given pane, but without using the ActualWidth() and +// ActualHeight() methods. This is used during the initialization of the +// Terminal, when we could be processing many "split-pane" commands _before_ +// we've ever laid out the Terminal for the first time. When this happens, the +// Pane's don't have an actual size yet. However, we'd still like to figure +// out how to do an "auto" split when these Panes are all laid out. +// - This method assumes that the Pane we're attempting to split is `target`, +// and this method should be called on the root of a tree of Panes. +// - We'll walk down the tree attempting to find `target`. As we traverse the +// tree, we'll reduce the size passed to each subsequent recursive call. The +// size passed to this method represents how much space this Pane _will_ have +// to use. +// * If this pane is a leaf, and it's the pane we're looking for, use the +// available space to calculate which direction to split in. +// * If this pane is _any other leaf_, then just return nullopt, to indicate +// that the `target` Pane is not down this branch. +// * If this pane is a parent, calculate how much space our children will be +// able to use, and recurse into them. +// Arguments: +// - target: The Pane we're attempting to split. +// - availableSpace: The theoretical space that's available for this pane to be able to split. +// Return Value: +// - nullopt if `target` is not this pane or a child of this pane, otherwise the +// SplitState that `target` would use for an `Automatic` split given +// `availableSpace` +std::optional Pane::PreCalculateAutoSplit(const std::shared_ptr target, + const winrt::Windows::Foundation::Size availableSpace) const +{ + if (_IsLeaf()) + { + if (target.get() == this) + { + //If this pane is a leaf, and it's the pane we're looking for, use + //the available space to calculate which direction to split in. + return availableSpace.Width > availableSpace.Height ? SplitState::Vertical : SplitState::Horizontal; + } + else + { + // If this pane is _any other leaf_, then just return nullopt, to + // indicate that the `target` Pane is not down this branch. + return std::nullopt; + } + } + else + { + // If this pane is a parent, calculate how much space our children will + // be able to use, and recurse into them. + + const bool isVerticalSplit = _splitState == SplitState::Vertical; + const float firstWidth = isVerticalSplit ? (availableSpace.Width * _desiredSplitPosition) : availableSpace.Width; + const float secondWidth = isVerticalSplit ? (availableSpace.Width - firstWidth) : availableSpace.Width; + const float firstHeight = !isVerticalSplit ? (availableSpace.Height * _desiredSplitPosition) : availableSpace.Height; + const float secondHeight = !isVerticalSplit ? (availableSpace.Height - firstHeight) : availableSpace.Height; + + const auto firstResult = _firstChild->PreCalculateAutoSplit(target, { firstWidth, firstHeight }); + return firstResult.has_value() ? firstResult : _secondChild->PreCalculateAutoSplit(target, { secondWidth, secondHeight }); + } + + // We should not possibly be getting here - both the above branches should + // return a value. + FAIL_FAST(); +} + DEFINE_EVENT(Pane, GotFocus, _GotFocusHandlers, winrt::delegate>); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index fc06c18ce91..3b514d52ff5 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -63,10 +63,13 @@ class Pane : public std::enable_shared_from_this const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const; + std::optional PreCalculateAutoSplit(const std::shared_ptr target, const winrt::Windows::Foundation::Size parentSize) const; void Shutdown(); void Close(); + int GetLeafPaneCount() const noexcept; + WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler); DECLARE_EVENT(GotFocus, _GotFocusHandlers, winrt::delegate>); @@ -107,8 +110,7 @@ class Pane : public std::enable_shared_from_this const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); - void _CreateRowColDefinitions(const winrt::Windows::Foundation::Size& rootSize); - void _CreateSplitContent(); + void _CreateRowColDefinitions(); void _ApplySplitDefinitions(); void _UpdateBorders(); @@ -133,6 +135,9 @@ class Pane : public std::enable_shared_from_this float _ClampSplitPosition(const bool widthOrHeight, const float requestedValue, const float totalSize) const; winrt::TerminalApp::SplitState _convertAutomaticSplitState(const winrt::TerminalApp::SplitState& splitType) const; + + std::optional _preCalculateAutoSplit(const std::shared_ptr target, const winrt::Windows::Foundation::Size parentSize) const; + // Function Description: // - Returns true if the given direction can be used with the given split // type. diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index e2377508815..1fb49c719a7 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -277,6 +277,12 @@ namespace winrt::TerminalApp::implementation // gains focus, we'll mark it as the new active pane. _AttachEventHandlersToPane(first); _AttachEventHandlersToPane(second); + + // Immediately update our tracker of the focused pane now. If we're + // splitting panes during startup (from a commandline), then it's + // possible that the focus events won't propagate immediately. Updating + // the focus here will give the same effect though. + _UpdateActivePane(second); } // Method Description: @@ -387,6 +393,28 @@ namespace winrt::TerminalApp::implementation }); } + // Method Description: + // - Mark the given pane as the active pane in this tab. All other panes + // will be marked as inactive. We'll also update our own UI state to + // reflect this newly active pane. + // Arguments: + // - pane: a Pane to mark as active. + // Return Value: + // - + void Tab::_UpdateActivePane(std::shared_ptr pane) + { + // Clear the active state of the entire tree, and mark only the pane as active. + _rootPane->ClearActive(); + _activePane = pane; + _activePane->SetActive(); + + // Update our own title text to match the newly-active pane. + SetTabText(GetActiveTitle()); + + // Raise our own ActivePaneChanged event. + _ActivePaneChangedHandlers(); + } + // Method Description: // - Add an event handler to this pane's GotFocus event. When that pane gains // focus, we'll mark it as the new active pane. We'll also query the title of @@ -406,19 +434,37 @@ namespace winrt::TerminalApp::implementation if (tab && sender != tab->_activePane) { - // Clear the active state of the entire tree, and mark only the sender as active. - tab->_rootPane->ClearActive(); - tab->_activePane = sender; - tab->_activePane->SetActive(); - - // Update our own title text to match the newly-active pane. - tab->SetTabText(tab->GetActiveTitle()); - - // Raise our own ActivePaneChanged event. - tab->_ActivePaneChangedHandlers(); + tab->_UpdateActivePane(sender); } }); } + // Method Description: + // - Get the total number of leaf panes in this tab. This will be the number + // of actual controls hosted by this tab. + // Arguments: + // - + // Return Value: + // - The total number of leaf panes hosted by this tab. + int Tab::_GetLeafPaneCount() const noexcept + { + return _rootPane->GetLeafPaneCount(); + } + + // Method Description: + // - This is a helper to determine which direction an "Automatic" split should + // happen in for the active pane of this tab, but without using the ActualWidth() and + // ActualHeight() methods. + // - See Pane::PreCalculateAutoSplit + // Arguments: + // - availableSpace: The theoretical space that's available for this Tab's content + // Return Value: + // - The SplitState that we should use for an `Automatic` split given + // `availableSpace` + SplitState Tab::PreCalculateAutoSplit(winrt::Windows::Foundation::Size availableSpace) const + { + return _rootPane->PreCalculateAutoSplit(_activePane, availableSpace).value_or(SplitState::Vertical); + } + DEFINE_EVENT(Tab, ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>); } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index af72398ffa0..93817b8f57b 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -5,6 +5,12 @@ #include "Pane.h" #include "Tab.g.h" +// fwdecl unittest classes +namespace TerminalAppLocalTests +{ + class TabTests; +}; + namespace winrt::TerminalApp::implementation { struct Tab : public TabT @@ -32,6 +38,7 @@ namespace winrt::TerminalApp::implementation winrt::fire_and_forget UpdateIcon(const winrt::hstring iconPath); float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const; + SplitState PreCalculateAutoSplit(winrt::Windows::Foundation::Size rootSize) const; void ResizeContent(const winrt::Windows::Foundation::Size& newSize); void ResizePane(const winrt::TerminalApp::Direction& direction); @@ -64,5 +71,10 @@ namespace winrt::TerminalApp::implementation void _AttachEventHandlersToControl(const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void _AttachEventHandlersToPane(std::shared_ptr pane); + + int _GetLeafPaneCount() const noexcept; + void _UpdateActivePane(std::shared_ptr pane); + + friend class ::TerminalAppLocalTests::TabTests; }; } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 47f24b4c86e..0f2773402dd 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -63,11 +63,20 @@ namespace winrt::TerminalApp::implementation _tabView = _tabRow.TabView(); _rearranging = false; - // GH#3581 - There's a platform limitation that causes us to crash when we rearrange tabs. - // Xaml tries to send a drag visual (to wit: a screenshot) to the drag hosting process, - // but that process is running at a different IL than us. - // For now, we're disabling elevated drag. - const auto isElevated = ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Logic().IsElevated(); + // GH#2455 - Make sure to try/catch calls to Application::Current, + // because that _won't_ be an instance of TerminalApp::App in the + // LocalTests + auto isElevated = false; + try + { + // GH#3581 - There's a platform limitation that causes us to crash when we rearrange tabs. + // Xaml tries to send a drag visual (to wit: a screenshot) to the drag hosting process, + // but that process is running at a different IL than us. + // For now, we're disabling elevated drag. + isElevated = ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Logic().IsUwp(); + } + CATCH_LOG(); + _tabView.CanReorderTabs(!isElevated); _tabView.CanDragTabs(!isElevated); @@ -137,51 +146,79 @@ namespace winrt::TerminalApp::implementation _tabContent.SizeChanged({ this, &TerminalPage::_OnContentSizeChanged }); - // Actually start the terminal. - if (_appArgs.GetStartupActions().empty()) - { - _OpenNewTab(nullptr); - } - else + // Once the page is actually laid out on the screen, trigger all our + // startup actions. Things like Panes need to know at least how big the + // window will be, so they can subdivide that space. + // + // _OnFirstLayout will remove this handler so it doesn't get called more than once. + _layoutUpdatedRevoker = _tabContent.LayoutUpdated(winrt::auto_revoke, { this, &TerminalPage::_OnFirstLayout }); + } + + // Method Description: + // - This method is called once on startup, on the first LayoutUpdated event. + // We'll use this event to know that we have an ActualWidth and + // ActualHeight, so we can now attempt to process our list of startup + // actions. + // - We'll remove this event handler when the event is first handled. + // - If there are no startup actions, we'll open a single tab with the + // default profile. + // Arguments: + // - + // Return Value: + // - + void TerminalPage::_OnFirstLayout(const IInspectable& /*sender*/, const IInspectable& /*eventArgs*/) + { + // Only let this succeed once. + _layoutUpdatedRevoker.revoke(); + + // This event fires every time the layout changes, but it is always the + // last one to fire in any layout change chain. That gives us great + // flexibility in finding the right point at which to initialize our + // renderer (and our terminal). Any earlier than the last layout update + // and we may not know the terminal's starting size. + if (_startupState == StartupState::NotInitialized) { + _startupState = StartupState::InStartup; _appArgs.ValidateStartupCommands(); - - // This will kick off a chain of events to perform each startup - // action. As each startup action is completed, the next will be - // fired. - _ProcessNextStartupAction(); + if (_appArgs.GetStartupActions().empty()) + { + _OpenNewTab(nullptr); + _startupState = StartupState::Initialized; + _InitializedHandlers(*this, nullptr); + } + else + { + _ProcessStartupActions(); + } } } // Method Description: - // - Process the next startup action in our list of startup actions. When - // that action is complete, fire the next (if there are any more). + // - Process all the startup actions in our list of startup actions. We'll + // do this all at once here. // Arguments: // - // Return Value: // - - fire_and_forget TerminalPage::_ProcessNextStartupAction() + winrt::fire_and_forget TerminalPage::_ProcessStartupActions() { // If there are no actions left, do nothing. if (_appArgs.GetStartupActions().empty()) { return; } - - // Get the next action to be processed - auto nextAction = _appArgs.GetStartupActions().front(); - _appArgs.GetStartupActions().pop_front(); - auto weakThis{ get_weak() }; - // Handle it on the UI thread. - co_await winrt::resume_foreground(Dispatcher(), CoreDispatcherPriority::Low); + // Handle it on a subsequent pass of the UI thread. + co_await winrt::resume_foreground(Dispatcher(), CoreDispatcherPriority::Normal); if (auto page{ weakThis.get() }) { - page->_actionDispatch->DoAction(nextAction); - - // Kick off the next action to be handled (if necessary) - page->_ProcessNextStartupAction(); + for (const auto& action : _appArgs.GetStartupActions()) + { + _actionDispatch->DoAction(action); + } + _startupState = StartupState::Initialized; + _InitializedHandlers(*this, nullptr); } } @@ -406,7 +443,15 @@ namespace winrt::TerminalApp::implementation // add static items { - const auto isUwp = ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Logic().IsUwp(); + // GH#2455 - Make sure to try/catch calls to Application::Current, + // because that _won't_ be an instance of TerminalApp::App in the + // LocalTests + auto isUwp = false; + try + { + isUwp = ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Logic().IsUwp(); + } + CATCH_LOG(); if (!isUwp) { @@ -476,6 +521,7 @@ namespace winrt::TerminalApp::implementation // control which profile is created and with possible other // configurations. See CascadiaSettings::BuildSettings for more details. void TerminalPage::_OpenNewTab(const winrt::TerminalApp::NewTerminalArgs& newTerminalArgs) + try { const auto [profileGuid, settings] = _settings->BuildSettings(newTerminalArgs); @@ -499,6 +545,7 @@ namespace winrt::TerminalApp::implementation TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); } + CATCH_LOG(); winrt::fire_and_forget TerminalPage::_RemoveOnCloseRoutine(Microsoft::UI::Xaml::Controls::TabViewItem tabViewItem, winrt::com_ptr page) { @@ -514,7 +561,6 @@ namespace winrt::TerminalApp::implementation // - settings: the TerminalSettings object to use to create the TerminalControl with. void TerminalPage::_CreateNewTabFromSettings(GUID profileGuid, TerminalSettings settings) { - const bool isFirstTab = _tabs.Size() == 0; // Initialize the new tab // Create a connection based on the values in our settings object. @@ -568,20 +614,9 @@ namespace winrt::TerminalApp::implementation } }); - // If this is the first tab, we don't need to kick off the event to get - // the tab's content added to the root of the page. just do it - // immediately. - if (isFirstTab) - { - _tabContent.Children().Clear(); - _tabContent.Children().Append(newTabImpl->GetRootElement()); - } - else - { - // This kicks off TabView::SelectionChanged, in response to which - // we'll attach the terminal's Xaml control to the Xaml root. - _tabView.SelectedItem(tabViewItem); - } + // This kicks off TabView::SelectionChanged, in response to which + // we'll attach the terminal's Xaml control to the Xaml root. + _tabView.SelectedItem(tabViewItem); } // Method Description: @@ -810,13 +845,30 @@ namespace winrt::TerminalApp::implementation { if (auto index{ _GetFocusedTabIndex() }) { - auto focusedTab = _GetStrongTabImpl(*index); - const auto& profileGuid = focusedTab->GetFocusedProfile(); - if (profileGuid.has_value()) + try { - const auto settings = _settings->BuildSettings(profileGuid.value()); - _CreateNewTabFromSettings(profileGuid.value(), settings); + auto focusedTab = _GetStrongTabImpl(*index); + // TODO: GH#5047 - In the future, we should get the Profile of + // the focused pane, and use that to build a new instance of the + // settings so we can duplicate this tab/pane. + // + // Currently, if the profile doesn't exist anymore in our + // settings, we'll silently do nothing. + // + // In the future, it will be preferable to just duplicate the + // current control's settings, but we can't do that currently, + // because we won't be able to create a new instance of the + // connection without keeping an instance of the original Profile + // object around. + + const auto& profileGuid = focusedTab->GetFocusedProfile(); + if (profileGuid.has_value()) + { + const auto settings = _settings->BuildSettings(profileGuid.value()); + _CreateNewTabFromSettings(profileGuid.value(), settings); + } } + CATCH_LOG(); } } @@ -901,20 +953,36 @@ namespace winrt::TerminalApp::implementation // Wraparound math. By adding tabCount and then calculating modulo tabCount, // we clamp the values to the range [0, tabCount) while still supporting moving // leftward from 0 to tabCount - 1. - _SetFocusedTabIndex(((tabCount + *index + (bMoveRight ? 1 : -1)) % tabCount)); + const auto newTabIndex = ((tabCount + *index + (bMoveRight ? 1 : -1)) % tabCount); + _SelectTab(newTabIndex); } } // Method Description: // - Sets focus to the desired tab. Returns false if the provided tabIndex // is greater than the number of tabs we have. + // - During startup, we'll immediately set the selected tab as focused. + // - After startup, we'll dispatch an async method to set the the selected + // item of the TabView, which will then also trigger a + // TabView::SelectionChanged, handled in + // TerminalPage::_OnTabSelectionChanged // Return Value: // true iff we were able to select that tab index, false otherwise bool TerminalPage::_SelectTab(const uint32_t tabIndex) { if (tabIndex >= 0 && tabIndex < _tabs.Size()) { - _SetFocusedTabIndex(tabIndex); + if (_startupState == StartupState::InStartup) + { + auto tab{ _GetStrongTabImpl(tabIndex) }; + _tabView.SelectedItem(tab->GetTabViewItem()); + _UpdatedSelectedTab(tabIndex); + } + else + { + _SetFocusedTabIndex(tabIndex); + } + return true; } return false; @@ -967,6 +1035,16 @@ namespace winrt::TerminalApp::implementation return std::nullopt; } + // Method Description: + // - An async method for changing the focused tab on the UI thread. This + // method will _only_ set the selected item of the TabView, which will + // then also trigger a TabView::SelectionChanged event, which we'll handle + // in TerminalPage::_OnTabSelectionChanged, where we'll mark the new tab + // as focused. + // Arguments: + // - tabIndex: the index in the list of tabs to focus. + // Return Value: + // - winrt::fire_and_forget TerminalPage::_SetFocusedTabIndex(const uint32_t tabIndex) { // GH#1117: This is a workaround because _tabView.SelectedIndex(tabIndex) @@ -1075,42 +1153,65 @@ namespace winrt::TerminalApp::implementation return; } - auto focusedTab = _GetStrongTabImpl(*indexOpt); - - winrt::Microsoft::Terminal::Settings::TerminalSettings controlSettings; - GUID realGuid; - bool profileFound = false; - - if (splitMode == TerminalApp::SplitType::Duplicate) + try { - std::optional current_guid = focusedTab->GetFocusedProfile(); - if (current_guid) + auto focusedTab = _GetStrongTabImpl(*indexOpt); + winrt::Microsoft::Terminal::Settings::TerminalSettings controlSettings; + GUID realGuid; + bool profileFound = false; + + if (splitMode == TerminalApp::SplitType::Duplicate) { - profileFound = true; - controlSettings = _settings->BuildSettings(current_guid.value()); - realGuid = current_guid.value(); + std::optional current_guid = focusedTab->GetFocusedProfile(); + if (current_guid) + { + profileFound = true; + controlSettings = _settings->BuildSettings(current_guid.value()); + realGuid = current_guid.value(); + } + // TODO: GH#5047 - In the future, we should get the Profile of + // the focused pane, and use that to build a new instance of the + // settings so we can duplicate this tab/pane. + // + // Currently, if the profile doesn't exist anymore in our + // settings, we'll silently do nothing. + // + // In the future, it will be preferable to just duplicate the + // current control's settings, but we can't do that currently, + // because we won't be able to create a new instance of the + // connection without keeping an instance of the original Profile + // object around. + } + if (!profileFound) + { + std::tie(realGuid, controlSettings) = _settings->BuildSettings(newTerminalArgs); } - } - if (!profileFound) - { - std::tie(realGuid, controlSettings) = _settings->BuildSettings(newTerminalArgs); - } - const auto controlConnection = _CreateConnectionFromSettings(realGuid, controlSettings); + const auto controlConnection = _CreateConnectionFromSettings(realGuid, controlSettings); - const auto canSplit = focusedTab->CanSplitPane(splitType); + const auto canSplit = focusedTab->CanSplitPane(splitType); - if (!canSplit) - { - return; - } + if (!canSplit && _startupState == StartupState::Initialized) + { + return; + } - TermControl newControl{ controlSettings, controlConnection }; + auto realSplitType = splitType; + if (realSplitType == SplitState::Automatic && _startupState < StartupState::Initialized) + { + float contentWidth = gsl::narrow_cast(_tabContent.ActualWidth()); + float contentHeight = gsl::narrow_cast(_tabContent.ActualHeight()); + realSplitType = focusedTab->PreCalculateAutoSplit({ contentWidth, contentHeight }); + } - // Hookup our event handlers to the new terminal - _RegisterTerminalEvents(newControl, *focusedTab); + TermControl newControl{ controlSettings, controlConnection }; - focusedTab->SplitPane(splitType, realGuid, newControl); + // Hookup our event handlers to the new terminal + _RegisterTerminalEvents(newControl, *focusedTab); + + focusedTab->SplitPane(realSplitType, realGuid, newControl); + } + CATCH_LOG(); } // Method Description: @@ -1426,6 +1527,31 @@ namespace winrt::TerminalApp::implementation } } + void TerminalPage::_UpdatedSelectedTab(const int32_t index) + { + // Unfocus all the tabs. + for (auto tab : _tabs) + { + auto tabImpl{ _GetStrongTabImpl(tab) }; + tabImpl->SetFocused(false); + } + + if (index >= 0) + { + try + { + auto tab{ _GetStrongTabImpl(index) }; + + _tabContent.Children().Clear(); + _tabContent.Children().Append(tab->GetRootElement()); + + tab->SetFocused(true); + _titleChangeHandlers(*this, Title()); + } + CATCH_LOG(); + } + } + // Method Description: // - Responds to the TabView control's Selection Changed event (to move a // new terminal control into focus) when not in in the middle of a tab rearrangement. @@ -1438,28 +1564,7 @@ namespace winrt::TerminalApp::implementation { auto tabView = sender.as(); auto selectedIndex = tabView.SelectedIndex(); - - // Unfocus all the tabs. - for (auto tab : _tabs) - { - auto tabImpl{ _GetStrongTabImpl(tab) }; - tabImpl->SetFocused(false); - } - - if (selectedIndex >= 0) - { - try - { - auto tab{ _GetStrongTabImpl(selectedIndex) }; - - _tabContent.Children().Clear(); - _tabContent.Children().Append(tab->GetRootElement()); - - tab->SetFocused(true); - _titleChangeHandlers(*this, Title()); - } - CATCH_LOG(); - } + _UpdatedSelectedTab(selectedIndex); } } @@ -1525,16 +1630,28 @@ namespace winrt::TerminalApp::implementation for (auto& profile : profiles) { const GUID profileGuid = profile.GetGuid(); - const auto settings = _settings->BuildSettings(profileGuid); - for (auto tab : _tabs) + try { - // Attempt to reload the settings of any panes with this profile - auto tabImpl{ _GetStrongTabImpl(tab) }; - tabImpl->UpdateSettings(settings, profileGuid); + // BuildSettings can throw an exception if the profileGuid does + // not belong to an actual profile in the list of profiles. + const auto settings = _settings->BuildSettings(profileGuid); + + for (auto tab : _tabs) + { + // Attempt to reload the settings of any panes with this profile + auto tabImpl{ _GetStrongTabImpl(tab) }; + tabImpl->UpdateSettings(settings, profileGuid); + } } + CATCH_LOG(); } + // GH#2455: If there are any panes with controls that had been + // initialized with a Profile that no longer exists in our list of + // profiles, we'll leave it unmodified. The profile doesn't exist + // anymore, so we can't possibly update its settings. + // Update the icon of the tab for the currently focused profile in that tab. for (auto tab : _tabs) { diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 684d55495f1..09c4fad95ed 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -20,6 +20,13 @@ namespace TerminalAppLocalTests namespace winrt::TerminalApp::implementation { + enum StartupState : int + { + NotInitialized = 0, + InStartup = 1, + Initialized = 2 + }; + struct TerminalPage : TerminalPageT { public: @@ -48,6 +55,7 @@ namespace winrt::TerminalApp::implementation DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(SetTitleBarContent, _setTitleBarContentHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::UIElement); DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(ShowDialog, _showDialogHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::Controls::ContentDialog); DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(ToggleFullscreen, _toggleFullscreenHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::ToggleFullscreenEventArgs); + TYPED_EVENT(Initialized, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::RoutedEventArgs); private: // If you add controls here, but forget to null them either here or in @@ -75,9 +83,12 @@ namespace winrt::TerminalApp::implementation winrt::com_ptr _actionDispatch{ winrt::make_self() }; + winrt::Windows::UI::Xaml::Controls::Grid::LayoutUpdated_revoker _layoutUpdatedRevoker; + StartupState _startupState{ StartupState::NotInitialized }; + ::TerminalApp::AppCommandlineArgs _appArgs; int _ParseArgs(winrt::array_view& args); - fire_and_forget _ProcessNextStartupAction(); + winrt::fire_and_forget _ProcessStartupActions(); void _ShowAboutDialog(); void _ShowCloseWarningDialog(); @@ -141,6 +152,8 @@ namespace winrt::TerminalApp::implementation void _OnTabItemsChanged(const IInspectable& sender, const Windows::Foundation::Collections::IVectorChangedEventArgs& eventArgs); void _OnContentSizeChanged(const IInspectable& /*sender*/, Windows::UI::Xaml::SizeChangedEventArgs const& e); void _OnTabCloseRequested(const IInspectable& sender, const Microsoft::UI::Xaml::Controls::TabViewTabCloseRequestedEventArgs& eventArgs); + void _OnFirstLayout(const IInspectable& sender, const IInspectable& eventArgs); + void _UpdatedSelectedTab(const int32_t index); void _Find(); diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl index a1bfcd98db2..d91d4da861f 100644 --- a/src/cascadia/TerminalApp/TerminalPage.idl +++ b/src/cascadia/TerminalApp/TerminalPage.idl @@ -18,5 +18,6 @@ namespace TerminalApp event Windows.Foundation.TypedEventHandler SetTitleBarContent; event Windows.Foundation.TypedEventHandler ShowDialog; event Windows.Foundation.TypedEventHandler ToggleFullscreen; + event Windows.Foundation.TypedEventHandler Initialized; } } diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index 2c14cb3a6d7..a82a4c0a4e4 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -172,8 +172,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation TextBlock().FontSize(fontSizePx); TextBlock().FontFamily(Media::FontFamily(fontArgs->FontFace())); - const auto widthToTerminalEnd = Canvas().ActualWidth() - ::base::ClampedNumeric(clientCursorPos.X); - TextBlock().MaxWidth(widthToTerminalEnd); + const auto canvasActualWidth = Canvas().ActualWidth(); + const auto widthToTerminalEnd = canvasActualWidth - ::base::ClampedNumeric(clientCursorPos.X); + // Make sure that we're setting the MaxWidth to a positive number - a + // negative number here will crash us in mysterious ways with a useless + // stack trace + const auto newMaxWidth = std::max(0.0, widthToTerminalEnd); + TextBlock().MaxWidth(newMaxWidth); // Set the text block bounds const auto yOffset = ::base::ClampedNumeric(TextBlock().ActualHeight()) - fontHeight; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 7797e9e3279..b8f2104c69c 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -878,6 +878,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - point: the PointerPoint object representing a mouse event from our XAML input handler bool TermControl::_CanSendVTMouseInput() { + if (!_terminal) + { + return false; + } // If the user is holding down Shift, suppress mouse events // TODO GH#4875: disable/customize this functionality const auto modifiers = _GetPressedModifierKeys(); @@ -2210,6 +2214,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // If we haven't initialized yet, just quick return. if (!_terminal) { + // fake it + eventArgs.CurrentPosition({ 0, 0 }); return; } const COORD cursorPos = _terminal->GetCursorPosition();