Skip to content

Commit

Permalink
Add the ability to split a pane and put the new pane first. (#11145)
Browse files Browse the repository at this point in the history 
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Adds directional modifiers for SplitState and convert those to the appropriate horizontal/vertical when splitting a pane.

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> 
## References

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #4340
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [x] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [x] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
"vertical" and "horizontal" splits were removed from `defaults.json`, but code was added to parse those as `right` and `down` respectively. It is also the case that if a user has a custom hotkey for `split: vertical` it will override the default for `split: right`.

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Split the pane using each of the new directional movements
  • Loading branch information
@Rosefield
Rosefield authored Sep 15, 2021
1 parent 121116c commit 87b695f
Show file tree
Hide file tree
Showing 22 changed files with 324 additions and 228 deletions.
14 changes: 8 additions & 6 deletions doc/cascadia/profiles.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,13 @@
],
"type": "string"
},
"SplitState": {
"SplitDirection": {
"enum": [
"vertical",
"horizontal",
"auto"
"auto",
"up",
"right",
"down",
"left"
],
"type": "string"
},
Expand Down Expand Up @@ -629,9 +631,9 @@
"properties": {
"action": { "type": "string", "pattern": "splitPane" },
"split": {
"$ref": "#/definitions/SplitState",
"$ref": "#/definitions/SplitDirection",
"default": "auto",
"description": "The orientation to split the pane in. Possible values:\n -\"auto\" (splits pane based on remaining space)\n -\"horizontal\" (think [-])\n -\"vertical\" (think [|])"
"description": "The orientation to split the pane in. Possible values:\n -\"auto\" (splits pane based on remaining space)\n -\"up\" (think [-] and above)\n -\"down\" (think [-] and below)\n -\"left\" (think [|] and to the left)\n -\"right\"(think [|] and to the right)"
},
"splitMode": {
"default": "duplicate",
Expand Down
104 changes: 87 additions & 17 deletions src/cascadia/LocalTests_SettingsModel/CommandTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ namespace SettingsModelLocalTests
{ "name": "command4", "command": { "action": "splitPane" } },
{ "name": "command5", "command": { "action": "splitPane", "split": "auto" } },
{ "name": "command6", "command": { "action": "splitPane", "size": 0.25 } },
{ "name": "command7", "command": { "action": "splitPane", "split": "right" } },
{ "name": "command8", "command": { "action": "splitPane", "split": "left" } },
{ "name": "command9", "command": { "action": "splitPane", "split": "up" } },
{ "name": "command10", "command": { "action": "splitPane", "split": "down" } },
])" };

const auto commands0Json = VerifyParseSucceeded(commands0String);
Expand All @@ -160,7 +164,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(5u, commands.Size());
VERIFY_ARE_EQUAL(9u, commands.Size());

{
auto command = commands.Lookup(L"command1");
Expand All @@ -170,7 +174,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
Expand All @@ -181,7 +185,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Down, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
Expand All @@ -192,7 +196,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().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(SplitDirection::Automatic, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
Expand All @@ -203,7 +207,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().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(SplitDirection::Automatic, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
Expand All @@ -214,9 +218,53 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().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(SplitDirection::Automatic, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.25, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command7");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command8");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitDirection::Left, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command9");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitDirection::Up, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
{
auto command = commands.Lookup(L"command10");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitDirection::Down, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.5, realArgs.SplitSize());
}
}

void CommandTests::TestSplitPaneBadSize()
Expand Down Expand Up @@ -244,7 +292,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().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(SplitDirection::Automatic, realArgs.SplitDirection());
VERIFY_ARE_EQUAL(0.25, realArgs.SplitSize());
}
}
Expand Down Expand Up @@ -286,8 +334,10 @@ namespace SettingsModelLocalTests

const std::string commands0String{ R"([
{ "command": { "action": "splitPane", "split": null } },
{ "command": { "action": "splitPane", "split": "vertical" } },
{ "command": { "action": "splitPane", "split": "horizontal" } },
{ "command": { "action": "splitPane", "split": "left" } },
{ "command": { "action": "splitPane", "split": "right" } },
{ "command": { "action": "splitPane", "split": "up" } },
{ "command": { "action": "splitPane", "split": "down" } },
{ "command": { "action": "splitPane", "split": "none" } },
{ "command": { "action": "splitPane" } },
{ "command": { "action": "splitPane", "split": "auto" } },
Expand All @@ -301,10 +351,10 @@ namespace SettingsModelLocalTests
auto warnings = implementation::Command::LayerJson(commands, commands0Json);
VERIFY_ARE_EQUAL(0u, warnings.size());

// There are only 3 commands here: all of the `"none"`, `"auto"`,
// There are only 5 commands here: all of the `"none"`, `"auto"`,
// `"foo"`, `null`, and <no args> bindings all generate the same action,
// which will generate just a single name for all of them.
VERIFY_ARE_EQUAL(3u, commands.Size());
VERIFY_ARE_EQUAL(5u, commands.Size());

{
auto command = commands.Lookup(L"Split pane");
Expand All @@ -314,27 +364,47 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().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(SplitDirection::Automatic, realArgs.SplitDirection());
}
{
auto command = commands.Lookup(L"Split pane, split: left");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitDirection::Left, realArgs.SplitDirection());
}
{
auto command = commands.Lookup(L"Split pane, split: right");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
}
{
auto command = commands.Lookup(L"Split pane, split: vertical");
auto command = commands.Lookup(L"Split pane, split: up");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Up, realArgs.SplitDirection());
}
{
auto command = commands.Lookup(L"Split pane, split: horizontal");
auto command = commands.Lookup(L"Split pane, split: down");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Down, realArgs.SplitDirection());
}
}
void CommandTests::TestLayerOnAutogeneratedName()
Expand All @@ -360,7 +430,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = command.ActionAndArgs().Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2130,7 +2130,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
Expand All @@ -2147,7 +2147,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
Expand All @@ -2161,7 +2161,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
Expand All @@ -2175,7 +2175,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Down, realArgs.SplitDirection());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
Expand All @@ -2189,7 +2189,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Down, realArgs.SplitDirection());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
Expand Down Expand Up @@ -2218,7 +2218,7 @@ namespace SettingsModelLocalTests
const auto& realArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
VERIFY_IS_NOT_NULL(realArgs);
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
VERIFY_IS_TRUE(realArgs.TerminalArgs().Commandline().empty());
VERIFY_IS_TRUE(realArgs.TerminalArgs().StartingDirectory().empty());
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,31 +432,31 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().as<SplitPaneArgs>();
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Vertical, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Right, realArgs.SplitDirection());
}
{
KeyChord kc{ true, false, false, false, static_cast<int32_t>('E'), 0 };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().as<SplitPaneArgs>();
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Horizontal, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Down, realArgs.SplitDirection());
}
{
KeyChord kc{ true, false, false, false, static_cast<int32_t>('G'), 0 };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().as<SplitPaneArgs>();
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Automatic, realArgs.SplitDirection());
}
{
KeyChord kc{ true, false, false, false, static_cast<int32_t>('H'), 0 };
auto actionAndArgs = ::TestUtils::GetActionAndArgs(*actionMap, kc);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
const auto& realArgs = actionAndArgs.Args().as<SplitPaneArgs>();
// Verify the args have the expected value
VERIFY_ARE_EQUAL(SplitState::Automatic, realArgs.SplitStyle());
VERIFY_ARE_EQUAL(SplitDirection::Automatic, realArgs.SplitDirection());
}
}

Expand Down
Loading

0 comments on commit 87b695f

Please sign in to comment.