Skip to content

Commit

Permalink
Add support for restoring non-terminal panes, and opening them with `…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
zadjii-msft committed Apr 5, 2024
1 parent c063d2b commit dc4026d
Show file tree
Hide file tree
Showing 25 changed files with 1,182 additions and 868 deletions.
593 changes: 320 additions & 273 deletions src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp

Large diffs are not rendered by default.

508 changes: 271 additions & 237 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp

Large diffs are not rendered by default.

91 changes: 61 additions & 30 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Expand Up @@ -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)
{
Expand All @@ -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 };
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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();

Expand All @@ -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
Expand All @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/IPaneContent.idl
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Pane.cpp
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Pane.h
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/ScratchpadContent.cpp
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/ScratchpadContent.h
Expand Up @@ -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; }
Expand Down
7 changes: 2 additions & 5 deletions src/cascadia/TerminalApp/SettingsPaneContent.cpp
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/SettingsPaneContent.h
Expand Up @@ -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; }
Expand Down
37 changes: 20 additions & 17 deletions src/cascadia/TerminalApp/TabManagement.cpp
Expand Up @@ -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();
Expand Down

0 comments on commit dc4026d

Please sign in to comment.