Add support for restoring non-terminal panes, and opening them with `…

…splitPane`, `newTab` (#16914) This changes `NewTabArgs`, `SplitPaneArgs`, and `NewWindowArgs` to accept a `INewContentArgs`, rather than just a `NewTerminalArgs`. This allows a couple things: * Users can open arbitrary types of panes with the existing `splitPane`, `newWindow` actions, just by passing `"type": "scartchpad"` (for example). This is a lot more flexible than re-defining different `"openScratchpad"`, `"openTasksPane"`, etc, etc actions for every kind of pane. * This allows us to use the existing machinery of session restore to also restore non-terminal panes. The `type` property was added to `newTab`, `splitPane`, `newWindow`. When omitted, we still just treat the json as a blob of NewTerminalArgs. There's not actually any other kinds of `INewContentArgs` in this PR (other than the placeholder `GenericContentArgs`). In [`dev/migrie/fhl/md-pane`](dev/migrie/f/tasks-pane...dev/migrie/fhl/md-pane), I have a type of pane that would LOVE to add some args here. So that's forward-thinking. There's really just two stealth types of pane for now: `settings`, and `scratchpad`. Those I DON'T have as constants or anything in this PR. They probably should be? Though, I suspect around the time of the tasks & MD panes, I'll come up with whatever structure I actually want them to take. ### future considerations here * In the future, this should allow extensions to say "I know how to host `foo` content", for 3p content. * The `wt` CLI args were not yet updated to also accept `--type` yet. There's no reason we couldn't easily do that. * I considered adding `ICanHasCommandline` to allow arbitrary content to generate a `wt` commandline-serializable string. Punted on that for now. ## other PRs * #16170 * #16171 * #16172 * #16895 * #16914 <-- you are here Closes #17014
microsoft · Apr 5, 2024 · dc4026d · dc4026d
1 parent c063d2b
commit dc4026d
Show file tree

Hide file tree

Showing 25 changed files with 1,182 additions and 868 deletions.
diff --git a/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp b/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -238,6 +238,32 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
+    // * Helper to try and get a ProfileIndex out of a NewTerminalArgs out of a
+    //   NewContentArgs. For the new tab and split pane action, we want to _not_
+    //   handle the event if an invalid profile index was passed.
+    //
+    // Return value:
+    // * True if the args are NewTerminalArgs, and the profile index was out of bounds.
+    // * False otherwise.
+    static bool _shouldBailForInvalidProfileIndex(const CascadiaSettings& settings, const INewContentArgs& args)
+    {
+        if (!args)
+        {
+            return false;
+        }
+        if (const auto& terminalArgs{ args.try_as<NewTerminalArgs>() })
+        {
+            if (const auto index = terminalArgs.ProfileIndex())
+            {
+                if (gsl::narrow<uint32_t>(index.Value()) >= settings.ActiveProfiles().Size())
+                {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     void TerminalPage::_HandleSplitPane(const IInspectable& sender,
                                         const ActionEventArgs& args)
     {
@@ -247,16 +273,10 @@ namespace winrt::TerminalApp::implementation
         }
         else if (const auto& realArgs = args.ActionArgs().try_as<SplitPaneArgs>())
         {
-            if (const auto& newTerminalArgs{ realArgs.TerminalArgs() })
+            if (_shouldBailForInvalidProfileIndex(_settings, realArgs.ContentArgs()))
             {
-                if (const auto index = realArgs.TerminalArgs().ProfileIndex())
-                {
-                    if (gsl::narrow<uint32_t>(index.Value()) >= _settings.ActiveProfiles().Size())
-                    {
-                        args.Handled(false);
-                        return;
-                    }
-                }
+                args.Handled(false);
+                return;
             }
 
             const auto& duplicateFromTab{ realArgs.SplitMode() == SplitType::Duplicate ? _GetFocusedTab() : nullptr };
@@ -267,7 +287,7 @@ namespace winrt::TerminalApp::implementation
                        realArgs.SplitDirection(),
                        // This is safe, we're already filtering so the value is (0, 1)
                        ::base::saturated_cast<float>(realArgs.SplitSize()),
-                       _MakePane(realArgs.TerminalArgs(), duplicateFromTab));
+                       _MakePane(realArgs.ContentArgs(), duplicateFromTab));
             args.Handled(true);
         }
     }
@@ -445,19 +465,13 @@ namespace winrt::TerminalApp::implementation
         }
         else if (const auto& realArgs = args.ActionArgs().try_as<NewTabArgs>())
         {
-            if (const auto& newTerminalArgs{ realArgs.TerminalArgs() })
+            if (_shouldBailForInvalidProfileIndex(_settings, realArgs.ContentArgs()))
             {
-                if (const auto index = newTerminalArgs.ProfileIndex())
-                {
-                    if (gsl::narrow<uint32_t>(index.Value()) >= _settings.ActiveProfiles().Size())
-                    {
-                        args.Handled(false);
-                        return;
-                    }
-                }
+                args.Handled(false);
+                return;
             }
 
-            LOG_IF_FAILED(_OpenNewTab(realArgs.TerminalArgs()));
+            LOG_IF_FAILED(_OpenNewTab(realArgs.ContentArgs()));
             args.Handled(true);
         }
     }
@@ -869,8 +883,23 @@ namespace winrt::TerminalApp::implementation
     // - <none>
     // Important: Don't take the param by reference, since we'll be doing work
     // on another thread.
-    fire_and_forget TerminalPage::_OpenNewWindow(const NewTerminalArgs newTerminalArgs)
+    fire_and_forget TerminalPage::_OpenNewWindow(const INewContentArgs newContentArgs)
     {
+        auto terminalArgs{ newContentArgs.try_as<NewTerminalArgs>() };
+
+        // Do nothing for non-terminal panes.
+        //
+        // Theoretically, we could define a `IHasCommandline` interface, and
+        // stick `ToCommandline` on that interface, for any kind of pane that
+        // wants to be convertable to a wt commandline.
+        //
+        // Another idea we're thinking about is just `wt do {literal json for an
+        // action}`, which might be less leaky
+        if (terminalArgs == nullptr)
+        {
+            co_return;
+        }
+
         // Hop to the BG thread
         co_await winrt::resume_background();
 
@@ -883,8 +912,7 @@ namespace winrt::TerminalApp::implementation
         // `-w -1` will ensure a new window is created.
         winrt::hstring cmdline{
             fmt::format(L"-w -1 new-tab {}",
-                        newTerminalArgs ? newTerminalArgs.ToCommandline().c_str() :
-                                          L"")
+                        terminalArgs.ToCommandline().c_str())
         };
 
         // Build the args to ShellExecuteEx. We need to use ShellExecuteEx so we
@@ -909,29 +937,32 @@ namespace winrt::TerminalApp::implementation
     void TerminalPage::_HandleNewWindow(const IInspectable& /*sender*/,
                                         const ActionEventArgs& actionArgs)
     {
-        NewTerminalArgs newTerminalArgs{ nullptr };
+        INewContentArgs newContentArgs{ nullptr };
         // If the caller provided NewTerminalArgs, then try to use those
         if (actionArgs)
         {
             if (const auto& realArgs = actionArgs.ActionArgs().try_as<NewWindowArgs>())
             {
-                newTerminalArgs = realArgs.TerminalArgs();
+                newContentArgs = realArgs.ContentArgs();
             }
         }
         // Otherwise, if no NewTerminalArgs were provided, then just use a
         // default-constructed one. The default-constructed one implies that
         // nothing about the launch should be modified (just use the default
         // profile).
-        if (!newTerminalArgs)
+        if (!newContentArgs)
         {
-            newTerminalArgs = NewTerminalArgs();
+            newContentArgs = NewTerminalArgs{};
         }
 
-        const auto profile{ _settings.GetProfileForArgs(newTerminalArgs) };
+        if (const auto& terminalArgs{ newContentArgs.try_as<NewTerminalArgs>() })
+        {
+            const auto profile{ _settings.GetProfileForArgs(terminalArgs) };
+            terminalArgs.Profile(::Microsoft::Console::Utils::GuidToString(profile.Guid()));
+        }
 
         // Manually fill in the evaluated profile.
-        newTerminalArgs.Profile(::Microsoft::Console::Utils::GuidToString(profile.Guid()));
-        _OpenNewWindow(newTerminalArgs);
+        _OpenNewWindow(newContentArgs);
         actionArgs.Handled(true);
     }
 

diff --git a/src/cascadia/TerminalApp/IPaneContent.idl b/src/cascadia/TerminalApp/IPaneContent.idl
@@ -31,7 +31,7 @@ namespace TerminalApp
         Windows.Foundation.IReference<Windows.UI.Color> TabColor { get; };
         Windows.UI.Xaml.Media.Brush BackgroundBrush { get; };
 
-        Microsoft.Terminal.Settings.Model.NewTerminalArgs GetNewTerminalArgs(BuildStartupKind kind);
+        Microsoft.Terminal.Settings.Model.INewContentArgs GetNewTerminalArgs(BuildStartupKind kind);
 
         void Focus(Windows.UI.Xaml.FocusState reason);
 

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
@@ -100,7 +100,7 @@ Pane::Pane(std::shared_ptr<Pane> first,
 
 // Extract the terminal settings from the current (leaf) pane's control
 // to be used to create an equivalent control
-NewTerminalArgs Pane::GetTerminalArgsForPane(BuildStartupKind kind) const
+INewContentArgs Pane::GetTerminalArgsForPane(BuildStartupKind kind) const
 {
     // Leaves are the only things that have controls
     assert(_IsLeaf());

diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
@@ -106,7 +106,7 @@ class Pane : public std::enable_shared_from_this<Pane>
         uint32_t panesCreated;
     };
     BuildStartupState BuildStartupActions(uint32_t currentId, uint32_t nextId, winrt::TerminalApp::BuildStartupKind kind);
-    winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetTerminalArgsForPane(winrt::TerminalApp::BuildStartupKind kind) const;
+    winrt::Microsoft::Terminal::Settings::Model::INewContentArgs GetTerminalArgsForPane(winrt::TerminalApp::BuildStartupKind kind) const;
 
     void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
     bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);

diff --git a/src/cascadia/TerminalApp/ScratchpadContent.cpp b/src/cascadia/TerminalApp/ScratchpadContent.cpp
@@ -48,9 +48,9 @@ namespace winrt::TerminalApp::implementation
         CloseRequested.raise(*this, nullptr);
     }
 
-    NewTerminalArgs ScratchpadContent::GetNewTerminalArgs(const BuildStartupKind /* kind */) const
+    INewContentArgs ScratchpadContent::GetNewTerminalArgs(const BuildStartupKind /* kind */) const
     {
-        return nullptr;
+        return BaseContentArgs(L"scratchpad");
     }
 
     winrt::hstring ScratchpadContent::Icon() const

diff --git a/src/cascadia/TerminalApp/ScratchpadContent.h b/src/cascadia/TerminalApp/ScratchpadContent.h
@@ -19,7 +19,7 @@ namespace winrt::TerminalApp::implementation
 
         void Focus(winrt::Windows::UI::Xaml::FocusState reason = winrt::Windows::UI::Xaml::FocusState::Programmatic);
         void Close();
-        winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetNewTerminalArgs(BuildStartupKind kind) const;
+        winrt::Microsoft::Terminal::Settings::Model::INewContentArgs GetNewTerminalArgs(BuildStartupKind kind) const;
 
         winrt::hstring Title() { return L"Scratchpad"; }
         uint64_t TaskbarState() { return 0; }

diff --git a/src/cascadia/TerminalApp/SettingsPaneContent.cpp b/src/cascadia/TerminalApp/SettingsPaneContent.cpp
@@ -50,12 +50,9 @@ namespace winrt::TerminalApp::implementation
         CloseRequested.raise(*this, nullptr);
     }
 
-    NewTerminalArgs SettingsPaneContent::GetNewTerminalArgs(const BuildStartupKind /*kind*/) const
+    INewContentArgs SettingsPaneContent::GetNewTerminalArgs(const BuildStartupKind /*kind*/) const
     {
-        // For now, we're doing a terrible thing in TerminalTab itself to
-        // generate an OpenSettings action manually, without asking for the pane
-        // structure.
-        return nullptr;
+        return BaseContentArgs(L"settings");
     }
 
     winrt::hstring SettingsPaneContent::Icon() const

diff --git a/src/cascadia/TerminalApp/SettingsPaneContent.h b/src/cascadia/TerminalApp/SettingsPaneContent.h
@@ -20,7 +20,7 @@ namespace winrt::TerminalApp::implementation
         winrt::Windows::Foundation::Size MinimumSize();
         void Focus(winrt::Windows::UI::Xaml::FocusState reason = winrt::Windows::UI::Xaml::FocusState::Programmatic);
         void Close();
-        winrt::Microsoft::Terminal::Settings::Model::NewTerminalArgs GetNewTerminalArgs(const BuildStartupKind kind) const;
+        winrt::Microsoft::Terminal::Settings::Model::INewContentArgs GetNewTerminalArgs(const BuildStartupKind kind) const;
 
         winrt::hstring Title() { return RS_(L"SettingsTab"); }
         uint64_t TaskbarState() { return 0; }

diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -62,30 +62,33 @@ namespace winrt::TerminalApp::implementation
     // - existingConnection: An optional connection that is already established to a PTY
     //   for this tab to host instead of creating one.
     //   If not defined, the tab will create the connection.
-    HRESULT TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs)
+    HRESULT TerminalPage::_OpenNewTab(const INewContentArgs& newContentArgs)
     try
     {
-        const auto profile{ _settings.GetProfileForArgs(newTerminalArgs) };
-        // GH#11114: GetProfileForArgs can return null if the index is higher
-        // than the number of available profiles.
-        if (!profile)
+        if (const auto& newTerminalArgs{ newContentArgs.try_as<NewTerminalArgs>() })
         {
-            return S_FALSE;
-        }
-        const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings) };
+            const auto profile{ _settings.GetProfileForArgs(newTerminalArgs) };
+            // GH#11114: GetProfileForArgs can return null if the index is higher
+            // than the number of available profiles.
+            if (!profile)
+            {
+                return S_FALSE;
+            }
+            const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings) };
 
-        // Try to handle auto-elevation
-        if (_maybeElevate(newTerminalArgs, settings, profile))
-        {
-            return S_OK;
+            // Try to handle auto-elevation
+            if (_maybeElevate(newTerminalArgs, settings, profile))
+            {
+                return S_OK;
+            }
+            // We can't go in the other direction (elevated->unelevated)
+            // unfortunately. This seems to be due to Centennial quirks. It works
+            // unpackaged, but not packaged.
         }
-        // We can't go in the other direction (elevated->unelevated)
-        // unfortunately. This seems to be due to Centennial quirks. It works
-        // unpackaged, but not packaged.
-        //
+
         // This call to _MakePane won't return nullptr, we already checked that
         // case above with the _maybeElevate call.
-        _CreateNewTabFromPane(_MakePane(newTerminalArgs, nullptr));
+        _CreateNewTabFromPane(_MakePane(newContentArgs, nullptr));
         return S_OK;
     }
     CATCH_RETURN();