add rowsToScroll to scrollUp/Down w/ fallback to system default (#7924)

- The number of lines to move upon scroll up scroll down can be defined
  in ScrollUp and ScrollDown commands (parameter is called
  "rowsToScroll").
- If the number are not provided, use the system default (the one we are
  using for mouse scrolls), rather than 1 line.

## Validation Steps Performed
* Manual testing
* Added custom bindings for scroll commands with different values,
  verified they and the default appear and behave as expected
* Checked that invalid values are not allowed

Closes #5078

doc/cascadia/profiles.schema.json

@@ -441,6 +441,38 @@
         }
       ]
     },
+    "ScrollUpAction": {
+      "description": "Arguments for a scrollUp action",
+      "allOf": [
+        { "$ref": "#/definitions/ShortcutAction" },
+        {
+          "properties": {
+            "action": { "type": "string", "pattern": "scrollUp" },
+            "rowsToScroll": {
+              "type": ["integer", "null"],
+              "default": null,
+              "description": "Scroll up rowsToScroll lines. If no value is provided, use the system-level defaults."
+            }
+          }
+        }
+      ]
+    },
+    "ScrollDownAction": {
+      "description": "Arguments for a scrollDown action",
+      "allOf": [
+        { "$ref": "#/definitions/ShortcutAction" },
+        {
+          "properties": {
+            "action": { "type": "string", "pattern": "scrollDown" },
+            "rowsToScroll": {
+              "type": ["integer", "null"],
+              "default": null,
+              "description": "Scroll down rowsToScroll lines. If no value is provided, use the system-level defaults."
+            }
+          }
+        }
+      ]
+    },
     "Keybinding": {
       "additionalProperties": false,
       "properties": {
@@ -462,6 +494,8 @@
               { "$ref": "#/definitions/WtAction" },
               { "$ref": "#/definitions/CloseOtherTabsAction" },
               { "$ref": "#/definitions/CloseTabsAfterAction" },
+              { "$ref": "#/definitions/ScrollUpAction" },
+              { "$ref": "#/definitions/ScrollDownAction" },
               { "type": "null" }
             ]
         },

src/cascadia/LocalTests_SettingsModel/KeyBindingsTests.cpp

@@ -46,6 +46,8 @@ namespace SettingsModelLocalTests
 
         TEST_METHOD(TestSetTabColorArgs);
 
+        TEST_METHOD(TestScrollArgs);
+
         TEST_CLASS_SETUP(ClassSetup)
         {
             InitializeJsonReader();
@@ -452,4 +454,89 @@ namespace SettingsModelLocalTests
             VERIFY_IS_FALSE(realArgs.SingleLine());
         }
     }
+
+    void KeyBindingsTests::TestScrollArgs()
+    {
+        const std::string bindings0String{ R"([
+            { "keys": ["up"], "command": "scrollUp" },
+            { "keys": ["down"], "command": "scrollDown" },
+            { "keys": ["ctrl+up"], "command": { "action": "scrollUp" } },
+            { "keys": ["ctrl+down"], "command": { "action": "scrollDown" } },
+            { "keys": ["ctrl+shift+up"], "command": { "action": "scrollUp", "rowsToScroll": 10 } },
+            { "keys": ["ctrl+shift+down"], "command": { "action": "scrollDown", "rowsToScroll": 10 } }
+        ])" };
+
+        const auto bindings0Json = VerifyParseSucceeded(bindings0String);
+
+        auto keymap = winrt::make_self<implementation::KeyMapping>();
+        VERIFY_IS_NOT_NULL(keymap);
+        VERIFY_ARE_EQUAL(0u, keymap->_keyShortcuts.size());
+        keymap->LayerJson(bindings0Json);
+        VERIFY_ARE_EQUAL(6u, keymap->_keyShortcuts.size());
+
+        {
+            KeyChord kc{ false, false, false, static_cast<int32_t>(VK_UP) };
+            auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
+            VERIFY_ARE_EQUAL(ShortcutAction::ScrollUp, actionAndArgs.Action());
+            const auto& realArgs = actionAndArgs.Args().try_as<ScrollUpArgs>();
+            VERIFY_IS_NOT_NULL(realArgs);
+            // Verify the args have the expected value
+            VERIFY_IS_NULL(realArgs.RowsToScroll());
+        }
+        {
+            KeyChord kc{ false, false, false, static_cast<int32_t>(VK_DOWN) };
+            auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
+            VERIFY_ARE_EQUAL(ShortcutAction::ScrollDown, actionAndArgs.Action());
+            const auto& realArgs = actionAndArgs.Args().try_as<ScrollDownArgs>();
+            VERIFY_IS_NOT_NULL(realArgs);
+            // Verify the args have the expected value
+            VERIFY_IS_NULL(realArgs.RowsToScroll());
+        }
+        {
+            KeyChord kc{ true, false, false, static_cast<int32_t>(VK_UP) };
+            auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
+            VERIFY_ARE_EQUAL(ShortcutAction::ScrollUp, actionAndArgs.Action());
+            const auto& realArgs = actionAndArgs.Args().try_as<ScrollUpArgs>();
+            VERIFY_IS_NOT_NULL(realArgs);
+            // Verify the args have the expected value
+            VERIFY_IS_NULL(realArgs.RowsToScroll());
+        }
+        {
+            KeyChord kc{ true, false, false, static_cast<int32_t>(VK_DOWN) };
+            auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
+            VERIFY_ARE_EQUAL(ShortcutAction::ScrollDown, actionAndArgs.Action());
+            const auto& realArgs = actionAndArgs.Args().try_as<ScrollDownArgs>();
+            VERIFY_IS_NOT_NULL(realArgs);
+            // Verify the args have the expected value
+            VERIFY_IS_NULL(realArgs.RowsToScroll());
+        }
+        {
+            KeyChord kc{ true, false, true, static_cast<int32_t>(VK_UP) };
+            auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
+            VERIFY_ARE_EQUAL(ShortcutAction::ScrollUp, actionAndArgs.Action());
+            const auto& realArgs = actionAndArgs.Args().try_as<ScrollUpArgs>();
+            VERIFY_IS_NOT_NULL(realArgs);
+            // Verify the args have the expected value
+            VERIFY_IS_NOT_NULL(realArgs.RowsToScroll());
+            VERIFY_ARE_EQUAL(10u, realArgs.RowsToScroll().Value());
+        }
+        {
+            KeyChord kc{ true, false, true, static_cast<int32_t>(VK_DOWN) };
+            auto actionAndArgs = ::TestUtils::GetActionAndArgs(*keymap, kc);
+            VERIFY_ARE_EQUAL(ShortcutAction::ScrollDown, actionAndArgs.Action());
+            const auto& realArgs = actionAndArgs.Args().try_as<ScrollDownArgs>();
+            VERIFY_IS_NOT_NULL(realArgs);
+            // Verify the args have the expected value
+            VERIFY_IS_NOT_NULL(realArgs.RowsToScroll());
+            VERIFY_ARE_EQUAL(10u, realArgs.RowsToScroll().Value());
+        }
+        {
+            const std::string bindingsInvalidString{ R"([{ "keys": ["up"], "command": { "action": "scrollDown", "rowsToScroll": -1 } }])" };
+            const auto bindingsInvalidJson = VerifyParseSucceeded(bindingsInvalidString);
+            auto invalidKeyMap = winrt::make_self<implementation::KeyMapping>();
+            VERIFY_IS_NOT_NULL(invalidKeyMap);
+            VERIFY_ARE_EQUAL(0u, invalidKeyMap->_keyShortcuts.size());
+            VERIFY_THROWS(invalidKeyMap->LayerJson(bindingsInvalidJson);, std::exception);
+        }
+    }
 }

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -65,15 +65,23 @@ namespace winrt::TerminalApp::implementation
     void TerminalPage::_HandleScrollUp(const IInspectable& /*sender*/,
                                        const ActionEventArgs& args)
     {
-        _Scroll(-1);
-        args.Handled(true);
+        const auto& realArgs = args.ActionArgs().try_as<ScrollUpArgs>();
+        if (realArgs)
+        {
+            _Scroll(ScrollUp, realArgs.RowsToScroll());
+            args.Handled(true);
+        }
     }
 
     void TerminalPage::_HandleScrollDown(const IInspectable& /*sender*/,
                                          const ActionEventArgs& args)
     {
-        _Scroll(1);
-        args.Handled(true);
+        const auto& realArgs = args.ActionArgs().try_as<ScrollDownArgs>();
+        if (realArgs)
+        {
+            _Scroll(ScrollDown, realArgs.RowsToScroll());
+            args.Handled(true);
+        }
     }
 
     void TerminalPage::_HandleNextTab(const IInspectable& /*sender*/,
@@ -143,14 +151,14 @@ namespace winrt::TerminalApp::implementation
     void TerminalPage::_HandleScrollUpPage(const IInspectable& /*sender*/,
                                            const ActionEventArgs& args)
     {
-        _ScrollPage(-1);
+        _ScrollPage(ScrollUp);
         args.Handled(true);
     }
 
     void TerminalPage::_HandleScrollDownPage(const IInspectable& /*sender*/,
                                              const ActionEventArgs& args)
     {
-        _ScrollPage(1);
+        _ScrollPage(ScrollDown);
         args.Handled(true);
     }
 

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -89,6 +89,10 @@ namespace winrt::TerminalApp::implementation
             _RefreshUIForSettingsReload();
         }
 
+        // Upon settings update we reload the system settings for scrolling as well.
+        // TODO: consider reloading this value periodically.
+        _systemRowsToScroll = _ReadSystemRowsToScroll();
+
         auto weakThis{ get_weak() };
         co_await winrt::resume_foreground(Dispatcher());
         if (auto page{ weakThis.get() })
@@ -1413,16 +1417,30 @@ namespace winrt::TerminalApp::implementation
 
     // Method Description:
     // - Move the viewport of the terminal of the currently focused tab up or
-    //      down a number of lines. Negative values of `delta` will move the
-    //      view up, and positive values will move the viewport down.
+    //      down a number of lines.
     // Arguments:
-    // - delta: a number of lines to move the viewport relative to the current viewport.
-    void TerminalPage::_Scroll(int delta)
+    // - scrollDirection: ScrollUp will move the viewport up, ScrollDown will move the viewport down
+    // - rowsToScroll: a number of lines to move the viewport. If not provided we will use a system default.
+    void TerminalPage::_Scroll(ScrollDirection scrollDirection, const Windows::Foundation::IReference<uint32_t>& rowsToScroll)
     {
         if (auto index{ _GetFocusedTabIndex() })
         {
             auto focusedTab{ _GetStrongTabImpl(*index) };
-            focusedTab->Scroll(delta);
+            uint32_t realRowsToScroll;
+            if (rowsToScroll == nullptr)
+            {
+                // The magic value of WHEEL_PAGESCROLL indicates that we need to scroll the entire page
+                realRowsToScroll = _systemRowsToScroll == WHEEL_PAGESCROLL ?
+                                       focusedTab->GetActiveTerminalControl().GetViewHeight() :
+                                       _systemRowsToScroll;
+            }
+            else
+            {
+                // use the custom value specified in the command
+                realRowsToScroll = rowsToScroll.Value();
+            }
+            auto scrollDelta = _ComputeScrollDelta(scrollDirection, realRowsToScroll);
+            focusedTab->Scroll(scrollDelta);
         }
     }
 
@@ -1540,12 +1558,9 @@ namespace winrt::TerminalApp::implementation
     // Method Description:
     // - Move the viewport of the terminal of the currently focused tab up or
     //      down a page. The page length will be dependent on the terminal view height.
-    //      Negative values of `delta` will move the view up by one page, and positive values
-    //      will move the viewport down by one page.
     // Arguments:
-    // - delta: The direction to move the view relative to the current viewport(it
-    //      is clamped between -1 and 1)
-    void TerminalPage::_ScrollPage(int delta)
+    // - scrollDirection: ScrollUp will move the viewport up, ScrollDown will move the viewport down
+    void TerminalPage::_ScrollPage(ScrollDirection scrollDirection)
     {
         auto indexOpt = _GetFocusedTabIndex();
         // Do nothing if for some reason, there's no tab in focus. We don't want to crash.
@@ -1554,11 +1569,11 @@ namespace winrt::TerminalApp::implementation
             return;
         }
 
-        delta = std::clamp(delta, -1, 1);
         const auto control = _GetActiveControl();
         const auto termHeight = control.GetViewHeight();
         auto focusedTab{ _GetStrongTabImpl(*indexOpt) };
-        focusedTab->Scroll(termHeight * delta);
+        auto scrollDelta = _ComputeScrollDelta(scrollDirection, termHeight);
+        focusedTab->Scroll(scrollDelta);
     }
 
     // Method Description:
@@ -2552,6 +2567,39 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
+    // Method Description:
+    // - Computes the delta for scrolling the tab's viewport.
+    // Arguments:
+    // - scrollDirection - direction (up / down) to scroll
+    // - rowsToScroll - the number of rows to scroll
+    // Return Value:
+    // - delta - Signed delta, where a negative value means scrolling up.
+    int TerminalPage::_ComputeScrollDelta(ScrollDirection scrollDirection, const uint32_t rowsToScroll)
+    {
+        return scrollDirection == ScrollUp ? -1 * rowsToScroll : rowsToScroll;
+    }
+
+    // Method Description:
+    // - Reads system settings for scrolling (based on the step of the mouse scroll).
+    // Upon failure fallbacks to default.
+    // Return Value:
+    // - The number of rows to scroll or a magic value of WHEEL_PAGESCROLL
+    // indicating that we need to scroll an entire view height
+    uint32_t TerminalPage::_ReadSystemRowsToScroll()
+    {
+        uint32_t systemRowsToScroll;
+        if (!SystemParametersInfoW(SPI_GETWHEELSCROLLLINES, 0, &systemRowsToScroll, 0))
+        {
+            LOG_LAST_ERROR();
+
+            // If SystemParametersInfoW fails, which it shouldn't, fall back to
+            // Windows' default value.
+            return DefaultRowsToScroll;
+        }
+
+        return systemRowsToScroll;
+    }
+
     // Method Description:
     // - Bumps the tab in its in-order index up to the top of the mru list.
     // Arguments:

src/cascadia/TerminalApp/TerminalPage.h

@@ -12,6 +12,8 @@
 
 #include "AppCommandlineArgs.h"
 
+static constexpr uint32_t DefaultRowsToScroll{ 3 };
+
 // fwdecl unittest classes
 namespace TerminalAppLocalTests
 {
@@ -28,6 +30,12 @@ namespace winrt::TerminalApp::implementation
         Initialized = 2
     };
 
+    enum ScrollDirection : int
+    {
+        ScrollUp = 0,
+        ScrollDown = 1
+    };
+
     struct TerminalPage : TerminalPageT<TerminalPage>
     {
     public:
@@ -102,6 +110,8 @@ namespace winrt::TerminalApp::implementation
         std::optional<int> _rearrangeFrom;
         std::optional<int> _rearrangeTo;
 
+        uint32_t _systemRowsToScroll{ DefaultRowsToScroll };
+
         // use a weak reference to prevent circular dependency with AppLogic
         winrt::weak_ref<winrt::TerminalApp::IDialogPresenter> _dialogPresenter;
 
@@ -165,10 +175,10 @@ namespace winrt::TerminalApp::implementation
 
         // Todo: add more event implementations here
         // MSFT:20641986: Add keybindings for New Window
-        void _Scroll(int delta);
+        void _Scroll(ScrollDirection scrollDirection, const Windows::Foundation::IReference<uint32_t>& 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 _ResizePane(const Microsoft::Terminal::Settings::Model::Direction& direction);
-        void _ScrollPage(int delta);
+        void _ScrollPage(ScrollDirection scrollDirection);
         void _SetAcceleratorForMenuItem(Windows::UI::Xaml::Controls::MenuFlyoutItem& menuItem, const winrt::Microsoft::Terminal::TerminalControl::KeyChord& keyChord);
 
         winrt::fire_and_forget _CopyToClipboardHandler(const IInspectable sender, const winrt::Microsoft::Terminal::TerminalControl::CopyToClipboardEventArgs copiedData);
@@ -206,6 +216,9 @@ namespace winrt::TerminalApp::implementation
 
         void _UnZoomIfNeeded();
 
+        static int _ComputeScrollDelta(ScrollDirection scrollDirection, const uint32_t rowsToScroll);
+        static uint32_t _ReadSystemRowsToScroll();
+
         void _UpdateTabSwitcherCommands(const bool mru);
         void _UpdateMRUTab(const uint32_t index);
 

src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp

@@ -129,6 +129,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         { ShortcutAction::SetTabColor, SetTabColorArgs::FromJson },
         { ShortcutAction::SplitPane, SplitPaneArgs::FromJson },
         { ShortcutAction::SwitchToTab, SwitchToTabArgs::FromJson },
+        { ShortcutAction::ScrollUp, ScrollUpArgs::FromJson },
+        { ShortcutAction::ScrollDown, ScrollDownArgs::FromJson },
 
         { ShortcutAction::Invalid, nullptr },
     };

src/cascadia/TerminalSettingsModel/ActionArgs.cpp

@@ -362,4 +362,28 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         }
         return RS_(L"CloseTabsAfterDefaultCommandKey");
     }
+
+    winrt::hstring ScrollUpArgs::GenerateName() const
+    {
+        if (_RowsToScroll)
+        {
+            return winrt::hstring{
+                fmt::format(std::wstring_view(RS_(L"ScrollUpSeveralRowsCommandKey")),
+                            _RowsToScroll.Value())
+            };
+        }
+        return RS_(L"ScrollUpCommandKey");
+    }
+
+    winrt::hstring ScrollDownArgs::GenerateName() const
+    {
+        if (_RowsToScroll)
+        {
+            return winrt::hstring{
+                fmt::format(std::wstring_view(RS_(L"ScrollDownSeveralRowsCommandKey")),
+                            _RowsToScroll.Value())
+            };
+        }
+        return RS_(L"ScrollDownCommandKey");
+    }
 }