maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Simplify TerminalPage by introducing _GetFocusedTabImpl (#8655)

Browse source 
A part of the https://github.com/microsoft/terminal/issues/8415.
Very technical commit to simplify the terminal page code
towards additional steps of simplifying tab management.
No business logic should change.

A firs step in splitting https://github.com/microsoft/terminal/pull/8427
This commit is contained in:
Don-Vito 2021-01-04 21:32:53 +02:00 committed by GitHub
parent 5220738d8e
commit 9b07cb8c71
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 117 additions and 165 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
src/cascadia/TerminalApp/AppActionHandlers.cpp
View file

@ -134,23 +134,20 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleTogglePaneZoom(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (auto focusedTab = _GetFocusedTab())
if (const auto activeTab{ _GetFocusedTabImpl() })
{
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
// Don't do anything if there's only one pane. It's already zoomed.
if (activeTab->GetLeafPaneCount() > 1)
{
// Don't do anything if there's only one pane. It's already zoomed.
if (activeTab && activeTab->GetLeafPaneCount() > 1)
{
// First thing's first, remove the current content from the UI
// tree. This is important, because we might be leaving zoom, and if
// a pane is zoomed, then it's currently in the UI tree, and should
// be removed before it's re-added in Pane::Restore
_tabContent.Children().Clear();
// First thing's first, remove the current content from the UI
// tree. This is important, because we might be leaving zoom, and if
// a pane is zoomed, then it's currently in the UI tree, and should
// be removed before it's re-added in Pane::Restore
_tabContent.Children().Clear();
// Togging the zoom on the tab will cause the tab to inform us of
// the new root Content for this tab.
activeTab->ToggleZoom();
}
// Togging the zoom on the tab will cause the tab to inform us of
// the new root Content for this tab.
activeTab->ToggleZoom();
}
}
@ -347,19 +344,16 @@ namespace winrt::TerminalApp::implementation
args.Handled(false);
if (const auto& realArgs = args.ActionArgs().try_as<SetColorSchemeArgs>())
{
if (auto focusedTab = _GetFocusedTab())
if (const auto activeTab{ _GetFocusedTabImpl() })
{
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
if (auto activeControl = activeTab->GetActiveTerminalControl())
{
if (auto activeControl = activeTab->GetActiveTerminalControl())
if (const auto scheme = _settings.GlobalSettings().ColorSchemes().TryLookup(realArgs.SchemeName()))
{
if (const auto scheme = _settings.GlobalSettings().ColorSchemes().TryLookup(realArgs.SchemeName()))
{
auto controlSettings = activeControl.Settings().as<TerminalSettings>();
controlSettings->ApplyColorScheme(scheme);
activeControl.UpdateSettings(*controlSettings);
args.Handled(true);
}
auto controlSettings = activeControl.Settings().as<TerminalSettings>();
controlSettings->ApplyColorScheme(scheme);
activeControl.UpdateSettings(*controlSettings);
args.Handled(true);
}
}
}
@ -376,18 +370,15 @@ namespace winrt::TerminalApp::implementation
tabColor = realArgs.TabColor();
}
if (auto focusedTab = _GetFocusedTab())
if (const auto activeTab{ _GetFocusedTabImpl() })
{
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
if (tabColor)
{
if (tabColor)
{
activeTab->SetRuntimeTabColor(tabColor.Value());
}
else
{
activeTab->ResetRuntimeTabColor();
}
activeTab->SetRuntimeTabColor(tabColor.Value());
}
else
{
activeTab->ResetRuntimeTabColor();
}
}
args.Handled(true);
@ -396,12 +387,9 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleOpenTabColorPicker(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (auto focusedTab = _GetFocusedTab())
if (const auto activeTab{ _GetFocusedTabImpl() })
{
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
{
activeTab->ActivateColorPicker();
}
activeTab->ActivateColorPicker();
}
args.Handled(true);
}
@ -416,18 +404,15 @@ namespace winrt::TerminalApp::implementation
title = realArgs.Title();
}
if (auto focusedTab = _GetFocusedTab())
if (const auto activeTab{ _GetFocusedTabImpl() })
{
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
if (title.has_value())
{
if (title.has_value())
{
activeTab->SetTabText(title.value());
}
else
{
activeTab->ResetTabText();
}
activeTab->SetTabText(title.value());
}
else
{
activeTab->ResetTabText();
}
}
args.Handled(true);
@ -436,12 +421,9 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleOpenTabRenamer(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (auto focusedTab = _GetFocusedTab())
if (const auto activeTab{ _GetFocusedTabImpl() })
{
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
{
activeTab->ActivateTabRenamer();
}
activeTab->ActivateTabRenamer();
}
args.Handled(true);
}

183
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -1095,34 +1095,31 @@ namespace winrt::TerminalApp::implementation
// - Duplicates the current focused tab
void TerminalPage::_DuplicateTabViewItem()
{
if (auto index{ _GetFocusedTabIndex() })
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*index)))
try
{
try
{
// TODO: GH#5047 - In the future, we should get the Profile of
// the focused pane, and use that to build a new instance of the
// settings so we can duplicate this tab/pane.
//
// Currently, if the profile doesn't exist anymore in our
// settings, we'll silently do nothing.
//
// In the future, it will be preferable to just duplicate the
// current control's settings, but we can't do that currently,
// because we won't be able to create a new instance of the
// connection without keeping an instance of the original Profile
// object around.
// TODO: GH#5047 - In the future, we should get the Profile of
// the focused pane, and use that to build a new instance of the
// settings so we can duplicate this tab/pane.
//
// Currently, if the profile doesn't exist anymore in our
// settings, we'll silently do nothing.
//
// In the future, it will be preferable to just duplicate the
// current control's settings, but we can't do that currently,
// because we won't be able to create a new instance of the
// connection without keeping an instance of the original Profile
// object around.
const auto& profileGuid = terminalTab->GetFocusedProfile();
if (profileGuid.has_value())
{
const auto settings{ winrt::make<TerminalSettings>(_settings, profileGuid.value(), *_bindings) };
_CreateNewTabFromSettings(profileGuid.value(), settings);
}
const auto& profileGuid = terminalTab->GetFocusedProfile();
if (profileGuid.has_value())
{
const auto settings{ winrt::make<TerminalSettings>(_settings, profileGuid.value(), *_bindings) };
_CreateNewTabFromSettings(profileGuid.value(), settings);
}
CATCH_LOG();
}
CATCH_LOG();
}
}
@ -1418,20 +1415,17 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalPage::_UnZoomIfNeeded()
{
if (auto focusedTab = _GetFocusedTab())
if (const auto activeTab{ _GetFocusedTabImpl() })
{
if (auto activeTab = _GetTerminalTabImpl(focusedTab))
if (activeTab->IsZoomed())
{
if (activeTab->IsZoomed())
{
// Remove the content from the tab first, so Pane::UnZoom can
// re-attach the content to the tree w/in the pane
_tabContent.Children().Clear();
// In ExitZoom, we'll change the Tab's Content(), triggering the
// content changed event, which will re-attach the tab's new content
// root to the tree.
activeTab->ExitZoom();
}
// Remove the content from the tab first, so Pane::UnZoom can
// re-attach the content to the tree w/in the pane
_tabContent.Children().Clear();
// In ExitZoom, we'll change the Tab's Content(), triggering the
// content changed event, which will re-attach the tab's new content
// root to the tree.
activeTab->ExitZoom();
}
}
}
@ -1446,24 +1440,18 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalPage::_MoveFocus(const FocusDirection& direction)
{
if (auto index{ _GetFocusedTabIndex() })
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*index)))
{
_UnZoomIfNeeded();
terminalTab->NavigateFocus(direction);
}
_UnZoomIfNeeded();
terminalTab->NavigateFocus(direction);
}
}
TermControl TerminalPage::_GetActiveControl()
{
if (auto index{ _GetFocusedTabIndex() })
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*index)))
{
return terminalTab->GetActiveTerminalControl();
}
return terminalTab->GetActiveTerminalControl();
}
return nullptr;
}
@ -1488,7 +1476,7 @@ namespace winrt::TerminalApp::implementation
// Method Description:
// - returns a com_ptr to the currently focused tab. This might return null,
// so make sure to check the result!
winrt::TerminalApp::TabBase TerminalPage::_GetFocusedTab()
winrt::TerminalApp::TabBase TerminalPage::_GetFocusedTab() const noexcept
{
if (auto index{ _GetFocusedTabIndex() })
{
@ -1497,6 +1485,18 @@ namespace winrt::TerminalApp::implementation
return nullptr;
}
// Method Description:
// - returns a com_ptr to the currently focused tab implementation. This might return null,
// so make sure to check the result!
winrt::com_ptr<TerminalTab> TerminalPage::_GetFocusedTabImpl() const noexcept
{
if (auto tab{ _GetFocusedTab() })
{
return _GetTerminalTabImpl(tab);
}
return nullptr;
}
// Method Description:
// - An async method for changing the focused tab on the UI thread. This
// method will _only_ set the selected item of the TabView, which will
@ -1538,13 +1538,10 @@ namespace winrt::TerminalApp::implementation
// tab's Closed event.
void TerminalPage::_CloseFocusedPane()
{
if (auto index{ _GetFocusedTabIndex() })
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*index)))
{
_UnZoomIfNeeded();
terminalTab->ClosePane();
}
_UnZoomIfNeeded();
terminalTab->ClosePane();
}
}
@ -1587,26 +1584,23 @@ namespace winrt::TerminalApp::implementation
// - rowsToScroll: a number of lines to move the viewport. If not provided we will use a system default.
void TerminalPage::_Scroll(ScrollDirection scrollDirection, const Windows::Foundation::IReference<uint32_t>& rowsToScroll)
{
if (auto index{ _GetFocusedTabIndex() })
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*index)))
uint32_t realRowsToScroll;
if (rowsToScroll == nullptr)
{
uint32_t realRowsToScroll;
if (rowsToScroll == nullptr)
{
// The magic value of WHEEL_PAGESCROLL indicates that we need to scroll the entire page
realRowsToScroll = _systemRowsToScroll == WHEEL_PAGESCROLL ?
terminalTab->GetActiveTerminalControl().GetViewHeight() :
_systemRowsToScroll;
}
else
{
// use the custom value specified in the command
realRowsToScroll = rowsToScroll.Value();
}
auto scrollDelta = _ComputeScrollDelta(scrollDirection, realRowsToScroll);
terminalTab->Scroll(scrollDelta);
// The magic value of WHEEL_PAGESCROLL indicates that we need to scroll the entire page
realRowsToScroll = _systemRowsToScroll == WHEEL_PAGESCROLL ?
terminalTab->GetActiveTerminalControl().GetViewHeight() :
_systemRowsToScroll;
}
else
{
// use the custom value specified in the command
realRowsToScroll = rowsToScroll.Value();
}
auto scrollDelta = _ComputeScrollDelta(scrollDirection, realRowsToScroll);
terminalTab->Scroll(scrollDelta);
}
}
@ -1632,17 +1626,9 @@ namespace winrt::TerminalApp::implementation
return;
}
auto indexOpt = _GetFocusedTabIndex();
const auto focusedTab{ _GetFocusedTabImpl() };
// Do nothing if for some reason, there's no tab in focus. We don't want to crash.
if (!indexOpt)
{
return;
}
auto focusedTab = _GetTerminalTabImpl(_tabs.GetAt(*indexOpt));
// Do nothing if the focused tab isn't a TerminalTab
// Do nothing if no TerminalTab is focused
if (!focusedTab)
{
return;
@ -1721,13 +1707,10 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalPage::_ResizePane(const ResizeDirection& direction)
{
if (auto index{ _GetFocusedTabIndex() })
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*index)))
{
_UnZoomIfNeeded();
terminalTab->ResizePane(direction);
}
_UnZoomIfNeeded();
terminalTab->ResizePane(direction);
}
}
@ -1738,14 +1721,8 @@ namespace winrt::TerminalApp::implementation
// - scrollDirection: ScrollUp will move the viewport up, ScrollDown will move the viewport down
void TerminalPage::_ScrollPage(ScrollDirection scrollDirection)
{
auto indexOpt = _GetFocusedTabIndex();
// Do nothing if for some reason, there's no tab in focus. We don't want to crash.
if (!indexOpt)
{
return;
}
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*indexOpt)))
// Do nothing if for some reason, there's no terminal tab in focus. We don't want to crash.
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
const auto control = _GetActiveControl();
const auto termHeight = control.GetViewHeight();
@ -1756,13 +1733,10 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_ScrollToBufferEdge(ScrollDirection scrollDirection)
{
if (const auto indexOpt = _GetFocusedTabIndex())
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*indexOpt)))
{
auto scrollDelta = _ComputeScrollDelta(scrollDirection, INT_MAX);
terminalTab->Scroll(scrollDelta);
}
auto scrollDelta = _ComputeScrollDelta(scrollDirection, INT_MAX);
terminalTab->Scroll(scrollDelta);
}
}
@ -1872,12 +1846,9 @@ namespace winrt::TerminalApp::implementation
{
if (_settings && _settings.GlobalSettings().SnapToGridOnResize())
{
if (auto index{ _GetFocusedTabIndex() })
if (const auto terminalTab{ _GetFocusedTabImpl() })
{
if (auto terminalTab = _GetTerminalTabImpl(_tabs.GetAt(*index)))
{
return terminalTab->CalcSnappedDimension(widthOrHeight, dimension);
}
return terminalTab->CalcSnappedDimension(widthOrHeight, dimension);
}
}
return dimension;
@ -2883,7 +2854,7 @@ namespace winrt::TerminalApp::implementation
// Return Value:
// - If the tab is a TerminalTab, a com_ptr to the implementation type.
// If the tab is not a TerminalTab, nullptr
winrt::com_ptr<TerminalTab> TerminalPage::_GetTerminalTabImpl(const TerminalApp::TabBase& tab) const
winrt::com_ptr<TerminalTab> TerminalPage::_GetTerminalTabImpl(const TerminalApp::TabBase& tab)
{
if (auto terminalTab = tab.try_as<TerminalApp::TerminalTab>())
{

9
src/cascadia/TerminalApp/TerminalPage.h
View file

@ -111,7 +111,7 @@ namespace winrt::TerminalApp::implementation
Windows::Foundation::Collections::IObservableVector<TerminalApp::TabBase> _tabs;
Windows::Foundation::Collections::IVector<TerminalApp::TabBase> _mruTabs;
winrt::com_ptr<TerminalTab> _GetTerminalTabImpl(const TerminalApp::TabBase& tab) const;
static winrt::com_ptr<TerminalTab> _GetTerminalTabImpl(const TerminalApp::TabBase& tab);
void _UpdateTabIndices();
@ -183,7 +183,9 @@ namespace winrt::TerminalApp::implementation
winrt::Microsoft::Terminal::TerminalControl::TermControl _GetActiveControl();
std::optional<uint32_t> _GetFocusedTabIndex() const noexcept;
TerminalApp::TabBase _GetFocusedTab();
TerminalApp::TabBase _GetFocusedTab() const noexcept;
winrt::com_ptr<TerminalTab> _GetFocusedTabImpl() const noexcept;
winrt::fire_and_forget _SetFocusedTabIndex(const uint32_t tabIndex);
void _CloseFocusedTab();
void _CloseFocusedPane();
@ -251,12 +253,9 @@ namespace winrt::TerminalApp::implementation
void _OpenSettingsUI();
void _MakeSwitchToTabCommand(const TerminalApp::TabBase& tab, const uint32_t index);
static int _ComputeScrollDelta(ScrollDirection scrollDirection, const uint32_t rowsToScroll);
static uint32_t _ReadSystemRowsToScroll();
void _UpdateTabSwitcherCommands(const bool mru);
void _UpdateMRUTab(const uint32_t index);
void _TryMoveTab(const uint32_t currentTabIndex, const int32_t suggestedNewTabIndex);