From 2e26c3e0c958f174d838049ce43cbb7dc52eb7e8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 19 Dec 2019 15:47:19 -0600 Subject: [PATCH] Add support for "Automatic" splits (#4025) ## Summary of the Pull Request Adds support for `auto` as a potential value for a `splitPane` keybinding's `split` argument. For example: ```json { "keys": [ "ctrl+shift+z" ], "command": { "action": "splitPane", "profile": "matrix", "commandline": "cmd.exe", "split":"auto" } }, ``` When set to `auto`, Panes will decide which direction to split based on the available space within the terminal. If the pane is wider than it is tall, the pane will introduce a new vertical split (and vice-versa). ## References ## PR Checklist * [x] Closes #3960 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Ran tests, played with it. --- .../KeyBindingsTests.cpp | 24 +++++++++- src/cascadia/TerminalApp/ActionArgs.h | 5 +++ src/cascadia/TerminalApp/ActionArgs.idl | 1 + src/cascadia/TerminalApp/Pane.cpp | 45 +++++++++++++++++-- src/cascadia/TerminalApp/Pane.h | 1 + 5 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp index 275fcd5bf78..2d939aaea4a 100644 --- a/src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp @@ -362,7 +362,9 @@ namespace TerminalAppLocalTests { "keys": ["ctrl+d"], "command": { "action": "splitPane", "split": "vertical" } }, { "keys": ["ctrl+e"], "command": { "action": "splitPane", "split": "horizontal" } }, { "keys": ["ctrl+f"], "command": { "action": "splitPane", "split": "none" } }, - { "keys": ["ctrl+g"], "command": { "action": "splitPane" } } + { "keys": ["ctrl+g"], "command": { "action": "splitPane" } }, + { "keys": ["ctrl+h"], "command": { "action": "splitPane", "split": "auto" } }, + { "keys": ["ctrl+i"], "command": { "action": "splitPane", "split": "foo" } } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); @@ -371,7 +373,7 @@ namespace TerminalAppLocalTests VERIFY_IS_NOT_NULL(appKeyBindings); VERIFY_ARE_EQUAL(0u, appKeyBindings->_keyShortcuts.size()); appKeyBindings->LayerJson(bindings0Json); - VERIFY_ARE_EQUAL(7u, appKeyBindings->_keyShortcuts.size()); + VERIFY_ARE_EQUAL(9u, appKeyBindings->_keyShortcuts.size()); { KeyChord kc{ true, false, false, static_cast('A') }; @@ -436,6 +438,24 @@ namespace TerminalAppLocalTests // Verify the args have the expected value VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::None, realArgs.SplitStyle()); } + { + KeyChord kc{ true, false, false, static_cast('H') }; + auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); + const auto& realArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(realArgs); + // Verify the args have the expected value + VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Automatic, realArgs.SplitStyle()); + } + { + KeyChord kc{ true, false, false, static_cast('I') }; + auto actionAndArgs = TestUtils::GetActionAndArgs(*appKeyBindings, kc); + VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action()); + const auto& realArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(realArgs); + // Verify the args have the expected value + VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::None, realArgs.SplitStyle()); + } } } diff --git a/src/cascadia/TerminalApp/ActionArgs.h b/src/cascadia/TerminalApp/ActionArgs.h index 5ccd365d88e..d3ef59058b8 100644 --- a/src/cascadia/TerminalApp/ActionArgs.h +++ b/src/cascadia/TerminalApp/ActionArgs.h @@ -295,6 +295,7 @@ namespace winrt::TerminalApp::implementation // TODO:GH#2550/#3475 - move these to a centralized deserializing place static constexpr std::string_view VerticalKey{ "vertical" }; static constexpr std::string_view HorizontalKey{ "horizontal" }; + static constexpr std::string_view AutomaticKey{ "auto" }; static TerminalApp::SplitState ParseSplitState(const std::string& stateString) { if (stateString == VerticalKey) @@ -305,6 +306,10 @@ namespace winrt::TerminalApp::implementation { return TerminalApp::SplitState::Horizontal; } + else if (stateString == AutomaticKey) + { + return TerminalApp::SplitState::Automatic; + } // default behavior for invalid data return TerminalApp::SplitState::None; }; diff --git a/src/cascadia/TerminalApp/ActionArgs.idl b/src/cascadia/TerminalApp/ActionArgs.idl index 2884afe0f80..92b21f6f7b5 100644 --- a/src/cascadia/TerminalApp/ActionArgs.idl +++ b/src/cascadia/TerminalApp/ActionArgs.idl @@ -25,6 +25,7 @@ namespace TerminalApp enum SplitState { + Automatic = -1, None = 0, Vertical = 1, Horizontal = 2 diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 5a53cb7fc5b..90bef8e32bf 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -935,6 +935,31 @@ std::pair, std::shared_ptr> Pane::Split(SplitState s return _Split(splitType, profile, control); } +// Method Description: +// - Converts an "automatic" split type into either Vertical or Horizontal, +// based upon the current dimensions of the Pane. +// - If any of the other SplitState values are passed in, they're returned +// unmodified. +// Arguments: +// - splitType: The SplitState to attempt to convert +// Return Value: +// - None if splitType was None, otherwise one of Horizontal or Vertical +SplitState Pane::_convertAutomaticSplitState(const SplitState& splitType) const +{ + // Careful here! If the pane doesn't yet have a size, these dimensions will + // be 0, and we'll always return Vertical. + + if (splitType == SplitState::Automatic) + { + // If the requested split type was "auto", determine which direction to + // split based on our current dimensions + const Size actualSize{ gsl::narrow_cast(_root.ActualWidth()), + gsl::narrow_cast(_root.ActualHeight()) }; + return actualSize.Width >= actualSize.Height ? SplitState::Vertical : SplitState::Horizontal; + } + return splitType; +} + // Method Description: // - Determines whether the pane can be split. // Arguments: @@ -948,7 +973,14 @@ bool Pane::_CanSplit(SplitState splitType) const Size minSize = _GetMinSize(); - if (splitType == SplitState::Vertical) + auto actualSplitType = _convertAutomaticSplitState(splitType); + + if (actualSplitType == SplitState::None) + { + return false; + } + + if (actualSplitType == SplitState::Vertical) { const auto widthMinusSeparator = actualSize.Width - CombinedPaneBorderSize; const auto newWidth = widthMinusSeparator * Half; @@ -956,7 +988,7 @@ bool Pane::_CanSplit(SplitState splitType) return newWidth > minSize.Width; } - if (splitType == SplitState::Horizontal) + if (actualSplitType == SplitState::Horizontal) { const auto heightMinusSeparator = actualSize.Height - CombinedPaneBorderSize; const auto newHeight = heightMinusSeparator * Half; @@ -978,6 +1010,13 @@ bool Pane::_CanSplit(SplitState splitType) // - The two newly created Panes std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, const GUID& profile, const TermControl& control) { + if (splitType == SplitState::None) + { + return { nullptr, nullptr }; + } + + auto actualSplitType = _convertAutomaticSplitState(splitType); + // Lock the create/close lock so that another operation won't concurrently // modify our tree std::unique_lock lock{ _createCloseLock }; @@ -991,7 +1030,7 @@ std::pair, std::shared_ptr> Pane::_Split(SplitState // parent. _gotFocusRevoker.revoke(); - _splitState = splitType; + _splitState = actualSplitType; _firstPercent = { Half }; _secondPercent = { Half }; diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index f93491171d1..7ed910f3238 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -120,6 +120,7 @@ class Pane : public std::enable_shared_from_this void _ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable const& sender, winrt::Windows::UI::Xaml::RoutedEventArgs const& e); + winrt::TerminalApp::SplitState _convertAutomaticSplitState(const winrt::TerminalApp::SplitState& splitType) const; // Function Description: // - Returns true if the given direction can be used with the given split // type.