[silmarillion] Detach on moveTab, but not on drag

It remains to be seen if this worked for movePane, but one step at a time
microsoft · Feb 6, 2023 · c282797 · c282797
1 parent a20201f
commit c282797
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 4 deletions.
diff --git a/src/cascadia/TerminalApp/ContentManager.cpp b/src/cascadia/TerminalApp/ContentManager.cpp
@@ -26,6 +26,9 @@ namespace winrt
 }
 namespace winrt::TerminalApp::implementation
 {
+    // TODO! These all probably need to be weak_refs, don't they, so that the
+    // contentmanager doesn't just keep every control around indefinitely.
+
     ControlInteractivity ContentManager::CreateCore(Microsoft::Terminal::Control::IControlSettings settings,
                                                     IControlAppearance unfocusedAppearance,
                                                     TerminalConnection::ITerminalConnection connection)
@@ -40,4 +43,23 @@ namespace winrt::TerminalApp::implementation
     {
         return _content.TryLookup(id);
     }
+
+    void ContentManager::Detach(const winrt::guid& contentGuid)
+    {
+        if (const auto& content{ LookupCore(contentGuid) })
+        {
+            content.Attached({ get_weak(), &ContentManager::_finalizeDetach });
+            _recentlyDetachedContent.Insert(contentGuid, content);
+        }
+    }
+
+    void ContentManager::_finalizeDetach(winrt::Windows::Foundation::IInspectable sender,
+                                         winrt::Windows::Foundation::IInspectable e)
+    {
+        if (const auto& content{ sender.try_as<winrt::Microsoft::Terminal::Control::ControlInteractivity>() })
+        {
+            _recentlyDetachedContent.Remove(content.Id());
+        }
+    }
+
 }
diff --git a/src/cascadia/TerminalApp/ContentManager.h b/src/cascadia/TerminalApp/ContentManager.h
@@ -17,9 +17,17 @@ namespace winrt::TerminalApp::implementation
                                                                       Microsoft::Terminal::TerminalConnection::ITerminalConnection connection);
         Microsoft::Terminal::Control::ControlInteractivity LookupCore(winrt::guid id);
 
+        void Detach(const winrt::guid& contentGuid);
+
     private:
         Windows::Foundation::Collections::IMap<winrt::guid, Microsoft::Terminal::Control::ControlInteractivity> _content{
             winrt::multi_threaded_map<winrt::guid, Microsoft::Terminal::Control::ControlInteractivity>()
         };
+        Windows::Foundation::Collections::IMap<winrt::guid, Microsoft::Terminal::Control::ControlInteractivity> _recentlyDetachedContent{
+            winrt::multi_threaded_map<winrt::guid, Microsoft::Terminal::Control::ControlInteractivity>()
+        };
+
+        void _finalizeDetach(winrt::Windows::Foundation::IInspectable sender,
+                             winrt::Windows::Foundation::IInspectable e);
     };
 }
diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
@@ -1232,8 +1232,9 @@ void Pane::Shutdown()
         // TOODO! if we call Close here, on a control that was moved to another thread, then it's Dispatcher is no longer this thread, and we'll crash.
         // Acn we get away with _not_ calling Close? Seems like shutdown is only called for RemoveTab(TerminalTab), so theoretically, removing the old control tree from the UI tree will release the core, calling it's dtor, which will call Close itself.
         // Alternatively, we could try and see if there's only one strong ref to a ControlCore and just noop if there's more than one.
-        // 
-        // _control.Close();
+        //
+        // I'm bringing this back for a second.
+        _control.Close();
     }
     else
     {
@@ -1415,7 +1416,7 @@ void Pane::UpdateVisuals()
     // TODO!
     // Hey remember when we made a static brush reference so we didn't have to look these up each time?
     // well, that's on the wrong thread always now. Great work.
-    // 
+    //
     // _borderFirst.BorderBrush(_lastActive ? s_focusedBorderBrush : s_unfocusedBorderBrush);
     // _borderSecond.BorderBrush(_lastActive ? s_focusedBorderBrush : s_unfocusedBorderBrush);
 }

diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -2039,6 +2039,18 @@ namespace winrt::TerminalApp::implementation
             if (const auto terminalTab{ _GetFocusedTabImpl() })
             {
                 auto startupActions = terminalTab->BuildStartupActions(true);
+
+                // Collect all the content we're about to detach.
+                if (const auto rootPane = terminalTab->GetRootPane())
+                {
+                    rootPane->WalkTree([&](auto p) {
+                        if (const auto& control{ p->GetTerminalControl() })
+                        {
+                            _manager.Detach(control.ContentGuid());
+                        }
+                    });
+                }
+
                 auto winRtActions{ winrt::single_threaded_vector<ActionAndArgs>(std::move(startupActions)) };
                 // Json::Value json{ Json::objectValue };
                 // SetValueForKey(json, "content", winRtActions);
@@ -2048,6 +2060,8 @@ namespace winrt::TerminalApp::implementation
                 auto request = winrt::make_self<RequestMoveContentArgs>(args.Window(),
                                                                         str,
                                                                         0);
+
+                _RemoveTab(*terminalTab);
                 _RequestMoveContentHandlers(*this, *request);
                 return true;
             }
@@ -2083,6 +2097,8 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
+    // TODO!  look at 87c840b381870e45bcc9e625fb88a8bdf5106420. That's what I was starting here
+
     // Method Description:
     // - Split the focused pane either horizontally or vertically, and place the
     //   given pane accordingly in the tree

diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl
@@ -14,6 +14,7 @@ namespace TerminalApp
                                                                    Microsoft.Terminal.Control.IControlAppearance unfocusedAppearance,
                                                                    Microsoft.Terminal.TerminalConnection.ITerminalConnection connection);
         Microsoft.Terminal.Control.ControlInteractivity LookupCore(Guid id);
+        void Detach(Guid id);
     }
 
     delegate void LastTabClosedEventArgs();

diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -231,9 +231,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
     void ControlCore::Reparent(const Microsoft::Terminal::Control::IKeyBindings& keyBindings)
     {
-        _CloseTerminalRequestedHandlers(*this, nullptr);
         _settings->KeyBindings(keyBindings);
         _setupDispatcherAndCallbacks();
+
+        _AttachedHandlers(*this, nullptr);
     }
 
     bool ControlCore::Initialize(const double actualWidth,

diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h
@@ -236,6 +236,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         TYPED_EVENT(UpdateSelectionMarkers,    IInspectable, Control::UpdateSelectionMarkersEventArgs);
         TYPED_EVENT(OpenHyperlink,             IInspectable, Control::OpenHyperlinkEventArgs);
         TYPED_EVENT(CloseTerminalRequested,    IInspectable, IInspectable);
+
+        TYPED_EVENT(Attached,                  IInspectable, IInspectable);
         // clang-format on
 
     private:

diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl
@@ -164,5 +164,7 @@ namespace Microsoft.Terminal.Control
         event Windows.Foundation.TypedEventHandler<Object, UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;
         event Windows.Foundation.TypedEventHandler<Object, OpenHyperlinkEventArgs> OpenHyperlink;
         event Windows.Foundation.TypedEventHandler<Object, Object> CloseTerminalRequested;
+
+        event Windows.Foundation.TypedEventHandler<Object, Object> Attached;
     };
 }
diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp
@@ -47,6 +47,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _guid = ::Microsoft::Console::Utils::CreateGuid();
 
         _core = winrt::make_self<ControlCore>(settings, unfocusedAppearance, connection);
+
+        _core->Attached([weakThis = get_weak()](auto&&, auto&&) {
+            if (auto self{ weakThis.get() })
+            {
+                self->_AttachedHandlers(*self, nullptr);
+            }
+        });
     }
 
     winrt::guid ControlInteractivity::Id()

diff --git a/src/cascadia/TerminalControl/ControlInteractivity.h b/src/cascadia/TerminalControl/ControlInteractivity.h
@@ -92,6 +92,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         TYPED_EVENT(PasteFromClipboard, IInspectable, Control::PasteFromClipboardEventArgs);
         TYPED_EVENT(ScrollPositionChanged, IInspectable, Control::ScrollPositionChangedArgs);
 
+        TYPED_EVENT(Attached, IInspectable, IInspectable);
+
     private:
         // NOTE: _uiaEngine must be ordered before _core.
         //

diff --git a/src/cascadia/TerminalControl/ControlInteractivity.idl b/src/cascadia/TerminalControl/ControlInteractivity.idl
@@ -68,6 +68,7 @@ namespace Microsoft.Terminal.Control
         event Windows.Foundation.TypedEventHandler<Object, ScrollPositionChangedArgs> ScrollPositionChanged;
         event Windows.Foundation.TypedEventHandler<Object, PasteFromClipboardEventArgs> PasteFromClipboard;
 
+        event Windows.Foundation.TypedEventHandler<Object, Object> Attached;
 
     };
 }