this works better than it has any right to

zadjii-msft · zadjii-msft · commit 29d0d576566c · 2023-07-27T16:29:26.000-05:00
diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -6,7 +6,6 @@
 
 #include "TerminalPage.h"
 #include "ScratchpadContent.h"
-#include "SettingsPaneContent.h"
 #include "../WinRTUtils/inc/WtExeUtils.h"
 #include "../../types/inc/utils.hpp"
 #include "Utils.h"
@@ -1323,8 +1322,8 @@ namespace winrt::TerminalApp::implementation
     {
         if (Feature_ScratchpadPane::IsEnabled())
         {
-            // auto scratchPane{ winrt::make_self<ScratchpadContent>() };
-            auto scratchPane{ winrt::make_self<SettingsPaneContent>(_settings) };
+            auto scratchPane{ winrt::make_self<ScratchpadContent>() };
+            // auto scratchPane{ winrt::make_self<SettingsPaneContent>(_settings) };
 
             // This is maybe a little wacky - add our key event handler to the pane
             // we made. So that we can get actions for keys that the content didn't
diff --git a/src/cascadia/TerminalApp/SettingsPaneContent.cpp b/src/cascadia/TerminalApp/SettingsPaneContent.cpp
@@ -53,6 +53,7 @@ namespace winrt::TerminalApp::implementation
 
     NewTerminalArgs SettingsPaneContent::GetNewTerminalArgs(const bool /* asContent */) const
     {
+        // TODO! hey, can we somehow replicate std::vector<ActionAndArgs> SettingsTab::BuildStartupActions?
         return nullptr;
     }
 }
diff --git a/src/cascadia/TerminalApp/SettingsPaneContent.h b/src/cascadia/TerminalApp/SettingsPaneContent.h
@@ -11,13 +11,14 @@ namespace winrt::TerminalApp::implementation
         SettingsPaneContent(winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings settings);
 
         winrt::Windows::UI::Xaml::FrameworkElement GetRoot();
+        winrt::Microsoft::Terminal::Settings::Editor::MainPage SettingsUI() { return _sui; }
 
         winrt::Windows::Foundation::Size MinSize();
         void Focus(winrt::Windows::UI::Xaml::FocusState reason = winrt::Windows::UI::Xaml::FocusState::Programmatic);
         void Close();
         winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetNewTerminalArgs(const bool asContent) const;
 
-        winrt::hstring Title() { return L"Scratchpad"; }
+        winrt::hstring Title() { return RS_(L"SettingsTab"); }
         uint64_t TaskbarState() { return 0; }
         uint64_t TaskbarProgress() { return 0; }
         bool ReadOnly() { return false; }
@@ -31,8 +32,6 @@ namespace winrt::TerminalApp::implementation
         til::typed_event<> FocusRequested;
 
     private:
-        winrt::Windows::UI::Xaml::Controls::Grid _root{ nullptr };
-        winrt::Windows::UI::Xaml::Controls::TextBox _box{ nullptr };
         winrt::Microsoft::Terminal::Settings::Editor::MainPage _sui{ nullptr };
     };
 }
diff --git a/src/cascadia/TerminalApp/SettingsTab.cpp b/src/cascadia/TerminalApp/SettingsTab.cpp
@@ -38,6 +38,7 @@ namespace winrt::TerminalApp::implementation
 
     void SettingsTab::UpdateSettings(CascadiaSettings settings)
     {
+        // TODO! oh noes, we need to do this too in the content
         ASSERT_UI_THREAD();
 
         auto settingsUI{ Content().as<MainPage>() };
@@ -109,6 +110,8 @@ namespace winrt::TerminalApp::implementation
     // - <none>
     void SettingsTab::_CreateIcon()
     {
+        // TODO! make sure this works
+
         // This is the Setting icon (looks like a gear)
         static constexpr std::wstring_view glyph{ L"\xE713" };
 
@@ -119,6 +122,9 @@ namespace winrt::TerminalApp::implementation
 
     winrt::Windows::UI::Xaml::Media::Brush SettingsTab::_BackgroundBrush()
     {
+        // TODO! make sure this still works. It would be ironic if this _just
+        // worked_ because the SUI was the same color as a tab with no styling.
+
         // Look up the color we should use for the settings tab item from our
         // resources. This should only be used for when "terminalBackground" is
         // requested.
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -275,13 +275,15 @@ namespace winrt::TerminalApp::implementation
     // Arguments:
     // - pane: The pane to use as the root.
     // - insertPosition: Optional parameter to indicate the position of tab.
-    void TerminalPage::_CreateNewTabFromPane(std::shared_ptr<Pane> pane, uint32_t insertPosition)
+    TerminalApp::TerminalTab TerminalPage::_CreateNewTabFromPane(std::shared_ptr<Pane> pane, uint32_t insertPosition)
     {
         if (pane)
         {
             auto newTabImpl = winrt::make_self<TerminalTab>(pane);
             _InitializeTab(newTabImpl, insertPosition);
+            return *newTabImpl;
         }
+        return nullptr;
     }
 
     // Method Description:
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -22,6 +22,7 @@
 #include "ColorHelper.h"
 #include "DebugTapConnection.h"
 #include "SettingsTab.h"
+#include "SettingsPaneContent.h"
 #include "TabRowControl.h"
 #include "Utils.h"
 
@@ -2264,6 +2265,8 @@ namespace winrt::TerminalApp::implementation
                                   const float splitSize,
                                   std::shared_ptr<Pane> newPane)
     {
+        // TODO! Prevent splitting the _settingsTab!
+
         // If the caller is calling us with the return value of _MakePane
         // directly, it's possible that nullptr was returned, if the connections
         // was supposed to be launched in an elevated window. In that case, do
@@ -3768,7 +3771,10 @@ namespace winrt::TerminalApp::implementation
                 }
             }
 
-            winrt::Microsoft::Terminal::Settings::Editor::MainPage sui{ _settings };
+            // Create the SUI pane content
+            auto settingsContent{ winrt::make_self<SettingsPaneContent>(_settings) };
+            auto sui = settingsContent->SettingsUI();
+
             if (_hostingHwnd)
             {
                 sui.SetHostingWindow(reinterpret_cast<uint64_t>(*_hostingHwnd));
@@ -3784,52 +3790,58 @@ namespace winrt::TerminalApp::implementation
                 }
             });
 
-            auto newTabImpl = winrt::make_self<SettingsTab>(sui, _settings.GlobalSettings().CurrentTheme().RequestedTheme());
+            // Create the tab
+            auto resultPane = std::make_shared<Pane>(*settingsContent);
+            _settingsTab = _CreateNewTabFromPane(resultPane);
 
-            // Add the new tab to the list of our tabs.
-            _tabs.Append(*newTabImpl);
-            _mruTabs.Append(*newTabImpl);
+            // auto newTabImpl = winrt::make_self<SettingsTab>(sui, _settings.GlobalSettings().CurrentTheme().RequestedTheme());
 
-            newTabImpl->SetDispatch(*_actionDispatch);
-            newTabImpl->SetActionMap(_settings.ActionMap());
+            // // Add the new tab to the list of our tabs.
+            // _tabs.Append(*newTabImpl);
+            // _mruTabs.Append(*newTabImpl);
 
-            // Give the tab its index in the _tabs vector so it can manage its own SwitchToTab command.
-            _UpdateTabIndices();
+            // newTabImpl->SetDispatch(*_actionDispatch);
+            // newTabImpl->SetActionMap(_settings.ActionMap());
 
-            // 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.
-            auto weakTab = make_weak(newTabImpl);
+            // // Give the tab its index in the _tabs vector so it can manage its own SwitchToTab command.
+            // _UpdateTabIndices();
 
-            auto tabViewItem = newTabImpl->TabViewItem();
-            _tabView.TabItems().Append(tabViewItem);
+            // // 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.
+            // auto weakTab = make_weak(newTabImpl);
 
-            tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick });
+            // auto tabViewItem = newTabImpl->TabViewItem();
+            // _tabView.TabItems().Append(tabViewItem);
 
-            // When the tab requests close, try to close it (prompt for approval, if required)
-            newTabImpl->CloseRequested([weakTab, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
-                auto page{ weakThis.get() };
-                auto tab{ weakTab.get() };
+            // tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick });
 
-                if (page && tab)
-                {
-                    page->_HandleCloseTabRequested(*tab);
-                }
-            });
+            // // When the tab requests close, try to close it (prompt for approval, if required)
+            // newTabImpl->CloseRequested([weakTab, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
+            //     auto page{ weakThis.get() };
+            //     auto tab{ weakTab.get() };
 
-            // When the tab is closed, remove it from our list of tabs.
-            newTabImpl->Closed([tabViewItem, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
-                if (auto page{ weakThis.get() })
-                {
-                    page->_settingsTab = nullptr;
-                    page->_RemoveOnCloseRoutine(tabViewItem, page);
-                }
-            });
+            //     if (page && tab)
+            //     {
+            //         page->_HandleCloseTabRequested(*tab);
+            //     }
+            // });
+
+            // TODO! Make sure we remove the _settingsTab if it is closed!
+
+            // // When the tab is closed, remove it from our list of tabs.
+            // newTabImpl->Closed([tabViewItem, weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
+            //     if (auto page{ weakThis.get() })
+            //     {
+            //         page->_settingsTab = nullptr;
+            //         page->_RemoveOnCloseRoutine(tabViewItem, page);
+            //     }
+            // });
 
-            _settingsTab = *newTabImpl;
+            // _settingsTab = *newTabImpl;
 
-            // This kicks off TabView::SelectionChanged, in response to which
-            // we'll attach the terminal's Xaml control to the Xaml root.
-            _tabView.SelectedItem(tabViewItem);
+            //// This kicks off TabView::SelectionChanged, in response to which
+            //// we'll attach the terminal's Xaml control to the Xaml root.
+            //_tabView.SelectedItem(tabViewItem);
         }
         else
         {
diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
@@ -220,7 +220,7 @@ namespace winrt::TerminalApp::implementation
 
         void _UpdateTabIndices();
 
-        TerminalApp::SettingsTab _settingsTab{ nullptr };
+        TerminalApp::TerminalTab _settingsTab{ nullptr };
 
         bool _isInFocusMode{ false };
         bool _isFullscreen{ false };
@@ -297,7 +297,7 @@ namespace winrt::TerminalApp::implementation
 
         void _OpenNewTabDropdown();
         HRESULT _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs);
-        void _CreateNewTabFromPane(std::shared_ptr<Pane> pane, uint32_t insertPosition = -1);
+        TerminalApp::TerminalTab _CreateNewTabFromPane(std::shared_ptr<Pane> pane, uint32_t insertPosition = -1);
 
         std::wstring _evaluatePathForCwd(std::wstring_view path);
 

Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,7 @@ namespace winrt::TerminalApp::implementation`
`53`	`53`
`54`	`54`	`NewTerminalArgs SettingsPaneContent::GetNewTerminalArgs(const bool /* asContent */) const`
`55`	`55`	`{`
	`56`	`+ // TODO! hey, can we somehow replicate std::vector<ActionAndArgs> SettingsTab::BuildStartupActions?`
`56`	`57`	`return nullptr;`
`57`	`58`	`}`
`58`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -275,13 +275,15 @@ namespace winrt::TerminalApp::implementation`
`275`	`275`	`// Arguments:`
`276`	`276`	`// - pane: The pane to use as the root.`
`277`	`277`	`// - insertPosition: Optional parameter to indicate the position of tab.`
`278`		`- void TerminalPage::_CreateNewTabFromPane(std::shared_ptr<Pane> pane, uint32_t insertPosition)`
	`278`	`+ TerminalApp::TerminalTab TerminalPage::_CreateNewTabFromPane(std::shared_ptr<Pane> pane, uint32_t insertPosition)`
`279`	`279`	`{`
`280`	`280`	`if (pane)`
`281`	`281`	`{`
`282`	`282`	`auto newTabImpl = winrt::make_self<TerminalTab>(pane);`
`283`	`283`	`_InitializeTab(newTabImpl, insertPosition);`
	`284`	`+ return *newTabImpl;`
`284`	`285`	`}`
	`286`	`+ return nullptr;`
`285`	`287`	`}`
`286`	`288`
`287`	`289`	`// Method Description:`