From 606dd576cc83ad7e3284a9a1e971b5568c72a71e Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 8 May 2019 17:28:44 -0500 Subject: [PATCH 01/33] Start working on adding support for panes --- src/cascadia/TerminalApp/App.cpp | 119 +++++++++++-------- src/cascadia/TerminalApp/App.h | 2 + src/cascadia/TerminalApp/Pane.cpp | 104 ++++++++++++++++ src/cascadia/TerminalApp/Pane.h | 55 +++++++++ src/cascadia/TerminalApp/Tab.cpp | 73 +++++++----- src/cascadia/TerminalApp/Tab.h | 15 ++- src/cascadia/TerminalApp/TerminalApp.vcxproj | 4 +- 7 files changed, 284 insertions(+), 88 deletions(-) create mode 100644 src/cascadia/TerminalApp/Pane.cpp create mode 100644 src/cascadia/TerminalApp/Pane.h diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index c984dda7fb7..5a890d94b16 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -417,20 +417,21 @@ namespace winrt::TerminalApp::implementation for (auto &tab : _tabs) { - const auto term = tab->GetTerminalControl(); - const GUID tabProfile = tab->GetProfile(); - - if (profileGuid == tabProfile) - { - term.UpdateSettings(settings); - - // Update the icons of the tabs with this profile open. - auto tabViewItem = tab->GetTabViewItem(); - tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [profile, tabViewItem]() { - // _GetIconFromProfile has to run on the main thread - tabViewItem.Icon(App::_GetIconFromProfile(profile)); - }); - } + // TODO + // const auto term = tab->GetTerminalControl(); + // const GUID tabProfile = tab->GetProfile(); + + // if (profileGuid == tabProfile) + // { + // term.UpdateSettings(settings); + + // // Update the icons of the tabs with this profile open. + // auto tabViewItem = tab->GetTabViewItem(); + // tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [profile, tabViewItem]() { + // // _GetIconFromProfile has to run on the main thread + // tabViewItem.Icon(App::_GetIconFromProfile(profile)); + // }); + // } } } @@ -597,17 +598,18 @@ namespace winrt::TerminalApp::implementation // Add the new tab to the list of our tabs. auto newTab = _tabs.emplace_back(std::make_shared(profileGuid, term)); - // Add an event handler when the terminal's title changes. When the - // title changes, we'll bubble it up to listeners of our own title - // changed event, so they can handle it. - newTab->GetTerminalControl().TitleChanged([=](auto newTitle){ - // Only bubble the change if this tab is the focused tab. - if (_settings->GlobalSettings().GetShowTitleInTitlebar() && - newTab->IsFocused()) - { - _titleChangeHandlers(newTitle); - } - }); + // TODO: + // // Add an event handler when the terminal's title changes. When the + // // title changes, we'll bubble it up to listeners of our own title + // // changed event, so they can handle it. + // newTab->GetTerminalControl().TitleChanged([=](auto newTitle){ + // // Only bubble the change if this tab is the focused tab. + // if (_settings->GlobalSettings().GetShowTitleInTitlebar() && + // newTab->IsFocused()) + // { + // _titleChangeHandlers(newTitle); + // } + // }); auto tabViewItem = newTab->GetTabViewItem(); _tabView.Items().Append(tabViewItem); @@ -620,22 +622,23 @@ namespace winrt::TerminalApp::implementation tabViewItem.Icon(_GetIconFromProfile(*profile)); } - // Add an event handler when the terminal's connection is closed. - newTab->GetTerminalControl().ConnectionClosed([=]() { - _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [newTab, tabViewItem, this]() { - const GUID tabProfile = newTab->GetProfile(); - // Don't just capture this pointer, because the profile might - // get destroyed before this is called (case in point - - // reloading settings) - const auto* const p = _settings->FindProfile(tabProfile); - - // TODO: MSFT:21268795: Need a better story for what should happen when the last tab is closed. - if (p != nullptr && p->GetCloseOnExit() && _tabs.size() > 1) - { - _RemoveTabViewItem(tabViewItem); - } - }); - }); + // TODO: + // // Add an event handler when the terminal's connection is closed. + // newTab->GetTerminalControl().ConnectionClosed([=]() { + // _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [newTab, tabViewItem, this]() { + // const GUID tabProfile = newTab->GetProfile(); + // // Don't just capture this pointer, because the profile might + // // get destroyed before this is called (case in point - + // // reloading settings) + // const auto* const p = _settings->FindProfile(tabProfile); + + // // TODO: MSFT:21268795: Need a better story for what should happen when the last tab is closed. + // if (p != nullptr && p->GetCloseOnExit() && _tabs.size() > 1) + // { + // _RemoveTabViewItem(tabViewItem); + // } + // }); + // }); tabViewItem.PointerPressed({ this, &App::_OnTabClick }); @@ -682,8 +685,12 @@ namespace winrt::TerminalApp::implementation // - delta: a number of lines to move the viewport relative to the current viewport. void App::_DoScroll(int delta) { - int focusedTabIndex = _GetFocusedTabIndex(); - _tabs[focusedTabIndex]->Scroll(delta); + // TODO: + // int focusedTabIndex = _GetFocusedTabIndex(); + // _tabs[focusedTabIndex]->Scroll(delta); + + // Maybe something like: + // _GetFocusedControl()._Scroll(delta); } // Method Description: @@ -693,10 +700,12 @@ namespace winrt::TerminalApp::implementation // and get text to appear on separate lines. void App::_CopyText(const bool trimTrailingWhitespace) { - const int focusedTabIndex = _GetFocusedTabIndex(); - std::shared_ptr focusedTab{ _tabs[focusedTabIndex] }; + // const int focusedTabIndex = _GetFocusedTabIndex(); + // std::shared_ptr focusedTab{ _tabs[focusedTabIndex] }; + + // const auto control = focusedTab->GetTerminalControl(); - const auto control = focusedTab->GetTerminalControl(); + const auto control = _GetFocusedControl(); control.CopySelectionToClipboard(trimTrailingWhitespace); } @@ -736,10 +745,12 @@ namespace winrt::TerminalApp::implementation try { auto tab = _tabs.at(selectedIndex); - auto control = tab->GetTerminalControl().GetControl(); + auto term = tab->GetFocusedTerminalControl(); + auto control = term.GetControl(); _tabContent.Children().Clear(); - _tabContent.Children().Append(control); + // _tabContent.Children().Append(control); + _tabContent.Children().Append(tab->GetRootElement()); tab->SetFocused(true); _titleChangeHandlers(GetTitle()); @@ -796,8 +807,9 @@ namespace winrt::TerminalApp::implementation { try { - auto tab = _tabs.at(selectedIndex); - return tab->GetTerminalControl().Title(); + return _GetFocusedControl().Title(); + // auto tab = _tabs.at(selectedIndex); + // return tab->GetTerminalControl().Title(); } CATCH_LOG(); } @@ -870,6 +882,13 @@ namespace winrt::TerminalApp::implementation } } + winrt::Microsoft::Terminal::TerminalControl::TermControl App::_GetFocusedControl() + { + int focusedTabIndex = _GetFocusedTabIndex(); + auto focusedTab = _tabs[focusedTabIndex]; + return focusedTab->GetFocusedTerminalControl(); + } + // -------------------------------- WinRT Events --------------------------------- // Winrt events need a method for adding a callback to the event and removing the callback. // These macros will define them both for you. diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 87b3915748f..31c1d60e1b9 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -106,6 +106,8 @@ namespace winrt::TerminalApp::implementation void _ApplyTheme(const Windows::UI::Xaml::ElementTheme& newTheme); static Windows::UI::Xaml::Controls::IconElement _GetIconFromProfile(const ::TerminalApp::Profile& profile); + + winrt::Microsoft::Terminal::TerminalControl::TermControl _GetFocusedControl(); }; } diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp new file mode 100644 index 00000000000..dc662b80c0f --- /dev/null +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -0,0 +1,104 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "pch.h" +#include "Pane.h" + +using namespace winrt::Windows::UI::Xaml; +using namespace winrt::Windows::UI::Core; + +Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) : + _control{ control }, + _focused{ false }, + _profile{ profile }, + _splitState{ SplitState::None }, + _firstChild{ nullptr }, + _secondChild{ nullptr } +{ + // _MakeTabViewItem(); + _root = Controls::Grid{}; + _root.Children().Append(_control.GetControl()); +} + +Pane::~Pane() +{ +} + +// void Pane::_MakeTabViewItem() +// { +// _tabViewItem = ::winrt::Microsoft::UI::Xaml::Controls::TabViewItem{}; +// const auto title = _control.Title(); + +// _tabViewItem.Header(title); + +// _control.TitleChanged([=](auto newTitle){ +// _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ +// _tabViewItem.Header(newTitle); +// }); +// }); +// } + +winrt::Windows::UI::Xaml::UIElement Pane::GetRootElement() +{ + return _root; +} + +winrt::Microsoft::Terminal::TerminalControl::TermControl Pane::GetFocusedTerminalControl() +{ + if (_IsLeaf()) + { + return _control; + } + else + { + // TODO determine a way of tracking the actually focused terminal control + return _firstChild->GetFocusedTerminalControl(); + } +} + +bool Pane::IsFocused() const noexcept +{ + return _focused; +} + +bool Pane::_IsLeaf() const noexcept +{ + return _splitState == SplitState::None; +} + +void Pane::SetFocused(bool focused) +{ + _focused = focused; + + if (_focused) + { + _Focus(); + } +} + +// GUID Pane::GetProfile() const noexcept +// { +// return _profile; +// } + +void Pane::_Focus() +{ + _focused = true; + _control.GetControl().Focus(FocusState::Programmatic); +} + +// // Method Description: +// // - Move the viewport of the terminal up or down a number of lines. Negative +// // values of `delta` will move the view up, and positive values will move +// // the viewport down. +// // Arguments: +// // - delta: a number of lines to move the viewport relative to the current viewport. +// // Return Value: +// // - +// void Pane::Scroll(int delta) +// { +// _control.GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ +// const auto currentOffset = _control.GetScrollOffset(); +// _control.ScrollViewport(currentOffset + delta); +// }); +// } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h new file mode 100644 index 00000000000..6cd21b9e8bc --- /dev/null +++ b/src/cascadia/TerminalApp/Pane.h @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once +#include +#include + + +class Pane +{ + +public: + + enum class SplitState : int + { + None = 0, + Vertical = 1, + Horizontal = 2 + }; + + Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + ~Pane(); + + winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); + // winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); + winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + winrt::Windows::UI::Xaml::UIElement GetRootElement(); + + bool IsFocused() const noexcept; + void SetFocused(bool focused); + + // GUID GetProfile() const noexcept; + + // void SplitHorizontally(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + // void SplitVertically(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + + // void Scroll(int delta); + +private: + winrt::Windows::UI::Xaml::Controls::Grid _root{ nullptr }; + winrt::Microsoft::Terminal::TerminalControl::TermControl _control{ nullptr }; + + std::shared_ptr _firstChild; + std::shared_ptr _secondChild; + SplitState _splitState; + + bool _focused; + std::optional _profile; + // winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem; + + void _MakeTabViewItem(); + void _Focus(); + + bool _IsLeaf() const noexcept; +}; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 48f71ab60a8..1bfbf170e21 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -8,11 +8,11 @@ using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Core; Tab::Tab(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) : - _control{ control }, _focused{ false }, - _profile{ profile }, - _tabViewItem{ nullptr } + _tabViewItem{ nullptr }, + _rootPane{ nullptr } { + _rootPane = std::make_shared(profile, control); _MakeTabViewItem(); } @@ -27,20 +27,26 @@ Tab::~Tab() void Tab::_MakeTabViewItem() { _tabViewItem = ::winrt::Microsoft::UI::Xaml::Controls::TabViewItem{}; - const auto title = _control.Title(); + // const auto title = _control.Title(); - _tabViewItem.Header(title); + // _tabViewItem.Header(title); - _control.TitleChanged([=](auto newTitle){ - _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - _tabViewItem.Header(newTitle); - }); - }); + // _control.TitleChanged([=](auto newTitle){ + // _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ + // _tabViewItem.Header(newTitle); + // }); + // }); } -winrt::Microsoft::Terminal::TerminalControl::TermControl Tab::GetTerminalControl() +winrt::Windows::UI::Xaml::UIElement Tab::GetRootElement() { - return _control; + return _rootPane->GetRootElement(); +} + +winrt::Microsoft::Terminal::TerminalControl::TermControl Tab::GetFocusedTerminalControl() +{ + return _rootPane->GetFocusedTerminalControl(); + // return _control; } winrt::Microsoft::UI::Xaml::Controls::TabViewItem Tab::GetTabViewItem() @@ -64,29 +70,32 @@ void Tab::SetFocused(bool focused) } } -GUID Tab::GetProfile() const noexcept -{ - return _profile; -} +// TODO +// GUID Tab::GetProfile() const noexcept +// { +// return _profile; +// } void Tab::_Focus() { _focused = true; - _control.GetControl().Focus(FocusState::Programmatic); + // _control.GetControl().Focus(FocusState::Programmatic); + _rootPane->SetFocused(true); } -// Method Description: -// - Move the viewport of the terminal up or down a number of lines. Negative -// values of `delta` will move the view up, and positive values will move -// the viewport down. -// Arguments: -// - delta: a number of lines to move the viewport relative to the current viewport. -// Return Value: -// - -void Tab::Scroll(int delta) -{ - _control.GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - const auto currentOffset = _control.GetScrollOffset(); - _control.ScrollViewport(currentOffset + delta); - }); -} +// TODO +// // Method Description: +// // - Move the viewport of the terminal up or down a number of lines. Negative +// // values of `delta` will move the view up, and positive values will move +// // the viewport down. +// // Arguments: +// // - delta: a number of lines to move the viewport relative to the current viewport. +// // Return Value: +// // - +// void Tab::Scroll(int delta) +// { +// _control.GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ +// const auto currentOffset = _control.GetScrollOffset(); +// _control.ScrollViewport(currentOffset + delta); +// }); +// } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index da3f4e3b620..17240ce3110 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -4,6 +4,7 @@ #pragma once #include #include +#include "Pane.h" class Tab { @@ -13,19 +14,23 @@ class Tab ~Tab(); winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); + winrt::Windows::UI::Xaml::UIElement GetRootElement(); + winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); bool IsFocused(); void SetFocused(bool focused); - GUID GetProfile() const noexcept; + // GUID GetProfile() const noexcept; - void Scroll(int delta); + // void Scroll(int delta); private: - winrt::Microsoft::Terminal::TerminalControl::TermControl _control; + + std::shared_ptr _rootPane; + + // winrt::Microsoft::Terminal::TerminalControl::TermControl _control; bool _focused; - GUID _profile; + // GUID _profile; winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem; void _MakeTabViewItem(); diff --git a/src/cascadia/TerminalApp/TerminalApp.vcxproj b/src/cascadia/TerminalApp/TerminalApp.vcxproj index 6a70d00dda9..363700158c8 100644 --- a/src/cascadia/TerminalApp/TerminalApp.vcxproj +++ b/src/cascadia/TerminalApp/TerminalApp.vcxproj @@ -37,6 +37,7 @@ + @@ -53,6 +54,7 @@ + @@ -128,4 +130,4 @@ - \ No newline at end of file + From 91dc273591cc030dbf95577fd98768a480bdb50c Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 9 May 2019 11:53:44 -0500 Subject: [PATCH 02/33] Add basic pane splitting * adds two keybindings, Ctrl+Shift++ and Ctrl+Shift+- for horizontal and vertical split, respectively. * Splits only open the default profile - This should probably be a setting, whether to use the default or a new copy of the current pane. There should also be a keystroke for opening a pane with a specific profile. * Only supports one pane ATM. * need to figure out which control is actively focused. Having an IsFocused method on the TermControl seems wrong. --- src/cascadia/TerminalApp/App.cpp | 37 ++++- src/cascadia/TerminalApp/App.h | 2 + src/cascadia/TerminalApp/AppKeyBindings.cpp | 9 ++ src/cascadia/TerminalApp/AppKeyBindings.h | 2 + src/cascadia/TerminalApp/AppKeyBindings.idl | 6 + src/cascadia/TerminalApp/CascadiaSettings.cpp | 7 + src/cascadia/TerminalApp/Pane.cpp | 127 ++++++++++++------ src/cascadia/TerminalApp/Pane.h | 8 +- src/cascadia/TerminalApp/Tab.cpp | 43 +++--- src/cascadia/TerminalApp/Tab.h | 8 +- src/cascadia/TerminalControl/TermControl.cpp | 12 +- 11 files changed, 189 insertions(+), 72 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 5a890d94b16..5fea8ce5e92 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -329,6 +329,8 @@ namespace winrt::TerminalApp::implementation bindings.ScrollDown([this]() { _DoScroll(1); }); bindings.NextTab([this]() { _SelectNextTab(true); }); bindings.PrevTab([this]() { _SelectNextTab(false); }); + bindings.SplitVertical([this]() { _SplitVertical(std::nullopt); }); + bindings.SplitHorizontal([this]() { _SplitHorizontal(std::nullopt); }); } // Method Description: @@ -685,12 +687,8 @@ namespace winrt::TerminalApp::implementation // - delta: a number of lines to move the viewport relative to the current viewport. void App::_DoScroll(int delta) { - // TODO: - // int focusedTabIndex = _GetFocusedTabIndex(); - // _tabs[focusedTabIndex]->Scroll(delta); - - // Maybe something like: - // _GetFocusedControl()._Scroll(delta); + int focusedTabIndex = _GetFocusedTabIndex(); + _tabs[focusedTabIndex]->Scroll(delta); } // Method Description: @@ -889,6 +887,33 @@ namespace winrt::TerminalApp::implementation return focusedTab->GetFocusedTerminalControl(); } + void App::_SplitVertical(std::optional profileGuid) + { + const GUID realGuid = profileGuid ? profileGuid.value() : + _settings->GlobalSettings().GetDefaultProfile(); + auto controlSettings = _settings->MakeSettings(realGuid); + TermControl newControl{ controlSettings }; + + int focusedTabIndex = _GetFocusedTabIndex(); + auto focusedTab = _tabs[focusedTabIndex]; + + return focusedTab->SplitVertical(realGuid, newControl); + } + + void App::_SplitHorizontal(std::optional profileGuid) + { + const GUID realGuid = profileGuid ? profileGuid.value() : + _settings->GlobalSettings().GetDefaultProfile(); + auto controlSettings = _settings->MakeSettings(realGuid); + TermControl newControl{ controlSettings }; + + int focusedTabIndex = _GetFocusedTabIndex(); + auto focusedTab = _tabs[focusedTabIndex]; + + return focusedTab->SplitHorizontal(realGuid, newControl); + } + + // -------------------------------- WinRT Events --------------------------------- // Winrt events need a method for adding a callback to the event and removing the callback. // These macros will define them both for you. diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 31c1d60e1b9..1d1882f48c0 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -93,6 +93,8 @@ namespace winrt::TerminalApp::implementation void _DoScroll(int delta); void _CopyText(const bool trimTrailingWhitespace); + void _SplitVertical(std::optional profileGuid); + void _SplitHorizontal(std::optional profileGuid); // Todo: add more event implementations here // MSFT:20641986: Add keybindings for New Window diff --git a/src/cascadia/TerminalApp/AppKeyBindings.cpp b/src/cascadia/TerminalApp/AppKeyBindings.cpp index 85fee19a373..fba1fa4d95a 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindings.cpp @@ -93,6 +93,13 @@ namespace winrt::TerminalApp::implementation case ShortcutAction::PrevTab: _PrevTabHandlers(); return true; + + case ShortcutAction::SplitVertical: + _SplitVerticalHandlers(); + return true; + case ShortcutAction::SplitHorizontal: + _SplitHorizontalHandlers(); + return true; } return false; } @@ -108,6 +115,8 @@ namespace winrt::TerminalApp::implementation DEFINE_EVENT(AppKeyBindings, SwitchToTab, _SwitchToTabHandlers, TerminalApp::SwitchToTabEventArgs); DEFINE_EVENT(AppKeyBindings, NextTab, _NextTabHandlers, TerminalApp::NextTabEventArgs); DEFINE_EVENT(AppKeyBindings, PrevTab, _PrevTabHandlers, TerminalApp::PrevTabEventArgs); + DEFINE_EVENT(AppKeyBindings, SplitVertical, _SplitVerticalHandlers, TerminalApp::SplitVerticalEventArgs); + DEFINE_EVENT(AppKeyBindings, SplitHorizontal, _SplitHorizontalHandlers, TerminalApp::SplitHorizontalEventArgs); DEFINE_EVENT(AppKeyBindings, IncreaseFontSize, _IncreaseFontSizeHandlers, TerminalApp::IncreaseFontSizeEventArgs); DEFINE_EVENT(AppKeyBindings, DecreaseFontSize, _DecreaseFontSizeHandlers, TerminalApp::DecreaseFontSizeEventArgs); DEFINE_EVENT(AppKeyBindings, ScrollUp, _ScrollUpHandlers, TerminalApp::ScrollUpEventArgs); diff --git a/src/cascadia/TerminalApp/AppKeyBindings.h b/src/cascadia/TerminalApp/AppKeyBindings.h index 800a72ea56d..04b1489e916 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.h +++ b/src/cascadia/TerminalApp/AppKeyBindings.h @@ -45,6 +45,8 @@ namespace winrt::TerminalApp::implementation DECLARE_EVENT(SwitchToTab, _SwitchToTabHandlers, TerminalApp::SwitchToTabEventArgs); DECLARE_EVENT(NextTab, _NextTabHandlers, TerminalApp::NextTabEventArgs); DECLARE_EVENT(PrevTab, _PrevTabHandlers, TerminalApp::PrevTabEventArgs); + DECLARE_EVENT(SplitVertical, _SplitVerticalHandlers, TerminalApp::SplitVerticalEventArgs); + DECLARE_EVENT(SplitHorizontal, _SplitHorizontalHandlers, TerminalApp::SplitHorizontalEventArgs); DECLARE_EVENT(IncreaseFontSize, _IncreaseFontSizeHandlers, TerminalApp::IncreaseFontSizeEventArgs); DECLARE_EVENT(DecreaseFontSize, _DecreaseFontSizeHandlers, TerminalApp::DecreaseFontSizeEventArgs); DECLARE_EVENT(ScrollUp, _ScrollUpHandlers, TerminalApp::ScrollUpEventArgs); diff --git a/src/cascadia/TerminalApp/AppKeyBindings.idl b/src/cascadia/TerminalApp/AppKeyBindings.idl index 00212eda60d..5200100ed2c 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.idl +++ b/src/cascadia/TerminalApp/AppKeyBindings.idl @@ -24,6 +24,8 @@ namespace TerminalApp SwitchToTab, NextTab, PrevTab, + SplitVertical, + SplitHorizontal, IncreaseFontSize, DecreaseFontSize, ScrollUp, @@ -40,6 +42,8 @@ namespace TerminalApp delegate void SwitchToTabEventArgs(); delegate void NextTabEventArgs(); delegate void PrevTabEventArgs(); + delegate void SplitVerticalEventArgs(); + delegate void SplitHorizontalEventArgs(); delegate void IncreaseFontSizeEventArgs(); delegate void DecreaseFontSizeEventArgs(); delegate void ScrollUpEventArgs(); @@ -62,6 +66,8 @@ namespace TerminalApp event SwitchToTabEventArgs SwitchToTab; event NextTabEventArgs NextTab; event PrevTabEventArgs PrevTab; + event SplitVerticalEventArgs SplitVertical; + event SplitHorizontalEventArgs SplitHorizontal; event IncreaseFontSizeEventArgs IncreaseFontSize; event DecreaseFontSizeEventArgs DecreaseFontSize; event ScrollUpEventArgs ScrollUp; diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index 0f36c880c69..347bb69502a 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -182,6 +182,13 @@ void CascadiaSettings::_CreateDefaultKeybindings() KeyChord{ KeyModifiers::Ctrl | KeyModifiers::Shift, VK_TAB }); + keyBindings.SetKeyBinding(ShortcutAction::SplitVertical, + KeyChord{ KeyModifiers::Ctrl | KeyModifiers::Shift, + VK_OEM_PLUS }); // For any country/region, the '+' key + keyBindings.SetKeyBinding(ShortcutAction::SplitHorizontal, + KeyChord{ KeyModifiers::Ctrl | KeyModifiers::Shift, + VK_OEM_MINUS }); // For any country/region, the '-' key + // Yes these are offset by one. // Ideally, you'd want C-S-1 to open the _first_ profile, which is index 0 keyBindings.SetKeyBinding(ShortcutAction::NewTabProfile0, diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index dc662b80c0f..d2fd1c3ea43 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -15,7 +15,6 @@ Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermContro _firstChild{ nullptr }, _secondChild{ nullptr } { - // _MakeTabViewItem(); _root = Controls::Grid{}; _root.Children().Append(_control.GetControl()); } @@ -24,21 +23,7 @@ Pane::~Pane() { } -// void Pane::_MakeTabViewItem() -// { -// _tabViewItem = ::winrt::Microsoft::UI::Xaml::Controls::TabViewItem{}; -// const auto title = _control.Title(); - -// _tabViewItem.Header(title); - -// _control.TitleChanged([=](auto newTitle){ -// _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ -// _tabViewItem.Header(newTitle); -// }); -// }); -// } - -winrt::Windows::UI::Xaml::UIElement Pane::GetRootElement() +winrt::Windows::UI::Xaml::Controls::Grid Pane::GetRootElement() { return _root; } @@ -76,29 +61,95 @@ void Pane::SetFocused(bool focused) } } -// GUID Pane::GetProfile() const noexcept -// { -// return _profile; -// } - void Pane::_Focus() { - _focused = true; - _control.GetControl().Focus(FocusState::Programmatic); + // _focused = true; + // _control.GetControl().Focus(FocusState::Programmatic); +} + +void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +{ + if (!_IsLeaf()) + { + return; + } + _splitState = SplitState::Horizontal; + + // Create two columns in this grid. + auto separatorColDef = Controls::ColumnDefinition(); + separatorColDef.Width(GridLengthHelper::Auto()); + + _root.ColumnDefinitions().Append(Controls::ColumnDefinition{}); + _root.ColumnDefinitions().Append(separatorColDef); + _root.ColumnDefinitions().Append(Controls::ColumnDefinition{}); + + _root.Children().Clear(); + + // Create two new Panes + // Move our control, guid into the first one. + // Move the new guid, control into the second. + _firstChild = std::make_shared(_profile.value(), _control); + _profile = std::nullopt; + _control = { nullptr }; + _secondChild = std::make_shared(profile, control); + + // add the first pane to row 0 + _root.Children().Append(_firstChild->GetRootElement()); + Controls::Grid::SetColumn(_firstChild->GetRootElement(), 0); + + _separatorRoot = Controls::Grid{}; + _separatorRoot.Width(8); + // NaN is the special value XAML uses for "Auto" sizing. + _separatorRoot.Height(NAN); + _root.Children().Append(_separatorRoot); + Controls::Grid::SetColumn(_separatorRoot, 1); + + // add the second pane to row 1 + _root.Children().Append(_secondChild->GetRootElement()); + Controls::Grid::SetColumn(_secondChild->GetRootElement(), 2); } -// // Method Description: -// // - Move the viewport of the terminal up or down a number of lines. Negative -// // values of `delta` will move the view up, and positive values will move -// // the viewport down. -// // Arguments: -// // - delta: a number of lines to move the viewport relative to the current viewport. -// // Return Value: -// // - -// void Pane::Scroll(int delta) -// { -// _control.GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ -// const auto currentOffset = _control.GetScrollOffset(); -// _control.ScrollViewport(currentOffset + delta); -// }); -// } +void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +{ + if (!_IsLeaf()) + { + return; + } + _splitState = SplitState::Vertical; + + // Create two rows in this grid. + + auto separatorRowDef = Controls::RowDefinition(); + separatorRowDef.Height(GridLengthHelper::Auto()); + + _root.RowDefinitions().Append(Controls::RowDefinition{}); + _root.RowDefinitions().Append(separatorRowDef); + _root.RowDefinitions().Append(Controls::RowDefinition{}); + + _root.Children().Clear(); + + // Create two new Panes + // Move our control, guid into the first one. + // Move the new guid, control into the second. + _firstChild = std::make_shared(_profile.value(), _control); + _profile = std::nullopt; + _control = { nullptr }; + _secondChild = std::make_shared(profile, control); + + + // add the first pane to row 0 + _root.Children().Append(_firstChild->GetRootElement()); + Controls::Grid::SetRow(_firstChild->GetRootElement(), 0); + + _separatorRoot = Controls::Grid{}; + _separatorRoot.Height(8); + // NaN is the special value XAML uses for "Auto" sizing. + _separatorRoot.Width(NAN); + _root.Children().Append(_separatorRoot); + Controls::Grid::SetRow(_separatorRoot, 1); + + // add the second pane to row 1 + _root.Children().Append(_secondChild->GetRootElement()); + Controls::Grid::SetRow(_secondChild->GetRootElement(), 2); + +} diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 6cd21b9e8bc..3d66d7623f2 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -24,20 +24,20 @@ class Pane winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); // winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); - winrt::Windows::UI::Xaml::UIElement GetRootElement(); + winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); bool IsFocused() const noexcept; void SetFocused(bool focused); // GUID GetProfile() const noexcept; - // void SplitHorizontally(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - // void SplitVertically(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - // void Scroll(int delta); private: winrt::Windows::UI::Xaml::Controls::Grid _root{ nullptr }; + winrt::Windows::UI::Xaml::Controls::Grid _separatorRoot{ nullptr }; winrt::Microsoft::Terminal::TerminalControl::TermControl _control{ nullptr }; std::shared_ptr _firstChild; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 1bfbf170e21..1875765291e 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -31,6 +31,7 @@ void Tab::_MakeTabViewItem() // _tabViewItem.Header(title); + // TODO // _control.TitleChanged([=](auto newTitle){ // _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ // _tabViewItem.Header(newTitle); @@ -83,19 +84,29 @@ void Tab::_Focus() _rootPane->SetFocused(true); } -// TODO -// // Method Description: -// // - Move the viewport of the terminal up or down a number of lines. Negative -// // values of `delta` will move the view up, and positive values will move -// // the viewport down. -// // Arguments: -// // - delta: a number of lines to move the viewport relative to the current viewport. -// // Return Value: -// // - -// void Tab::Scroll(int delta) -// { -// _control.GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ -// const auto currentOffset = _control.GetScrollOffset(); -// _control.ScrollViewport(currentOffset + delta); -// }); -// } +// Method Description: +// - Move the viewport of the terminal up or down a number of lines. Negative +// values of `delta` will move the view up, and positive values will move +// the viewport down. +// Arguments: +// - delta: a number of lines to move the viewport relative to the current viewport. +// Return Value: +// - +void Tab::Scroll(int delta) +{ + GetFocusedTerminalControl().GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ + auto control = GetFocusedTerminalControl(); + const auto currentOffset = control.GetScrollOffset(); + control.ScrollViewport(currentOffset + delta); + }); +} + +void Tab::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +{ + _rootPane->SplitVertical(profile, control); +} + +void Tab::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +{ + _rootPane->SplitHorizontal(profile, control); +} diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 17240ce3110..ac360e65c75 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -20,17 +20,15 @@ class Tab bool IsFocused(); void SetFocused(bool focused); - // GUID GetProfile() const noexcept; - - // void Scroll(int delta); + void Scroll(int delta); + void SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); private: std::shared_ptr _rootPane; - // winrt::Microsoft::Terminal::TerminalControl::TermControl _control; bool _focused; - // GUID _profile; winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem; void _MakeTabViewItem(); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 185c4d8c82f..e33c2bfa24a 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -230,15 +230,21 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation TermControl::~TermControl() { _closing = true; - // Don't let anyone else do something to the buffer. - auto lock = _terminal->LockForWriting(); + if (_initializedTerminal) + { + // Don't let anyone else do something to the buffer. + auto lock = _terminal->LockForWriting(); + } if (_connection != nullptr) { _connection.Close(); } - _renderer->TriggerTeardown(); + if (_renderer != nullptr) + { + _renderer->TriggerTeardown(); + } _swapChainPanel = nullptr; _root = nullptr; From a600b204a5939d9df14b6bbc3d7aeaee61ec5f51 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 9 May 2019 12:54:58 -0500 Subject: [PATCH 03/33] This works for nesting panes Moving focus between panes is still a janky - selection isn't dismissed when the TermControl loses focus, and the cursor remains visible. Switching to a tab with panes doesn't focus _any_ control, so that's bad. --- src/cascadia/TerminalApp/Pane.cpp | 32 +++++++++++++++++++++++++++++-- src/cascadia/TerminalApp/Pane.h | 1 + 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index d2fd1c3ea43..322f3e48e0f 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -51,6 +51,16 @@ bool Pane::_IsLeaf() const noexcept return _splitState == SplitState::None; } +bool Pane::_HasFocusedChild() const noexcept +{ + const bool controlFocused = _control != nullptr && + _control.GetControl().FocusState() != FocusState::Unfocused; + const bool firstFocused = _firstChild != nullptr && _firstChild->_HasFocusedChild(); + const bool secondFocused = _secondChild != nullptr && _secondChild->_HasFocusedChild(); + + return controlFocused || firstFocused || secondFocused; +} + void Pane::SetFocused(bool focused) { _focused = focused; @@ -71,9 +81,18 @@ void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalContr { if (!_IsLeaf()) { + if (_firstChild->_HasFocusedChild()) + { + _firstChild->SplitVertical(profile, control); + } + else if (_secondChild->_HasFocusedChild()) + { + _secondChild->SplitVertical(profile, control); + } + return; } - _splitState = SplitState::Horizontal; + _splitState = SplitState::Vertical; // Create two columns in this grid. auto separatorColDef = Controls::ColumnDefinition(); @@ -113,9 +132,18 @@ void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalCon { if (!_IsLeaf()) { + if (_firstChild->_HasFocusedChild()) + { + _firstChild->SplitHorizontal(profile, control); + } + else if (_secondChild->_HasFocusedChild()) + { + _secondChild->SplitHorizontal(profile, control); + } + return; } - _splitState = SplitState::Vertical; + _splitState = SplitState::Horizontal; // Create two rows in this grid. diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 3d66d7623f2..a46ad5dd4b3 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -52,4 +52,5 @@ class Pane void _Focus(); bool _IsLeaf() const noexcept; + bool _HasFocusedChild() const noexcept; }; From faaba470aaf1c8e42983509144bb614d5bc1dd30 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 10 May 2019 10:10:18 -0500 Subject: [PATCH 04/33] Switching tabs keeps focus on the last active pane now --- src/cascadia/TerminalApp/App.cpp | 19 ++++++++++++ src/cascadia/TerminalApp/Pane.cpp | 51 ++++++++++++++++++++++--------- src/cascadia/TerminalApp/Pane.h | 13 +++----- src/cascadia/TerminalApp/Tab.cpp | 15 +++++++-- src/cascadia/TerminalApp/Tab.h | 2 ++ 5 files changed, 76 insertions(+), 24 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 5fea8ce5e92..a07a282333a 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -576,6 +576,9 @@ namespace winrt::TerminalApp::implementation // Initialize the new tab TermControl term{ settings }; + // TODO" All these event handles we hook up to the termControl, we need + // to put it intp a single method that the Split* methods use too. + // Add an event handler when the terminal's selection wants to be copied. // When the text buffer data is retrieved, we'll copy the data into the Clipboard term.CopyToClipboard([=](auto copiedData) { @@ -613,6 +616,12 @@ namespace winrt::TerminalApp::implementation // } // }); + term.GetControl().GotFocus([newTab](auto&&, auto&&) + { + newTab->CheckFocus(); + }); + + auto tabViewItem = newTab->GetTabViewItem(); _tabView.Items().Append(tabViewItem); @@ -897,6 +906,11 @@ namespace winrt::TerminalApp::implementation int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; + newControl.GetControl().GotFocus([focusedTab](auto&&, auto&&) + { + focusedTab->CheckFocus(); + }); + return focusedTab->SplitVertical(realGuid, newControl); } @@ -910,6 +924,11 @@ namespace winrt::TerminalApp::implementation int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; + newControl.GetControl().GotFocus([focusedTab](auto&&, auto&&) + { + focusedTab->CheckFocus(); + }); + return focusedTab->SplitHorizontal(realGuid, newControl); } diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 322f3e48e0f..2306d260ec3 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -7,9 +7,9 @@ using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Core; -Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) : +Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused) : _control{ control }, - _focused{ false }, + _lastFocused{ lastFocused }, _profile{ profile }, _splitState{ SplitState::None }, _firstChild{ nullptr }, @@ -41,9 +41,27 @@ winrt::Microsoft::Terminal::TerminalControl::TermControl Pane::GetFocusedTermina } } -bool Pane::IsFocused() const noexcept +winrt::Microsoft::Terminal::TerminalControl::TermControl Pane::GetLastFocusedTerminalControl() { - return _focused; + if (_IsLeaf()) + { + return _lastFocused ? _control : nullptr; + } + else + { + auto firstFocused = _firstChild->GetLastFocusedTerminalControl(); + if (firstFocused != nullptr) + { + return firstFocused; + } + auto secondFocused = _secondChild->GetLastFocusedTerminalControl(); + return secondFocused; + } +} + +bool Pane::WasLastFocused() const noexcept +{ + return _lastFocused; } bool Pane::_IsLeaf() const noexcept @@ -61,22 +79,23 @@ bool Pane::_HasFocusedChild() const noexcept return controlFocused || firstFocused || secondFocused; } -void Pane::SetFocused(bool focused) +void Pane::CheckFocus() { - _focused = focused; + if (_IsLeaf()) + { + const bool controlFocused = _control != nullptr && + _control.GetControl().FocusState() != FocusState::Unfocused; - if (_focused) + _lastFocused = controlFocused; + } + else { - _Focus(); + _lastFocused = false; + _firstChild->CheckFocus(); + _secondChild->CheckFocus(); } } -void Pane::_Focus() -{ - // _focused = true; - // _control.GetControl().Focus(FocusState::Programmatic); -} - void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) { if (!_IsLeaf()) @@ -126,6 +145,9 @@ void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalContr // add the second pane to row 1 _root.Children().Append(_secondChild->GetRootElement()); Controls::Grid::SetColumn(_secondChild->GetRootElement(), 2); + + + _lastFocused = false; } void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) @@ -180,4 +202,5 @@ void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalCon _root.Children().Append(_secondChild->GetRootElement()); Controls::Grid::SetRow(_secondChild->GetRootElement(), 2); + _lastFocused = false; } diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index a46ad5dd4b3..f824c2708db 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -18,16 +18,17 @@ class Pane Horizontal = 2 }; - Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused = false); ~Pane(); winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); // winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + winrt::Microsoft::Terminal::TerminalControl::TermControl GetLastFocusedTerminalControl(); winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); - bool IsFocused() const noexcept; - void SetFocused(bool focused); + bool WasLastFocused() const noexcept; + void CheckFocus(); // GUID GetProfile() const noexcept; @@ -44,12 +45,8 @@ class Pane std::shared_ptr _secondChild; SplitState _splitState; - bool _focused; + bool _lastFocused; std::optional _profile; - // winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem; - - void _MakeTabViewItem(); - void _Focus(); bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 1875765291e..eccf7e32f93 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -12,7 +12,7 @@ Tab::Tab(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl _tabViewItem{ nullptr }, _rootPane{ nullptr } { - _rootPane = std::make_shared(profile, control); + _rootPane = std::make_shared(profile, control, true); _MakeTabViewItem(); } @@ -81,7 +81,18 @@ void Tab::_Focus() { _focused = true; // _control.GetControl().Focus(FocusState::Programmatic); - _rootPane->SetFocused(true); + // _rootPane->SetFocused(true); + auto lastFocusedControl = _rootPane->GetLastFocusedTerminalControl(); + if (lastFocusedControl) + { + lastFocusedControl.GetControl().Focus(FocusState::Programmatic); + } +} + + +void Tab::CheckFocus() +{ + _rootPane->CheckFocus(); } // Method Description: diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index ac360e65c75..b25eec729e8 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -24,6 +24,8 @@ class Tab void SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void CheckFocus(); + private: std::shared_ptr _rootPane; From 9b792d3faf9f4bb28166ced8acd8b488369e3074 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 09:38:32 -0500 Subject: [PATCH 05/33] Reload settings for the panes. On reload of settings, change the icon of the tab to that of the last focused pane --- src/cascadia/TerminalApp/App.cpp | 36 ++++++++++++++++++------------- src/cascadia/TerminalApp/Pane.cpp | 35 ++++++++++++++++++++++++++++++ src/cascadia/TerminalApp/Pane.h | 7 +++--- src/cascadia/TerminalApp/Tab.cpp | 16 ++++++++------ src/cascadia/TerminalApp/Tab.h | 4 +++- 5 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index a07a282333a..2a039d14ea4 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -419,24 +419,30 @@ namespace winrt::TerminalApp::implementation for (auto &tab : _tabs) { - // TODO - // const auto term = tab->GetTerminalControl(); - // const GUID tabProfile = tab->GetProfile(); - - // if (profileGuid == tabProfile) - // { - // term.UpdateSettings(settings); - - // // Update the icons of the tabs with this profile open. - // auto tabViewItem = tab->GetTabViewItem(); - // tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [profile, tabViewItem]() { - // // _GetIconFromProfile has to run on the main thread - // tabViewItem.Icon(App::_GetIconFromProfile(profile)); - // }); - // } + // Attempt to reload the settings of any panes with this profile + tab->CheckUpdateSettings(settings, profileGuid); } } + // Update the icon of the tab for the currently focused profile in that tab. + for (auto &tab : _tabs) + { + const auto lastFocusedProfileOpt = tab->GetLastFocusedProfile(); + if (lastFocusedProfileOpt.has_value()) + { + const auto lastFocusedProfile = lastFocusedProfileOpt.value(); + + auto tabViewItem = tab->GetTabViewItem(); + tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this, lastFocusedProfile, tabViewItem]() { + // _GetIconFromProfile has to run on the main thread + const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile); + if (matchingProfile) + { + tabViewItem.Icon(App::_GetIconFromProfile(*matchingProfile)); + } + }); + } + } _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { // Refresh the UI theme diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 2306d260ec3..366ad2b7837 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -6,6 +6,7 @@ using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Core; +using namespace winrt::Microsoft::Terminal::Settings; Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused) : _control{ control }, @@ -59,6 +60,24 @@ winrt::Microsoft::Terminal::TerminalControl::TermControl Pane::GetLastFocusedTer } } +std::optional Pane::GetLastFocusedProfile() const noexcept +{ + if (_IsLeaf()) + { + return _lastFocused ? _profile : std::nullopt; + } + else + { + auto firstFocused = _firstChild->GetLastFocusedProfile(); + if (firstFocused.has_value()) + { + return firstFocused; + } + auto secondFocused = _secondChild->GetLastFocusedProfile(); + return secondFocused; + } +} + bool Pane::WasLastFocused() const noexcept { return _lastFocused; @@ -96,6 +115,22 @@ void Pane::CheckFocus() } } +void Pane::CheckUpdateSettings(TerminalSettings settings, GUID profile) +{ + if (!_IsLeaf()) + { + _firstChild->CheckUpdateSettings(settings, profile); + _secondChild->CheckUpdateSettings(settings, profile); + } + else + { + if (profile == _profile) + { + _control.UpdateSettings(settings); + } + } +} + void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) { if (!_IsLeaf()) diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index f824c2708db..0b437567b9e 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -2,10 +2,8 @@ // Licensed under the MIT license. #pragma once -#include #include - class Pane { @@ -21,15 +19,18 @@ class Pane Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused = false); ~Pane(); - winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); + // winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); // winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetLastFocusedTerminalControl(); + std::optional GetLastFocusedProfile() const noexcept; winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); bool WasLastFocused() const noexcept; void CheckFocus(); + void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); + // GUID GetProfile() const noexcept; void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index eccf7e32f93..cc75400124c 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -6,6 +6,7 @@ using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Core; +using namespace winrt::Microsoft::Terminal::Settings; Tab::Tab(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) : _focused{ false }, @@ -71,11 +72,15 @@ void Tab::SetFocused(bool focused) } } -// TODO -// GUID Tab::GetProfile() const noexcept -// { -// return _profile; -// } +std::optional Tab::GetLastFocusedProfile() const noexcept +{ + return _rootPane->GetLastFocusedProfile(); +} + +void Tab::CheckUpdateSettings(TerminalSettings settings, GUID profile) +{ + _rootPane->CheckUpdateSettings(settings, profile); +} void Tab::_Focus() { @@ -89,7 +94,6 @@ void Tab::_Focus() } } - void Tab::CheckFocus() { _rootPane->CheckFocus(); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index b25eec729e8..bd71f969bc6 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -3,7 +3,6 @@ #pragma once #include -#include #include "Pane.h" class Tab @@ -16,6 +15,7 @@ class Tab winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); winrt::Windows::UI::Xaml::UIElement GetRootElement(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + std::optional GetLastFocusedProfile() const noexcept; bool IsFocused(); void SetFocused(bool focused); @@ -26,6 +26,8 @@ class Tab void CheckFocus(); + void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); + private: std::shared_ptr _rootPane; From 7ca3e8df2c5c4c93f18c941df6b3e9940a212614 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 10:46:32 -0500 Subject: [PATCH 06/33] Get titles working again --- src/cascadia/TerminalApp/App.cpp | 90 +++++++++++++++++++------------- src/cascadia/TerminalApp/App.h | 1 + src/cascadia/TerminalApp/Tab.cpp | 21 ++++++-- src/cascadia/TerminalApp/Tab.h | 2 + 4 files changed, 75 insertions(+), 39 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 2a039d14ea4..81d0227204a 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -424,7 +424,8 @@ namespace winrt::TerminalApp::implementation } } - // Update the icon of the tab for the currently focused profile in that tab. + // Update the icon of the tab for the currently focused profile in that + // tab. for (auto &tab : _tabs) { const auto lastFocusedProfileOpt = tab->GetLastFocusedProfile(); @@ -572,22 +573,15 @@ namespace winrt::TerminalApp::implementation eventArgs.HandleClipboardData(text); } - // Method Description: - // - Creates a new tab with the given settings. If the tab bar is not being - // currently displayed, it will be shown. - // Arguments: - // - settings: the TerminalSettings object to use to create the TerminalControl with. - void App::_CreateNewTabFromSettings(GUID profileGuid, TerminalSettings settings) + void App::_RegisterTerminalEvents(TermControl term, std::shared_ptr hostingTab) { - // Initialize the new tab - TermControl term{ settings }; // TODO" All these event handles we hook up to the termControl, we need // to put it intp a single method that the Split* methods use too. // Add an event handler when the terminal's selection wants to be copied. // When the text buffer data is retrieved, we'll copy the data into the Clipboard - term.CopyToClipboard([=](auto copiedData) { + term.CopyToClipboard([this](auto copiedData) { _root.Dispatcher().RunAsync(CoreDispatcherPriority::High, [copiedData]() { DataPackage dataPack = DataPackage(); dataPack.RequestedOperation(DataPackageOperation::Copy); @@ -600,32 +594,62 @@ namespace winrt::TerminalApp::implementation }); // Add an event handler when the terminal wants to paste data from the Clipboard. - term.PasteFromClipboard([=](auto /*sender*/, auto eventArgs) { + term.PasteFromClipboard([this](auto /*sender*/, auto eventArgs) { _root.Dispatcher().RunAsync(CoreDispatcherPriority::High, [eventArgs]() { PasteFromClipboard(eventArgs); }); }); - // Add the new tab to the list of our tabs. - auto newTab = _tabs.emplace_back(std::make_shared(profileGuid, term)); - - // TODO: - // // Add an event handler when the terminal's title changes. When the - // // title changes, we'll bubble it up to listeners of our own title - // // changed event, so they can handle it. - // newTab->GetTerminalControl().TitleChanged([=](auto newTitle){ - // // Only bubble the change if this tab is the focused tab. - // if (_settings->GlobalSettings().GetShowTitleInTitlebar() && - // newTab->IsFocused()) - // { - // _titleChangeHandlers(newTitle); - // } - // }); + term.TitleChanged([this, hostingTab](auto newTitle){ + // The title of the control changed, but not necessarily the title + // of the tab. Get the title of the focused pane of the tab, and set + // the tab's text to the focused panes' text. + auto newTabTitle = hostingTab->CheckTitleUpdate(); + + // TODO #608: If the settings don't want the terminal's text in the + // tab, then display something else. + hostingTab->SetTabText(newTabTitle); + if (_settings->GlobalSettings().GetShowTitleInTitlebar() && + hostingTab->IsFocused()) + { + _titleChangeHandlers(newTabTitle); + } + }); - term.GetControl().GotFocus([newTab](auto&&, auto&&) + // TODO: this feels like it should be a weak ref, not a strong ref. + term.GetControl().GotFocus([this, hostingTab](auto&&, auto&&) { - newTab->CheckFocus(); + hostingTab->CheckFocus(); + // Possibly update the title of the tab, window to match the newly + // focused pane. + auto newTabTitle = hostingTab->CheckTitleUpdate(); + + // TODO #608: If the settings don't want the terminal's text in the + // tab, then display something else. + hostingTab->SetTabText(newTabTitle); + if (_settings->GlobalSettings().GetShowTitleInTitlebar() && + hostingTab->IsFocused()) + { + _titleChangeHandlers(newTabTitle); + } }); + } + + // Method Description: + // - Creates a new tab with the given settings. If the tab bar is not being + // currently displayed, it will be shown. + // Arguments: + // - settings: the TerminalSettings object to use to create the TerminalControl with. + void App::_CreateNewTabFromSettings(GUID profileGuid, TerminalSettings settings) + { + // Initialize the new tab + TermControl term{ settings }; + + + // Add the new tab to the list of our tabs. + auto newTab = _tabs.emplace_back(std::make_shared(profileGuid, term)); + + _RegisterTerminalEvents(term, newTab); auto tabViewItem = newTab->GetTabViewItem(); @@ -912,10 +936,7 @@ namespace winrt::TerminalApp::implementation int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; - newControl.GetControl().GotFocus([focusedTab](auto&&, auto&&) - { - focusedTab->CheckFocus(); - }); + _RegisterTerminalEvents(newControl, focusedTab); return focusedTab->SplitVertical(realGuid, newControl); } @@ -930,10 +951,7 @@ namespace winrt::TerminalApp::implementation int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; - newControl.GetControl().GotFocus([focusedTab](auto&&, auto&&) - { - focusedTab->CheckFocus(); - }); + _RegisterTerminalEvents(newControl, focusedTab); return focusedTab->SplitHorizontal(realGuid, newControl); } diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 1d1882f48c0..67adb48fff8 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -81,6 +81,7 @@ namespace winrt::TerminalApp::implementation void _FeedbackButtonOnClick(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs); void _UpdateTabView(); + void _RegisterTerminalEvents(Microsoft::Terminal::TerminalControl::TermControl term, std::shared_ptr hostingTab); void _CreateNewTabFromSettings(GUID profileGuid, winrt::Microsoft::Terminal::Settings::TerminalSettings settings); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index cc75400124c..68136d4d2c4 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -28,15 +28,17 @@ Tab::~Tab() void Tab::_MakeTabViewItem() { _tabViewItem = ::winrt::Microsoft::UI::Xaml::Controls::TabViewItem{}; + // auto f = _tabViewItem.FontSize(); + _tabViewItem.FontSize(12); + // auto a = 0; + // a++; + // const auto title = _control.Title(); // _tabViewItem.Header(title); // TODO // _control.TitleChanged([=](auto newTitle){ - // _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - // _tabViewItem.Header(newTitle); - // }); // }); } @@ -99,6 +101,19 @@ void Tab::CheckFocus() _rootPane->CheckFocus(); } +winrt::hstring Tab::CheckTitleUpdate() +{ + auto lastFocusedControl = _rootPane->GetLastFocusedTerminalControl(); + return lastFocusedControl ? lastFocusedControl.Title() : L""; +} + +void Tab::SetTabText(const winrt::hstring& text) +{ + _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ + _tabViewItem.Header(text); + }); +} + // Method Description: // - Move the viewport of the terminal up or down a number of lines. Negative // values of `delta` will move the view up, and positive values will move diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index bd71f969bc6..3d496c141c4 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -27,6 +27,8 @@ class Tab void CheckFocus(); void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); + winrt::hstring CheckTitleUpdate(); + void SetTabText(const winrt::hstring& text); private: From db7523af84cf45198873922d24ae575060c93df4 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 10:52:08 -0500 Subject: [PATCH 07/33] Update the icon too --- src/cascadia/TerminalApp/App.cpp | 17 +++++++++++++++++ src/cascadia/TerminalApp/Tab.cpp | 16 +--------------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 81d0227204a..4f838c1be24 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -632,6 +632,23 @@ namespace winrt::TerminalApp::implementation { _titleChangeHandlers(newTabTitle); } + + // Copied from _ReloadSettings + const auto lastFocusedProfileOpt = hostingTab->GetLastFocusedProfile(); + if (lastFocusedProfileOpt.has_value()) + { + const auto lastFocusedProfile = lastFocusedProfileOpt.value(); + + auto tabViewItem = hostingTab->GetTabViewItem(); + tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this, lastFocusedProfile, tabViewItem]() { + // _GetIconFromProfile has to run on the main thread + const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile); + if (matchingProfile) + { + tabViewItem.Icon(App::_GetIconFromProfile(*matchingProfile)); + } + }); + } }); } diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 68136d4d2c4..770c41d3fb6 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -28,18 +28,7 @@ Tab::~Tab() void Tab::_MakeTabViewItem() { _tabViewItem = ::winrt::Microsoft::UI::Xaml::Controls::TabViewItem{}; - // auto f = _tabViewItem.FontSize(); _tabViewItem.FontSize(12); - // auto a = 0; - // a++; - - // const auto title = _control.Title(); - - // _tabViewItem.Header(title); - - // TODO - // _control.TitleChanged([=](auto newTitle){ - // }); } winrt::Windows::UI::Xaml::UIElement Tab::GetRootElement() @@ -50,7 +39,6 @@ winrt::Windows::UI::Xaml::UIElement Tab::GetRootElement() winrt::Microsoft::Terminal::TerminalControl::TermControl Tab::GetFocusedTerminalControl() { return _rootPane->GetFocusedTerminalControl(); - // return _control; } winrt::Microsoft::UI::Xaml::Controls::TabViewItem Tab::GetTabViewItem() @@ -58,7 +46,6 @@ winrt::Microsoft::UI::Xaml::Controls::TabViewItem Tab::GetTabViewItem() return _tabViewItem; } - bool Tab::IsFocused() { return _focused; @@ -87,8 +74,7 @@ void Tab::CheckUpdateSettings(TerminalSettings settings, GUID profile) void Tab::_Focus() { _focused = true; - // _control.GetControl().Focus(FocusState::Programmatic); - // _rootPane->SetFocused(true); + auto lastFocusedControl = _rootPane->GetLastFocusedTerminalControl(); if (lastFocusedControl) { From 7258a7179c47fd8a348e843839e016e6e4054d11 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 11:04:31 -0500 Subject: [PATCH 08/33] Pull these two guys into helpers --- src/cascadia/TerminalApp/App.cpp | 100 ++++++++++++++----------------- src/cascadia/TerminalApp/App.h | 4 ++ 2 files changed, 49 insertions(+), 55 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 4f838c1be24..f03f605aed7 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -424,25 +424,10 @@ namespace winrt::TerminalApp::implementation } } - // Update the icon of the tab for the currently focused profile in that - // tab. - for (auto &tab : _tabs) + // Update the icon of the tab for the currently focused profile in that tab. + for (auto& tab : _tabs) { - const auto lastFocusedProfileOpt = tab->GetLastFocusedProfile(); - if (lastFocusedProfileOpt.has_value()) - { - const auto lastFocusedProfile = lastFocusedProfileOpt.value(); - - auto tabViewItem = tab->GetTabViewItem(); - tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this, lastFocusedProfile, tabViewItem]() { - // _GetIconFromProfile has to run on the main thread - const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile); - if (matchingProfile) - { - tabViewItem.Icon(App::_GetIconFromProfile(*matchingProfile)); - } - }); - } + _UpdateTabIcon(tab); } _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() { @@ -456,6 +441,40 @@ namespace winrt::TerminalApp::implementation } + void App::_UpdateTabIcon(std::shared_ptr tab) + { + const auto lastFocusedProfileOpt = tab->GetLastFocusedProfile(); + if (lastFocusedProfileOpt.has_value()) + { + const auto lastFocusedProfile = lastFocusedProfileOpt.value(); + + auto tabViewItem = tab->GetTabViewItem(); + tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this, lastFocusedProfile, tabViewItem]() { + // _GetIconFromProfile has to run on the main thread + const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile); + if (matchingProfile) + { + tabViewItem.Icon(App::_GetIconFromProfile(*matchingProfile)); + } + }); + } + } + + void App::_CheckTitleUpdate(std::shared_ptr tab) + { + auto newTabTitle = tab->CheckTitleUpdate(); + + // TODO #608: If the settings don't want the terminal's text in the + // tab, then display something else. + tab->SetTabText(newTabTitle); + if (_settings->GlobalSettings().GetShowTitleInTitlebar() && + tab->IsFocused()) + { + _titleChangeHandlers(newTabTitle); + } + + } + // Method Description: // - Update the current theme of the application. This will manually update // all of the elements in our UI to match the given theme. @@ -600,55 +619,26 @@ namespace winrt::TerminalApp::implementation }); }); + // TODO: hostingTab feels like it should be a weak ref, not a strong ref. term.TitleChanged([this, hostingTab](auto newTitle){ // The title of the control changed, but not necessarily the title // of the tab. Get the title of the focused pane of the tab, and set // the tab's text to the focused panes' text. - auto newTabTitle = hostingTab->CheckTitleUpdate(); - - // TODO #608: If the settings don't want the terminal's text in the - // tab, then display something else. - hostingTab->SetTabText(newTabTitle); - if (_settings->GlobalSettings().GetShowTitleInTitlebar() && - hostingTab->IsFocused()) - { - _titleChangeHandlers(newTabTitle); - } + _CheckTitleUpdate(hostingTab); }); - // TODO: this feels like it should be a weak ref, not a strong ref. + // TODO: hostingTab feels like it should be a weak ref, not a strong ref. term.GetControl().GotFocus([this, hostingTab](auto&&, auto&&) { + // Update the focus of the tab's panes hostingTab->CheckFocus(); + // Possibly update the title of the tab, window to match the newly // focused pane. - auto newTabTitle = hostingTab->CheckTitleUpdate(); + _CheckTitleUpdate(hostingTab); - // TODO #608: If the settings don't want the terminal's text in the - // tab, then display something else. - hostingTab->SetTabText(newTabTitle); - if (_settings->GlobalSettings().GetShowTitleInTitlebar() && - hostingTab->IsFocused()) - { - _titleChangeHandlers(newTabTitle); - } - - // Copied from _ReloadSettings - const auto lastFocusedProfileOpt = hostingTab->GetLastFocusedProfile(); - if (lastFocusedProfileOpt.has_value()) - { - const auto lastFocusedProfile = lastFocusedProfileOpt.value(); - - auto tabViewItem = hostingTab->GetTabViewItem(); - tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this, lastFocusedProfile, tabViewItem]() { - // _GetIconFromProfile has to run on the main thread - const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile); - if (matchingProfile) - { - tabViewItem.Icon(App::_GetIconFromProfile(*matchingProfile)); - } - }); - } + // Possibly update the icon of the tab. + _UpdateTabIcon(hostingTab); }); } diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 67adb48fff8..d2aef9e4ffd 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -81,6 +81,10 @@ namespace winrt::TerminalApp::implementation void _FeedbackButtonOnClick(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs); void _UpdateTabView(); + void _UpdateTabIcon(std::shared_ptr tab); + void _CheckTitleUpdate(std::shared_ptr tab); + + void _RegisterTerminalEvents(Microsoft::Terminal::TerminalControl::TermControl term, std::shared_ptr hostingTab); void _CreateNewTabFromSettings(GUID profileGuid, winrt::Microsoft::Terminal::Settings::TerminalSettings settings); From 377aa4f3053ebfeb18f3a867c246ea3a90a1eab9 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 13:21:30 -0500 Subject: [PATCH 09/33] Hook up the terminal's closing to the pane's to the tab's But something seems fucky with the closing of the tabs. I'm getting crashes, when I close with `exit` from the commandline and `closeOnExit:true`, but I don't think it has anything to with my change. Gonna mess with master and see if it's busted. --- src/cascadia/TerminalApp/App.cpp | 53 +++++++------ src/cascadia/TerminalApp/Pane.cpp | 78 +++++++++++++++++++- src/cascadia/TerminalApp/Pane.h | 5 ++ src/cascadia/TerminalApp/Tab.cpp | 8 ++ src/cascadia/TerminalApp/Tab.h | 2 + src/cascadia/TerminalControl/TermControl.cpp | 8 +- src/cascadia/TerminalControl/TermControl.h | 1 + src/cascadia/TerminalControl/TermControl.idl | 1 + 8 files changed, 128 insertions(+), 28 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 63176688e54..08101bb5d1b 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -596,10 +596,6 @@ namespace winrt::TerminalApp::implementation void App::_RegisterTerminalEvents(TermControl term, std::shared_ptr hostingTab) { - - // TODO" All these event handles we hook up to the termControl, we need - // to put it intp a single method that the Split* methods use too. - // Add an event handler when the terminal's selection wants to be copied. // When the text buffer data is retrieved, we'll copy the data into the Clipboard term.CopyToClipboard([this](auto copiedData) { @@ -642,6 +638,30 @@ namespace winrt::TerminalApp::implementation // Possibly update the icon of the tab. _UpdateTabIcon(hostingTab); }); + + // hostingTab->Closed([](){ + // auto tabViewItem = hostingTab->GetTabViewItem(); + // // _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [hostingTab, tabViewItem, this]() { + + // _RemoveTabViewItem(tabViewItem); + + // }); + // TODO: + // // Add an event handler when the terminal's connection is closed. + // hostingTab->GetTerminalControl().ConnectionClosed([=]() { + // _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [hostingTab, tabViewItem, this]() { + // const GUID tabProfile = hostingTab->GetProfile(); + // // Don't just capture this pointer, because the profile might + // // get destroyed before this is called (case in point - + // // reloading settings) + // const auto* const p = _settings->FindProfile(tabProfile); + + // if (p != nullptr && p->GetCloseOnExit()) + // { + // _RemoveTabViewItem(tabViewItem); + // } + // }); + // }); } // Method Description: @@ -654,13 +674,11 @@ namespace winrt::TerminalApp::implementation // Initialize the new tab TermControl term{ settings }; - // Add the new tab to the list of our tabs. auto newTab = _tabs.emplace_back(std::make_shared(profileGuid, term)); _RegisterTerminalEvents(term, newTab); - auto tabViewItem = newTab->GetTabViewItem(); _tabView.Items().Append(tabViewItem); @@ -672,25 +690,14 @@ namespace winrt::TerminalApp::implementation tabViewItem.Icon(_GetIconFromProfile(*profile)); } - // TODO: - // // Add an event handler when the terminal's connection is closed. - // newTab->GetTerminalControl().ConnectionClosed([=]() { - // _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [newTab, tabViewItem, this]() { - // const GUID tabProfile = newTab->GetProfile(); - // // Don't just capture this pointer, because the profile might - // // get destroyed before this is called (case in point - - // // reloading settings) - // const auto* const p = _settings->FindProfile(tabProfile); - - // if (p != nullptr && p->GetCloseOnExit()) - // { - // _RemoveTabViewItem(tabViewItem); - // } - // }); - // }); - tabViewItem.PointerPressed({ this, &App::_OnTabClick }); + newTab->Closed([tabViewItem, this](){ + _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tabViewItem, this]() { + _RemoveTabViewItem(tabViewItem); + }); + }); + // This is one way to set the tab's selected background color. // tabViewItem.Resources().Insert(winrt::box_value(L"TabViewItemHeaderBackgroundSelected"), a Brush?); diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 366ad2b7837..5d59622b72c 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -7,6 +7,7 @@ using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; +using namespace winrt::Microsoft::Terminal::TerminalControl; Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused) : _control{ control }, @@ -14,22 +15,36 @@ Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermContro _profile{ profile }, _splitState{ SplitState::None }, _firstChild{ nullptr }, - _secondChild{ nullptr } + _secondChild{ nullptr }, + _connectionClosedToken{} { _root = Controls::Grid{}; - _root.Children().Append(_control.GetControl()); + _AddControlToRoot(_control); } Pane::~Pane() { } -winrt::Windows::UI::Xaml::Controls::Grid Pane::GetRootElement() +void Pane::_AddControlToRoot(TermControl control) +{ + _root.Children().Append(control.GetControl()); + + _connectionClosedToken = control.ConnectionClosed([=]() { + if (control.CloseOnExit()) + { + // Fire our ConnectionClosed event + _closedHandlers(); + } + }); +} + +Controls::Grid Pane::GetRootElement() { return _root; } -winrt::Microsoft::Terminal::TerminalControl::TermControl Pane::GetFocusedTerminalControl() +TermControl Pane::GetFocusedTerminalControl() { if (_IsLeaf()) { @@ -131,6 +146,53 @@ void Pane::CheckUpdateSettings(TerminalSettings settings, GUID profile) } } +void Pane::_SetupChildCloseHandlers() +{ + + _firstChild->Closed([this](){ + + _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ + _control = _secondChild->_control; + _profile = _secondChild->_profile; + _secondChild->_root.Children().Clear(); + _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; + _root.Children().Clear(); + _root.ColumnDefinitions().Clear(); + _root.RowDefinitions().Clear(); + _separatorRoot = { nullptr }; + _AddControlToRoot(_control); + if (_lastFocused) + { + _control.GetControl().Focus(FocusState::Programmatic); + } + _splitState = SplitState::None; + _firstChild = nullptr; + _secondChild = nullptr; + }); + }); + + _secondChild->Closed([this](){ + _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ + _control = _firstChild->_control; + _profile = _firstChild->_profile; + _firstChild->_root.Children().Clear(); + _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; + _root.Children().Clear(); + _root.ColumnDefinitions().Clear(); + _root.RowDefinitions().Clear(); + _separatorRoot = { nullptr }; + _AddControlToRoot(_control); + if (_lastFocused) + { + _control.GetControl().Focus(FocusState::Programmatic); + } + _splitState = SplitState::None; + _firstChild = nullptr; + _secondChild = nullptr; + }); + }); +} + void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) { if (!_IsLeaf()) @@ -146,6 +208,9 @@ void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalContr return; } + // revoke our handler - the child will take care of the control now. + _control.ConnectionClosed(_connectionClosedToken); + _splitState = SplitState::Vertical; // Create two columns in this grid. @@ -181,6 +246,7 @@ void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalContr _root.Children().Append(_secondChild->GetRootElement()); Controls::Grid::SetColumn(_secondChild->GetRootElement(), 2); + _SetupChildCloseHandlers(); _lastFocused = false; } @@ -237,5 +303,9 @@ void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalCon _root.Children().Append(_secondChild->GetRootElement()); Controls::Grid::SetRow(_secondChild->GetRootElement(), 2); + _SetupChildCloseHandlers(); + _lastFocused = false; } + +DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 0b437567b9e..941c4d3900c 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -3,6 +3,7 @@ #pragma once #include +#include "../../cascadia/inc/cppwinrt_utils.h" class Pane { @@ -36,6 +37,7 @@ class Pane void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); void SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); private: winrt::Windows::UI::Xaml::Controls::Grid _root{ nullptr }; @@ -48,7 +50,10 @@ class Pane bool _lastFocused; std::optional _profile; + winrt::event_token _connectionClosedToken; bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; + void _SetupChildCloseHandlers(); + void _AddControlToRoot(winrt::Microsoft::Terminal::TerminalControl::TermControl control); }; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 770c41d3fb6..1abfe7e71aa 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -7,6 +7,7 @@ using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; +using namespace winrt::Microsoft::Terminal::TerminalControl; Tab::Tab(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) : _focused{ false }, @@ -14,6 +15,11 @@ Tab::Tab(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl _rootPane{ nullptr } { _rootPane = std::make_shared(profile, control, true); + + _rootPane->Closed([=]() { + _closedHandlers(); + }); + _MakeTabViewItem(); } @@ -126,3 +132,5 @@ void Tab::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalCont { _rootPane->SplitHorizontal(profile, control); } + +DEFINE_EVENT(Tab, Closed, _closedHandlers, ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 3d496c141c4..4393c9c4a26 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -30,6 +30,8 @@ class Tab winrt::hstring CheckTitleUpdate(); void SetTabText(const winrt::hstring& text); + DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); + private: std::shared_ptr _rootPane; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 803fa969fe4..15eaf2f079e 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1173,7 +1173,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // don't necessarily include that state. // Return Value: // - a KeyModifiers value with flags set for each key that's pressed. - Settings::KeyModifiers TermControl::_GetPressedModifierKeys() const{ + Settings::KeyModifiers TermControl::_GetPressedModifierKeys() const + { CoreWindow window = CoreWindow::GetForCurrentThread(); // DONT USE // != CoreVirtualKeyStates::None @@ -1195,6 +1196,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation (shift ? Settings::KeyModifiers::Shift : Settings::KeyModifiers::None) }; } + bool TermControl::CloseOnExit() const noexcept + { + return _settings.CloseOnExit(); + } + // -------------------------------- WinRT Events --------------------------------- // Winrt events need a method for adding a callback to the event and removing the callback. // These macros will define them both for you. diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index fa0b2d364d1..fcfca8723fc 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -41,6 +41,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation hstring Title(); void CopySelectionToClipboard(bool trimTrailingWhitespace); void Close(); + bool CloseOnExit() const noexcept; void ScrollViewport(int viewTop); int GetScrollOffset(); diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl index 2a5ccfc0619..7fad65d857c 100644 --- a/src/cascadia/TerminalControl/TermControl.idl +++ b/src/cascadia/TerminalControl/TermControl.idl @@ -33,6 +33,7 @@ namespace Microsoft.Terminal.TerminalControl String Title { get; }; void CopySelectionToClipboard(Boolean trimTrailingWhitespace); void Close(); + Boolean CloseOnExit { get; }; void ScrollViewport(Int32 viewTop); Int32 GetScrollOffset(); From 23930b508a09e69b4f1fb0bd02804ca0b460b1b6 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 13:21:54 -0500 Subject: [PATCH 10/33] This is me trying to mess with the tab closing, but I think it's probably _more_ wrong. --- src/cascadia/TerminalApp/App.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 08101bb5d1b..43638e71fea 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -901,17 +901,19 @@ namespace winrt::TerminalApp::implementation uint32_t tabIndexFromControl = 0; _tabView.Items().IndexOf(tabViewItem, tabIndexFromControl); - if (tabIndexFromControl == _GetFocusedTabIndex()) - { - _tabView.SelectedIndex((tabIndexFromControl > 0) ? tabIndexFromControl - 1 : 1); - } + // if (tabIndexFromControl == _GetFocusedTabIndex()) + // { + // auto newTabIndex = (tabIndexFromControl > 0) ? tabIndexFromControl - 1 : 1; + // _tabView.SelectedIndex(newTabIndex); + // } + + // ensure tabs and focus is sync + _tabView.SelectedIndex(tabIndexFromControl > 0 ? tabIndexFromControl - 1 : 0); // Removing the tab from the collection will destroy its control and disconnect its connection. _tabs.erase(_tabs.begin() + tabIndexFromControl); _tabView.Items().RemoveAt(tabIndexFromControl); - // ensure tabs and focus is sync - _tabView.SelectedIndex(tabIndexFromControl > 0 ? tabIndexFromControl - 1 : 0); } // Method Description: From a4fbc5327aff2c780b3f281efa1688dea5e7a268 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 13:22:01 -0500 Subject: [PATCH 11/33] Revert "This is me trying to mess with the tab closing, but I think it's probably _more_ wrong." This reverts commit 23930b508a09e69b4f1fb0bd02804ca0b460b1b6. --- src/cascadia/TerminalApp/App.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 43638e71fea..08101bb5d1b 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -901,19 +901,17 @@ namespace winrt::TerminalApp::implementation uint32_t tabIndexFromControl = 0; _tabView.Items().IndexOf(tabViewItem, tabIndexFromControl); - // if (tabIndexFromControl == _GetFocusedTabIndex()) - // { - // auto newTabIndex = (tabIndexFromControl > 0) ? tabIndexFromControl - 1 : 1; - // _tabView.SelectedIndex(newTabIndex); - // } - - // ensure tabs and focus is sync - _tabView.SelectedIndex(tabIndexFromControl > 0 ? tabIndexFromControl - 1 : 0); + if (tabIndexFromControl == _GetFocusedTabIndex()) + { + _tabView.SelectedIndex((tabIndexFromControl > 0) ? tabIndexFromControl - 1 : 1); + } // Removing the tab from the collection will destroy its control and disconnect its connection. _tabs.erase(_tabs.begin() + tabIndexFromControl); _tabView.Items().RemoveAt(tabIndexFromControl); + // ensure tabs and focus is sync + _tabView.SelectedIndex(tabIndexFromControl > 0 ? tabIndexFromControl - 1 : 0); } // Method Description: From b353e89d160e02215985468a9ce779abda8da6e8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 14 May 2019 15:11:41 -0500 Subject: [PATCH 12/33] TONS of polish. Doc comments, etc. --- src/cascadia/TerminalApp/App.cpp | 71 ++++------- src/cascadia/TerminalApp/App.h | 1 + src/cascadia/TerminalApp/Pane.cpp | 196 +++++++++++++++++++++--------- src/cascadia/TerminalApp/Pane.h | 8 +- src/cascadia/TerminalApp/Tab.cpp | 12 +- src/cascadia/TerminalApp/Tab.h | 10 +- 6 files changed, 176 insertions(+), 122 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 08101bb5d1b..b271bdba4bb 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -594,6 +594,17 @@ namespace winrt::TerminalApp::implementation eventArgs.HandleClipboardData(text); } + // Method Description: + // - Connects event handlers to the TermControl for events that we want to + // handle. This includes: + // * the Copy and Paste events, for setting and retrieving clipboard data + // on the right thread + // * the TitleChanged event, for changing the text of the tab + // * the GotFocus event, for chnging the title/icon in the tab when a new + // control is focused + // Arguments: + // - term: The newly created TermControl to connect the events for + // - hostingTab: The Tab that's hosting this TermControl instance void App::_RegisterTerminalEvents(TermControl term, std::shared_ptr hostingTab) { // Add an event handler when the terminal's selection wants to be copied. @@ -638,30 +649,6 @@ namespace winrt::TerminalApp::implementation // Possibly update the icon of the tab. _UpdateTabIcon(hostingTab); }); - - // hostingTab->Closed([](){ - // auto tabViewItem = hostingTab->GetTabViewItem(); - // // _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [hostingTab, tabViewItem, this]() { - - // _RemoveTabViewItem(tabViewItem); - - // }); - // TODO: - // // Add an event handler when the terminal's connection is closed. - // hostingTab->GetTerminalControl().ConnectionClosed([=]() { - // _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [hostingTab, tabViewItem, this]() { - // const GUID tabProfile = hostingTab->GetProfile(); - // // Don't just capture this pointer, because the profile might - // // get destroyed before this is called (case in point - - // // reloading settings) - // const auto* const p = _settings->FindProfile(tabProfile); - - // if (p != nullptr && p->GetCloseOnExit()) - // { - // _RemoveTabViewItem(tabViewItem); - // } - // }); - // }); } // Method Description: @@ -677,6 +664,7 @@ namespace winrt::TerminalApp::implementation // Add the new tab to the list of our tabs. auto newTab = _tabs.emplace_back(std::make_shared(profileGuid, term)); + // Hookp our event handlers to the new terminal _RegisterTerminalEvents(term, newTab); auto tabViewItem = newTab->GetTabViewItem(); @@ -692,6 +680,7 @@ namespace winrt::TerminalApp::implementation tabViewItem.PointerPressed({ this, &App::_OnTabClick }); + // When the tab is closed, remove it from our list of tabs. newTab->Closed([tabViewItem, this](){ _tabView.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [tabViewItem, this]() { _RemoveTabViewItem(tabViewItem); @@ -749,11 +738,6 @@ namespace winrt::TerminalApp::implementation // and get text to appear on separate lines. void App::_CopyText(const bool trimTrailingWhitespace) { - // const int focusedTabIndex = _GetFocusedTabIndex(); - // std::shared_ptr focusedTab{ _tabs[focusedTabIndex] }; - - // const auto control = focusedTab->GetTerminalControl(); - const auto control = _GetFocusedControl(); control.CopySelectionToClipboard(trimTrailingWhitespace); } @@ -804,11 +788,8 @@ namespace winrt::TerminalApp::implementation try { auto tab = _tabs.at(selectedIndex); - auto term = tab->GetFocusedTerminalControl(); - auto control = term.GetControl(); _tabContent.Children().Clear(); - // _tabContent.Children().Append(control); _tabContent.Children().Append(tab->GetRootElement()); tab->SetFocused(true); @@ -863,8 +844,6 @@ namespace winrt::TerminalApp::implementation try { return _GetFocusedControl().Title(); - // auto tab = _tabs.at(selectedIndex); - // return tab->GetTerminalControl().Title(); } CATCH_LOG(); } @@ -950,25 +929,20 @@ namespace winrt::TerminalApp::implementation { int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; - return focusedTab->GetFocusedTerminalControl(); + return focusedTab->GetLastFocusedTerminalControl(); } void App::_SplitVertical(std::optional profileGuid) { - const GUID realGuid = profileGuid ? profileGuid.value() : - _settings->GlobalSettings().GetDefaultProfile(); - auto controlSettings = _settings->MakeSettings(realGuid); - TermControl newControl{ controlSettings }; - - int focusedTabIndex = _GetFocusedTabIndex(); - auto focusedTab = _tabs[focusedTabIndex]; - - _RegisterTerminalEvents(newControl, focusedTab); - - return focusedTab->SplitVertical(realGuid, newControl); + _SplitPane(false, profileGuid); } void App::_SplitHorizontal(std::optional profileGuid) + { + _SplitPane(true, profileGuid); + } + + void App::_SplitPane(const bool splitHorizontal, std::optional profileGuid) { const GUID realGuid = profileGuid ? profileGuid.value() : _settings->GlobalSettings().GetDefaultProfile(); @@ -978,12 +952,13 @@ namespace winrt::TerminalApp::implementation int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; + // Hookp our event handlers to the new terminal _RegisterTerminalEvents(newControl, focusedTab); - return focusedTab->SplitHorizontal(realGuid, newControl); + return splitHorizontal ? focusedTab->SplitHorizontal(realGuid, newControl) : + focusedTab->SplitVertical(realGuid, newControl); } - // -------------------------------- WinRT Events --------------------------------- // Winrt events need a method for adding a callback to the event and removing the callback. // These macros will define them both for you. diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 68827cf5b9e..1731f030b2f 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -103,6 +103,7 @@ namespace winrt::TerminalApp::implementation void _CopyText(const bool trimTrailingWhitespace); void _SplitVertical(std::optional profileGuid); void _SplitHorizontal(std::optional profileGuid); + void _SplitPane(const bool splitHorizontal, std::optional profileGuid); // Todo: add more event implementations here // MSFT:20641986: Add keybindings for New Window diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 5d59622b72c..51c040c58ee 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -9,6 +9,8 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; +static const int SEPERATOR_SIZE = 8; + Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused) : _control{ control }, _lastFocused{ lastFocused }, @@ -20,12 +22,29 @@ Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermContro { _root = Controls::Grid{}; _AddControlToRoot(_control); + + // Set the background of the pane to match that of the theme's default grid + // background. This way, we'll match the small underline under the tabs, and + // the UI will be consistent on bot light and dark modes. + auto res = Application::Current().Resources(); + winrt::Windows::Foundation::IInspectable key = winrt::box_value(L"BackgroundGridThemeStyle"); + if (res.HasKey(key)) + { + winrt::Windows::Foundation::IInspectable g = res.Lookup(key); + winrt::Windows::UI::Xaml::Style style = g.try_as(); + _root.Style(style); + } } Pane::~Pane() { } +// Method Description: +// - Adds a given Terminal Control to our root Grid. Also registers an event +// handler to know when that control closed. +// Arguments: +// - control: The new TermControl to use as the content of our root Grid. void Pane::_AddControlToRoot(TermControl control) { _root.Children().Append(control.GetControl()); @@ -33,31 +52,32 @@ void Pane::_AddControlToRoot(TermControl control) _connectionClosedToken = control.ConnectionClosed([=]() { if (control.CloseOnExit()) { - // Fire our ConnectionClosed event + // Fire our Closed event to tell our parent that we should be removed. _closedHandlers(); } }); } +// Method Description: +// - Get the root UIElement of this pane. There may be a single TermControl as a +// child, or an entire tree of grids and panes as children of this element. +// Return Value: +// - the Grid acting as the root of this pane. Controls::Grid Pane::GetRootElement() { return _root; } -TermControl Pane::GetFocusedTerminalControl() -{ - if (_IsLeaf()) - { - return _control; - } - else - { - // TODO determine a way of tracking the actually focused terminal control - return _firstChild->GetFocusedTerminalControl(); - } -} - -winrt::Microsoft::Terminal::TerminalControl::TermControl Pane::GetLastFocusedTerminalControl() +// Method Description: +// - Returns nullptr if no children of this pane were the last control to be +// focused, or the TermControl that _was_ the last control to be focused (if +// there was one). +// - This control might not currently be focused, if the tab itself is not +// currently focused. +// Return Value: +// - nullptr if no children were marked `_lastFocused`, else the TermControl +// that was last focused. +TermControl Pane::GetLastFocusedTerminalControl() { if (_IsLeaf()) { @@ -75,6 +95,13 @@ winrt::Microsoft::Terminal::TerminalControl::TermControl Pane::GetLastFocusedTer } } +// Method Description: +// - Returns nullopt if no children of this pane were the last control to be +// focused, or the GUID of the profile of the last control to be focused (if +// there was one). +// Return Value: +// - nullopt if no children of this pane were the last control to be +// focused, else the GUID of the profile of the last control to be focused std::optional Pane::GetLastFocusedProfile() const noexcept { if (_IsLeaf()) @@ -93,16 +120,36 @@ std::optional Pane::GetLastFocusedProfile() const noexcept } } +// Method Description: +// - Returns true if this pane was the last pane to be focused in a tree of panes. +// Arguments: +// - +// Return Value: +// - true iff we were the last pane focused in this tree of panes. bool Pane::WasLastFocused() const noexcept { return _lastFocused; } +// Method Description: +// - Returns true iff this pane has no child panes. +// Arguments: +// - +// Return Value: +// - true iff this pane has no child panes. bool Pane::_IsLeaf() const noexcept { return _splitState == SplitState::None; } +// Method Description: +// - Returns true if this pane is currently focused, or there is a pane which is +// a child of this pane that is actively focused +// Arguments: +// - +// Return Value: +// - true if the currently focused pane is either this pane, or one of this +// pane's descendants bool Pane::_HasFocusedChild() const noexcept { const bool controlFocused = _control != nullptr && @@ -113,6 +160,11 @@ bool Pane::_HasFocusedChild() const noexcept return controlFocused || firstFocused || secondFocused; } +// Method Description: +// - Update the focus state of this pane, and all it's descendants. +// * If this is a leaf node, and our control is actively focused, well mark +// ourselves as the _lastFocused. +// * If we're not a leaf, we'll recurse on our children to check them. void Pane::CheckFocus() { if (_IsLeaf()) @@ -130,6 +182,14 @@ void Pane::CheckFocus() } } +// Method Description: +// - Attempts to update the settings of this pane or any children of this pane. +// * If this pane is a leaf, and our profile guid matches the parameter, then +// we'll apply the new settings to our control. +// * If we're not a leaf, we'll recurse on our children. +// Arguments: +// - settings: The new TerminalSettings to apply to any matching controls +// - profile: The GUID of the profile these settings should apply to. void Pane::CheckUpdateSettings(TerminalSettings settings, GUID profile) { if (!_IsLeaf()) @@ -146,55 +206,68 @@ void Pane::CheckUpdateSettings(TerminalSettings settings, GUID profile) } } -void Pane::_SetupChildCloseHandlers() +// Method Description: +// - Closes one of our children. In doing so, takes the control from the other +// child, and makes this pane a leaf node again. +// Arguments: +// - closeFirst: if true, the first child should be closed, and the second +// should be preserved, and vice-versa for false. +void Pane::_CloseChild(const bool closeFirst) { + // take the control and profile of the pane that _wasn't_ closed. + _control = closeFirst ? _secondChild->_control : _firstChild->_control; + _profile = closeFirst ? _secondChild->_profile : _firstChild->_profile; - _firstChild->Closed([this](){ + // Remove all the ui elements of our children. This'll make sure we can + // re-attach the TermControl to our Grid. + _firstChild->_root.Children().Clear(); + _secondChild->_root.Children().Clear(); + // If either of our children was focused, we want to take that focus from + // them. + _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; + + // Reset our UI: + _root.Children().Clear(); + _root.ColumnDefinitions().Clear(); + _root.RowDefinitions().Clear(); + _separatorRoot = { nullptr }; + + // Reattach the TermControl to our grid. + _AddControlToRoot(_control); + + if (_lastFocused) + { + _control.GetControl().Focus(FocusState::Programmatic); + } + + _splitState = SplitState::None; + + // Release our children. + _firstChild = nullptr; + _secondChild = nullptr; +} + +// Method Description: +// - Adds event handlers to our chilcren to handle their close events. +void Pane::_SetupChildCloseHandlers() +{ + _firstChild->Closed([this](){ _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - _control = _secondChild->_control; - _profile = _secondChild->_profile; - _secondChild->_root.Children().Clear(); - _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; - _root.Children().Clear(); - _root.ColumnDefinitions().Clear(); - _root.RowDefinitions().Clear(); - _separatorRoot = { nullptr }; - _AddControlToRoot(_control); - if (_lastFocused) - { - _control.GetControl().Focus(FocusState::Programmatic); - } - _splitState = SplitState::None; - _firstChild = nullptr; - _secondChild = nullptr; + _CloseChild(true); }); }); _secondChild->Closed([this](){ _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - _control = _firstChild->_control; - _profile = _firstChild->_profile; - _firstChild->_root.Children().Clear(); - _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; - _root.Children().Clear(); - _root.ColumnDefinitions().Clear(); - _root.RowDefinitions().Clear(); - _separatorRoot = { nullptr }; - _AddControlToRoot(_control); - if (_lastFocused) - { - _control.GetControl().Focus(FocusState::Programmatic); - } - _splitState = SplitState::None; - _firstChild = nullptr; - _secondChild = nullptr; + _CloseChild(false); }); }); } -void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +void Pane::SplitVertical(const GUID profile, TermControl control) { + // If we're not the leaf, recurse into our children to split them. if (!_IsLeaf()) { if (_firstChild->_HasFocusedChild()) @@ -208,12 +281,13 @@ void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalContr return; } + // revoke our handler - the child will take care of the control now. _control.ConnectionClosed(_connectionClosedToken); _splitState = SplitState::Vertical; - // Create two columns in this grid. + // Create three columns in this grid: one for each pane, and one for the separator. auto separatorColDef = Controls::ColumnDefinition(); separatorColDef.Width(GridLengthHelper::Auto()); @@ -221,6 +295,8 @@ void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalContr _root.ColumnDefinitions().Append(separatorColDef); _root.ColumnDefinitions().Append(Controls::ColumnDefinition{}); + // Remove any children we currently have. We can't add the existing + // TermControl to a new grid until we do this. _root.Children().Clear(); // Create two new Panes @@ -235,23 +311,25 @@ void Pane::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalContr _root.Children().Append(_firstChild->GetRootElement()); Controls::Grid::SetColumn(_firstChild->GetRootElement(), 0); + // Create the pane separator, and place it in column 1 _separatorRoot = Controls::Grid{}; - _separatorRoot.Width(8); + _separatorRoot.Width(SEPERATOR_SIZE); // NaN is the special value XAML uses for "Auto" sizing. _separatorRoot.Height(NAN); _root.Children().Append(_separatorRoot); Controls::Grid::SetColumn(_separatorRoot, 1); - // add the second pane to row 1 + // add the second pane to column 2 _root.Children().Append(_secondChild->GetRootElement()); Controls::Grid::SetColumn(_secondChild->GetRootElement(), 2); + // Register event handlers on our children to handle their Close events _SetupChildCloseHandlers(); _lastFocused = false; } -void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +void Pane::SplitHorizontal(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) { if (!_IsLeaf()) { @@ -266,10 +344,13 @@ void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalCon return; } - _splitState = SplitState::Horizontal; - // Create two rows in this grid. + // revoke our handler - the child will take care of the control now. + _control.ConnectionClosed(_connectionClosedToken); + + _splitState = SplitState::Horizontal; + // Create three rows in this grid: one for each pane, and one for the separator. auto separatorRowDef = Controls::RowDefinition(); separatorRowDef.Height(GridLengthHelper::Auto()); @@ -287,13 +368,12 @@ void Pane::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalCon _control = { nullptr }; _secondChild = std::make_shared(profile, control); - // add the first pane to row 0 _root.Children().Append(_firstChild->GetRootElement()); Controls::Grid::SetRow(_firstChild->GetRootElement(), 0); _separatorRoot = Controls::Grid{}; - _separatorRoot.Height(8); + _separatorRoot.Height(SEPERATOR_SIZE); // NaN is the special value XAML uses for "Auto" sizing. _separatorRoot.Width(NAN); _root.Children().Append(_separatorRoot); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 941c4d3900c..7453d585323 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -20,9 +20,6 @@ class Pane Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused = false); ~Pane(); - // winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); - // winrt::Microsoft::Terminal::TerminalControl::TermControl GetTerminalControl(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetLastFocusedTerminalControl(); std::optional GetLastFocusedProfile() const noexcept; winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); @@ -32,8 +29,6 @@ class Pane void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); - // GUID GetProfile() const noexcept; - void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); void SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); @@ -56,4 +51,7 @@ class Pane bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); void _AddControlToRoot(winrt::Microsoft::Terminal::TerminalControl::TermControl control); + + void _CloseChild(const bool closeFirst); + }; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 1abfe7e71aa..fde07856c84 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -42,9 +42,9 @@ winrt::Windows::UI::Xaml::UIElement Tab::GetRootElement() return _rootPane->GetRootElement(); } -winrt::Microsoft::Terminal::TerminalControl::TermControl Tab::GetFocusedTerminalControl() +winrt::Microsoft::Terminal::TerminalControl::TermControl Tab::GetLastFocusedTerminalControl() { - return _rootPane->GetFocusedTerminalControl(); + return _rootPane->GetLastFocusedTerminalControl(); } winrt::Microsoft::UI::Xaml::Controls::TabViewItem Tab::GetTabViewItem() @@ -57,7 +57,7 @@ bool Tab::IsFocused() return _focused; } -void Tab::SetFocused(bool focused) +void Tab::SetFocused(const bool focused) { _focused = focused; @@ -114,10 +114,10 @@ void Tab::SetTabText(const winrt::hstring& text) // - delta: a number of lines to move the viewport relative to the current viewport. // Return Value: // - -void Tab::Scroll(int delta) +void Tab::Scroll(const int delta) { - GetFocusedTerminalControl().GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - auto control = GetFocusedTerminalControl(); + GetLastFocusedTerminalControl().GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ + auto control = GetLastFocusedTerminalControl(); const auto currentOffset = control.GetScrollOffset(); control.ScrollViewport(currentOffset + delta); }); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 4393c9c4a26..bd03a08ce98 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -14,15 +14,15 @@ class Tab winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); winrt::Windows::UI::Xaml::UIElement GetRootElement(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + winrt::Microsoft::Terminal::TerminalControl::TermControl GetLastFocusedTerminalControl(); std::optional GetLastFocusedProfile() const noexcept; bool IsFocused(); - void SetFocused(bool focused); + void SetFocused(const bool focused); - void Scroll(int delta); - void SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void Scroll(const int delta); + void SplitVertical(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void SplitHorizontal(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); void CheckFocus(); From dc3f5224e092dabec913b63c0b4497385cf7d6c7 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 15 May 2019 09:39:35 -0500 Subject: [PATCH 13/33] Correctly close a pane when one of it's children has children --- src/cascadia/TerminalApp/Pane.cpp | 116 +++++++++++++++++++++++------- src/cascadia/TerminalApp/Pane.h | 2 + 2 files changed, 92 insertions(+), 26 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 51c040c58ee..8013a7eccbc 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -18,7 +18,9 @@ Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermContro _splitState{ SplitState::None }, _firstChild{ nullptr }, _secondChild{ nullptr }, - _connectionClosedToken{} + _connectionClosedToken{}, + _firstClosedToken{}, + _secondClosedToken{} { _root = Controls::Grid{}; _AddControlToRoot(_control); @@ -214,51 +216,113 @@ void Pane::CheckUpdateSettings(TerminalSettings settings, GUID profile) // should be preserved, and vice-versa for false. void Pane::_CloseChild(const bool closeFirst) { - // take the control and profile of the pane that _wasn't_ closed. - _control = closeFirst ? _secondChild->_control : _firstChild->_control; - _profile = closeFirst ? _secondChild->_profile : _firstChild->_profile; + auto remainingChild = closeFirst ? _secondChild : _firstChild; - // Remove all the ui elements of our children. This'll make sure we can - // re-attach the TermControl to our Grid. - _firstChild->_root.Children().Clear(); - _secondChild->_root.Children().Clear(); + // If the only child left is a leaf, that means we're a leaf now. + if (remainingChild->_IsLeaf()) + { + // take the control and profile of the pane that _wasn't_ closed. + _control = closeFirst ? _secondChild->_control : _firstChild->_control; + _profile = closeFirst ? _secondChild->_profile : _firstChild->_profile; - // If either of our children was focused, we want to take that focus from - // them. - _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; + // If either of our children was focused, we want to take that focus from + // them. + _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; - // Reset our UI: - _root.Children().Clear(); - _root.ColumnDefinitions().Clear(); - _root.RowDefinitions().Clear(); - _separatorRoot = { nullptr }; + // Remove all the ui elements of our children. This'll make sure we can + // re-attach the TermControl to our Grid. + _firstChild->_root.Children().Clear(); + _secondChild->_root.Children().Clear(); - // Reattach the TermControl to our grid. - _AddControlToRoot(_control); + // Reset our UI: + _root.Children().Clear(); + _root.ColumnDefinitions().Clear(); + _root.RowDefinitions().Clear(); + _separatorRoot = { nullptr }; + + // Reattach the TermControl to our grid. + _AddControlToRoot(_control); + + if (_lastFocused) + { + _control.GetControl().Focus(FocusState::Programmatic); + } + + _splitState = SplitState::None; - if (_lastFocused) + // Release our children. + _firstChild = nullptr; + _secondChild = nullptr; + } + else { - _control.GetControl().Focus(FocusState::Programmatic); + // Revoke the old event handlers. + _firstChild->Closed(_firstClosedToken); + _secondChild->Closed(_secondClosedToken); + + // Steal all the state from our child + _splitState = remainingChild->_splitState; + _separatorRoot = remainingChild->_separatorRoot; + _firstChild = remainingChild->_firstChild; + _secondChild = remainingChild->_secondChild; + + // remainingChild->_root.Children().Clear(); + _root.Children().Clear(); + _root.ColumnDefinitions().Clear(); + _root.RowDefinitions().Clear(); + + // Copy the UI over to our grid. + auto oldCols = remainingChild->_root.ColumnDefinitions(); + auto oldRows = remainingChild->_root.RowDefinitions(); + + // remainingChild->_root.Children().Clear(); + // remainingChild->_root.ColumnDefinitions().Clear(); + // remainingChild->_root.RowDefinitions().Clear(); + // TODO: These throw, because apparently the definitions are still + // parented to another (the old) element. maybe we need to just + // regenerate them? + while (remainingChild->_root.ColumnDefinitions().Size() > 0) + { + auto col = remainingChild->_root.ColumnDefinitions().GetAt(0); + remainingChild->_root.ColumnDefinitions().RemoveAt(0); + _root.ColumnDefinitions().Append(col); + } + while (remainingChild->_root.RowDefinitions().Size() > 0) + { + auto row = remainingChild->_root.RowDefinitions().GetAt(0); + remainingChild->_root.RowDefinitions().RemoveAt(0); + _root.RowDefinitions().Append(row); + } + + remainingChild->_root.Children().Clear(); + + _root.Children().Append(_firstChild->GetRootElement()); + _root.Children().Append(_separatorRoot); + _root.Children().Append(_secondChild->GetRootElement()); + + + _SetupChildCloseHandlers(); + + remainingChild->_firstChild = nullptr; + remainingChild->_secondChild = nullptr; + remainingChild->_separatorRoot = { nullptr }; + } - _splitState = SplitState::None; - // Release our children. - _firstChild = nullptr; - _secondChild = nullptr; } // Method Description: // - Adds event handlers to our chilcren to handle their close events. void Pane::_SetupChildCloseHandlers() { - _firstChild->Closed([this](){ + _firstClosedToken = _firstChild->Closed([this](){ _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ _CloseChild(true); }); }); - _secondChild->Closed([this](){ + _secondClosedToken = _secondChild->Closed([this](){ _root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ _CloseChild(false); }); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 7453d585323..2cc6306c379 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -46,6 +46,8 @@ class Pane bool _lastFocused; std::optional _profile; winrt::event_token _connectionClosedToken; + winrt::event_token _firstClosedToken; + winrt::event_token _secondClosedToken; bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; From d45a9aa17cb9e1d369dca3fb75b3bb01b62443a9 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 15 May 2019 10:40:05 -0500 Subject: [PATCH 14/33] Correctly move focus to a child with children when the focused pane is closed. --- src/cascadia/TerminalApp/Pane.cpp | 67 ++++++++++++++++++++++--------- src/cascadia/TerminalApp/Pane.h | 1 + 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 8013a7eccbc..b2ae7096221 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -9,9 +9,9 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; -static const int SEPERATOR_SIZE = 8; +static const int SEPERATOR_SIZE = 4; -Pane::Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused) : +Pane::Pane(GUID profile, TermControl control, const bool lastFocused) : _control{ control }, _lastFocused{ lastFocused }, _profile{ profile }, @@ -184,6 +184,21 @@ void Pane::CheckFocus() } } +// Method Description: +// - Focuses this control if we're a leaf, or attempts to focus the first leaf +// of our first child, recursively. +void Pane::_FocusFirstChild() +{ + if (_IsLeaf()) + { + _control.GetControl().Focus(FocusState::Programmatic); + } + else + { + _firstChild->_FocusFirstChild(); + } +} + // Method Description: // - Attempts to update the settings of this pane or any children of this pane. // * If this pane is a leaf, and our profile guid matches the parameter, then @@ -216,6 +231,7 @@ void Pane::CheckUpdateSettings(TerminalSettings settings, GUID profile) // should be preserved, and vice-versa for false. void Pane::_CloseChild(const bool closeFirst) { + auto closedChild = closeFirst ? _firstChild : _secondChild; auto remainingChild = closeFirst ? _secondChild : _firstChild; // If the only child left is a leaf, that means we're a leaf now. @@ -266,21 +282,15 @@ void Pane::_CloseChild(const bool closeFirst) _firstChild = remainingChild->_firstChild; _secondChild = remainingChild->_secondChild; - // remainingChild->_root.Children().Clear(); + // Reset our UI: _root.Children().Clear(); _root.ColumnDefinitions().Clear(); _root.RowDefinitions().Clear(); - // Copy the UI over to our grid. - auto oldCols = remainingChild->_root.ColumnDefinitions(); - auto oldRows = remainingChild->_root.RowDefinitions(); - - // remainingChild->_root.Children().Clear(); - // remainingChild->_root.ColumnDefinitions().Clear(); - // remainingChild->_root.RowDefinitions().Clear(); - // TODO: These throw, because apparently the definitions are still - // parented to another (the old) element. maybe we need to just - // regenerate them? + // Copy the old UI over to our grid. + // Start by copying the row/column definitions. Iterate over the + // rows/cols, and remove each one from the old grid, and attach it to + // our grid instead. while (remainingChild->_root.ColumnDefinitions().Size() > 0) { auto col = remainingChild->_root.ColumnDefinitions().GetAt(0); @@ -294,22 +304,29 @@ void Pane::_CloseChild(const bool closeFirst) _root.RowDefinitions().Append(row); } + // Remove the child's UI elements from the child's grid, so we can + // attach them to us instead. remainingChild->_root.Children().Clear(); _root.Children().Append(_firstChild->GetRootElement()); _root.Children().Append(_separatorRoot); _root.Children().Append(_secondChild->GetRootElement()); - + // Set up new close handlers on the children, so thay'll notify us + // instead of their old parent. _SetupChildCloseHandlers(); + // If the closed child was focused, transfer the focus to it's first sibling. + if (closedChild->_lastFocused) + { + _FocusFirstChild(); + } + + // Release the pointers that the child was holding. remainingChild->_firstChild = nullptr; remainingChild->_secondChild = nullptr; remainingChild->_separatorRoot = { nullptr }; - } - - } // Method Description: @@ -329,6 +346,13 @@ void Pane::_SetupChildCloseHandlers() }); } +// Method Description: +// - Vertically split the focused pane in our tree of panes, and place the given +// TermControl into the newly created pane. If we're the focused pane, then +// we'll create two new children, and place them side-by-side in our Grid. +// Arguments: +// - profile: The profile GUID to associate with the newly created pane. +// - control: A TermControl to use in the new pane. void Pane::SplitVertical(const GUID profile, TermControl control) { // If we're not the leaf, recurse into our children to split them. @@ -393,7 +417,14 @@ void Pane::SplitVertical(const GUID profile, TermControl control) _lastFocused = false; } -void Pane::SplitHorizontal(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +// Method Description: +// - Horizontally split the focused pane in our tree of panes, and place the given +// TermControl into the newly created pane. If we're the focused pane, then +// we'll create two new children, and place them side-by-side in our Grid. +// Arguments: +// - profile: The profile GUID to associate with the newly created pane. +// - control: A TermControl to use in the new pane. +void Pane::SplitHorizontal(const GUID profile, TermControl control) { if (!_IsLeaf()) { diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 2cc6306c379..dcd60014928 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -56,4 +56,5 @@ class Pane void _CloseChild(const bool closeFirst); + void _FocusFirstChild(); }; From 0a96ed4c869579a02a7ff39bd59fe44fa58f81a5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 15 May 2019 10:40:26 -0500 Subject: [PATCH 15/33] doc comments for days --- src/cascadia/TerminalApp/App.cpp | 33 ++++++++++++++- src/cascadia/TerminalApp/Tab.cpp | 69 +++++++++++++++++++++++++++++--- src/cascadia/TerminalApp/Tab.h | 2 +- 3 files changed, 96 insertions(+), 8 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index b271bdba4bb..cd9b8aad8e7 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -443,6 +443,11 @@ namespace winrt::TerminalApp::implementation } + // Method Description: + // - Get the icon of the currently focused terminal control, and set it's + // tab's icon to that icon. + // Arguments: + // - tab: the Tab to update the title for. void App::_UpdateTabIcon(std::shared_ptr tab) { const auto lastFocusedProfileOpt = tab->GetLastFocusedProfile(); @@ -462,9 +467,15 @@ namespace winrt::TerminalApp::implementation } } + // Method Description: + // - Get the title of the currently focused terminal control, and set it's + // tab's text to that text. If this tab is the focused tab, then also + // bubble this title to any listeners of our TitleChanged event. + // Arguments: + // - tab: the Tab to update the title for. void App::_CheckTitleUpdate(std::shared_ptr tab) { - auto newTabTitle = tab->CheckTitleUpdate(); + auto newTabTitle = tab->GetLastFocusedTitle(); // TODO #608: If the settings don't want the terminal's text in the // tab, then display something else. @@ -932,16 +943,36 @@ namespace winrt::TerminalApp::implementation return focusedTab->GetLastFocusedTerminalControl(); } + // Method Description: + // - Vertically split the focused pane, and place the given TermControl into + // the newly created pane. + // Arguments: + // - profile: The profile GUID to associate with the newly created pane. If + // this is nullopt, use the default profile. void App::_SplitVertical(std::optional profileGuid) { _SplitPane(false, profileGuid); } + // Method Description: + // - Horizontally split the focused pane and place the given TermControl + // into the newly created pane. + // Arguments: + // - profile: The profile GUID to associate with the newly created pane. If + // this is nullopt, use the default profile. void App::_SplitHorizontal(std::optional profileGuid) { _SplitPane(true, profileGuid); } + // Method Description: + // - Split the focused pane either horizontally or vertically, and place the + // given TermControl into the newly created pane. + // Arguments: + // - splitHorizontal: if true, split the pane horizontally. Else split + // vertically. + // - profile: The profile GUID to associate with the newly created pane. If + // this is nullopt, use the default profile. void App::_SplitPane(const bool splitHorizontal, std::optional profileGuid) { const GUID realGuid = profileGuid ? profileGuid.value() : diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index fde07856c84..6a6aacbf95a 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -9,7 +9,7 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; -Tab::Tab(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) : +Tab::Tab(GUID profile, TermControl control) : _focused{ false }, _tabViewItem{ nullptr }, _rootPane{ nullptr } @@ -37,12 +37,21 @@ void Tab::_MakeTabViewItem() _tabViewItem.FontSize(12); } -winrt::Windows::UI::Xaml::UIElement Tab::GetRootElement() +UIElement Tab::GetRootElement() { return _rootPane->GetRootElement(); } -winrt::Microsoft::Terminal::TerminalControl::TermControl Tab::GetLastFocusedTerminalControl() +// Method Description: +// - Returns nullptr if no children of this tab were the last control to be +// focused, or the TermControl that _was_ the last control to be focused (if +// there was one). +// - This control might not currently be focused, if the tab itself is not +// currently focused. +// Return Value: +// - nullptr if no children were marked `_lastFocused`, else the TermControl +// that was last focused. +TermControl Tab::GetLastFocusedTerminalControl() { return _rootPane->GetLastFocusedTerminalControl(); } @@ -57,6 +66,14 @@ bool Tab::IsFocused() return _focused; } +// Method Description: +// - Updates our focus state. If we're gaining focus, make sure to transfer +// focus to the last focused terminal control in our tree of controls. +// Arguments: +// - focused: our new focus state. If true, we should be focused. If false, we +// should be unfocused. +// Return Value: +// - void Tab::SetFocused(const bool focused) { _focused = focused; @@ -67,16 +84,30 @@ void Tab::SetFocused(const bool focused) } } +// Method Description: +// - Returns nullopt if no children of this tab were the last control to be +// focused, or the GUID of the profile of the last control to be focused (if +// there was one). +// Return Value: +// - nullopt if no children of this tab were the last control to be +// focused, else the GUID of the profile of the last control to be focused std::optional Tab::GetLastFocusedProfile() const noexcept { return _rootPane->GetLastFocusedProfile(); } +// Method Description: +// - Attempts to update the settings of this tab's tree of panes. +// Arguments: +// - settings: The new TerminalSettings to apply to any matching controls +// - profile: The GUID of the profile these settings should apply to. void Tab::CheckUpdateSettings(TerminalSettings settings, GUID profile) { _rootPane->CheckUpdateSettings(settings, profile); } +// Method Description: +// - Focus the last focused control in our tree of panes. void Tab::_Focus() { _focused = true; @@ -88,17 +119,31 @@ void Tab::_Focus() } } +// Method Description: +// - Update the focus state of this tab's tree of panes. If one of the controls +// under this tab is focused, then it will be marked as the last focused. If +// there are no focused panes, then there will not be a last focused contrl +// when this returns. void Tab::CheckFocus() { _rootPane->CheckFocus(); } -winrt::hstring Tab::CheckTitleUpdate() +// Method Description: +// - Gets the title string of the last focused terminal control in our tree. +// Returns the empty string if there is no such control. +// Return Value: +// - the title string of the last focused terminal control in our tree. +winrt::hstring Tab::GetLastFocusedTitle() { auto lastFocusedControl = _rootPane->GetLastFocusedTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; } +// Method Description: +// - Set the text on the TabViewItem for this tab. +// Arguments: +// - text: The new text string to use as the Header for our TabViewItem void Tab::SetTabText(const winrt::hstring& text) { _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ @@ -123,12 +168,24 @@ void Tab::Scroll(const int delta) }); } -void Tab::SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +// Method Description: +// - Vertically split the focused pane in our tree of panes, and place the +// given TermControl into the newly created pane. +// Arguments: +// - profile: The profile GUID to associate with the newly created pane. +// - control: A TermControl to use in the new pane. +void Tab::SplitVertical(GUID profile, TermControl control) { _rootPane->SplitVertical(profile, control); } -void Tab::SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control) +// Method Description: +// - Horizontally split the focused pane in our tree of panes, and place the +// given TermControl into the newly created pane. +// Arguments: +// - profile: The profile GUID to associate with the newly created pane. +// - control: A TermControl to use in the new pane. +void Tab::SplitHorizontal(GUID profile, TermControl control) { _rootPane->SplitHorizontal(profile, control); } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index bd03a08ce98..637cc84f1d5 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -27,7 +27,7 @@ class Tab void CheckFocus(); void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); - winrt::hstring CheckTitleUpdate(); + winrt::hstring GetLastFocusedTitle(); void SetTabText(const winrt::hstring& text); DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); From ff21bd24a909a61399f84634fab161f21ce47978 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 May 2019 11:48:45 -0500 Subject: [PATCH 16/33] This is most all of the PR feedback --- doc/cascadia/Panes.md | 231 +++++++++++++++++++ src/cascadia/TerminalApp/App.cpp | 89 ++++--- src/cascadia/TerminalApp/App.h | 9 +- src/cascadia/TerminalApp/Pane.cpp | 89 +++---- src/cascadia/TerminalApp/Pane.h | 34 ++- src/cascadia/TerminalApp/Tab.cpp | 12 +- src/cascadia/TerminalApp/Tab.h | 6 +- src/cascadia/TerminalControl/TermControl.cpp | 7 +- 8 files changed, 380 insertions(+), 97 deletions(-) create mode 100644 doc/cascadia/Panes.md diff --git a/doc/cascadia/Panes.md b/doc/cascadia/Panes.md new file mode 100644 index 00000000000..21ac4f9a9a0 --- /dev/null +++ b/doc/cascadia/Panes.md @@ -0,0 +1,231 @@ +# Panes in the Windows Terminal + +* author: Mike Griese __[@zadjii-msft](https://github.com/zadjii-msft)__ +* created on: 2019-May-16 + +## Abstract + +Panes are an abstraction by which the terminal can dislay multiple terminal +instances simultaneously in a single terminal window. While tabs allow for a +single terminal window to have many terminal sessions running simultaneously +within a single window, only one tab can be visible at a time. Panes, on the +other hand, allow a user to have many different terminal sessions visible to the +user within the context of a single window at the same time. This can enable +greater productivity from the user, as they can see the output of one terminal +window while working in another. + +This spec will help outline the design of the implementation of panes in the +Windows Terminal. + + +## Inspirations + +Panes within the context of a single terminal window are not a new idea. The +design of the panes for the Windows Terminal was heavily inspired by the +application `tmux`, which is a commandline application which acts as a "terminal +multiplexer", allowing for the easy managment of many terminal sessions from a +single application. + +Other applications that include pane-like functionality include (but are not +limited to): + +* screen +* terminator +* emacs & vim +* Iterm2 + +## Design + +The architecture of the Windows Terminal can be broken into two main pieces: +Tabs and Panes. The Windows Terminal supports _top-level_ tabs, with nested +panes inside the tabs. This means that there's a single strip of tabs along the +application, and each tab has a set of panes that are visible within the context +of that tab. + +Panes are implemented as a binary tree of panes. A Pane can either be a leaf +pane, with it's own terminal control that it displays, or it could be a parent +pane, where it has two children, each with their own terminal control. + +When a pane is a parent, its two children are either split vertically or +horizontally. Parent nodes don't have a terminal of their own, the merely +display the terminals of their children. + + * If a Pane is split vertically, the two panes are seperated by a vertical + split, as to appear side-by-side. Think `[|]` + * If a Pane is split horizontally, the two panes are split by a horizontal + separator, and appear above/below one another. Think `[-]`. + +As additional panes are created, panes will continue to subdivide the space of +their parent. It's up to the parent pane to control the sizing and display of +it's children. + +### Example + +We'll start by taking the terminal and creating a single vertical split. There +are now two panes in the terminal, side by side. The original terminal is `A`, +and the newly created one is `B`. The terminal now looks like this: + +``` + +---------------+ + | | | 1: parent [|] + | | | ├── 2: A + | | | └── 3: B + | A | B | + | | | + | | | + | | | + +---------------+ +``` + +Here, there are actually 3 nodes: 1 is the parent of both 2 and 3. 2 is the node +containing the `A` terminal, and 3 is the node with the `B` terminal. + + +We could now split `B` in two horizontally, creating a third terminal pane `C`. + +``` + +---------------+ + | | | 1: parent [|] + | | B | ├── 2: A + | | | └── 3: parent [-] + | A +-------+ ├── 4: B + | | | └── 5: C + | | C | + | | | + +---------------+ +``` + +Node 3 is now a parent node, and the terminal `B` has moved into a new node as a +sibling of the new terminal `C`. + +We could also split `A` in horizontally, creating a fourth terminal pane `D`. + +``` + +---------------+ + | | | 1: parent [|] + | A | B | ├── 2: parent [-] + | | | | ├── 4: A + +-------+-------+ | └── 5: D + | | | └── 3: parent [-] + | D | C | ├── 4: B + | | | └── 5: C + +---------------+ +``` + +While it may appear that there's a single horizonal separator and a single +vertical separator here, that's not actually the case. Due to the tree-like +structure of the pane splitting, the horizontal splits exist only between the +two panes they're splitting. So, the user could move each of the horizontal +splits independently, without affecting the other set of panes. As an example: + +``` + +---------------+ + | | | + | A | | + +-------+ B | + | | | + | D | | + | +-------+ + | | C | + +---------------+ +``` + +### Creating a pane + +In the basic use case, the user will decide to split the currently focused pane. +The currently focused pane is always a leaf, because as parent's can't be +focused (they don't have their own terminal). When a user decides to add a new +pane, the child will: + + 1. Convert into a parent + 2. Move its terminal into it's first child + 3. Split it's UI in half, and display each child in one half. + +It's up to the app hosting the panes to tell the pane what kind of terminal in +wants created in the new pane. Implemented by default is the new pane will be +created with the default settings profile. + +### While panes are open + +When a tab has multiple panes open, only one is the "active" pane. This is the +pane that was last focused in the tab. If the tab is the currently open tab, +then this is the pane with the currectly focused terminal control. When the user +brings the tab into focus, the last focused pane is the pane that should become +focused again. + +The Tab's state will be updated to reflect the state of it's focused pane. The +title text and icon of the tab will reflect that of the focused pane. Should the +focus switch from one pane to another, the tab's text and icon should update to +reflect the newly focused control. Any additional state that the tab would +display for a single pane should also be reflected in the tab for a tab with +multiple panes. + +While panes are open, the user should be able to move any split between panes. +In moving the split, the sizes of the terminal controls should be resized to +match. + +### Closing a pane + +A pane can either be close by the user manually, or when the terminal it's +attached to raises it's ConnectionClosed event. When this happens, we should +remove this pane from the tree. The parent of the closing pane will have to +remove the pane as one of it's children. If the sibling of the closing pane is a +leaf, then the parent should just take all of the state from the remaining pane. +This will cause the remaining pane's content to expand to take the entire +boundaries of the parent's pane. If the remaining child was a parent itself, +then the parent will take both the children of the remaining pane, and make them +the parent's children, as if the parent node was taken from the tree and +replaced by the remaining child. + +## Future considerations + +The Pane implementation isn't complete in it's current form. There are many +additional things that could be done to improve the user experience. This is by +no means a comprehensive list. + +* [ ] Panes should be resizable with the mouse. The user should be able to drag + the separator for a pair of panes, and have the content between them resize as + the separator moves. +* [ ] There's no keyboard shortcut for "ClosePane" +* [ ] The user should be able to configure what profile is used for splitting a + pane. Currently, the default profile is used, but it's possible a user might + want to create a new pane with the parent pane's profile. +* [ ] There should be some sort of UI to indicate that a particular pane is + focused, more than just the blinking cursor. `tmux` accomplishes this by + colorizing the separators adjacent to the active pane. Another idea is + displaying a small outline around the focused pane (like when tabbing through + controls on a webpage). +* [ ] The user should be able to navigate the focus of panes with the keyboard, + instead of requiring the mouse. +* [ ] The user should be able to zoom a pane, to make the pane take the entire + size of the terminal window temporarily. +* [ ] A pane doesn't necessarily need to host a terminal. It could potentially + host another UIElement. One could imagine enabling a user to quickly open up a + Browser pane to search for a particular string without needing to leave the + terminal. + +## Footnotes + +### Why not top-Level Panes, and nested tabs? + +If each pane were to have it's own set of tabs, then each pane would need to +reserve screen real estate for a row of tabs. As a user continued to split the +window, more and more of the screen would be dedicated to just displaying a row +of tabs, which isn't really the important part of the application, the terminal +is. + +Additionally, if there were top-level panes, once the root was split, it would +not be possible to move a single pane to be the full size of the window. The +user would need to somehow close the other panes, to be able to make the split +the size of the dull window. + +One con of this design is that if a control is hosted in a pane, the current +design makes it hard to move out of a pane into it's own tab, or into another +pane. This could be solved a number of ways. There could be keyboard shortcuts +for swapping the positions of tabs, or a shortcut for both "zooming" a tab +(temporarily making it the full size) or even popping a pane out to it's own +tab. Additionally, a right-click menu option could be added to do the +afformentioned actions. Discoverability of these two actions is not as high as +just dragging a tab from one pane to another, however, it's believed that panes +are more of a power-user scenario, and power users will not neccessarily be +turned off by the feature's discoverability. diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index a8cc2693c1f..8b8015fe3bf 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -620,45 +620,42 @@ namespace winrt::TerminalApp::implementation { // Add an event handler when the terminal's selection wants to be copied. // When the text buffer data is retrieved, we'll copy the data into the Clipboard - term.CopyToClipboard([this](auto copiedData) { - _root.Dispatcher().RunAsync(CoreDispatcherPriority::High, [copiedData]() { - DataPackage dataPack = DataPackage(); - dataPack.RequestedOperation(DataPackageOperation::Copy); - dataPack.SetText(copiedData); - Clipboard::SetContent(dataPack); - - // TODO: MSFT 20642290 and 20642291 - // rtf copy and html copy - }); - }); + term.CopyToClipboard({ this, &App::_CopyToClipboardHandler }); // Add an event handler when the terminal wants to paste data from the Clipboard. - term.PasteFromClipboard([this](auto /*sender*/, auto eventArgs) { - _root.Dispatcher().RunAsync(CoreDispatcherPriority::High, [eventArgs]() { - PasteFromClipboard(eventArgs); - }); - }); - - // TODO: hostingTab feels like it should be a weak ref, not a strong ref. - term.TitleChanged([this, hostingTab](auto newTitle){ + term.PasteFromClipboard({ this, &App::_PasteFromClipboardHandler }); + + // Don't capture a strong ref to the tab. If the tab is removed as this + // is called, we don't really care anymore about handling the event. + std::weak_ptr weakTabPtr = hostingTab; + term.TitleChanged([this, weakTabPtr](auto newTitle){ + auto tab = weakTabPtr.lock(); + if (!tab) + { + return; + } // The title of the control changed, but not necessarily the title // of the tab. Get the title of the focused pane of the tab, and set // the tab's text to the focused panes' text. - _CheckTitleUpdate(hostingTab); + _CheckTitleUpdate(tab); }); - // TODO: hostingTab feels like it should be a weak ref, not a strong ref. - term.GetControl().GotFocus([this, hostingTab](auto&&, auto&&) + term.GetControl().GotFocus([this, weakTabPtr](auto&&, auto&&) { + auto tab = weakTabPtr.lock(); + if (!tab) + { + return; + } // Update the focus of the tab's panes - hostingTab->CheckFocus(); + tab->UpdateFocus(); // Possibly update the title of the tab, window to match the newly // focused pane. - _CheckTitleUpdate(hostingTab); + _CheckTitleUpdate(tab); // Possibly update the icon of the tab. - _UpdateTabIcon(hostingTab); + _UpdateTabIcon(tab); }); } @@ -965,7 +962,7 @@ namespace winrt::TerminalApp::implementation // Arguments: // - profile: The profile GUID to associate with the newly created pane. If // this is nullopt, use the default profile. - void App::_SplitVertical(std::optional profileGuid) + void App::_SplitVertical(const std::optional& profileGuid) { _SplitPane(false, profileGuid); } @@ -976,7 +973,7 @@ namespace winrt::TerminalApp::implementation // Arguments: // - profile: The profile GUID to associate with the newly created pane. If // this is nullopt, use the default profile. - void App::_SplitHorizontal(std::optional profileGuid) + void App::_SplitHorizontal(const std::optional& profileGuid) { _SplitPane(true, profileGuid); } @@ -989,7 +986,7 @@ namespace winrt::TerminalApp::implementation // vertically. // - profile: The profile GUID to associate with the newly created pane. If // this is nullopt, use the default profile. - void App::_SplitPane(const bool splitHorizontal, std::optional profileGuid) + void App::_SplitPane(const bool splitHorizontal, const std::optional& profileGuid) { const GUID realGuid = profileGuid ? profileGuid.value() : _settings->GlobalSettings().GetDefaultProfile(); @@ -1002,8 +999,40 @@ namespace winrt::TerminalApp::implementation // Hookp our event handlers to the new terminal _RegisterTerminalEvents(newControl, focusedTab); - return splitHorizontal ? focusedTab->SplitHorizontal(realGuid, newControl) : - focusedTab->SplitVertical(realGuid, newControl); + return splitHorizontal ? focusedTab->AddHorizontalSplit(realGuid, newControl) : + focusedTab->AddVerticalSplit(realGuid, newControl); + } + + // Method Description: + // - Place `copiedData` into the clipboard as text. Triggered when a + // terminal control raises it's CopyToClipboard event. + // Arguments: + // - copiedData: the new string content to place on the clipboard. + void App::_CopyToClipboardHandler(const winrt::hstring& copiedData) + { + _root.Dispatcher().RunAsync(CoreDispatcherPriority::High, [copiedData]() { + DataPackage dataPack = DataPackage(); + dataPack.RequestedOperation(DataPackageOperation::Copy); + dataPack.SetText(copiedData); + Clipboard::SetContent(dataPack); + + // TODO: MSFT 20642290 and 20642291 + // rtf copy and html copy + }); + } + + // Method Description: + // - Fires an async event to get data from the clipboard, and paste it to + // the terminal. Triggered when the Terminal Control requests clipboard + // data with it's PasteFromClipboard event. + // Arguments: + // - eventArgs: the PasteFromClipboard event sent from the TermControl + void App::_PasteFromClipboardHandler(const IInspectable& /*sender*/, + const PasteFromClipboardEventArgs& eventArgs) + { + _root.Dispatcher().RunAsync(CoreDispatcherPriority::High, [eventArgs]() { + PasteFromClipboard(eventArgs); + }); } // -------------------------------- WinRT Events --------------------------------- diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index ae5a8f45339..b0f296bde98 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -102,9 +102,9 @@ namespace winrt::TerminalApp::implementation void _Scroll(int delta); void _CopyText(const bool trimTrailingWhitespace); - void _SplitVertical(std::optional profileGuid); - void _SplitHorizontal(std::optional profileGuid); - void _SplitPane(const bool splitHorizontal, std::optional profileGuid); + void _SplitVertical(const std::optional& profileGuid); + void _SplitHorizontal(const std::optional& profileGuid); + void _SplitPane(const bool splitHorizontal, const std::optional& profileGuid); // Todo: add more event implementations here // MSFT:20641986: Add keybindings for New Window void _ScrollPage(int delta); @@ -121,6 +121,9 @@ namespace winrt::TerminalApp::implementation static Windows::UI::Xaml::Controls::IconElement _GetIconFromProfile(const ::TerminalApp::Profile& profile); winrt::Microsoft::Terminal::TerminalControl::TermControl _GetFocusedControl(); + + void _CopyToClipboardHandler(const winrt::hstring& copiedData); + void _PasteFromClipboardHandler(const IInspectable& sender, const Microsoft::Terminal::TerminalControl::PasteFromClipboardEventArgs& eventArgs); }; } diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index b2ae7096221..e6b2f57fe4c 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -9,7 +9,7 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; -static const int SEPERATOR_SIZE = 4; +static const int SEPARATOR_SIZE = 4; Pane::Pane(GUID profile, TermControl control, const bool lastFocused) : _control{ control }, @@ -20,9 +20,9 @@ Pane::Pane(GUID profile, TermControl control, const bool lastFocused) : _secondChild{ nullptr }, _connectionClosedToken{}, _firstClosedToken{}, - _secondClosedToken{} + _secondClosedToken{}, + _root{} { - _root = Controls::Grid{}; _AddControlToRoot(_control); // Set the background of the pane to match that of the theme's default grid @@ -34,14 +34,14 @@ Pane::Pane(GUID profile, TermControl control, const bool lastFocused) : { winrt::Windows::Foundation::IInspectable g = res.Lookup(key); winrt::Windows::UI::Xaml::Style style = g.try_as(); - _root.Style(style); + // try_as fails by returning nullptr + if (style) + { + _root.Style(style); + } } } -Pane::~Pane() -{ -} - // Method Description: // - Adds a given Terminal Control to our root Grid. Also registers an event // handler to know when that control closed. @@ -71,32 +71,47 @@ Controls::Grid Pane::GetRootElement() } // Method Description: -// - Returns nullptr if no children of this pane were the last control to be -// focused, or the TermControl that _was_ the last control to be focused (if -// there was one). -// - This control might not currently be focused, if the tab itself is not -// currently focused. +// - If this is the last focused pane, returns itself. Returns nullptr if this +// is a leaf and it's not focused. If it's a parent, it returns nullptr if no +// children of this pane were the last pane to be focused, or the Pane that +// _was_ the last pane to be focused (if there was one). +// - This Pane's control might not currently be focused, if the tab itself is +// not currently focused. // Return Value: -// - nullptr if no children were marked `_lastFocused`, else the TermControl -// that was last focused. -TermControl Pane::GetLastFocusedTerminalControl() +// - nullptr if we're a leaf and unfocused, or no children were marked +// `_lastFocused`, else returns this +std::shared_ptr Pane::GetLastFocusedPane() { if (_IsLeaf()) { - return _lastFocused ? _control : nullptr; + return _lastFocused ? shared_from_this() : nullptr; } else { - auto firstFocused = _firstChild->GetLastFocusedTerminalControl(); + auto firstFocused = _firstChild->GetLastFocusedPane(); if (firstFocused != nullptr) { return firstFocused; } - auto secondFocused = _secondChild->GetLastFocusedTerminalControl(); - return secondFocused; + return _secondChild->GetLastFocusedPane(); } } +// Method Description: +// - Returns nullptr if no children of this pane were the last control to be +// focused, or the TermControl that _was_ the last control to be focused (if +// there was one). +// - This control might not currently be focused, if the tab itself is not +// currently focused. +// Return Value: +// - nullptr if no children were marked `_lastFocused`, else the TermControl +// that was last focused. +TermControl Pane::GetLastFocusedTerminalControl() +{ + auto lastFocused = GetLastFocusedPane(); + return lastFocused ? lastFocused->_control : nullptr; +} + // Method Description: // - Returns nullopt if no children of this pane were the last control to be // focused, or the GUID of the profile of the last control to be focused (if @@ -104,22 +119,10 @@ TermControl Pane::GetLastFocusedTerminalControl() // Return Value: // - nullopt if no children of this pane were the last control to be // focused, else the GUID of the profile of the last control to be focused -std::optional Pane::GetLastFocusedProfile() const noexcept +std::optional Pane::GetLastFocusedProfile() { - if (_IsLeaf()) - { - return _lastFocused ? _profile : std::nullopt; - } - else - { - auto firstFocused = _firstChild->GetLastFocusedProfile(); - if (firstFocused.has_value()) - { - return firstFocused; - } - auto secondFocused = _secondChild->GetLastFocusedProfile(); - return secondFocused; - } + auto lastFocused = GetLastFocusedPane(); + return lastFocused ? lastFocused->_profile : std::nullopt; } // Method Description: @@ -167,7 +170,7 @@ bool Pane::_HasFocusedChild() const noexcept // * If this is a leaf node, and our control is actively focused, well mark // ourselves as the _lastFocused. // * If we're not a leaf, we'll recurse on our children to check them. -void Pane::CheckFocus() +void Pane::UpdateFocus() { if (_IsLeaf()) { @@ -179,8 +182,8 @@ void Pane::CheckFocus() else { _lastFocused = false; - _firstChild->CheckFocus(); - _secondChild->CheckFocus(); + _firstChild->UpdateFocus(); + _secondChild->UpdateFocus(); } } @@ -207,7 +210,7 @@ void Pane::_FocusFirstChild() // Arguments: // - settings: The new TerminalSettings to apply to any matching controls // - profile: The GUID of the profile these settings should apply to. -void Pane::CheckUpdateSettings(TerminalSettings settings, GUID profile) +void Pane::CheckUpdateSettings(const TerminalSettings& settings, const GUID& profile) { if (!_IsLeaf()) { @@ -353,7 +356,7 @@ void Pane::_SetupChildCloseHandlers() // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Pane::SplitVertical(const GUID profile, TermControl control) +void Pane::SplitVertical(const GUID& profile, TermControl control) { // If we're not the leaf, recurse into our children to split them. if (!_IsLeaf()) @@ -401,7 +404,7 @@ void Pane::SplitVertical(const GUID profile, TermControl control) // Create the pane separator, and place it in column 1 _separatorRoot = Controls::Grid{}; - _separatorRoot.Width(SEPERATOR_SIZE); + _separatorRoot.Width(SEPARATOR_SIZE); // NaN is the special value XAML uses for "Auto" sizing. _separatorRoot.Height(NAN); _root.Children().Append(_separatorRoot); @@ -424,7 +427,7 @@ void Pane::SplitVertical(const GUID profile, TermControl control) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Pane::SplitHorizontal(const GUID profile, TermControl control) +void Pane::SplitHorizontal(const GUID& profile, TermControl control) { if (!_IsLeaf()) { @@ -468,7 +471,7 @@ void Pane::SplitHorizontal(const GUID profile, TermControl control) Controls::Grid::SetRow(_firstChild->GetRootElement(), 0); _separatorRoot = Controls::Grid{}; - _separatorRoot.Height(SEPERATOR_SIZE); + _separatorRoot.Height(SEPARATOR_SIZE); // NaN is the special value XAML uses for "Auto" sizing. _separatorRoot.Width(NAN); _root.Children().Append(_separatorRoot); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index dcd60014928..902d38c9c7e 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -1,11 +1,29 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +// +// Module Name: +// - Pane.h +// +// Abstract: +// - Panes are an abstraction by which the terminal can dislay multiple terminal +// instances simultaneously in a single terminal window. While tabs allow for +// a single terminal window to have many terminal sessions running +// simultaneously within a single window, only one tab can be visible at a +// time. Panes, on the other hand, allow a user to have many different +// terminal sessions visible to the user within the context of a single window +// at the same time. This can enable greater productivity from the user, as +// they can see the output of one terminal window while working in another. +// - See doc/cascadia/Panes.md for a detailed description. +// +// Author: +// - Mike Griese (zadjii-msft) 16-May-2019 + #pragma once #include #include "../../cascadia/inc/cppwinrt_utils.h" -class Pane +class Pane : public std::enable_shared_from_this { public: @@ -18,19 +36,21 @@ class Pane }; Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused = false); - ~Pane(); + ~Pane() = default; + std::shared_ptr GetLastFocusedPane(); winrt::Microsoft::Terminal::TerminalControl::TermControl GetLastFocusedTerminalControl(); - std::optional GetLastFocusedProfile() const noexcept; + std::optional GetLastFocusedProfile(); + winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); bool WasLastFocused() const noexcept; - void CheckFocus(); + void UpdateFocus(); - void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); + void CheckUpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); - void SplitHorizontal(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - void SplitVertical(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void SplitHorizontal(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void SplitVertical(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 6a6aacbf95a..e1e732153e3 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -124,9 +124,9 @@ void Tab::_Focus() // under this tab is focused, then it will be marked as the last focused. If // there are no focused panes, then there will not be a last focused contrl // when this returns. -void Tab::CheckFocus() +void Tab::UpdateFocus() { - _rootPane->CheckFocus(); + _rootPane->UpdateFocus(); } // Method Description: @@ -161,8 +161,8 @@ void Tab::SetTabText(const winrt::hstring& text) // - void Tab::Scroll(const int delta) { - GetLastFocusedTerminalControl().GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - auto control = GetLastFocusedTerminalControl(); + auto control = GetLastFocusedTerminalControl(); + control.GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [control, delta](){ const auto currentOffset = control.GetScrollOffset(); control.ScrollViewport(currentOffset + delta); }); @@ -174,7 +174,7 @@ void Tab::Scroll(const int delta) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Tab::SplitVertical(GUID profile, TermControl control) +void Tab::AddVerticalSplit(GUID profile, TermControl control) { _rootPane->SplitVertical(profile, control); } @@ -185,7 +185,7 @@ void Tab::SplitVertical(GUID profile, TermControl control) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Tab::SplitHorizontal(GUID profile, TermControl control) +void Tab::AddHorizontalSplit(GUID profile, TermControl control) { _rootPane->SplitHorizontal(profile, control); } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 637cc84f1d5..7e85a0ad4f3 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -21,10 +21,10 @@ class Tab void SetFocused(const bool focused); void Scroll(const int delta); - void SplitVertical(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - void SplitHorizontal(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void AddVerticalSplit(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void AddHorizontalSplit(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - void CheckFocus(); + void UpdateFocus(); void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); winrt::hstring GetLastFocusedTitle(); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index be2aa073d13..146acb2bacf 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -227,11 +227,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation TermControl::~TermControl() { _closing = true; - if (_initializedTerminal) - { - // Don't let anyone else do something to the buffer. - auto lock = _terminal->LockForWriting(); - } + // Don't let anyone else do something to the buffer. + auto lock = _terminal->LockForWriting(); if (_connection != nullptr) { From ba805e85267f738b5096e7f270864adb672c50d5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 17 May 2019 12:07:06 -0500 Subject: [PATCH 17/33] Update doc/cascadia/Panes.md Co-Authored-By: Dustin L. Howett (MSFT) --- doc/cascadia/Panes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/cascadia/Panes.md b/doc/cascadia/Panes.md index 21ac4f9a9a0..32c3b78d632 100644 --- a/doc/cascadia/Panes.md +++ b/doc/cascadia/Panes.md @@ -139,7 +139,7 @@ pane, the child will: 1. Convert into a parent 2. Move its terminal into it's first child - 3. Split it's UI in half, and display each child in one half. + 3. Split its UI in half, and display each child in one half. It's up to the app hosting the panes to tell the pane what kind of terminal in wants created in the new pane. Implemented by default is the new pane will be From 3d1c4cace79c53c291d787ddb496777084307971 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 May 2019 15:49:14 -0500 Subject: [PATCH 18/33] Mostly just PR nits --- doc/cascadia/Panes.md | 13 +++---- src/cascadia/TerminalApp/App.cpp | 8 ++--- src/cascadia/TerminalApp/CascadiaSettings.cpp | 4 +-- src/cascadia/TerminalApp/Pane.cpp | 26 +++++++------- src/cascadia/TerminalApp/Pane.h | 15 ++++---- src/cascadia/TerminalApp/Tab.cpp | 36 ++++++++----------- src/cascadia/TerminalApp/Tab.h | 17 +++++---- 7 files changed, 55 insertions(+), 64 deletions(-) diff --git a/doc/cascadia/Panes.md b/doc/cascadia/Panes.md index 32c3b78d632..f633ca13c5b 100644 --- a/doc/cascadia/Panes.md +++ b/doc/cascadia/Panes.md @@ -1,7 +1,9 @@ -# Panes in the Windows Terminal +--- +author: "Mike Griese @zadjii-msft" +created on: 2019-May-16 +--- -* author: Mike Griese __[@zadjii-msft](https://github.com/zadjii-msft)__ -* created on: 2019-May-16 +# Panes in the Windows Terminal ## Abstract @@ -17,7 +19,6 @@ window while working in another. This spec will help outline the design of the implementation of panes in the Windows Terminal. - ## Inspirations Panes within the context of a single terminal window are not a new idea. The @@ -142,8 +143,8 @@ pane, the child will: 3. Split its UI in half, and display each child in one half. It's up to the app hosting the panes to tell the pane what kind of terminal in -wants created in the new pane. Implemented by default is the new pane will be -created with the default settings profile. +wants created in the new pane. By default, the new pane will be created with the +default settings profile. ### While panes are open diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 8b8015fe3bf..dd3137a53da 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -422,7 +422,7 @@ namespace winrt::TerminalApp::implementation for (auto &tab : _tabs) { // Attempt to reload the settings of any panes with this profile - tab->CheckUpdateSettings(settings, profileGuid); + tab->UpdateSettings(settings, profileGuid); } } @@ -450,7 +450,7 @@ namespace winrt::TerminalApp::implementation // - tab: the Tab to update the title for. void App::_UpdateTabIcon(std::shared_ptr tab) { - const auto lastFocusedProfileOpt = tab->GetLastFocusedProfile(); + const auto lastFocusedProfileOpt = tab->GetFocusedProfile(); if (lastFocusedProfileOpt.has_value()) { const auto lastFocusedProfile = lastFocusedProfileOpt.value(); @@ -475,7 +475,7 @@ namespace winrt::TerminalApp::implementation // - tab: the Tab to update the title for. void App::_CheckTitleUpdate(std::shared_ptr tab) { - auto newTabTitle = tab->GetLastFocusedTitle(); + auto newTabTitle = tab->GetFocusedTitle(); // TODO #608: If the settings don't want the terminal's text in the // tab, then display something else. @@ -953,7 +953,7 @@ namespace winrt::TerminalApp::implementation { int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; - return focusedTab->GetLastFocusedTerminalControl(); + return focusedTab->GetFocusedTerminalControl(); } // Method Description: diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index dd3405bf145..182e107ef47 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -250,10 +250,10 @@ void CascadiaSettings::_CreateDefaultKeybindings() VK_TAB }); keyBindings.SetKeyBinding(ShortcutAction::SplitVertical, - KeyChord{ KeyModifiers::Ctrl | KeyModifiers::Shift, + KeyChord{ KeyModifiers::Alt | KeyModifiers::Shift, VK_OEM_PLUS }); // For any country/region, the '+' key keyBindings.SetKeyBinding(ShortcutAction::SplitHorizontal, - KeyChord{ KeyModifiers::Ctrl | KeyModifiers::Shift, + KeyChord{ KeyModifiers::Alt | KeyModifiers::Shift, VK_OEM_MINUS }); // For any country/region, the '-' key // Yes these are offset by one. diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index e6b2f57fe4c..089646062a1 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -11,7 +11,7 @@ using namespace winrt::Microsoft::Terminal::TerminalControl; static const int SEPARATOR_SIZE = 4; -Pane::Pane(GUID profile, TermControl control, const bool lastFocused) : +Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocused) : _control{ control }, _lastFocused{ lastFocused }, _profile{ profile }, @@ -80,7 +80,7 @@ Controls::Grid Pane::GetRootElement() // Return Value: // - nullptr if we're a leaf and unfocused, or no children were marked // `_lastFocused`, else returns this -std::shared_ptr Pane::GetLastFocusedPane() +std::shared_ptr Pane::GetFocusedPane() { if (_IsLeaf()) { @@ -88,12 +88,12 @@ std::shared_ptr Pane::GetLastFocusedPane() } else { - auto firstFocused = _firstChild->GetLastFocusedPane(); + auto firstFocused = _firstChild->GetFocusedPane(); if (firstFocused != nullptr) { return firstFocused; } - return _secondChild->GetLastFocusedPane(); + return _secondChild->GetFocusedPane(); } } @@ -106,9 +106,9 @@ std::shared_ptr Pane::GetLastFocusedPane() // Return Value: // - nullptr if no children were marked `_lastFocused`, else the TermControl // that was last focused. -TermControl Pane::GetLastFocusedTerminalControl() +TermControl Pane::GetFocusedTerminalControl() { - auto lastFocused = GetLastFocusedPane(); + auto lastFocused = GetFocusedPane(); return lastFocused ? lastFocused->_control : nullptr; } @@ -119,9 +119,9 @@ TermControl Pane::GetLastFocusedTerminalControl() // Return Value: // - nullopt if no children of this pane were the last control to be // focused, else the GUID of the profile of the last control to be focused -std::optional Pane::GetLastFocusedProfile() +std::optional Pane::GetFocusedProfile() { - auto lastFocused = GetLastFocusedPane(); + auto lastFocused = GetFocusedPane(); return lastFocused ? lastFocused->_profile : std::nullopt; } @@ -210,12 +210,12 @@ void Pane::_FocusFirstChild() // Arguments: // - settings: The new TerminalSettings to apply to any matching controls // - profile: The GUID of the profile these settings should apply to. -void Pane::CheckUpdateSettings(const TerminalSettings& settings, const GUID& profile) +void Pane::UpdateSettings(const TerminalSettings& settings, const GUID& profile) { if (!_IsLeaf()) { - _firstChild->CheckUpdateSettings(settings, profile); - _secondChild->CheckUpdateSettings(settings, profile); + _firstChild->UpdateSettings(settings, profile); + _secondChild->UpdateSettings(settings, profile); } else { @@ -356,7 +356,7 @@ void Pane::_SetupChildCloseHandlers() // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Pane::SplitVertical(const GUID& profile, TermControl control) +void Pane::SplitVertical(const GUID& profile, const TermControl& control) { // If we're not the leaf, recurse into our children to split them. if (!_IsLeaf()) @@ -427,7 +427,7 @@ void Pane::SplitVertical(const GUID& profile, TermControl control) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Pane::SplitHorizontal(const GUID& profile, TermControl control) +void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) { if (!_IsLeaf()) { diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 902d38c9c7e..64c7c316d58 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -35,22 +35,21 @@ class Pane : public std::enable_shared_from_this Horizontal = 2 }; - Pane(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control, const bool lastFocused = false); - ~Pane() = default; + Pane(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control, const bool lastFocused = false); - std::shared_ptr GetLastFocusedPane(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetLastFocusedTerminalControl(); - std::optional GetLastFocusedProfile(); + std::shared_ptr GetFocusedPane(); + winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + std::optional GetFocusedProfile(); winrt::Windows::UI::Xaml::Controls::Grid GetRootElement(); bool WasLastFocused() const noexcept; void UpdateFocus(); - void CheckUpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); + void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); - void SplitHorizontal(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - void SplitVertical(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void SplitHorizontal(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void SplitVertical(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index e1e732153e3..4abbe2d1177 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -9,7 +9,7 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; -Tab::Tab(GUID profile, TermControl control) : +Tab::Tab(const GUID& profile, const TermControl& control) : _focused{ false }, _tabViewItem{ nullptr }, _rootPane{ nullptr } @@ -23,14 +23,6 @@ Tab::Tab(GUID profile, TermControl control) : _MakeTabViewItem(); } -Tab::~Tab() -{ - // When we're destructed, winrt will automatically decrement the refcount - // of our terminalcontrol. - // Assuming that refcount hits 0, it'll destruct it on it's own, including - // calling Close on the terminal and connection. -} - void Tab::_MakeTabViewItem() { _tabViewItem = ::winrt::Microsoft::UI::Xaml::Controls::TabViewItem{}; @@ -51,9 +43,9 @@ UIElement Tab::GetRootElement() // Return Value: // - nullptr if no children were marked `_lastFocused`, else the TermControl // that was last focused. -TermControl Tab::GetLastFocusedTerminalControl() +TermControl Tab::GetFocusedTerminalControl() { - return _rootPane->GetLastFocusedTerminalControl(); + return _rootPane->GetFocusedTerminalControl(); } winrt::Microsoft::UI::Xaml::Controls::TabViewItem Tab::GetTabViewItem() @@ -61,7 +53,7 @@ winrt::Microsoft::UI::Xaml::Controls::TabViewItem Tab::GetTabViewItem() return _tabViewItem; } -bool Tab::IsFocused() +bool Tab::IsFocused() const noexcept { return _focused; } @@ -91,9 +83,9 @@ void Tab::SetFocused(const bool focused) // Return Value: // - nullopt if no children of this tab were the last control to be // focused, else the GUID of the profile of the last control to be focused -std::optional Tab::GetLastFocusedProfile() const noexcept +std::optional Tab::GetFocusedProfile() const noexcept { - return _rootPane->GetLastFocusedProfile(); + return _rootPane->GetFocusedProfile(); } // Method Description: @@ -101,9 +93,9 @@ std::optional Tab::GetLastFocusedProfile() const noexcept // Arguments: // - settings: The new TerminalSettings to apply to any matching controls // - profile: The GUID of the profile these settings should apply to. -void Tab::CheckUpdateSettings(TerminalSettings settings, GUID profile) +void Tab::UpdateSettings(const TerminalSettings& settings, const GUID& profile) { - _rootPane->CheckUpdateSettings(settings, profile); + _rootPane->UpdateSettings(settings, profile); } // Method Description: @@ -112,7 +104,7 @@ void Tab::_Focus() { _focused = true; - auto lastFocusedControl = _rootPane->GetLastFocusedTerminalControl(); + auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); if (lastFocusedControl) { lastFocusedControl.GetControl().Focus(FocusState::Programmatic); @@ -134,9 +126,9 @@ void Tab::UpdateFocus() // Returns the empty string if there is no such control. // Return Value: // - the title string of the last focused terminal control in our tree. -winrt::hstring Tab::GetLastFocusedTitle() +winrt::hstring Tab::GetFocusedTitle() { - auto lastFocusedControl = _rootPane->GetLastFocusedTerminalControl(); + auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; } @@ -161,7 +153,7 @@ void Tab::SetTabText(const winrt::hstring& text) // - void Tab::Scroll(const int delta) { - auto control = GetLastFocusedTerminalControl(); + auto control = GetFocusedTerminalControl(); control.GetControl().Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [control, delta](){ const auto currentOffset = control.GetScrollOffset(); control.ScrollViewport(currentOffset + delta); @@ -174,7 +166,7 @@ void Tab::Scroll(const int delta) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Tab::AddVerticalSplit(GUID profile, TermControl control) +void Tab::AddVerticalSplit(const GUID& profile, TermControl& control) { _rootPane->SplitVertical(profile, control); } @@ -185,7 +177,7 @@ void Tab::AddVerticalSplit(GUID profile, TermControl control) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. -void Tab::AddHorizontalSplit(GUID profile, TermControl control) +void Tab::AddHorizontalSplit(const GUID& profile, TermControl& control) { _rootPane->SplitHorizontal(profile, control); } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 7e85a0ad4f3..bad22153eb8 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -9,25 +9,24 @@ class Tab { public: - Tab(GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - ~Tab(); + Tab(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); winrt::Microsoft::UI::Xaml::Controls::TabViewItem GetTabViewItem(); winrt::Windows::UI::Xaml::UIElement GetRootElement(); - winrt::Microsoft::Terminal::TerminalControl::TermControl GetLastFocusedTerminalControl(); - std::optional GetLastFocusedProfile() const noexcept; + winrt::Microsoft::Terminal::TerminalControl::TermControl GetFocusedTerminalControl(); + std::optional GetFocusedProfile() const noexcept; - bool IsFocused(); + bool IsFocused() const noexcept; void SetFocused(const bool focused); void Scroll(const int delta); - void AddVerticalSplit(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); - void AddHorizontalSplit(const GUID profile, winrt::Microsoft::Terminal::TerminalControl::TermControl control); + void AddVerticalSplit(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void AddHorizontalSplit(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control); void UpdateFocus(); - void CheckUpdateSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings settings, GUID profile); - winrt::hstring GetLastFocusedTitle(); + void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); + winrt::hstring GetFocusedTitle(); void SetTabText(const winrt::hstring& text); DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); From 7c5c22216004aa554afd6b943aff52b9b14a4073 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 21 May 2019 09:22:46 -0500 Subject: [PATCH 19/33] Fix some bugs with closing a leaf _after_ it was a parent. if you open 3 panes like so: ``` 1:[|] 2:A 3:[-] 4:B 5:C ``` Then `exit` C, the tree will look like this: ``` 1:[|] 2:A 3:B ``` At this point, if you closed B, it wouldn't work, because pane 4 was still registered as the ConnectionColsed handler for control B, even though it's been destructed. Also fixes a couple crashes with the cursor timer. --- src/cascadia/TerminalApp/Pane.cpp | 18 +++++++++++++----- src/cascadia/TerminalControl/TermControl.cpp | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 089646062a1..218c14538df 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -18,9 +18,9 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus _splitState{ SplitState::None }, _firstChild{ nullptr }, _secondChild{ nullptr }, - _connectionClosedToken{}, - _firstClosedToken{}, - _secondClosedToken{}, + _connectionClosedToken{ 0 }, + _firstClosedToken{ 0 }, + _secondClosedToken{ 0 }, _root{} { _AddControlToRoot(_control); @@ -51,8 +51,8 @@ void Pane::_AddControlToRoot(TermControl control) { _root.Children().Append(control.GetControl()); - _connectionClosedToken = control.ConnectionClosed([=]() { - if (control.CloseOnExit()) + _connectionClosedToken = control.ConnectionClosed([this]() { + if (_control.CloseOnExit()) { // Fire our Closed event to tell our parent that we should be removed. _closedHandlers(); @@ -240,6 +240,12 @@ void Pane::_CloseChild(const bool closeFirst) // If the only child left is a leaf, that means we're a leaf now. if (remainingChild->_IsLeaf()) { + // Revoke the old event handlers. + _firstChild->Closed(_firstClosedToken); + _secondChild->Closed(_secondClosedToken); + closedChild->_control.ConnectionClosed(closedChild->_connectionClosedToken); + remainingChild->_control.ConnectionClosed(remainingChild->_connectionClosedToken); + // take the control and profile of the pane that _wasn't_ closed. _control = closeFirst ? _secondChild->_control : _firstChild->_control; _profile = closeFirst ? _secondChild->_profile : _firstChild->_profile; @@ -375,6 +381,7 @@ void Pane::SplitVertical(const GUID& profile, const TermControl& control) // revoke our handler - the child will take care of the control now. _control.ConnectionClosed(_connectionClosedToken); + _connectionClosedToken.value = 0; _splitState = SplitState::Vertical; @@ -445,6 +452,7 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) // revoke our handler - the child will take care of the control now. _control.ConnectionClosed(_connectionClosedToken); + _connectionClosedToken.value = 0; _splitState = SplitState::Horizontal; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 21847920b23..ddac8e8d983 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -230,6 +230,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Don't let anyone else do something to the buffer. auto lock = _terminal->LockForWriting(); + // Clear out the cursor timer, so it doesn't trigger again on us once we're destructed. + if (_cursorTimer) + { + _cursorTimer.value().Stop(); + _cursorTimer = std::nullopt; + } + if (_connection != nullptr) { _connection.Close(); @@ -840,6 +847,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void TermControl::_GotFocusHandler(Windows::Foundation::IInspectable const& /* sender */, RoutedEventArgs const& /* args */) { + if (_closing) + { + return; + } _focused = true; if (_cursorTimer.has_value()) @@ -852,6 +863,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void TermControl::_LostFocusHandler(Windows::Foundation::IInspectable const& /* sender */, RoutedEventArgs const& /* args */) { + if (_closing) + { + return; + } _focused = false; if (_cursorTimer.has_value()) @@ -923,6 +938,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void TermControl::_BlinkCursor(Windows::Foundation::IInspectable const& /* sender */, Windows::Foundation::IInspectable const& /* e */) { + if (_closing) + { + return; + } _terminal->SetCursorVisible(!_terminal->IsCursorVisible()); } From 0f848e7f9535d463fa8bb9d1956bb620c876b646 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 24 May 2019 10:50:03 -0500 Subject: [PATCH 20/33] Remove the default keybinding. The functionality will be there, but the key won't be there, so people have to opt-in to it. --- src/cascadia/TerminalApp/CascadiaSettings.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index de20a840b36..74242976ab7 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -256,13 +256,6 @@ void CascadiaSettings::_CreateDefaultKeybindings() KeyChord{ KeyModifiers::Ctrl | KeyModifiers::Shift, VK_TAB }); - keyBindings.SetKeyBinding(ShortcutAction::SplitVertical, - KeyChord{ KeyModifiers::Alt | KeyModifiers::Shift, - VK_OEM_PLUS }); // For any country/region, the '+' key - keyBindings.SetKeyBinding(ShortcutAction::SplitHorizontal, - KeyChord{ KeyModifiers::Alt | KeyModifiers::Shift, - VK_OEM_MINUS }); // For any country/region, the '-' key - // Yes these are offset by one. // Ideally, you'd want C-S-1 to open the _first_ profile, which is index 0 keyBindings.SetKeyBinding(ShortcutAction::NewTabProfile0, From bbed9ed512cc13f1652181b049b241831390b037 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 24 May 2019 10:50:27 -0500 Subject: [PATCH 21/33] switch to in-class initializers --- src/cascadia/TerminalApp/Pane.cpp | 9 +-------- src/cascadia/TerminalApp/Pane.h | 18 +++++++++--------- src/cascadia/TerminalApp/Tab.cpp | 5 +---- src/cascadia/TerminalApp/Tab.h | 6 +++--- 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 218c14538df..073a558cd37 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -14,14 +14,7 @@ static const int SEPARATOR_SIZE = 4; Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocused) : _control{ control }, _lastFocused{ lastFocused }, - _profile{ profile }, - _splitState{ SplitState::None }, - _firstChild{ nullptr }, - _secondChild{ nullptr }, - _connectionClosedToken{ 0 }, - _firstClosedToken{ 0 }, - _secondClosedToken{ 0 }, - _root{} + _profile{ profile } { _AddControlToRoot(_control); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 64c7c316d58..09b85c71e49 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -54,19 +54,19 @@ class Pane : public std::enable_shared_from_this DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); private: - winrt::Windows::UI::Xaml::Controls::Grid _root{ nullptr }; + winrt::Windows::UI::Xaml::Controls::Grid _root{}; winrt::Windows::UI::Xaml::Controls::Grid _separatorRoot{ nullptr }; winrt::Microsoft::Terminal::TerminalControl::TermControl _control{ nullptr }; - std::shared_ptr _firstChild; - std::shared_ptr _secondChild; - SplitState _splitState; + std::shared_ptr _firstChild{ nullptr }; + std::shared_ptr _secondChild{ nullptr }; + SplitState _splitState{ SplitState::None }; - bool _lastFocused; - std::optional _profile; - winrt::event_token _connectionClosedToken; - winrt::event_token _firstClosedToken; - winrt::event_token _secondClosedToken; + bool _lastFocused{ false }; + std::optional _profile{ std::nullopt }; + winrt::event_token _connectionClosedToken{ 0 }; + winrt::event_token _firstClosedToken{ 0 }; + winrt::event_token _secondClosedToken{ 0 }; bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 4abbe2d1177..4800a79e28a 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -9,10 +9,7 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; -Tab::Tab(const GUID& profile, const TermControl& control) : - _focused{ false }, - _tabViewItem{ nullptr }, - _rootPane{ nullptr } +Tab::Tab(const GUID& profile, const TermControl& control) { _rootPane = std::make_shared(profile, control, true); diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index bad22153eb8..40983a52875 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -33,10 +33,10 @@ class Tab private: - std::shared_ptr _rootPane; + std::shared_ptr _rootPane{ nullptr }; - bool _focused; - winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem; + bool _focused{ false }; + winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem{ nullptr }; void _MakeTabViewItem(); void _Focus(); From 704f6cc162612f7837ec02902508340a2b0a1f57 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 30 May 2019 07:25:13 -0500 Subject: [PATCH 22/33] Update doc/cascadia/Panes.md Co-Authored-By: Summon528 --- doc/cascadia/Panes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/cascadia/Panes.md b/doc/cascadia/Panes.md index f633ca13c5b..878eed1a743 100644 --- a/doc/cascadia/Panes.md +++ b/doc/cascadia/Panes.md @@ -7,7 +7,7 @@ created on: 2019-May-16 ## Abstract -Panes are an abstraction by which the terminal can dislay multiple terminal +Panes are an abstraction by which the terminal can display multiple terminal instances simultaneously in a single terminal window. While tabs allow for a single terminal window to have many terminal sessions running simultaneously within a single window, only one tab can be visible at a time. Panes, on the From 40c560022f74f6dbf3e24d88a4cb29de7394f84b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 31 May 2019 14:45:05 -0500 Subject: [PATCH 23/33] Apply suggestions from code review These are all the really simple suggestions that could be done from github Co-Authored-By: Michael Niksa --- doc/cascadia/Panes.md | 4 ++-- src/cascadia/TerminalApp/Pane.cpp | 20 ++++++++++---------- src/cascadia/TerminalApp/Tab.cpp | 6 +++--- src/cascadia/TerminalApp/Tab.h | 2 +- src/cascadia/TerminalControl/TermControl.cpp | 4 ++-- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/doc/cascadia/Panes.md b/doc/cascadia/Panes.md index 878eed1a743..83d20fa4272 100644 --- a/doc/cascadia/Panes.md +++ b/doc/cascadia/Panes.md @@ -226,7 +226,7 @@ pane. This could be solved a number of ways. There could be keyboard shortcuts for swapping the positions of tabs, or a shortcut for both "zooming" a tab (temporarily making it the full size) or even popping a pane out to it's own tab. Additionally, a right-click menu option could be added to do the -afformentioned actions. Discoverability of these two actions is not as high as -just dragging a tab from one pane to another, however, it's believed that panes +aformentioned actions. Discoverability of these two actions is not as high as +just dragging a tab from one pane to another; however, it's believed that panes are more of a power-user scenario, and power users will not neccessarily be turned off by the feature's discoverability. diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 073a558cd37..9efc4185fc2 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -21,12 +21,12 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus // Set the background of the pane to match that of the theme's default grid // background. This way, we'll match the small underline under the tabs, and // the UI will be consistent on bot light and dark modes. - auto res = Application::Current().Resources(); - winrt::Windows::Foundation::IInspectable key = winrt::box_value(L"BackgroundGridThemeStyle"); + const auto res = Application::Current().Resources(); + const auto key = winrt::box_value(L"BackgroundGridThemeStyle"); if (res.HasKey(key)) { - winrt::Windows::Foundation::IInspectable g = res.Lookup(key); - winrt::Windows::UI::Xaml::Style style = g.try_as(); + const auto g = res.Lookup(key); + const auto style = g.try_as(); // try_as fails by returning nullptr if (style) { @@ -150,24 +150,24 @@ bool Pane::_IsLeaf() const noexcept // pane's descendants bool Pane::_HasFocusedChild() const noexcept { - const bool controlFocused = _control != nullptr && + const auto controlFocused = _control && _control.GetControl().FocusState() != FocusState::Unfocused; - const bool firstFocused = _firstChild != nullptr && _firstChild->_HasFocusedChild(); - const bool secondFocused = _secondChild != nullptr && _secondChild->_HasFocusedChild(); + const auto firstFocused = _firstChild && _firstChild->_HasFocusedChild(); + const auto secondFocused = _secondChild && _secondChild->_HasFocusedChild(); return controlFocused || firstFocused || secondFocused; } // Method Description: -// - Update the focus state of this pane, and all it's descendants. -// * If this is a leaf node, and our control is actively focused, well mark +// - Update the focus state of this pane, and all its descendants. +// * If this is a leaf node, and our control is actively focused, we'll mark // ourselves as the _lastFocused. // * If we're not a leaf, we'll recurse on our children to check them. void Pane::UpdateFocus() { if (_IsLeaf()) { - const bool controlFocused = _control != nullptr && + const auto controlFocused = _control && _control.GetControl().FocusState() != FocusState::Unfocused; _lastFocused = controlFocused; diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 4f535d0ec2e..15a7c8c3a94 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -111,7 +111,7 @@ void Tab::_Focus() // Method Description: // - Update the focus state of this tab's tree of panes. If one of the controls // under this tab is focused, then it will be marked as the last focused. If -// there are no focused panes, then there will not be a last focused contrl +// there are no focused panes, then there will not be a last focused control // when this returns. void Tab::UpdateFocus() { @@ -123,9 +123,9 @@ void Tab::UpdateFocus() // Returns the empty string if there is no such control. // Return Value: // - the title string of the last focused terminal control in our tree. -winrt::hstring Tab::GetFocusedTitle() +winrt::hstring Tab::GetFocusedTitle() const { - auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); + const auto lastFocusedControl = _rootPane->GetFocusedTerminalControl(); return lastFocusedControl ? lastFocusedControl.Title() : L""; } diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h index 40983a52875..5cb3833946e 100644 --- a/src/cascadia/TerminalApp/Tab.h +++ b/src/cascadia/TerminalApp/Tab.h @@ -26,7 +26,7 @@ class Tab void UpdateFocus(); void UpdateSettings(const winrt::Microsoft::Terminal::Settings::TerminalSettings& settings, const GUID& profile); - winrt::hstring GetFocusedTitle(); + winrt::hstring GetFocusedTitle() const; void SetTabText(const winrt::hstring& text); DECLARE_EVENT(Closed, _closedHandlers, winrt::Microsoft::Terminal::TerminalControl::ConnectionClosedEventArgs); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index c2eadbe888a..ff42bc6c63e 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -354,12 +354,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _cursorTimer = std::nullopt; } - if (_connection != nullptr) + if (_connection) { _connection.Close(); } - if (_renderer != nullptr) + if (_renderer) { _renderer->TriggerTeardown(); } From 890fb3a98c35d54a6148d2f61124f05e6e855537 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Fri, 31 May 2019 14:12:51 -0700 Subject: [PATCH 24/33] Add a NOTICES file. --- NOTICE.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 NOTICE.md diff --git a/NOTICE.md b/NOTICE.md new file mode 100644 index 00000000000..623e3fb70d4 --- /dev/null +++ b/NOTICE.md @@ -0,0 +1,49 @@ +# NOTICES AND INFORMATION +Do Not Translate or Localize + +This software incorporates material from third parties. Microsoft makes certain +open source code available at http://3rdpartysource.microsoft.com, or you may +send a check or money order for US $5.00, including the product name, the open +source component name, and version number, to: + +``` +Source Code Compliance Team +Microsoft Corporation +One Microsoft Way +Redmond, WA 98052 +USA +``` + +Notwithstanding any other terms, you may reverse engineer this software to the +extent required to debug changes to any libraries licensed under the GNU Lesser +General Public License. + +## jsoncpp + +**Source**: https://github.com/open-source-parsers/jsoncpp + +### License + +``` +Copyright (c) 2007-2010 Baptiste Lepilleur and The JsonCpp Authors + +Permission is hereby granted, free of charge, to any person +obtaining a copy of this software and associated documentation +files (the "Software"), to deal in the Software without +restriction, including without limitation the rights to use, copy, +modify, merge, publish, distribute, sublicense, and/or sell copies +of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be +included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS +BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN +ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. +``` From 0ea8b85e99bbb2eeb78969af01ded8486e904fe5 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Fri, 31 May 2019 14:43:23 -0700 Subject: [PATCH 25/33] move the NOTICE to the right branch... --- NOTICE.md | 49 ------------------------------------------------- 1 file changed, 49 deletions(-) delete mode 100644 NOTICE.md diff --git a/NOTICE.md b/NOTICE.md deleted file mode 100644 index 623e3fb70d4..00000000000 --- a/NOTICE.md +++ /dev/null @@ -1,49 +0,0 @@ -# NOTICES AND INFORMATION -Do Not Translate or Localize - -This software incorporates material from third parties. Microsoft makes certain -open source code available at http://3rdpartysource.microsoft.com, or you may -send a check or money order for US $5.00, including the product name, the open -source component name, and version number, to: - -``` -Source Code Compliance Team -Microsoft Corporation -One Microsoft Way -Redmond, WA 98052 -USA -``` - -Notwithstanding any other terms, you may reverse engineer this software to the -extent required to debug changes to any libraries licensed under the GNU Lesser -General Public License. - -## jsoncpp - -**Source**: https://github.com/open-source-parsers/jsoncpp - -### License - -``` -Copyright (c) 2007-2010 Baptiste Lepilleur and The JsonCpp Authors - -Permission is hereby granted, free of charge, to any person -obtaining a copy of this software and associated documentation -files (the "Software"), to deal in the Software without -restriction, including without limitation the rights to use, copy, -modify, merge, publish, distribute, sublicense, and/or sell copies -of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be -included in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS -BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN -ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN -CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. -``` From 80036e28bf335babce35caffdfd210ceeb562874 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 3 Jun 2019 12:44:40 -0500 Subject: [PATCH 26/33] Much of the easier PR feedback --- src/cascadia/TerminalApp/App.cpp | 34 ++++++++------ src/cascadia/TerminalApp/App.h | 4 +- src/cascadia/TerminalApp/Pane.cpp | 48 ++++++++++++++++---- src/cascadia/TerminalApp/Tab.cpp | 34 +++++++++++++- src/cascadia/TerminalControl/TermControl.cpp | 12 +++-- src/cascadia/TerminalControl/TermControl.h | 2 +- src/cascadia/TerminalControl/TermControl.idl | 2 +- 7 files changed, 104 insertions(+), 32 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index a0df58c1baf..465254c914d 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -633,7 +633,7 @@ namespace winrt::TerminalApp::implementation // bubble this title to any listeners of our TitleChanged event. // Arguments: // - tab: the Tab to update the title for. - void App::_CheckTitleUpdate(std::shared_ptr tab) + void App::_UpdateTitle(std::shared_ptr tab) { auto newTabTitle = tab->GetFocusedTitle(); @@ -645,7 +645,6 @@ namespace winrt::TerminalApp::implementation { _titleChangeHandlers(newTabTitle); } - } // Method Description: @@ -797,7 +796,7 @@ namespace winrt::TerminalApp::implementation // The title of the control changed, but not necessarily the title // of the tab. Get the title of the focused pane of the tab, and set // the tab's text to the focused panes' text. - _CheckTitleUpdate(tab); + _UpdateTitle(tab); }); term.GetControl().GotFocus([this, weakTabPtr](auto&&, auto&&) @@ -812,7 +811,7 @@ namespace winrt::TerminalApp::implementation // Possibly update the title of the tab, window to match the newly // focused pane. - _CheckTitleUpdate(tab); + _UpdateTitle(tab); // Possibly update the icon of the tab. _UpdateTabIcon(tab); @@ -1132,7 +1131,7 @@ namespace winrt::TerminalApp::implementation // this is nullopt, use the default profile. void App::_SplitVertical(const std::optional& profileGuid) { - _SplitPane(false, profileGuid); + _SplitPane(Pane::SplitState::Vertical, profileGuid); } // Method Description: @@ -1143,32 +1142,39 @@ namespace winrt::TerminalApp::implementation // this is nullopt, use the default profile. void App::_SplitHorizontal(const std::optional& profileGuid) { - _SplitPane(true, profileGuid); + _SplitPane(Pane::SplitState::Horizontal, profileGuid); } // Method Description: // - Split the focused pane either horizontally or vertically, and place the // given TermControl into the newly created pane. + // - If splitType == SplitState::None, this method does nothing. // Arguments: - // - splitHorizontal: if true, split the pane horizontally. Else split - // vertically. + // - splitType: one value from the Pane::SplitState enum, indicating how the + // new pane should be split from its parent. // - profile: The profile GUID to associate with the newly created pane. If // this is nullopt, use the default profile. - void App::_SplitPane(const bool splitHorizontal, const std::optional& profileGuid) + void App::_SplitPane(const Pane::SplitState splitType, const std::optional& profileGuid) { - const GUID realGuid = profileGuid ? profileGuid.value() : + // No nothing if we're requesting no split. + if (splitType == Pane::SplitState::None) + { + return; + } + + const auto realGuid = profileGuid ? profileGuid.value() : _settings->GlobalSettings().GetDefaultProfile(); - auto controlSettings = _settings->MakeSettings(realGuid); + const auto controlSettings = _settings->MakeSettings(realGuid); TermControl newControl{ controlSettings }; - int focusedTabIndex = _GetFocusedTabIndex(); + const int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; // Hookp our event handlers to the new terminal _RegisterTerminalEvents(newControl, focusedTab); - return splitHorizontal ? focusedTab->AddHorizontalSplit(realGuid, newControl) : - focusedTab->AddVerticalSplit(realGuid, newControl); + return splitType == Pane::SplitState::Horizontal ? focusedTab->AddHorizontalSplit(realGuid, newControl) : + focusedTab->AddVerticalSplit(realGuid, newControl); } // Method Description: diff --git a/src/cascadia/TerminalApp/App.h b/src/cascadia/TerminalApp/App.h index 49b63df42c5..2ccb3530eab 100644 --- a/src/cascadia/TerminalApp/App.h +++ b/src/cascadia/TerminalApp/App.h @@ -91,7 +91,7 @@ namespace winrt::TerminalApp::implementation void _UpdateTabView(); void _UpdateTabIcon(std::shared_ptr tab); - void _CheckTitleUpdate(std::shared_ptr tab); + void _UpdateTitle(std::shared_ptr tab); void _RegisterTerminalEvents(Microsoft::Terminal::TerminalControl::TermControl term, std::shared_ptr hostingTab); @@ -110,7 +110,7 @@ namespace winrt::TerminalApp::implementation void _CopyText(const bool trimTrailingWhitespace); void _SplitVertical(const std::optional& profileGuid); void _SplitHorizontal(const std::optional& profileGuid); - void _SplitPane(const bool splitHorizontal, const std::optional& profileGuid); + void _SplitPane(const Pane::SplitState splitType, const std::optional& profileGuid); // Todo: add more event implementations here // MSFT:20641986: Add keybindings for New Window void _ScrollPage(int delta); diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 9efc4185fc2..fed054ba86f 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -9,7 +9,7 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; -static const int SEPARATOR_SIZE = 4; +static const int PaneSeparatorSize = 4; Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocused) : _control{ control }, @@ -40,12 +40,14 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus // handler to know when that control closed. // Arguments: // - control: The new TermControl to use as the content of our root Grid. +// Return Value: +// - void Pane::_AddControlToRoot(TermControl control) { _root.Children().Append(control.GetControl()); _connectionClosedToken = control.ConnectionClosed([this]() { - if (_control.CloseOnExit()) + if (_control.ShouldCloseOnExit()) { // Fire our Closed event to tell our parent that we should be removed. _closedHandlers(); @@ -56,6 +58,8 @@ void Pane::_AddControlToRoot(TermControl control) // Method Description: // - Get the root UIElement of this pane. There may be a single TermControl as a // child, or an entire tree of grids and panes as children of this element. +// Arguments: +// - // Return Value: // - the Grid acting as the root of this pane. Controls::Grid Pane::GetRootElement() @@ -96,6 +100,8 @@ std::shared_ptr Pane::GetFocusedPane() // there was one). // - This control might not currently be focused, if the tab itself is not // currently focused. +// Arguments: +// - // Return Value: // - nullptr if no children were marked `_lastFocused`, else the TermControl // that was last focused. @@ -109,6 +115,8 @@ TermControl Pane::GetFocusedTerminalControl() // - Returns nullopt if no children of this pane were the last control to be // focused, or the GUID of the profile of the last control to be focused (if // there was one). +// Arguments: +// - // Return Value: // - nullopt if no children of this pane were the last control to be // focused, else the GUID of the profile of the last control to be focused @@ -150,12 +158,12 @@ bool Pane::_IsLeaf() const noexcept // pane's descendants bool Pane::_HasFocusedChild() const noexcept { - const auto controlFocused = _control && - _control.GetControl().FocusState() != FocusState::Unfocused; - const auto firstFocused = _firstChild && _firstChild->_HasFocusedChild(); - const auto secondFocused = _secondChild && _secondChild->_HasFocusedChild(); - - return controlFocused || firstFocused || secondFocused; + // We're intentionally making this one giant expression, so the compiler + // will skip the following lookups if one of the lookups before it returns + // true + return (_control && _control.GetControl().FocusState() != FocusState::Unfocused) || + (_firstChild && _firstChild->_HasFocusedChild()) || + (_secondChild && _secondChild->_HasFocusedChild()); } // Method Description: @@ -163,6 +171,10 @@ bool Pane::_HasFocusedChild() const noexcept // * If this is a leaf node, and our control is actively focused, we'll mark // ourselves as the _lastFocused. // * If we're not a leaf, we'll recurse on our children to check them. +// Arguments: +// - +// Return Value: +// - void Pane::UpdateFocus() { if (_IsLeaf()) @@ -183,6 +195,10 @@ void Pane::UpdateFocus() // Method Description: // - Focuses this control if we're a leaf, or attempts to focus the first leaf // of our first child, recursively. +// Arguments: +// - +// Return Value: +// - void Pane::_FocusFirstChild() { if (_IsLeaf()) @@ -203,6 +219,8 @@ void Pane::_FocusFirstChild() // Arguments: // - settings: The new TerminalSettings to apply to any matching controls // - profile: The GUID of the profile these settings should apply to. +// Return Value: +// - void Pane::UpdateSettings(const TerminalSettings& settings, const GUID& profile) { if (!_IsLeaf()) @@ -225,6 +243,8 @@ void Pane::UpdateSettings(const TerminalSettings& settings, const GUID& profile) // Arguments: // - closeFirst: if true, the first child should be closed, and the second // should be preserved, and vice-versa for false. +// Return Value: +// - void Pane::_CloseChild(const bool closeFirst) { auto closedChild = closeFirst ? _firstChild : _secondChild; @@ -333,6 +353,10 @@ void Pane::_CloseChild(const bool closeFirst) // Method Description: // - Adds event handlers to our chilcren to handle their close events. +// Arguments: +// - +// Return Value: +// - void Pane::_SetupChildCloseHandlers() { _firstClosedToken = _firstChild->Closed([this](){ @@ -355,6 +379,8 @@ void Pane::_SetupChildCloseHandlers() // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. +// Return Value: +// - void Pane::SplitVertical(const GUID& profile, const TermControl& control) { // If we're not the leaf, recurse into our children to split them. @@ -404,7 +430,7 @@ void Pane::SplitVertical(const GUID& profile, const TermControl& control) // Create the pane separator, and place it in column 1 _separatorRoot = Controls::Grid{}; - _separatorRoot.Width(SEPARATOR_SIZE); + _separatorRoot.Width(PaneSeparatorSize); // NaN is the special value XAML uses for "Auto" sizing. _separatorRoot.Height(NAN); _root.Children().Append(_separatorRoot); @@ -427,6 +453,8 @@ void Pane::SplitVertical(const GUID& profile, const TermControl& control) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. +// Return Value: +// - void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) { if (!_IsLeaf()) @@ -472,7 +500,7 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) Controls::Grid::SetRow(_firstChild->GetRootElement(), 0); _separatorRoot = Controls::Grid{}; - _separatorRoot.Height(SEPARATOR_SIZE); + _separatorRoot.Height(PaneSeparatorSize); // NaN is the special value XAML uses for "Auto" sizing. _separatorRoot.Width(NAN); _root.Children().Append(_separatorRoot); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 15a7c8c3a94..f25721dd04f 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -9,6 +9,8 @@ using namespace winrt::Windows::UI::Core; using namespace winrt::Microsoft::Terminal::Settings; using namespace winrt::Microsoft::Terminal::TerminalControl; +static const int TabViewFontSize = 12; + Tab::Tab(const GUID& profile, const TermControl& control) { _rootPane = std::make_shared(profile, control, true); @@ -23,7 +25,7 @@ Tab::Tab(const GUID& profile, const TermControl& control) void Tab::_MakeTabViewItem() { _tabViewItem = ::winrt::Microsoft::UI::Xaml::Controls::TabViewItem{}; - _tabViewItem.FontSize(12); + _tabViewItem.FontSize(TabViewFontSize); } UIElement Tab::GetRootElement() @@ -37,6 +39,8 @@ UIElement Tab::GetRootElement() // there was one). // - This control might not currently be focused, if the tab itself is not // currently focused. +// Arguments: +// - // Return Value: // - nullptr if no children were marked `_lastFocused`, else the TermControl // that was last focused. @@ -50,6 +54,14 @@ winrt::Microsoft::UI::Xaml::Controls::TabViewItem Tab::GetTabViewItem() return _tabViewItem; } +// Method Description: +// - Returns true if this is the currently focused tab. For any set of tabs, +// there should only be one tab that is marked as focused, though each tab has +// no control over the other tabs in the set. +// Arguments: +// - +// Return Value: +// - true iff this tab is focused. bool Tab::IsFocused() const noexcept { return _focused; @@ -77,6 +89,8 @@ void Tab::SetFocused(const bool focused) // - Returns nullopt if no children of this tab were the last control to be // focused, or the GUID of the profile of the last control to be focused (if // there was one). +// Arguments: +// - // Return Value: // - nullopt if no children of this tab were the last control to be // focused, else the GUID of the profile of the last control to be focused @@ -90,6 +104,8 @@ std::optional Tab::GetFocusedProfile() const noexcept // Arguments: // - settings: The new TerminalSettings to apply to any matching controls // - profile: The GUID of the profile these settings should apply to. +// Return Value: +// - void Tab::UpdateSettings(const TerminalSettings& settings, const GUID& profile) { _rootPane->UpdateSettings(settings, profile); @@ -97,6 +113,10 @@ void Tab::UpdateSettings(const TerminalSettings& settings, const GUID& profile) // Method Description: // - Focus the last focused control in our tree of panes. +// Arguments: +// - +// Return Value: +// - void Tab::_Focus() { _focused = true; @@ -113,6 +133,10 @@ void Tab::_Focus() // under this tab is focused, then it will be marked as the last focused. If // there are no focused panes, then there will not be a last focused control // when this returns. +// Arguments: +// - +// Return Value: +// - void Tab::UpdateFocus() { _rootPane->UpdateFocus(); @@ -121,6 +145,8 @@ void Tab::UpdateFocus() // Method Description: // - Gets the title string of the last focused terminal control in our tree. // Returns the empty string if there is no such control. +// Arguments: +// - // Return Value: // - the title string of the last focused terminal control in our tree. winrt::hstring Tab::GetFocusedTitle() const @@ -133,6 +159,8 @@ winrt::hstring Tab::GetFocusedTitle() const // - Set the text on the TabViewItem for this tab. // Arguments: // - text: The new text string to use as the Header for our TabViewItem +// Return Value: +// - void Tab::SetTabText(const winrt::hstring& text) { _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ @@ -163,6 +191,8 @@ void Tab::Scroll(const int delta) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. +// Return Value: +// - void Tab::AddVerticalSplit(const GUID& profile, TermControl& control) { _rootPane->SplitVertical(profile, control); @@ -174,6 +204,8 @@ void Tab::AddVerticalSplit(const GUID& profile, TermControl& control) // Arguments: // - profile: The profile GUID to associate with the newly created pane. // - control: A TermControl to use in the new pane. +// Return Value: +// - void Tab::AddHorizontalSplit(const GUID& profile, TermControl& control) { _rootPane->SplitHorizontal(profile, control); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index ff42bc6c63e..962ab748fca 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -254,7 +254,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation || imageSource.UriSource().RawUri() != imageUri.RawUri()) { // Note that BitmapImage handles the image load asynchronously, - // which is especially important since the image + // which is especially important since the image // may well be both large and somewhere out on the // internet. Media::Imaging::BitmapImage image(imageUri); @@ -1197,7 +1197,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - Scrolls the viewport of the terminal and updates the scroll bar accordingly // Arguments: // - viewTop: the viewTop to scroll to - // The difference between this function and ScrollViewport is that this one also + // The difference between this function and ScrollViewport is that this one also // updates the _scrollBar after the viewport scroll. The reason _scrollBar is not updated in // ScrollViewport is because ScrollViewport is being called by _ScrollbarChangeHandler void TermControl::KeyboardScrollViewport(int viewTop) @@ -1375,7 +1375,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation (shift ? Settings::KeyModifiers::Shift : Settings::KeyModifiers::None) }; } - bool TermControl::CloseOnExit() const noexcept + // Method Description: + // - Returns true if this control should close when its connection is closed. + // Arguments: + // - + // Return Value: + // - true iff the control should close when the connection is closed. + bool TermControl::ShouldCloseOnExit() const noexcept { return _settings.CloseOnExit(); } diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index d6a387ae701..5abe60e2057 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -41,7 +41,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation hstring Title(); void CopySelectionToClipboard(bool trimTrailingWhitespace); void Close(); - bool CloseOnExit() const noexcept; + bool ShouldCloseOnExit() const noexcept; void ScrollViewport(int viewTop); void KeyboardScrollViewport(int viewTop); diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl index 4f6b808fd46..c098fe3393c 100644 --- a/src/cascadia/TerminalControl/TermControl.idl +++ b/src/cascadia/TerminalControl/TermControl.idl @@ -33,7 +33,7 @@ namespace Microsoft.Terminal.TerminalControl String Title { get; }; void CopySelectionToClipboard(Boolean trimTrailingWhitespace); void Close(); - Boolean CloseOnExit { get; }; + Boolean ShouldCloseOnExit { get; }; void ScrollViewport(Int32 viewTop); void KeyboardScrollViewport(Int32 viewTop); From 9165d0250408d000c821a8b0aceeb21af0f8ade0 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 3 Jun 2019 15:34:04 -0500 Subject: [PATCH 27/33] Lock up the panes when they're getting opened/closed --- src/cascadia/TerminalApp/Pane.cpp | 86 ++++++++++++++++++++++++------- src/cascadia/TerminalApp/Pane.h | 3 ++ 2 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index fed054ba86f..4f7b18131b6 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -46,13 +46,29 @@ void Pane::_AddControlToRoot(TermControl control) { _root.Children().Append(control.GetControl()); - _connectionClosedToken = control.ConnectionClosed([this]() { - if (_control.ShouldCloseOnExit()) - { - // Fire our Closed event to tell our parent that we should be removed. - _closedHandlers(); - } - }); + _connectionClosedToken = control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); +} + +void Pane::_ControlClosedHandler() +{ + std::unique_lock lock{ _createCloseLock }; + // It's possible that this event handler started being executed, then before + // we got the lock, another thread created another child. So our control is + // actually no longer _our_ control, and instead could be a descendant. + // + // When the control's new Pane takes ownership of the control, the new + // parent will register it's own event handler. That event handler will get + // fired after this handler returns, and will properly cleanup state. + if (!_IsLeaf()) + { + return; + } + + if (_control.ShouldCloseOnExit()) + { + // Fire our Closed event to tell our parent that we should be removed. + _closedHandlers(); + } } // Method Description: @@ -247,22 +263,42 @@ void Pane::UpdateSettings(const TerminalSettings& settings, const GUID& profile) // - void Pane::_CloseChild(const bool closeFirst) { + // Lock the create/close lock so that another operation won't concurrently + // modify our tree + std::unique_lock lock{ _createCloseLock }; + + // If we're a leaf, then chances are both our children closed in close + // succession. We waited on the lock while the other child was closed, so + // now we don't have a child to close anymore. Return here. When we moved + // the non-closed child into us, we also set up event handlers that will be + // triggered when we return from this. + if (_IsLeaf()) + { + return; + } + auto closedChild = closeFirst ? _firstChild : _secondChild; auto remainingChild = closeFirst ? _secondChild : _firstChild; // If the only child left is a leaf, that means we're a leaf now. if (remainingChild->_IsLeaf()) { - // Revoke the old event handlers. + // take the control and profile of the pane that _wasn't_ closed. + _control = remainingChild->_control; + _profile = remainingChild->_profile; + + // TODO: Add our new event handler before revoking the old one. + _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); + + // Revoke the old event handlers. Remove both the handlers for the panes + // themselves closing, and remove their handlers for their controls + // closing. At this point, if the remaining child's control is closed, + // they'll trigger only our event handler for the control's close. _firstChild->Closed(_firstClosedToken); _secondChild->Closed(_secondClosedToken); closedChild->_control.ConnectionClosed(closedChild->_connectionClosedToken); remainingChild->_control.ConnectionClosed(remainingChild->_connectionClosedToken); - // take the control and profile of the pane that _wasn't_ closed. - _control = closeFirst ? _secondChild->_control : _firstChild->_control; - _profile = closeFirst ? _secondChild->_profile : _firstChild->_profile; - // If either of our children was focused, we want to take that focus from // them. _lastFocused = _firstChild->_lastFocused || _secondChild->_lastFocused; @@ -294,9 +330,11 @@ void Pane::_CloseChild(const bool closeFirst) } else { - // Revoke the old event handlers. - _firstChild->Closed(_firstClosedToken); - _secondChild->Closed(_secondClosedToken); + // First stash away references to the old panes and their tokens + const auto oldFirstToken = _firstClosedToken; + const auto oldSecondToken = _secondClosedToken; + const auto oldFirst = _firstChild; + const auto oldSecond = _secondClosedToken; // Steal all the state from our child _splitState = remainingChild->_splitState; @@ -304,6 +342,13 @@ void Pane::_CloseChild(const bool closeFirst) _firstChild = remainingChild->_firstChild; _secondChild = remainingChild->_secondChild; + // Set up new close handlers on the children + _SetupChildCloseHandlers(); + + // Revoke the old event handlers. + _firstChild->Closed(_firstClosedToken); + _secondChild->Closed(_secondClosedToken); + // Reset our UI: _root.Children().Clear(); _root.ColumnDefinitions().Clear(); @@ -334,9 +379,6 @@ void Pane::_CloseChild(const bool closeFirst) _root.Children().Append(_separatorRoot); _root.Children().Append(_secondChild->GetRootElement()); - // Set up new close handlers on the children, so thay'll notify us - // instead of their old parent. - _SetupChildCloseHandlers(); // If the closed child was focused, transfer the focus to it's first sibling. if (closedChild->_lastFocused) @@ -352,7 +394,7 @@ void Pane::_CloseChild(const bool closeFirst) } // Method Description: -// - Adds event handlers to our chilcren to handle their close events. +// - Adds event handlers to our children to handle their close events. // Arguments: // - // Return Value: @@ -397,6 +439,9 @@ void Pane::SplitVertical(const GUID& profile, const TermControl& control) return; } + // Lock the create/close lock so that another operation won't concurrently + // modify our tree + std::unique_lock lock{ _createCloseLock }; // revoke our handler - the child will take care of the control now. _control.ConnectionClosed(_connectionClosedToken); @@ -470,6 +515,9 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) return; } + // Lock the create/close lock so that another operation won't concurrently + // modify our tree + std::unique_lock lock{ _createCloseLock }; // revoke our handler - the child will take care of the control now. _control.ConnectionClosed(_connectionClosedToken); diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 09b85c71e49..4d75396b7fd 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -68,6 +68,8 @@ class Pane : public std::enable_shared_from_this winrt::event_token _firstClosedToken{ 0 }; winrt::event_token _secondClosedToken{ 0 }; + std::shared_mutex _createCloseLock{}; + bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); @@ -76,4 +78,5 @@ class Pane : public std::enable_shared_from_this void _CloseChild(const bool closeFirst); void _FocusFirstChild(); + void _ControlClosedHandler(); }; From 7f50a0fcf1b104df1050ed0bb025f09ef769b470 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 3 Jun 2019 15:37:25 -0500 Subject: [PATCH 28/33] A little bit of cleanup on the comments here --- src/cascadia/TerminalApp/Pane.cpp | 23 ++++++++++------------- src/cascadia/TerminalApp/Pane.h | 1 - 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 4f7b18131b6..8b066e14e4a 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -16,7 +16,8 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus _lastFocused{ lastFocused }, _profile{ profile } { - _AddControlToRoot(_control); + _root.Children().Append(_control.GetControl()); + _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); // Set the background of the pane to match that of the theme's default grid // background. This way, we'll match the small underline under the tabs, and @@ -36,19 +37,15 @@ Pane::Pane(const GUID& profile, const TermControl& control, const bool lastFocus } // Method Description: -// - Adds a given Terminal Control to our root Grid. Also registers an event -// handler to know when that control closed. +// - Called when our attached control is closed. Triggers listeners to our close +// event, if we're a leaf pane. +// - If this was called, and we became a parent pane (due to work on another +// thread), this function will do nothing (allowing the control's new parent +// to handle the event instead). // Arguments: -// - control: The new TermControl to use as the content of our root Grid. +// - // Return Value: // - -void Pane::_AddControlToRoot(TermControl control) -{ - _root.Children().Append(control.GetControl()); - - _connectionClosedToken = control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); -} - void Pane::_ControlClosedHandler() { std::unique_lock lock{ _createCloseLock }; @@ -287,7 +284,7 @@ void Pane::_CloseChild(const bool closeFirst) _control = remainingChild->_control; _profile = remainingChild->_profile; - // TODO: Add our new event handler before revoking the old one. + // Add our new event handler before revoking the old one. _connectionClosedToken = _control.ConnectionClosed({ this, &Pane::_ControlClosedHandler }); // Revoke the old event handlers. Remove both the handlers for the panes @@ -315,7 +312,7 @@ void Pane::_CloseChild(const bool closeFirst) _separatorRoot = { nullptr }; // Reattach the TermControl to our grid. - _AddControlToRoot(_control); + _root.Children().Append(_control.GetControl()); if (_lastFocused) { diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 4d75396b7fd..9778b61dd7c 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -73,7 +73,6 @@ class Pane : public std::enable_shared_from_this bool _IsLeaf() const noexcept; bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); - void _AddControlToRoot(winrt::Microsoft::Terminal::TerminalControl::TermControl control); void _CloseChild(const bool closeFirst); From b5d954ed4a18cfe9c1399c4e291c05f8008102e3 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 4 Jun 2019 09:16:19 -0500 Subject: [PATCH 29/33] Refactor the code for actually doing a split It's much easier to read as a few more functions --- src/cascadia/TerminalApp/Pane.cpp | 159 +++++++++++++++++------------- src/cascadia/TerminalApp/Pane.h | 4 + 2 files changed, 95 insertions(+), 68 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 8b066e14e4a..1c24f6f2ab0 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -411,6 +411,74 @@ void Pane::_SetupChildCloseHandlers() }); } +// 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() +{ + if (_splitState == SplitState::Vertical) + { + // Create three columns in this grid: one for each pane, and one for the separator. + auto separatorColDef = Controls::ColumnDefinition(); + separatorColDef.Width(GridLengthHelper::Auto()); + + _root.ColumnDefinitions().Append(Controls::ColumnDefinition{}); + _root.ColumnDefinitions().Append(separatorColDef); + _root.ColumnDefinitions().Append(Controls::ColumnDefinition{}); + + // Create the pane separator + _separatorRoot = Controls::Grid{}; + _separatorRoot.Width(PaneSeparatorSize); + // NaN is the special value XAML uses for "Auto" sizing. + _separatorRoot.Height(NAN); + } + else if (_splitState == SplitState::Horizontal) + { + // Create three rows in this grid: one for each pane, and one for the separator. + auto separatorRowDef = Controls::RowDefinition(); + separatorRowDef.Height(GridLengthHelper::Auto()); + + _root.RowDefinitions().Append(Controls::RowDefinition{}); + _root.RowDefinitions().Append(separatorRowDef); + _root.RowDefinitions().Append(Controls::RowDefinition{}); + + // Create the pane separator + _separatorRoot = Controls::Grid{}; + _separatorRoot.Height(PaneSeparatorSize); + // NaN is the special value XAML uses for "Auto" sizing. + _separatorRoot.Width(NAN); + } +} + +// Method Description: +// - Sets the row/column of our child UI elements, to match our current split type. +// Arguments: +// - +// Return Value: +// - +void Pane::_ApplySplitDefinitions() +{ + if (_splitState == SplitState::Vertical) + { + Controls::Grid::SetColumn(_firstChild->GetRootElement(), 0); + Controls::Grid::SetColumn(_separatorRoot, 1); + Controls::Grid::SetColumn(_secondChild->GetRootElement(), 2); + + } + else if (_splitState == SplitState::Horizontal) + { + Controls::Grid::SetRow(_firstChild->GetRootElement(), 0); + Controls::Grid::SetRow(_separatorRoot, 1); + Controls::Grid::SetRow(_secondChild->GetRootElement(), 2); + } +} + + // Method Description: // - Vertically split the focused pane in our tree of panes, and place the given // TermControl into the newly created pane. If we're the focused pane, then @@ -436,56 +504,8 @@ void Pane::SplitVertical(const GUID& profile, const TermControl& control) return; } - // Lock the create/close lock so that another operation won't concurrently - // modify our tree - std::unique_lock lock{ _createCloseLock }; - // revoke our handler - the child will take care of the control now. - _control.ConnectionClosed(_connectionClosedToken); - _connectionClosedToken.value = 0; - - _splitState = SplitState::Vertical; - - // Create three columns in this grid: one for each pane, and one for the separator. - auto separatorColDef = Controls::ColumnDefinition(); - separatorColDef.Width(GridLengthHelper::Auto()); - - _root.ColumnDefinitions().Append(Controls::ColumnDefinition{}); - _root.ColumnDefinitions().Append(separatorColDef); - _root.ColumnDefinitions().Append(Controls::ColumnDefinition{}); - - // Remove any children we currently have. We can't add the existing - // TermControl to a new grid until we do this. - _root.Children().Clear(); - - // Create two new Panes - // Move our control, guid into the first one. - // Move the new guid, control into the second. - _firstChild = std::make_shared(_profile.value(), _control); - _profile = std::nullopt; - _control = { nullptr }; - _secondChild = std::make_shared(profile, control); - - // add the first pane to row 0 - _root.Children().Append(_firstChild->GetRootElement()); - Controls::Grid::SetColumn(_firstChild->GetRootElement(), 0); - - // Create the pane separator, and place it in column 1 - _separatorRoot = Controls::Grid{}; - _separatorRoot.Width(PaneSeparatorSize); - // NaN is the special value XAML uses for "Auto" sizing. - _separatorRoot.Height(NAN); - _root.Children().Append(_separatorRoot); - Controls::Grid::SetColumn(_separatorRoot, 1); - - // add the second pane to column 2 - _root.Children().Append(_secondChild->GetRootElement()); - Controls::Grid::SetColumn(_secondChild->GetRootElement(), 2); - - // Register event handlers on our children to handle their Close events - _SetupChildCloseHandlers(); - - _lastFocused = false; + _DoSplit(SplitState::Vertical, profile, control); } // Method Description: @@ -512,6 +532,21 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) return; } + + _DoSplit(SplitState::Horizontal, profile, control); +} + +// Method Description: +// - Does the bulk of the work of creating a new split. Initializes our UI, +// creates a new Pane to host the control, registers event handlers. +// Arguments: +// - splitType: what type of split we should create. +// - profile: The profile GUID to associate with the newly created pane. +// - control: A TermControl to use in the new pane. +// Return Value: +// - +void Pane::_DoSplit(SplitState splitType, const GUID& profile, const TermControl& control) +{ // Lock the create/close lock so that another operation won't concurrently // modify our tree std::unique_lock lock{ _createCloseLock }; @@ -520,16 +555,12 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) _control.ConnectionClosed(_connectionClosedToken); _connectionClosedToken.value = 0; - _splitState = SplitState::Horizontal; - - // Create three rows in this grid: one for each pane, and one for the separator. - auto separatorRowDef = Controls::RowDefinition(); - separatorRowDef.Height(GridLengthHelper::Auto()); + _splitState = splitType; - _root.RowDefinitions().Append(Controls::RowDefinition{}); - _root.RowDefinitions().Append(separatorRowDef); - _root.RowDefinitions().Append(Controls::RowDefinition{}); + _CreateSplitContent(); + // Remove any children we currently have. We can't add the existing + // TermControl to a new grid until we do this. _root.Children().Clear(); // Create two new Panes @@ -540,21 +571,13 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control) _control = { nullptr }; _secondChild = std::make_shared(profile, control); - // add the first pane to row 0 _root.Children().Append(_firstChild->GetRootElement()); - Controls::Grid::SetRow(_firstChild->GetRootElement(), 0); - - _separatorRoot = Controls::Grid{}; - _separatorRoot.Height(PaneSeparatorSize); - // NaN is the special value XAML uses for "Auto" sizing. - _separatorRoot.Width(NAN); _root.Children().Append(_separatorRoot); - Controls::Grid::SetRow(_separatorRoot, 1); - - // add the second pane to row 1 _root.Children().Append(_secondChild->GetRootElement()); - Controls::Grid::SetRow(_secondChild->GetRootElement(), 2); + _ApplySplitDefinitions(); + + // Register event handlers on our children to handle their Close events _SetupChildCloseHandlers(); _lastFocused = false; diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 9778b61dd7c..737b9f0b8e2 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -74,6 +74,10 @@ class Pane : public std::enable_shared_from_this bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); + void _DoSplit(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void _CreateSplitContent(); + void _ApplySplitDefinitions(); + void _CloseChild(const bool closeFirst); void _FocusFirstChild(); From c6311f7522811f87b069a03b6bde7222cacc0115 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 7 Jun 2019 08:59:04 -0500 Subject: [PATCH 30/33] Clean up some typos --- doc/cascadia/Panes.md | 12 ++++++------ src/cascadia/TerminalApp/App.cpp | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/cascadia/Panes.md b/doc/cascadia/Panes.md index 83d20fa4272..6dd430748cb 100644 --- a/doc/cascadia/Panes.md +++ b/doc/cascadia/Panes.md @@ -48,7 +48,7 @@ pane, with it's own terminal control that it displays, or it could be a parent pane, where it has two children, each with their own terminal control. When a pane is a parent, its two children are either split vertically or -horizontally. Parent nodes don't have a terminal of their own, the merely +horizontally. Parent nodes don't have a terminal of their own, they merely display the terminals of their children. * If a Pane is split vertically, the two panes are seperated by a vertical @@ -150,11 +150,11 @@ default settings profile. When a tab has multiple panes open, only one is the "active" pane. This is the pane that was last focused in the tab. If the tab is the currently open tab, -then this is the pane with the currectly focused terminal control. When the user +then this is the pane with the currently focused terminal control. When the user brings the tab into focus, the last focused pane is the pane that should become focused again. -The Tab's state will be updated to reflect the state of it's focused pane. The +The tab's state will be updated to reflect the state of it's focused pane. The title text and icon of the tab will reflect that of the focused pane. Should the focus switch from one pane to another, the tab's text and icon should update to reflect the newly focused control. Any additional state that the tab would @@ -167,8 +167,8 @@ match. ### Closing a pane -A pane can either be close by the user manually, or when the terminal it's -attached to raises it's ConnectionClosed event. When this happens, we should +A pane can either be closed by the user manually, or when the terminal it's +attached to raises its ConnectionClosed event. When this happens, we should remove this pane from the tree. The parent of the closing pane will have to remove the pane as one of it's children. If the sibling of the closing pane is a leaf, then the parent should just take all of the state from the remaining pane. @@ -207,7 +207,7 @@ no means a comprehensive list. ## Footnotes -### Why not top-Level Panes, and nested tabs? +### Why not top-level panes, and nested tabs? If each pane were to have it's own set of tabs, then each pane would need to reserve screen real estate for a row of tabs. As a user continued to split the diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 465254c914d..19e0714a489 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -604,7 +604,7 @@ namespace winrt::TerminalApp::implementation } // Method Description: - // - Get the icon of the currently focused terminal control, and set it's + // - Get the icon of the currently focused terminal control, and set its // tab's icon to that icon. // Arguments: // - tab: the Tab to update the title for. @@ -770,7 +770,7 @@ namespace winrt::TerminalApp::implementation // * the Copy and Paste events, for setting and retrieving clipboard data // on the right thread // * the TitleChanged event, for changing the text of the tab - // * the GotFocus event, for chnging the title/icon in the tab when a new + // * the GotFocus event, for changing the title/icon in the tab when a new // control is focused // Arguments: // - term: The newly created TermControl to connect the events for @@ -831,7 +831,7 @@ namespace winrt::TerminalApp::implementation // Add the new tab to the list of our tabs. auto newTab = _tabs.emplace_back(std::make_shared(profileGuid, term)); - // Hookp our event handlers to the new terminal + // Hookup our event handlers to the new terminal _RegisterTerminalEvents(term, newTab); auto tabViewItem = newTab->GetTabViewItem(); @@ -1156,7 +1156,7 @@ namespace winrt::TerminalApp::implementation // this is nullopt, use the default profile. void App::_SplitPane(const Pane::SplitState splitType, const std::optional& profileGuid) { - // No nothing if we're requesting no split. + // Do nothing if we're requesting no split. if (splitType == Pane::SplitState::None) { return; @@ -1170,7 +1170,7 @@ namespace winrt::TerminalApp::implementation const int focusedTabIndex = _GetFocusedTabIndex(); auto focusedTab = _tabs[focusedTabIndex]; - // Hookp our event handlers to the new terminal + // Hookup our event handlers to the new terminal _RegisterTerminalEvents(newControl, focusedTab); return splitType == Pane::SplitState::Horizontal ? focusedTab->AddHorizontalSplit(realGuid, newControl) : From c2653173e8860904d5c4660ac70e43c06a80421b Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 7 Jun 2019 11:02:18 -0700 Subject: [PATCH 31/33] Fixed build error --- src/cascadia/TerminalApp/AppKeyBindings.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cascadia/TerminalApp/AppKeyBindings.cpp b/src/cascadia/TerminalApp/AppKeyBindings.cpp index a2af8bcfd88..91c459d9128 100644 --- a/src/cascadia/TerminalApp/AppKeyBindings.cpp +++ b/src/cascadia/TerminalApp/AppKeyBindings.cpp @@ -218,6 +218,8 @@ namespace winrt::TerminalApp::implementation DEFINE_EVENT(AppKeyBindings, SwitchToTab, _SwitchToTabHandlers, TerminalApp::SwitchToTabEventArgs); DEFINE_EVENT(AppKeyBindings, NextTab, _NextTabHandlers, TerminalApp::NextTabEventArgs); DEFINE_EVENT(AppKeyBindings, PrevTab, _PrevTabHandlers, TerminalApp::PrevTabEventArgs); + DEFINE_EVENT(AppKeyBindings, SplitVertical, _SplitVerticalHandlers, TerminalApp::SplitVerticalEventArgs); + DEFINE_EVENT(AppKeyBindings, SplitHorizontal, _SplitHorizontalHandlers, TerminalApp::SplitHorizontalEventArgs); DEFINE_EVENT(AppKeyBindings, IncreaseFontSize, _IncreaseFontSizeHandlers, TerminalApp::IncreaseFontSizeEventArgs); DEFINE_EVENT(AppKeyBindings, DecreaseFontSize, _DecreaseFontSizeHandlers, TerminalApp::DecreaseFontSizeEventArgs); DEFINE_EVENT(AppKeyBindings, ScrollUp, _ScrollUpHandlers, TerminalApp::ScrollUpEventArgs); From 14f3ffa56d25bbacd95e1e060a22e3b76f6a0f1f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 7 Jun 2019 15:49:37 -0500 Subject: [PATCH 32/33] Apply suggestions from code review Don't capture a dead reference Co-Authored-By: Dustin L. Howett (MSFT) --- doc/cascadia/Panes.md | 2 +- src/cascadia/TerminalApp/Tab.cpp | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/cascadia/Panes.md b/doc/cascadia/Panes.md index 6dd430748cb..e165c208ab4 100644 --- a/doc/cascadia/Panes.md +++ b/doc/cascadia/Panes.md @@ -139,7 +139,7 @@ focused (they don't have their own terminal). When a user decides to add a new pane, the child will: 1. Convert into a parent - 2. Move its terminal into it's first child + 2. Move its terminal into its first child 3. Split its UI in half, and display each child in one half. It's up to the app hosting the panes to tell the pane what kind of terminal in diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index f25721dd04f..0878e83e916 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -163,8 +163,10 @@ winrt::hstring Tab::GetFocusedTitle() const // - void Tab::SetTabText(const winrt::hstring& text) { - _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [=](){ - _tabViewItem.Header(text); + // Copy the hstring, so we don't capture a dead reference + winrt::hstring textCopy{ text }; + _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [textCopy](){ + _tabViewItem.Header(textCopy); }); } From f945c18ae2d9998be57959b9159ae6f088740815 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 7 Jun 2019 16:21:30 -0500 Subject: [PATCH 33/33] Apply suggestions from code review Let's try and write code in github --- src/cascadia/TerminalApp/Tab.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index 0878e83e916..cd239337f82 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -165,8 +165,8 @@ void Tab::SetTabText(const winrt::hstring& text) { // Copy the hstring, so we don't capture a dead reference winrt::hstring textCopy{ text }; - _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [textCopy](){ - _tabViewItem.Header(textCopy); + _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [text = std::move(textCopy), this](){ + _tabViewItem.Header(text); }); }