From a313863b7e7a577eb6bb6f682db7ac03722338f3 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 10 Dec 2020 07:01:46 -0600 Subject: [PATCH 1/6] Add a size param to splitPane to control the size of the created pane --- .../TerminalApp/AppActionHandlers.cpp | 6 +- src/cascadia/TerminalApp/AppLogic.cpp | 3 +- src/cascadia/TerminalApp/Pane.cpp | 19 ++++-- src/cascadia/TerminalApp/Pane.h | 2 + .../Resources/en-US/Resources.resw | 60 ++++++++++--------- src/cascadia/TerminalApp/TerminalPage.cpp | 7 ++- src/cascadia/TerminalApp/TerminalPage.h | 5 +- src/cascadia/TerminalApp/TerminalTab.cpp | 7 ++- src/cascadia/TerminalApp/TerminalTab.h | 5 +- .../TerminalSettingsModel/ActionArgs.cpp | 9 ++- .../TerminalSettingsModel/ActionArgs.h | 9 +++ .../TerminalSettingsModel/ActionArgs.idl | 1 + .../TerminalWarnings.idl | 1 + 13 files changed, 90 insertions(+), 44 deletions(-) diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index a558d9e162c..d56df377237 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -122,7 +122,11 @@ namespace winrt::TerminalApp::implementation } else if (const auto& realArgs = args.ActionArgs().try_as()) { - _SplitPane(realArgs.SplitStyle(), realArgs.SplitMode(), realArgs.TerminalArgs()); + _SplitPane(realArgs.SplitStyle(), + realArgs.SplitMode(), + // This is safe, we're alrady filtering so the value is (0, 1) + ::base::saturated_cast(realArgs.SplitSize()), + realArgs.TerminalArgs()); args.Handled(true); } } diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 1f6c1e7a227..a063010ea80 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -42,7 +42,8 @@ static const std::array(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(SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels { USES_RESOURCE(L"NoProfilesText"), diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index b561ea6acd3..810648aebd3 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -1270,6 +1270,7 @@ bool Pane::CanSplit(SplitState splitType) // Note: // - This method is highly similar to Pane::PreCalculateAutoSplit std::optional Pane::PreCalculateCanSplit(const std::shared_ptr target, + // TODO: const float splitSize SplitState splitType, const winrt::Windows::Foundation::Size availableSpace) const { @@ -1347,23 +1348,26 @@ std::optional Pane::PreCalculateCanSplit(const std::shared_ptr targe // - control: A TermControl to use in the new pane. // Return Value: // - The two newly created Panes -std::pair, std::shared_ptr> Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control) +std::pair, std::shared_ptr> 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: @@ -1439,7 +1443,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::_Split(SplitState splitType, const GUID& profile, const TermControl& control) +std::pair, std::shared_ptr> Pane::_Split(SplitState splitType, + const float splitSize, + const GUID& profile, + const TermControl& control) { if (splitType == SplitState::None) { @@ -1464,7 +1471,7 @@ std::pair, std::shared_ptr> Pane::_Split(SplitState _gotFocusRevoker.revoke(); _splitState = actualSplitType; - _desiredSplitPosition = Half; + _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. diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index 925dd436c9a..ebaf0cb707f 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -60,6 +60,7 @@ class Pane : public std::enable_shared_from_this bool CanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType); std::pair, std::shared_ptr> 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; @@ -116,6 +117,7 @@ class Pane : public std::enable_shared_from_this bool _CanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType); std::pair, std::shared_ptr> _Split(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, + const float splitSize, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control); diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index d1973062029..c3c9a2784e0 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -1,17 +1,17 @@  - @@ -249,6 +249,10 @@ 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. {Locked="\"colorScheme\"","\"name\"","\"schemes\""} + + Found a "splitPane" command with an invalid "size". This command will be ignored. Make sure the size is between 0 and 1, exclusive. + {Locked="\"splitPane\"","\"size\""} + An optional command, with arguments, to be spawned in the new tab or pane @@ -510,4 +514,4 @@ Warning - \ No newline at end of file + diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index a7320c31067..c8e70dbab69 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -208,6 +208,7 @@ namespace winrt::TerminalApp::implementation { page->_SplitPane(SplitState::Automatic, SplitType::Manual, + 0.5f, nullptr); } else @@ -559,6 +560,7 @@ namespace winrt::TerminalApp::implementation { page->_SplitPane(SplitState::Automatic, SplitType::Manual, + 0.5f, newTerminalArgs); } else @@ -808,7 +810,7 @@ namespace winrt::TerminalApp::implementation TermControl newControl{ settings, debugConnection }; _RegisterTerminalEvents(newControl, *newTabImpl); // Split (auto) with the debug tap. - newTabImpl->SplitPane(SplitState::Automatic, profileGuid, newControl); + newTabImpl->SplitPane(SplitState::Automatic, .5f, profileGuid, newControl); } // This kicks off TabView::SelectionChanged, in response to which @@ -1574,6 +1576,7 @@ namespace winrt::TerminalApp::implementation // configurations. See CascadiaSettings::BuildSettings for more details. void TerminalPage::_SplitPane(const SplitState splitType, const SplitType splitMode, + const float splitSize, const NewTerminalArgs& newTerminalArgs) { // Do nothing if we're requesting no split. @@ -1656,7 +1659,7 @@ namespace winrt::TerminalApp::implementation _UnZoomIfNeeded(); - focusedTab->SplitPane(realSplitType, realGuid, newControl); + focusedTab->SplitPane(realSplitType, splitSize, realGuid, newControl); } CATCH_LOG(); } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index ed87a877180..381b4da5d90 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -190,7 +190,10 @@ namespace winrt::TerminalApp::implementation // Todo: add more event implementations here // MSFT:20641986: Add keybindings for New Window void _Scroll(ScrollDirection scrollDirection, const Windows::Foundation::IReference& rowsToScroll); - void _SplitPane(const Microsoft::Terminal::Settings::Model::SplitState splitType, const Microsoft::Terminal::Settings::Model::SplitType splitMode = Microsoft::Terminal::Settings::Model::SplitType::Manual, const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr); + void _SplitPane(const Microsoft::Terminal::Settings::Model::SplitState splitType, + const Microsoft::Terminal::Settings::Model::SplitType splitMode = Microsoft::Terminal::Settings::Model::SplitType::Manual, + const float splitSize = 0.5f, + const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs = nullptr); void _ResizePane(const Microsoft::Terminal::Settings::Model::Direction& direction); void _ScrollPage(ScrollDirection scrollDirection); void _ScrollToBufferEdge(ScrollDirection scrollDirection); diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 919e44ea2c9..68c690a5de5 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -266,9 +266,12 @@ namespace winrt::TerminalApp::implementation // - control: A TermControl to use in the new pane. // Return Value: // - - void TerminalTab::SplitPane(SplitState splitType, const GUID& profile, TermControl& control) + void TerminalTab::SplitPane(SplitState splitType, + const float splitSize, + const GUID& profile, + TermControl& control) { - auto [first, second] = _activePane->Split(splitType, profile, control); + auto [first, second] = _activePane->Split(splitType, splitSize, profile, control); _activePane = first; _AttachEventHandlersToControl(control); diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index b228b45a1de..721b4bc8e3d 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -31,7 +31,10 @@ namespace winrt::TerminalApp::implementation winrt::fire_and_forget Scroll(const int delta); bool CanSplitPane(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType); - void SplitPane(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control); + void SplitPane(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, + const float splitSize, + const GUID& profile, + winrt::Microsoft::Terminal::TerminalControl::TermControl& control); winrt::fire_and_forget UpdateIcon(const winrt::hstring iconPath); diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp index 611e65793a3..3817bbc4a03 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp @@ -229,8 +229,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::hstring SplitPaneArgs::GenerateName() const { // The string will be similar to the following: - // * "Duplicate pane[, split: ][, new terminal arguments...]" - // * "Split pane[, split: ][, new terminal arguments...]" + // * "Duplicate pane[, split: ][, size: %][, new terminal arguments...]" + // * "Split pane[, split: ][, size: %][, new terminal arguments...]" // // Direction will only be added to the string if the split direction is // not "auto". @@ -260,6 +260,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation break; } + if (_SplitSize != .5f) + { + ss << L"size: " << (_SplitSize * 100) << L"%, "; + } + winrt::hstring newTerminalArgsStr; if (_TerminalArgs) { diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h index dbe385059b1..c7b02341bb3 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.h +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h @@ -387,9 +387,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation GETSET_PROPERTY(SplitState, SplitStyle, SplitState::Automatic); GETSET_PROPERTY(Model::NewTerminalArgs, TerminalArgs, nullptr); GETSET_PROPERTY(SplitType, SplitMode, SplitType::Manual); + GETSET_PROPERTY(double, SplitSize, .5); static constexpr std::string_view SplitKey{ "split" }; static constexpr std::string_view SplitModeKey{ "splitMode" }; + static constexpr std::string_view SplitSizeKey{ "size" }; public: hstring GenerateName() const; @@ -402,6 +404,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return otherAsUs->_SplitStyle == _SplitStyle && (otherAsUs->_TerminalArgs ? otherAsUs->_TerminalArgs.Equals(_TerminalArgs) : otherAsUs->_TerminalArgs == _TerminalArgs) && + otherAsUs->_SplitSize == _SplitSize && otherAsUs->_SplitMode == _SplitMode; } return false; @@ -413,6 +416,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation args->_TerminalArgs = NewTerminalArgs::FromJson(json); JsonUtils::GetValueForKey(json, SplitKey, args->_SplitStyle); JsonUtils::GetValueForKey(json, SplitModeKey, args->_SplitMode); + JsonUtils::GetValueForKey(json, SplitSizeKey, args->_SplitSize); + if (args->_SplitSize >= 1 || args->_SplitSize <= 0) + { + return { nullptr, { SettingsLoadWarnings::InvalidSplitSize } }; + } return { *args, {} }; } IActionArgs Copy() const @@ -421,6 +429,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation copy->_SplitStyle = _SplitStyle; copy->_TerminalArgs = _TerminalArgs.Copy(); copy->_SplitMode = _SplitMode; + copy->_SplitSize = _SplitSize; return *copy; } }; diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl index 9bb545ece54..5a5d44e51ea 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl @@ -130,6 +130,7 @@ namespace Microsoft.Terminal.Settings.Model SplitState SplitStyle { get; }; NewTerminalArgs TerminalArgs { get; }; SplitType SplitMode { get; }; + Double SplitSize { get; }; }; [default_interface] runtimeclass OpenSettingsArgs : IActionArgs diff --git a/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl b/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl index 1446ff12ea4..ff8b9636995 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl +++ b/src/cascadia/TerminalSettingsModel/TerminalWarnings.idl @@ -19,6 +19,7 @@ namespace Microsoft.Terminal.Settings.Model FailedToParseCommandJson = 9, FailedToWriteToSettings = 10, InvalidColorSchemeInCmd = 11, + InvalidSplitSize = 12, WARNINGS_SIZE // IMPORTANT: This MUST be the last value in this enum. It's an unused placeholder. }; From 49759212807861f367951ae372ef625c25c50761 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 10 Dec 2020 07:32:43 -0600 Subject: [PATCH 2/6] Add to the commandline args too --- src/cascadia/TerminalApp/AppCommandlineArgs.cpp | 6 +++++- src/cascadia/TerminalApp/AppCommandlineArgs.h | 1 + src/cascadia/TerminalApp/Resources/en-US/Resources.resw | 3 +++ src/cascadia/TerminalSettingsModel/ActionArgs.h | 4 ++++ src/cascadia/TerminalSettingsModel/ActionArgs.idl | 1 + 5 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp index 7b7e2138265..464cea2ad11 100644 --- a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp +++ b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp @@ -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 @@ -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); }); diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.h b/src/cascadia/TerminalApp/AppCommandlineArgs.h index 6a7d08b0e7d..835022ec600 100644 --- a/src/cascadia/TerminalApp/AppCommandlineArgs.h +++ b/src/cascadia/TerminalApp/AppCommandlineArgs.h @@ -84,6 +84,7 @@ class TerminalApp::AppCommandlineArgs final bool _splitVertical{ false }; bool _splitHorizontal{ false }; + float _splitPaneSize{ 0.5f }; int _focusTabIndex{ -1 }; bool _focusNextTab{ false }; diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index c3c9a2784e0..e4623e834a8 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -272,6 +272,9 @@ Move focus the tab at the given index + + Specify the size as a percentage of the parent pane. Valid values are between (0,1), exclusive. + Create a new tab diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h index c7b02341bb3..2297110b0a9 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.h +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h @@ -379,6 +379,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation struct SplitPaneArgs : public SplitPaneArgsT { SplitPaneArgs() = default; + SplitPaneArgs(SplitState style, double size, const Model::NewTerminalArgs& terminalArgs) : + _SplitStyle{ style }, + _SplitSize{ size }, + _TerminalArgs{ terminalArgs } {}; SplitPaneArgs(SplitState style, const Model::NewTerminalArgs& terminalArgs) : _SplitStyle{ style }, _TerminalArgs{ terminalArgs } {}; diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl index 5a5d44e51ea..c751ebd3ba6 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl @@ -124,6 +124,7 @@ namespace Microsoft.Terminal.Settings.Model [default_interface] runtimeclass SplitPaneArgs : IActionArgs { + SplitPaneArgs(SplitState split, Double size, NewTerminalArgs terminalArgs); SplitPaneArgs(SplitState split, NewTerminalArgs terminalArgs); SplitPaneArgs(SplitType splitMode); From d0f543a1b2bbe6f713f10a639e2fe5d8140bcc67 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 10 Dec 2020 08:18:31 -0600 Subject: [PATCH 3/6] You know what else everybody likes? Tests. Have you ever met a person, you say, 'Let's add some tests,' they say, 'Heck no, I don't like no tests'? --- .../LocalTests_SettingsModel/CommandTests.cpp | 52 ++++++++++++++++++- .../LocalTests_TerminalApp/TabTests.cpp | 4 +- tools/runut.cmd | 1 + 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp index b3a0951f78d..5aea3598502 100644 --- a/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/CommandTests.cpp @@ -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); @@ -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" } }, + { "name": "command6", "command": { "action": "splitPane", "size": 0.25 } }, ])" }; const auto commands0Json = VerifyParseSucceeded(commands0String); @@ -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"); @@ -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"); @@ -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"); @@ -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"); @@ -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(); + 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 commands = winrt::single_threaded_map(); + 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(); + 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. diff --git a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp index b0b391cbd27..e5a1480f333 100644 --- a/src/cascadia/LocalTests_TerminalApp/TabTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/TabTests.cpp @@ -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)); @@ -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)); diff --git a/tools/runut.cmd b/tools/runut.cmd index 9df937cf1a9..6f28b883fb5 100644 --- a/tools/runut.cmd +++ b/tools/runut.cmd @@ -24,5 +24,6 @@ call %TAEF% ^ %OPENCON%\bin\%PLATFORM%\%_LAST_BUILD_CONF%\til.unit.tests.dll ^ %OPENCON%\bin\%PLATFORM%\%_LAST_BUILD_CONF%\UnitTests_TerminalApp\Terminal.App.Unit.Tests.dll ^ %_TestHostAppPath%\TerminalApp.LocalTests.dll ^ + %_TestHostAppPath%\SettingsModel.LocalTests.dll ^ %* From 05d778348cbcc038064e4e292205ce3933926511 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 15 Dec 2020 05:26:24 -0600 Subject: [PATCH 4/6] good bot --- src/cascadia/TerminalApp/AppActionHandlers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 76431d3d10d..7f72182dbf1 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -124,7 +124,7 @@ namespace winrt::TerminalApp::implementation { _SplitPane(realArgs.SplitStyle(), realArgs.SplitMode(), - // This is safe, we're alrady filtering so the value is (0, 1) + // This is safe, we're already filtering so the value is (0, 1) ::base::saturated_cast(realArgs.SplitSize()), realArgs.TerminalArgs()); args.Handled(true); From c7839b9ba6c6a817de2c3fdc646ea89bf39ef7e2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 15 Dec 2020 05:45:33 -0600 Subject: [PATCH 5/6] update the schema too, because I'm a good person --- doc/cascadia/profiles.schema.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 79159589474..7b77d302c25 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -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" } } } From 40babcffd980c89f61a1cd9782f5769f0d3a515e Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 17 Dec 2020 08:48:37 -0600 Subject: [PATCH 6/6] also account for the size in PrecalculateCanSplit, remove the old, unused CanSplit --- src/cascadia/TerminalApp/Pane.cpp | 84 +++-------------------- src/cascadia/TerminalApp/Pane.h | 6 +- src/cascadia/TerminalApp/TerminalPage.cpp | 4 +- src/cascadia/TerminalApp/TerminalTab.cpp | 17 ++--- src/cascadia/TerminalApp/TerminalTab.h | 5 +- 5 files changed, 23 insertions(+), 93 deletions(-) diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 2a5b4ce4740..a9f1767e13d 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -21,7 +21,6 @@ using namespace TerminalApp; static const int PaneBorderSize = 2; static const int CombinedPaneBorderSize = 2 * PaneBorderSize; -static const float Half = 0.50f; // WARNING: Don't do this! This won't work // Duration duration{ std::chrono::milliseconds{ 200 } }; @@ -1216,32 +1215,6 @@ void Pane::_SetupEntranceAnimation() setupAnimation(secondSize, false); } -// Method Description: -// - Determines whether the pane can be split -// Arguments: -// - splitType: what type of split we want to create. -// Return Value: -// - True if the pane can be split. False otherwise. -bool Pane::CanSplit(SplitState splitType) -{ - if (_IsLeaf()) - { - return _CanSplit(splitType); - } - - if (_firstChild->_HasFocusedChild()) - { - return _firstChild->CanSplit(splitType); - } - - if (_secondChild->_HasFocusedChild()) - { - return _secondChild->CanSplit(splitType); - } - - return false; -} - // Method Description: // - This is a helper to determine if a given Pane can be split, but without // using the ActualWidth() and ActualHeight() methods. This is used during @@ -1271,14 +1244,16 @@ bool Pane::CanSplit(SplitState splitType) // Note: // - This method is highly similar to Pane::PreCalculateAutoSplit std::optional Pane::PreCalculateCanSplit(const std::shared_ptr target, - // TODO: const float splitSize SplitState splitType, + const float splitSize, const winrt::Windows::Foundation::Size availableSpace) const { if (_IsLeaf()) { if (target.get() == this) { + const auto firstPrecent = 1.0f - splitSize; + const auto secondPercent = splitSize; // If this pane is a leaf, and it's the pane we're looking for, use // the available space to calculate which direction to split in. const Size minSize = _GetMinSize(); @@ -1291,17 +1266,19 @@ std::optional Pane::PreCalculateCanSplit(const std::shared_ptr targe else if (splitType == SplitState::Vertical) { const auto widthMinusSeparator = availableSpace.Width - CombinedPaneBorderSize; - const auto newWidth = widthMinusSeparator * Half; + const auto newFirstWidth = widthMinusSeparator * firstPrecent; + const auto newSecondWidth = widthMinusSeparator * secondPercent; - return { newWidth > minSize.Width }; + return { newFirstWidth > minSize.Width && newSecondWidth > minSize.Width }; } else if (splitType == SplitState::Horizontal) { const auto heightMinusSeparator = availableSpace.Height - CombinedPaneBorderSize; - const auto newHeight = heightMinusSeparator * Half; + const auto newFirstHeight = heightMinusSeparator * firstPrecent; + const auto newSecondHeight = heightMinusSeparator * secondPercent; - return { newHeight > minSize.Height }; + return { newFirstHeight > minSize.Height && newSecondHeight > minSize.Height }; } } else @@ -1330,8 +1307,8 @@ std::optional Pane::PreCalculateCanSplit(const std::shared_ptr targe (availableSpace.Height - firstHeight) - PaneBorderSize : availableSpace.Height; - const auto firstResult = _firstChild->PreCalculateCanSplit(target, splitType, { firstWidth, firstHeight }); - return firstResult.has_value() ? firstResult : _secondChild->PreCalculateCanSplit(target, splitType, { secondWidth, secondHeight }); + const auto firstResult = _firstChild->PreCalculateCanSplit(target, splitType, splitSize, { firstWidth, firstHeight }); + return firstResult.has_value() ? firstResult : _secondChild->PreCalculateCanSplit(target, splitType, splitSize, { secondWidth, secondHeight }); } // We should not possibly be getting here - both the above branches should @@ -1396,45 +1373,6 @@ SplitState Pane::_convertAutomaticSplitState(const SplitState& splitType) const return splitType; } -// Method Description: -// - Determines whether the pane can be split. -// Arguments: -// - splitType: what type of split we want to create. -// Return Value: -// - True if the pane can be split. False otherwise. -bool Pane::_CanSplit(SplitState splitType) -{ - const Size actualSize{ gsl::narrow_cast(_root.ActualWidth()), - gsl::narrow_cast(_root.ActualHeight()) }; - - const Size minSize = _GetMinSize(); - - 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; - - return newWidth > minSize.Width; - } - - if (actualSplitType == SplitState::Horizontal) - { - const auto heightMinusSeparator = actualSize.Height - CombinedPaneBorderSize; - const auto newHeight = heightMinusSeparator * Half; - - return newHeight > minSize.Height; - } - - return false; -} - // Method Description: // - Does the bulk of the work of creating a new split. Initializes our UI, // creates a new Pane to host the control, registers event handlers. diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index ecf2eebeee2..01e6cc237c8 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -58,15 +58,16 @@ class Pane : public std::enable_shared_from_this bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction); bool NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); - bool CanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType); std::pair, std::shared_ptr> 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; - std::optional PreCalculateAutoSplit(const std::shared_ptr target, const winrt::Windows::Foundation::Size parentSize) const; + std::optional PreCalculateAutoSplit(const std::shared_ptr target, + const winrt::Windows::Foundation::Size parentSize) const; std::optional PreCalculateCanSplit(const std::shared_ptr target, winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, + const float splitSize, const winrt::Windows::Foundation::Size availableSpace) const; void Shutdown(); void Close(); @@ -121,7 +122,6 @@ class Pane : public std::enable_shared_from_this bool _HasFocusedChild() const noexcept; void _SetupChildCloseHandlers(); - bool _CanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType); std::pair, std::shared_ptr> _Split(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, const float splitSize, const GUID& profile, diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 9bcc221a300..e8c2ca2f6f9 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -811,7 +811,7 @@ namespace winrt::TerminalApp::implementation TermControl newControl{ settings, debugConnection }; _RegisterTerminalEvents(newControl, *newTabImpl); // Split (auto) with the debug tap. - newTabImpl->SplitPane(SplitState::Automatic, .5f, profileGuid, newControl); + newTabImpl->SplitPane(SplitState::Automatic, 0.5f, profileGuid, newControl); } // This kicks off TabView::SelectionChanged, in response to which @@ -1650,7 +1650,7 @@ namespace winrt::TerminalApp::implementation realSplitType = focusedTab->PreCalculateAutoSplit(availableSpace); } - const auto canSplit = focusedTab->PreCalculateCanSplit(realSplitType, availableSpace); + const auto canSplit = focusedTab->PreCalculateCanSplit(realSplitType, splitSize, availableSpace); if (!canSplit) { return; diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 7e6685dc805..a0f1bc994cb 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -286,17 +286,6 @@ namespace winrt::TerminalApp::implementation control.ScrollViewport(currentOffset + delta); } - // Method Description: - // - Determines whether the focused pane has sufficient space to be split. - // Arguments: - // - splitType: The type of split we want to create. - // Return Value: - // - True if the focused pane can be split. False otherwise. - bool TerminalTab::CanSplitPane(SplitState splitType) - { - return _activePane->CanSplit(splitType); - } - // Method Description: // - Split the focused pane in our tree of panes, and place the // given TermControl into the newly created pane. @@ -939,9 +928,11 @@ namespace winrt::TerminalApp::implementation return _rootPane->PreCalculateAutoSplit(_activePane, availableSpace).value_or(SplitState::Vertical); } - bool TerminalTab::PreCalculateCanSplit(SplitState splitType, winrt::Windows::Foundation::Size availableSpace) const + bool TerminalTab::PreCalculateCanSplit(SplitState splitType, + const float splitSize, + winrt::Windows::Foundation::Size availableSpace) const { - return _rootPane->PreCalculateCanSplit(_activePane, splitType, availableSpace).value_or(false); + return _rootPane->PreCalculateCanSplit(_activePane, splitType, splitSize, availableSpace).value_or(false); } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index 389799f2629..d5a4dfff998 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -30,7 +30,6 @@ namespace winrt::TerminalApp::implementation winrt::fire_and_forget Scroll(const int delta); - bool CanSplitPane(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType); void SplitPane(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, const float splitSize, const GUID& profile, @@ -41,7 +40,9 @@ namespace winrt::TerminalApp::implementation float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const; winrt::Microsoft::Terminal::Settings::Model::SplitState PreCalculateAutoSplit(winrt::Windows::Foundation::Size rootSize) const; - bool PreCalculateCanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, winrt::Windows::Foundation::Size availableSpace) const; + bool PreCalculateCanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, + const float splitSize, + winrt::Windows::Foundation::Size availableSpace) const; void ResizeContent(const winrt::Windows::Foundation::Size& newSize); void ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);