Remove the rowsToScroll setting and just always use the system setting (

#6891)

This parameter was added as a workaround for our fast trackpad
scrolling. Since that was fixed before 1.0 shipped, in #4554, it has
been largely vestigial. There is no reason for us to keep it around any
longer.

It was also the only "logic" in TerminalSettings, which is otherwise a
library that only transits data between two other libraries.

I have not removed it from the schema, as I do not want to mark folks'
settings files invalid to a strict schema parser.

While I was in the area, I added support for "scroll one screen at a
time" (which is represented by the API returning WHEEL_PAGESCROLL),
fixing #5610. We were also storing it in an int (whoops) instead of a
uint.

Fixes #5610

.github/actions/spell-check/dictionary/apis.txt

@@ -8,8 +8,8 @@ EXPCMDSTATE
 fullkbd
 href
 IAsync
-IBox
 IBind
+IBox
 IClass
 IComparable
 ICustom
@@ -28,6 +28,7 @@ NOAGGREGATION
 NOREDIRECTIONBITMAP
 oaidl
 ocidl
+PAGESCROLL
 RETURNCMD
 rfind
 roundf

doc/cascadia/SettingsSchema.md

@@ -16,7 +16,6 @@ Properties listed below affect the entire window, regardless of the profile sett
 | `initialPosition` | Optional | String | `","` | The position of the top left corner of the window upon first load. On a system with multiple displays, these coordinates are relative to the top left of the primary display. If `launchMode` is set to `"maximized"`, the window will be maximized on the monitor specified by those coordinates. |
 | `initialRows` | _Required_ | Integer | `30` | The number of rows displayed in the window upon first load. |
 | `launchMode` | Optional | String | `default` | Defines whether the Terminal will launch as maximized or not. Possible values: `"default"`, `"maximized"` |
-| `rowsToScroll` | Optional | Integer | `system` | The number of rows to scroll at a time with the mouse wheel. This will override the system setting if the value is not zero or "system". |
 | `theme` | _Required_ | String | `system` | Sets the theme of the application. Possible values: `"light"`, `"dark"`, `"system"` |
 | `showTerminalTitleInTitlebar` | _Required_ | Boolean | `true` | When set to `true`, titlebar displays the title of the selected tab. When set to `false`, titlebar displays "Windows Terminal". |
 | `showTabsInTitlebar` | Optional | Boolean | `true` | When set to `true`, the tabs are moved into the titlebar and the titlebar disappears. When set to `false`, the titlebar sits above the tabs. |

doc/cascadia/profiles.schema.json

@@ -1,6 +1,6 @@
 {
   "$id": "https://github.com/microsoft/terminal/blob/master/doc/cascadia/profiles.schema.json",
-  "$schema": "http://json-schema.org/draft-07/schema#",
+  "$schema": "https://json-schema.org/draft/2019-09/schema#",
   "title": "Microsoft's Windows Terminal Settings Profile Schema",
   "definitions": {
     "KeyChordSegment": {
@@ -397,10 +397,11 @@
         },
         "rowsToScroll": {
           "default": "system",
-          "description": "The number of rows to scroll at a time with the mouse wheel. This will override the system setting if the value is not zero or \"system\".",
+          "description": "This parameter once allowed you to override the systemwide \"choose how many lines to scroll at one time\" setting. It no longer does so.",
           "maximum": 999,
           "minimum": 0,
-          "type": [ "integer", "string" ]
+          "type": [ "integer", "string" ],
+          "deprecated": true
         },
         "keybindings": {
           "description": "Properties are specific to each custom key binding.",

src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp

@@ -466,15 +466,13 @@ namespace TerminalAppLocalTests
         {
             "alwaysShowTabs": true,
             "initialCols" : 120,
-            "initialRows" : 30,
-            "rowsToScroll" :  4
+            "initialRows" : 30
         })" };
         const std::string settings1String{ R"(
         {
             "showTabsInTitlebar": false,
             "initialCols" : 240,
-            "initialRows" : 60,
-            "rowsToScroll" : 8
+            "initialRows" : 60
         })" };
         const auto settings0Json = VerifyParseSucceeded(settings0String);
         const auto settings1Json = VerifyParseSucceeded(settings1String);
@@ -485,14 +483,12 @@ namespace TerminalAppLocalTests
         VERIFY_ARE_EQUAL(true, settings._globals._AlwaysShowTabs);
         VERIFY_ARE_EQUAL(120, settings._globals._InitialCols);
         VERIFY_ARE_EQUAL(30, settings._globals._InitialRows);
-        VERIFY_ARE_EQUAL(4, settings._globals._RowsToScroll);
         VERIFY_ARE_EQUAL(true, settings._globals._ShowTabsInTitlebar);
 
         settings.LayerJson(settings1Json);
         VERIFY_ARE_EQUAL(true, settings._globals._AlwaysShowTabs);
         VERIFY_ARE_EQUAL(240, settings._globals._InitialCols);
         VERIFY_ARE_EQUAL(60, settings._globals._InitialRows);
-        VERIFY_ARE_EQUAL(8, settings._globals._RowsToScroll);
         VERIFY_ARE_EQUAL(false, settings._globals._ShowTabsInTitlebar);
     }
 

src/cascadia/TerminalApp/GlobalAppSettings.cpp

@@ -22,7 +22,6 @@ static constexpr std::string_view DefaultProfileKey{ "defaultProfile" };
 static constexpr std::string_view AlwaysShowTabsKey{ "alwaysShowTabs" };
 static constexpr std::string_view InitialRowsKey{ "initialRows" };
 static constexpr std::string_view InitialColsKey{ "initialCols" };
-static constexpr std::string_view RowsToScrollKey{ "rowsToScroll" };
 static constexpr std::string_view InitialPositionKey{ "initialPosition" };
 static constexpr std::string_view ShowTitleInTitlebarKey{ "showTerminalTitleInTitlebar" };
 static constexpr std::string_view ThemeKey{ "theme" };
@@ -73,7 +72,6 @@ GlobalAppSettings::GlobalAppSettings() :
     _defaultProfile{},
     _InitialRows{ DEFAULT_ROWS },
     _InitialCols{ DEFAULT_COLS },
-    _RowsToScroll{ DEFAULT_ROWSTOSCROLL },
     _WordDelimiters{ DEFAULT_WORD_DELIMITERS },
     _DebugFeaturesEnabled{ debugFeaturesDefault }
 {
@@ -127,7 +125,6 @@ void GlobalAppSettings::ApplyToSettings(TerminalSettings& settings) const noexce
     settings.KeyBindings(GetKeybindings());
     settings.InitialRows(_InitialRows);
     settings.InitialCols(_InitialCols);
-    settings.RowsToScroll(_RowsToScroll);
 
     settings.WordDelimiters(_WordDelimiters);
     settings.CopyOnSelect(_CopyOnSelect);
@@ -164,19 +161,6 @@ void GlobalAppSettings::LayerJson(const Json::Value& json)
 
     JsonUtils::GetInt(json, InitialColsKey, _InitialCols);
 
-    if (auto rowsToScroll{ json[JsonKey(RowsToScrollKey)] })
-    {
-        //if it's not an int we fall back to setting it to 0, which implies using the system setting. This will be the case if it's set to "system"
-        if (rowsToScroll.isInt())
-        {
-            _RowsToScroll = rowsToScroll.asInt();
-        }
-        else
-        {
-            _RowsToScroll = 0;
-        }
-    }
-
     if (auto initialPosition{ json[JsonKey(InitialPositionKey)] })
     {
         _ParseInitialPosition(initialPosition.asString(), _InitialPosition);

src/cascadia/TerminalApp/GlobalAppSettings.h

@@ -70,7 +70,6 @@ class TerminalApp::GlobalAppSettings final
     GETSET_PROPERTY(bool, ConfirmCloseAllTabs, true);
     GETSET_PROPERTY(winrt::Windows::UI::Xaml::ElementTheme, Theme, winrt::Windows::UI::Xaml::ElementTheme::Default);
     GETSET_PROPERTY(winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode, TabWidthMode, winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode::Equal);
-    GETSET_PROPERTY(int, RowsToScroll); // default value set in constructor
     GETSET_PROPERTY(bool, ShowTabsInTitlebar, true);
     GETSET_PROPERTY(std::wstring, WordDelimiters); // default value set in constructor
     GETSET_PROPERTY(bool, CopyOnSelect, false);

src/cascadia/TerminalApp/defaults-universal.json

@@ -21,7 +21,6 @@
     // Miscellaneous
     "confirmCloseAllTabs": true,
     "theme": "system",
-    "rowsToScroll": "system",
     "snapToGridOnResize": true,
 
     "profiles":

src/cascadia/TerminalApp/defaults.json

@@ -22,7 +22,6 @@
     "confirmCloseAllTabs": true,
     "startOnUserLogin":  false,
     "theme": "system",
-    "rowsToScroll": "system",
     "snapToGridOnResize": true,
 
     "profiles":

src/cascadia/TerminalControl/TermControl.cpp

@@ -361,8 +361,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             ScrollBar().Visibility(Visibility::Visible);
         }
 
-        // set number of rows to scroll at a time
-        _rowsToScroll = _settings.RowsToScroll();
+        _UpdateSystemParameterSettings();
     }
 
     // Method Description:
@@ -1491,12 +1490,15 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
         // negative = down, positive = up
         // However, for us, the signs are flipped.
-        const auto rowDelta = mouseDelta / (-1.0 * WHEEL_DELTA);
-
-        // With one of the precision mouses, one click is always a multiple of 120,
+        // With one of the precision mice, one click is always a multiple of 120 (WHEEL_DELTA),
         // but the "smooth scrolling" mode results in non-int values
+        const auto rowDelta = mouseDelta / (-1.0 * WHEEL_DELTA);
 
-        double newValue = (_rowsToScroll * rowDelta) + (currentOffset);
+        // WHEEL_PAGESCROLL is a Win32 constant that represents the "scroll one page
+        // at a time" setting. If we ignore it, we will scroll a truly absurd number
+        // of rows.
+        const auto rowsToScroll{ _rowsToScroll == WHEEL_PAGESCROLL ? GetViewHeight() : _rowsToScroll };
+        double newValue = (rowsToScroll * rowDelta) + (currentOffset);
 
         // The scroll bar's ValueChanged handler will actually move the viewport
         //      for us.
@@ -1697,7 +1699,21 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             _terminal->SetCursorOn(true);
             _cursorTimer.value().Start();
         }
-        _rowsToScroll = _settings.RowsToScroll();
+
+        _UpdateSystemParameterSettings();
+    }
+
+    // Method Description
+    // - Updates internal params based on system parameters
+    void TermControl::_UpdateSystemParameterSettings() noexcept
+    {
+        if (!SystemParametersInfoW(SPI_GETWHEELSCROLLLINES, 0, &_rowsToScroll, 0))
+        {
+            LOG_LAST_ERROR();
+            // If SystemParametersInfoW fails, which it shouldn't, fall back to
+            // Windows' default value.
+            _rowsToScroll = 3;
+        }
     }
 
     // Method Description:

src/cascadia/TerminalControl/TermControl.h

@@ -150,7 +150,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         std::shared_ptr<ThrottledFunc<ScrollBarUpdate>> _updateScrollBar;
         bool _isInternalScrollBarUpdate;
 
-        int _rowsToScroll;
+        unsigned int _rowsToScroll;
 
         // Auto scroll occurs when user, while selecting, drags cursor outside viewport. View is then scrolled to 'follow' the cursor.
         double _autoScrollVelocity;
@@ -185,6 +185,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker;
 
         void _ApplyUISettings();
+        void _UpdateSystemParameterSettings() noexcept;
         void _InitializeBackgroundBrush();
         winrt::fire_and_forget _BackgroundColorChanged(const COLORREF color);
         bool _InitializeTerminal();

src/cascadia/TerminalSettings/ICoreSettings.idl

@@ -22,7 +22,6 @@ namespace Microsoft.Terminal.Settings
         Int32 HistorySize;
         Int32 InitialRows;
         Int32 InitialCols;
-        Int32 RowsToScroll;
 
         Boolean SnapOnInput;
         Boolean AltGrAliasing;

src/cascadia/TerminalSettings/TerminalSettings.cpp

@@ -19,24 +19,4 @@ namespace winrt::Microsoft::Terminal::Settings::implementation
         THROW_HR_IF(E_INVALIDARG, index >= colorTableCount);
         _colorTable.at(index) = value;
     }
-
-    int32_t TerminalSettings::RowsToScroll() noexcept
-    {
-        if (_rowsToScroll != 0)
-        {
-            return _rowsToScroll;
-        }
-        int systemRowsToScroll;
-        if (!SystemParametersInfo(SPI_GETWHEELSCROLLLINES, 0, &systemRowsToScroll, 0))
-        {
-            systemRowsToScroll = 4;
-        }
-        return systemRowsToScroll;
-    }
-
-    void TerminalSettings::RowsToScroll(int32_t value) noexcept
-    {
-        _rowsToScroll = value;
-    }
-
 }

src/cascadia/TerminalSettings/terminalsettings.h

@@ -39,11 +39,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation
         uint32_t GetColorTableEntry(int32_t index) const noexcept;
         void SetColorTableEntry(int32_t index, uint32_t value);
 
-        // the RowsToScroll getter needs to be implemented manually, so it can
-        // default to SPI_GETWHEELSCROLLLINES
-        int32_t RowsToScroll() noexcept;
-        void RowsToScroll(int32_t value) noexcept;
-
         GETSET_PROPERTY(uint32_t, DefaultForeground, DEFAULT_FOREGROUND_WITH_ALPHA);
         GETSET_PROPERTY(uint32_t, DefaultBackground, DEFAULT_BACKGROUND_WITH_ALPHA);
         GETSET_PROPERTY(uint32_t, SelectionBackground, DEFAULT_FOREGROUND);
@@ -104,7 +99,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation
 
     private:
         std::array<uint32_t, COLOR_TABLE_SIZE> _colorTable{};
-        int32_t _rowsToScroll{ 0 };
     };
 }
 

src/cascadia/UnitTests_TerminalCore/MockTermSettings.h

@@ -25,7 +25,6 @@ namespace TerminalCoreUnitTests
         int32_t HistorySize() { return _historySize; }
         int32_t InitialRows() { return _initialRows; }
         int32_t InitialCols() { return _initialCols; }
-        int32_t RowsToScroll() { return 4; }
         uint32_t DefaultForeground() { return COLOR_WHITE; }
         uint32_t DefaultBackground() { return COLOR_BLACK; }
         bool SnapOnInput() { return false; }
@@ -47,7 +46,6 @@ namespace TerminalCoreUnitTests
         void HistorySize(int32_t) {}
         void InitialRows(int32_t) {}
         void InitialCols(int32_t) {}
-        void RowsToScroll(int32_t) {}
         void DefaultForeground(uint32_t) {}
         void DefaultBackground(uint32_t) {}
         void SnapOnInput(bool) {}

src/inc/DefaultSettings.h

@@ -34,7 +34,6 @@ constexpr uint16_t DEFAULT_FONT_WEIGHT = 400; // normal
 
 constexpr int DEFAULT_ROWS = 30;
 constexpr int DEFAULT_COLS = 120;
-constexpr int DEFAULT_ROWSTOSCROLL = 0;
 
 const std::wstring DEFAULT_PADDING{ L"8, 8, 8, 8" };
 const std::wstring DEFAULT_STARTING_DIRECTORY{ L"%USERPROFILE%" };