Fixes mouse selection within scrolloff setting to not cause the viewp…

…ort to jump anymore (#1019).

Signed-off-by: Christian Parpart <christian@parpart.family>

metainfo.xml

@@ -111,6 +111,7 @@
           <li>Fixes abnormal termination on incomplete foreground/background color-pair specification.</li>
           <li>Fixes `SendChars` input acion to actually send the chars as-is to the standard input of the connected application.</li>
           <li>Fixes mouse selection to only be initiated if actually meant to, i.e. in alt screen mode only if bypass-modifier was pressed (#1017).</li>
+          <li>Fixes mouse selection within scrolloff setting to not cause the viewport to jump anymore (#1019).</li>
           <li>Adds normal mode motion `[[`, `]]`, `[]`, `][` mimmicking exactly what vim does.</li>
           <li>Adds normal mode motion `[m` and `]m` to jump line marks up/down.</li>
           <li>Adds normal mode motion `mm` to toggle the line mark at the current active cursor position.</li>

src/contour/TerminalSession.cpp

@@ -729,16 +729,16 @@ bool TerminalSession::operator()(actions::DecreaseOpacity)
 bool TerminalSession::operator()(actions::FocusNextSearchMatch)
 {
     if (terminal_.state().viCommands.jumpToNextMatch(1))
-        terminal_.viewport().makeVisible(terminal_.state().viCommands.cursorPosition.line);
-    // TODO why didn't the makeVisible() call from inside jumpToNextMatch not work?
+        terminal_.viewport().makeVisibleWithinSafeArea(terminal_.state().viCommands.cursorPosition.line);
+    // TODO why didn't the makeVisibleWithinSafeArea() call from inside jumpToNextMatch not work?
     return true;
 }
 
 bool TerminalSession::operator()(actions::FocusPreviousSearchMatch)
 {
     if (terminal_.state().viCommands.jumpToPreviousMatch(1))
-        terminal_.viewport().makeVisible(terminal_.state().viCommands.cursorPosition.line);
-    // TODO why didn't the makeVisible() call from inside jumpToPreviousMatch not work?
+        terminal_.viewport().makeVisibleWithinSafeArea(terminal_.state().viCommands.cursorPosition.line);
+    // TODO why didn't the makeVisibleWithinSafeArea() call from inside jumpToPreviousMatch not work?
     return true;
 }
 

src/vtbackend/Terminal.cpp

@@ -803,11 +803,8 @@ void Terminal::sendMouseMoveEvent(Modifier modifier,
         && (selector()->state() != Selection::State::Complete && _leftMouseButtonPressed))
     {
         if (currentScreen().isCellEmpty(relativePos) && !currentScreen().compareCellTextAt(relativePos, 0x20))
-        {
             relativePos.column = ColumnOffset { 0 } + *(_settings.pageSize.columns - 1);
-        }
         _state.viCommands.cursorPosition = relativePos;
-        _viewport.makeVisible(_state.viCommands.cursorPosition.line);
         if (_state.inputHandler.mode() != ViMode::Insert)
             _state.inputHandler.setMode(selector()->viMode());
         if (selector()->extend(relativePos))
@@ -1976,7 +1973,7 @@ optional<CellLocation> Terminal::search(CellLocation searchPosition)
     auto const matchLocation = currentScreen().search(searchText, searchPosition);
 
     if (matchLocation)
-        viewport().makeVisible(matchLocation.value().line);
+        viewport().makeVisibleWithinSafeArea(matchLocation.value().line);
 
     screenUpdated();
     return matchLocation;
@@ -2022,7 +2019,7 @@ optional<CellLocation> Terminal::searchReverse(CellLocation searchPosition)
     auto const matchLocation = currentScreen().searchReverse(searchText, searchPosition);
 
     if (matchLocation)
-        viewport().makeVisible(matchLocation.value().line);
+        viewport().makeVisibleWithinSafeArea(matchLocation.value().line);
 
     screenUpdated();
     return matchLocation;

src/vtbackend/ViCommands.cpp

@@ -1100,7 +1100,7 @@ void ViCommands::moveCursorTo(CellLocation position)
 {
     cursorPosition = position;
 
-    _terminal.viewport().makeVisible(cursorPosition.line);
+    _terminal.viewport().makeVisibleWithinSafeArea(cursorPosition.line);
 
     switch (_terminal.inputHandler().mode())
     {

src/vtbackend/Viewport.cpp

@@ -52,11 +52,16 @@ bool Viewport::forceScrollToBottom()
     return scrollTo(ScrollOffset(0));
 }
 
-bool Viewport::makeVisible(LineOffset lineOffset)
+bool Viewport::makeVisibleWithinSafeArea(LineOffset lineOffset)
+{
+    return makeVisibleWithinSafeArea(lineOffset, _scrollOff);
+}
+
+bool Viewport::makeVisibleWithinSafeArea(LineOffset lineOffset, LineCount paddingLines)
 {
-    auto const viewportTop = -_scrollOffset.as<LineOffset>() + boxed_cast<LineOffset>(_scrollOff);
+    auto const viewportTop = -_scrollOffset.as<LineOffset>() + boxed_cast<LineOffset>(paddingLines);
     auto const viewportBottom = boxed_cast<LineOffset>(screenLineCount() - 1) - _scrollOffset.as<int>()
-                                - boxed_cast<LineOffset>(_scrollOff);
+                                - boxed_cast<LineOffset>(paddingLines);
 
     // Is the line above the viewport?
     if (!(viewportTop < lineOffset))
@@ -69,6 +74,11 @@ bool Viewport::makeVisible(LineOffset lineOffset)
     return false;
 }
 
+bool Viewport::makeVisible(LineOffset lineOffset)
+{
+    return makeVisibleWithinSafeArea(lineOffset, LineCount(0));
+}
+
 bool Viewport::scrollTo(ScrollOffset offset)
 {
     if (scrollingDisabled() && offset != ScrollOffset(0))

src/vtbackend/Viewport.h

@@ -77,6 +77,9 @@ class Viewport
     /// If the line is already visible, no scrolling is applied.
     bool makeVisible(LineOffset line);
 
+    bool makeVisibleWithinSafeArea(LineOffset line);
+    bool makeVisibleWithinSafeArea(LineOffset line, LineCount paddingLines);
+
     /// Translates a screen coordinate to a Grid-coordinate by applying
     /// the scroll-offset to it.
     constexpr CellLocation translateScreenToGridCoordinate(CellLocation p) const noexcept