Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add size param to splitPane action, split-pane subcommand #8543

Merged
8 commits merged into from
Dec 18, 2020
7 changes: 7 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,13 @@
"splitMode": {
"default": "duplicate",
"description": "Control how the pane splits. Only accepts \"duplicate\" which will duplicate the focused pane's profile into a new pane."
},
"size": {
"default": 0.5,
"description": "Specify how large the new pane should be, as a fraction of the current pane's size. 1.0 would be 'all of the current pane', and 0.0 is 'None of the parent'. Accepts floating point values from 0-1 (default 0.5).",
"maximum": 1,
"minimum": 0,
"type": "number"
}
}
}
Expand Down
52 changes: 50 additions & 2 deletions src/cascadia/LocalTests_SettingsModel/CommandTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ namespace SettingsModelLocalTests
TEST_METHOD(ManyCommandsSameAction);
TEST_METHOD(LayerCommand);
TEST_METHOD(TestSplitPaneArgs);
TEST_METHOD(TestSplitPaneBadSize);
TEST_METHOD(TestResourceKeyName);
TEST_METHOD(TestAutogeneratedName);
TEST_METHOD(TestLayerOnAutogeneratedName);
Expand Down Expand Up @@ -147,7 +148,8 @@ namespace SettingsModelLocalTests
{ "name": "command1", "command": { "action": "splitPane", "split": "vertical" } },
{ "name": "command2", "command": { "action": "splitPane", "split": "horizontal" } },
{ "name": "command4", "command": { "action": "splitPane" } },
{ "name": "command5", "command": { "action": "splitPane", "split": "auto" } }
{ "name": "command5", "command": { "action": "splitPane", "split": "auto" } },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe add a test for "split" and "size" both defined?

{ "name": "command6", "command": { "action": "splitPane", "size": 0.25 } },
])" };

const auto commands0Json = VerifyParseSucceeded(commands0String);
Expand All @@ -156,7 +158,7 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(0u, commands.Size());
auto warnings = implementation::Command::LayerJson(commands, commands0Json);
VERIFY_ARE_EQUAL(0u, warnings.size());
VERIFY_ARE_EQUAL(4u, commands.Size());
VERIFY_ARE_EQUAL(5u, commands.Size());

{
auto command = commands.Lookup(L"command1");
Expand All @@ -167,6 +169,7 @@ namespace SettingsModelLocalTests
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command2");
Expand All @@ -177,6 +180,7 @@ namespace SettingsModelLocalTests
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command4");
Expand All @@ -187,6 +191,7 @@ namespace SettingsModelLocalTests
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command5");
Expand All @@ -197,8 +202,51 @@ namespace SettingsModelLocalTests
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command6");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action());
const auto& realArgs = command.Action().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(0.25, realArgs.SplitSize());
}
}

void CommandTests::TestSplitPaneBadSize()
{
const std::string commands0String{ R"([
{ "name": "command1", "command": { "action": "splitPane", "size": 0.25 } },
{ "name": "command2", "command": { "action": "splitPane", "size": 1.0 } },
{ "name": "command3", "command": { "action": "splitPane", "size": 0 } },
{ "name": "command4", "command": { "action": "splitPane", "size": 50 } },
])" };

const auto commands0Json = VerifyParseSucceeded(commands0String);

IMap<winrt::hstring, Command> commands = winrt::single_threaded_map<winrt::hstring, Command>();
VERIFY_ARE_EQUAL(0u, commands.Size());
auto warnings = implementation::Command::LayerJson(commands, commands0Json);
VERIFY_ARE_EQUAL(3u, warnings.size());
VERIFY_ARE_EQUAL(1u, commands.Size());

{
auto command = commands.Lookup(L"command1");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.Action());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.Action().Action());
const auto& realArgs = command.Action().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(0.25, realArgs.SplitSize());
}
}

void CommandTests::TestResourceKeyName()
{
// This test checks looking up a name from a resource key.
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ namespace TerminalAppLocalTests

Log::Comment(NoThrowString().Format(L"Duplicate the first pane"));
result = RunOnUIThread([&page]() {
page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, nullptr);
page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, 0.5f, nullptr);

VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
Expand All @@ -504,7 +504,7 @@ namespace TerminalAppLocalTests

Log::Comment(NoThrowString().Format(L"Duplicate the pane, and don't crash"));
result = RunOnUIThread([&page]() {
page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, nullptr);
page->_SplitPane(SplitState::Automatic, SplitType::Duplicate, 0.5f, nullptr);

VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ namespace winrt::TerminalApp::implementation
}
else if (const auto& realArgs = args.ActionArgs().try_as<SplitPaneArgs>())
{
_SplitPane(realArgs.SplitStyle(), realArgs.SplitMode(), realArgs.TerminalArgs());
_SplitPane(realArgs.SplitStyle(),
realArgs.SplitMode(),
// This is safe, we're already filtering so the value is (0, 1)
::base::saturated_cast<float>(realArgs.SplitSize()),
realArgs.TerminalArgs());
args.Handled(true);
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalApp/AppCommandlineArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ void AppCommandlineArgs::_buildSplitPaneParser()
_splitVertical,
RS_A(L"CmdSplitPaneVerticalArgDesc"));
subcommand._verticalOption->excludes(subcommand._horizontalOption);
auto* sizeOpt = subcommand.subcommand->add_option("-s,--size",
_splitPaneSize,
RS_A(L"CmdSplitPaneSizeArgDesc"));
sizeOpt->check(CLI::Range(0.01f, 0.99f));

// When ParseCommand is called, if this subcommand was provided, this
// callback function will be triggered on the same thread. We can be sure
Expand Down Expand Up @@ -274,7 +278,7 @@ void AppCommandlineArgs::_buildSplitPaneParser()
style = SplitState::Vertical;
}
}
SplitPaneArgs args{ style, terminalArgs };
SplitPaneArgs args{ style, _splitPaneSize, terminalArgs };
splitPaneActionAndArgs.Args(args);
_startupActions.push_back(splitPaneActionAndArgs);
});
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class TerminalApp::AppCommandlineArgs final

bool _splitVertical{ false };
bool _splitHorizontal{ false };
float _splitPaneSize{ 0.5f };

int _focusTabIndex{ -1 };
bool _focusNextTab{ false };
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ static const std::array<std::wstring_view, static_cast<uint32_t>(SettingsLoadWar
USES_RESOURCE(L"LegacyGlobalsProperty"),
USES_RESOURCE(L"FailedToParseCommandJson"),
USES_RESOURCE(L"FailedToWriteToSettings"),
USES_RESOURCE(L"InvalidColorSchemeInCmd")
USES_RESOURCE(L"InvalidColorSchemeInCmd"),
USES_RESOURCE(L"InvalidSplitSize")
};
static const std::array<std::wstring_view, static_cast<uint32_t>(SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels {
USES_RESOURCE(L"NoProfilesText"),
Expand Down
19 changes: 13 additions & 6 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1271,6 +1271,7 @@ bool Pane::CanSplit(SplitState splitType)
// Note:
// - This method is highly similar to Pane::PreCalculateAutoSplit
std::optional<bool> Pane::PreCalculateCanSplit(const std::shared_ptr<Pane> target,
// TODO: const float splitSize
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
SplitState splitType,
const winrt::Windows::Foundation::Size availableSpace) const
{
Expand Down Expand Up @@ -1348,23 +1349,26 @@ std::optional<bool> Pane::PreCalculateCanSplit(const std::shared_ptr<Pane> targe
// - control: A TermControl to use in the new pane.
// Return Value:
// - The two newly created Panes
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control)
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::Split(SplitState splitType,
const float splitSize,
const GUID& profile,
const TermControl& control)
{
if (!_IsLeaf())
{
if (_firstChild->_HasFocusedChild())
{
return _firstChild->Split(splitType, profile, control);
return _firstChild->Split(splitType, splitSize, profile, control);
}
else if (_secondChild->_HasFocusedChild())
{
return _secondChild->Split(splitType, profile, control);
return _secondChild->Split(splitType, splitSize, profile, control);
}

return { nullptr, nullptr };
}

return _Split(splitType, profile, control);
return _Split(splitType, splitSize, profile, control);
}

// Method Description:
Expand Down Expand Up @@ -1440,7 +1444,10 @@ bool Pane::_CanSplit(SplitState splitType)
// - control: A TermControl to use in the new pane.
// Return Value:
// - The two newly created Panes
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control)
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitState splitType,
const float splitSize,
const GUID& profile,
const TermControl& control)
{
if (splitType == SplitState::None)
{
Expand All @@ -1465,7 +1472,7 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitState
_gotFocusRevoker.revoke();

_splitState = actualSplitType;
_desiredSplitPosition = Half;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
_desiredSplitPosition = 1.0f - splitSize;

// Remove any children we currently have. We can't add the existing
// TermControl to a new grid until we do this.
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class Pane : public std::enable_shared_from_this<Pane>

bool CanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType);
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Split(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType,
const float splitSize,
const GUID& profile,
const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const;
Expand Down Expand Up @@ -122,6 +123,7 @@ class Pane : public std::enable_shared_from_this<Pane>

bool _CanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType);
std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> _Split(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType,
const float splitSize,
const GUID& profile,
const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);

Expand Down
63 changes: 35 additions & 28 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema

Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes

The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.

Example:

... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple

There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the

Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not

The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can

Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.

mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -249,6 +249,10 @@
<value>Found a command with an invalid "colorScheme". This command will be ignored. Make sure that when setting a "colorScheme", the value matches the "name" of a color scheme in the "schemes" list.</value>
<comment>{Locked="\"colorScheme\"","\"name\"","\"schemes\""}</comment>
</data>
<data name="InvalidSplitSize" xml:space="preserve">
<value>Found a "splitPane" command with an invalid "size". This command will be ignored. Make sure the size is between 0 and 1, exclusive.</value>
<comment>{Locked="\"splitPane\"","\"size\""}</comment>
</data>
<data name="CmdCommandArgDesc" xml:space="preserve">
<value>An optional command, with arguments, to be spawned in the new tab or pane</value>
</data>
Expand All @@ -268,6 +272,9 @@
<data name="CmdFocusTabTargetArgDesc" xml:space="preserve">
<value>Move focus the tab at the given index</value>
</data>
<data name="CmdSplitPaneSizeArgDesc" xml:space="preserve">
<value>Specify the size as a percentage of the parent pane. Valid values are between (0,1), exclusive.</value>
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="CmdNewTabDesc" xml:space="preserve">
<value>Create a new tab</value>
</data>
Expand Down Expand Up @@ -513,4 +520,4 @@
<data name="NoticeWarning" xml:space="preserve">
<value>Warning</value>
</data>
</root>
</root>
Loading