Merge the LineFeed functionality into AdaptDispatch

src/cascadia/TerminalCore/Terminal.cpp

@@ -490,9 +490,7 @@ void Terminal::Write(std::wstring_view stringView)
     const til::point cursorPosAfter{ cursor.GetPosition() };
 
     // Firing the CursorPositionChanged event is very expensive so we try not to
-    // do that when the cursor does not need to be redrawn. We don't do this
-    // inside _AdjustCursorPosition, only once we're done writing the whole run
-    // of output.
+    // do that when the cursor does not need to be redrawn.
     if (cursorPosBefore != cursorPosAfter)
     {
         _NotifyTerminalCursorPositionChanged();
@@ -1078,146 +1076,16 @@ Viewport Terminal::_GetVisibleViewport() const noexcept
                                     size);
 }
 
-void Terminal::_AdjustCursorPosition(const til::point proposedPosition)
+void Terminal::_PreserveUserScrollOffset(const int viewportDelta) noexcept
 {
-#pragma warning(suppress : 26496) // cpp core checks wants this const but it's modified below.
-    auto proposedCursorPosition = proposedPosition;
-    auto& cursor = _activeBuffer().GetCursor();
-    const auto bufferSize = _activeBuffer().GetSize();
-
-    // If we're about to scroll past the bottom of the buffer, instead cycle the
-    // buffer.
-    til::CoordType rowsPushedOffTopOfBuffer = 0;
-    const auto newRows = std::max(0, proposedCursorPosition.y - bufferSize.Height() + 1);
-    if (proposedCursorPosition.y >= bufferSize.Height())
+    // When the mutable viewport is moved down, and there's an active selection,
+    // or the visible viewport isn't already at the bottom, then we want to keep
+    // the visible viewport where it is. To do this, we adjust the scroll offset
+    // by the same amount that we've just moved down.
+    if (viewportDelta > 0 && (IsSelectionActive() || _scrollOffset != 0))
     {
-        for (auto dy = 0; dy < newRows; dy++)
-        {
-            _activeBuffer().IncrementCircularBuffer();
-            proposedCursorPosition.y--;
-            rowsPushedOffTopOfBuffer++;
-
-            // Update our selection too, so it doesn't move as the buffer is cycled
-            if (_selection)
-            {
-                // Stash this, so we can make sure to update the pivot to match later
-                const auto pivotWasStart = _selection->start == _selection->pivot;
-                // If the start of the selection is above 0, we can reduce both the start and end by 1
-                if (_selection->start.y > 0)
-                {
-                    _selection->start.y -= 1;
-                    _selection->end.y -= 1;
-                }
-                else
-                {
-                    // The start of the selection is at 0, if the end is greater than 0, then only reduce the end
-                    if (_selection->end.y > 0)
-                    {
-                        _selection->start.x = 0;
-                        _selection->end.y -= 1;
-                    }
-                    else
-                    {
-                        // Both the start and end of the selection are at 0, clear the selection
-                        _selection.reset();
-                    }
-                }
-
-                // If we still have a selection, make sure to sync the pivot
-                // with whichever value is the right one.
-                //
-                // Failure to do this might lead to GH #14462
-                if (_selection.has_value())
-                {
-                    _selection->pivot = pivotWasStart ? _selection->start : _selection->end;
-                }
-            }
-        }
-
-        // manually erase our pattern intervals since the locations have changed now
-        _patternIntervalTree = {};
-    }
-
-    // Update Cursor Position
-    cursor.SetPosition(proposedCursorPosition);
-
-    // Move the viewport down if the cursor moved below the viewport.
-    // Obviously, don't need to do this in the alt buffer.
-    if (!_inAltBuffer())
-    {
-        auto updatedViewport = false;
-        const auto scrollAmount = std::max(0, proposedCursorPosition.y - _mutableViewport.BottomInclusive());
-        if (scrollAmount > 0)
-        {
-            const auto newViewTop = std::max(0, proposedCursorPosition.y - (_mutableViewport.Height() - 1));
-            // In the alt buffer, we never need to adjust _mutableViewport, which is the viewport of the main buffer.
-            if (newViewTop != _mutableViewport.Top())
-            {
-                _mutableViewport = Viewport::FromDimensions({ 0, newViewTop },
-                                                            _mutableViewport.Dimensions());
-                updatedViewport = true;
-            }
-        }
-
-        // If the viewport moved, or we circled the buffer, we might need to update
-        // our _scrollOffset
-        if (updatedViewport || newRows != 0)
-        {
-            const auto oldScrollOffset = _scrollOffset;
-
-            // scroll if...
-            //   - no selection is active
-            //   - viewport is already at the bottom
-            const auto scrollToOutput = !IsSelectionActive() && _scrollOffset == 0;
-
-            _scrollOffset = scrollToOutput ? 0 : _scrollOffset + scrollAmount + newRows;
-
-            // Clamp the range to make sure that we don't scroll way off the top of the buffer
-            _scrollOffset = std::clamp(_scrollOffset,
-                                       0,
-                                       _activeBuffer().GetSize().Height() - _mutableViewport.Height());
-
-            // If the new scroll offset is different, then we'll still want to raise a scroll event
-            updatedViewport = updatedViewport || (oldScrollOffset != _scrollOffset);
-        }
-
-        // If the viewport moved, then send a scrolling notification.
-        if (updatedViewport)
-        {
-            _NotifyScrollEvent();
-        }
-    }
-
-    if (rowsPushedOffTopOfBuffer != 0)
-    {
-        if (_scrollMarks.size() > 0)
-        {
-            for (auto& mark : _scrollMarks)
-            {
-                // Move the mark up
-                mark.start.y -= rowsPushedOffTopOfBuffer;
-
-                // If the mark had sub-regions, then move those pointers too
-                if (mark.commandEnd.has_value())
-                {
-                    (*mark.commandEnd).y -= rowsPushedOffTopOfBuffer;
-                }
-                if (mark.outputEnd.has_value())
-                {
-                    (*mark.outputEnd).y -= rowsPushedOffTopOfBuffer;
-                }
-            }
-
-            _scrollMarks.erase(std::remove_if(_scrollMarks.begin(),
-                                              _scrollMarks.end(),
-                                              [](const VirtualTerminal::DispatchTypes::ScrollMark& m) { return m.start.y < 0; }),
-                               _scrollMarks.end());
-        }
-        // We have to report the delta here because we might have circled the text buffer.
-        // That didn't change the viewport and therefore the TriggerScroll(void)
-        // method can't detect the delta on its own.
-        const til::point delta{ 0, -rowsPushedOffTopOfBuffer };
-        _activeBuffer().TriggerScroll(delta);
+        const auto maxScrollOffset = _activeBuffer().GetSize().Height() - _mutableViewport.Height();
+        _scrollOffset = std::min(_scrollOffset + viewportDelta, maxScrollOffset);
     }
 }
 

src/cascadia/TerminalCore/Terminal.hpp

@@ -113,10 +113,8 @@ class Microsoft::Terminal::Core::Terminal final :
     void SetTextAttributes(const TextAttribute& attrs) noexcept override;
     void SetAutoWrapMode(const bool wrapAtEOL) noexcept override;
     bool GetAutoWrapMode() const noexcept override;
-    void SetScrollingRegion(const til::inclusive_rect& scrollMargins) noexcept override;
     void WarningBell() override;
     bool GetLineFeedMode() const noexcept override;
-    void LineFeed(const bool withReturn, const bool wrapForced) override;
     void SetWindowTitle(const std::wstring_view title) override;
     CursorType GetUserDefaultCursorStyle() const noexcept override;
     bool ResizeWindow(const til::CoordType width, const til::CoordType height) noexcept override;
@@ -140,6 +138,7 @@ class Microsoft::Terminal::Core::Terminal final :
     bool IsConsolePty() const noexcept override;
     bool IsVtInputEnabled() const noexcept override;
     void NotifyAccessibilityChange(const til::rect& changedRect) noexcept override;
+    void NotifyBufferRotation(const int delta) override;
 #pragma endregion
 
     void ClearMark();
@@ -420,7 +419,7 @@ class Microsoft::Terminal::Core::Terminal final :
     Microsoft::Console::Types::Viewport _GetMutableViewport() const noexcept;
     Microsoft::Console::Types::Viewport _GetVisibleViewport() const noexcept;
 
-    void _AdjustCursorPosition(const til::point proposedPosition);
+    void _PreserveUserScrollOffset(const int viewportDelta) noexcept;
 
     void _NotifyScrollEvent() noexcept;
 

src/cascadia/TerminalCore/TerminalApi.cpp

@@ -48,9 +48,11 @@ void Terminal::SetViewportPosition(const til::point position) noexcept
     // The viewport is fixed at 0,0 for the alt buffer, so this is a no-op.
     if (!_inAltBuffer())
     {
+        const auto viewportDelta = position.y - _GetMutableViewport().Origin().y;
         const auto dimensions = _GetMutableViewport().Dimensions();
         _mutableViewport = Viewport::FromDimensions(position, dimensions);
-        Terminal::_NotifyScrollEvent();
+        _PreserveUserScrollOffset(viewportDelta);
+        _NotifyScrollEvent();
     }
 }
 
@@ -70,11 +72,6 @@ bool Terminal::GetAutoWrapMode() const noexcept
     return true;
 }
 
-void Terminal::SetScrollingRegion(const til::inclusive_rect& /*scrollMargins*/) noexcept
-{
-    // TODO: This will be needed to fully support DECSTBM.
-}
-
 void Terminal::WarningBell()
 {
     _pfnWarningBell();
@@ -86,22 +83,6 @@ bool Terminal::GetLineFeedMode() const noexcept
     return false;
 }
 
-void Terminal::LineFeed(const bool withReturn, const bool wrapForced)
-{
-    auto cursorPos = _activeBuffer().GetCursor().GetPosition();
-
-    // If the line was forced to wrap, set the wrap status.
-    // When explicitly moving down a row, clear the wrap status.
-    _activeBuffer().GetRowByOffset(cursorPos.y).SetWrapForced(wrapForced);
-
-    cursorPos.y++;
-    if (withReturn)
-    {
-        cursorPos.x = 0;
-    }
-    _AdjustCursorPosition(cursorPos);
-}
-
 void Terminal::SetWindowTitle(const std::wstring_view title)
 {
     if (!_suppressApplicationTitle)
@@ -467,3 +448,62 @@ void Terminal::NotifyAccessibilityChange(const til::rect& /*changedRect*/) noexc
 {
     // This is only needed in conhost. Terminal handles accessibility in another way.
 }
+
+void Terminal::NotifyBufferRotation(const int delta)
+{
+    // Update our selection, so it doesn't move as the buffer is cycled
+    if (_selection)
+    {
+        // If the end of the selection will be out of range after the move, we just
+        // clear the selection. Otherwise we move both the start and end points up
+        // by the given delta and clamp to the first row.
+        if (_selection->end.y < delta)
+        {
+            _selection.reset();
+        }
+        else
+        {
+            // Stash this, so we can make sure to update the pivot to match later.
+            const auto pivotWasStart = _selection->start == _selection->pivot;
+            _selection->start.y = std::max(_selection->start.y - delta, 0);
+            _selection->end.y = std::max(_selection->end.y - delta, 0);
+            // Make sure to sync the pivot with whichever value is the right one.
+            _selection->pivot = pivotWasStart ? _selection->start : _selection->end;
+        }
+    }
+
+    // manually erase our pattern intervals since the locations have changed now
+    _patternIntervalTree = {};
+
+    const auto hasScrollMarks = _scrollMarks.size() > 0;
+    if (hasScrollMarks)
+    {
+        for (auto& mark : _scrollMarks)
+        {
+            // Move the mark up
+            mark.start.y -= delta;
+
+            // If the mark had sub-regions, then move those pointers too
+            if (mark.commandEnd.has_value())
+            {
+                (*mark.commandEnd).y -= delta;
+            }
+            if (mark.outputEnd.has_value())
+            {
+                (*mark.outputEnd).y -= delta;
+            }
+        }
+
+        _scrollMarks.erase(std::remove_if(_scrollMarks.begin(),
+                                          _scrollMarks.end(),
+                                          [](const auto& m) { return m.start.y < 0; }),
+                           _scrollMarks.end());
+    }
+
+    const auto oldScrollOffset = _scrollOffset;
+    _PreserveUserScrollOffset(delta);
+    if (_scrollOffset != oldScrollOffset || hasScrollMarks)
+    {
+        _NotifyScrollEvent();
+    }
+}

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -1700,56 +1700,59 @@ void ConptyRoundtripTests::ScrollWithMargins()
     hostSm.ProcessString(completeCursorAtPromptLine);
 
     // Set up the verifications like above.
-    auto verifyBufferAfter = [&](const TextBuffer& tb) {
+    auto verifyBufferAfter = [&](const TextBuffer& tb, const auto panOffset) {
         auto& cursor = tb.GetCursor();
         // Verify the cursor is waiting on the freshly revealed line (1 above mode line)
         // and in the left most column.
-        VERIFY_ARE_EQUAL(initialTermView.Height() - 2, cursor.GetPosition().y);
+        const auto bottomLine = initialTermView.BottomInclusive() + panOffset;
+        VERIFY_ARE_EQUAL(bottomLine - 1, cursor.GetPosition().y);
         VERIFY_ARE_EQUAL(0, cursor.GetPosition().x);
 
         // For all rows except the last two, verify that we have a run of four letters.
         for (auto i = 0; i < rowsToWrite - 1; ++i)
         {
             // Start with B this time because the A line got scrolled off the top.
             const std::wstring expectedString(4, static_cast<wchar_t>(L'B' + i));
-            const til::point expectedPos{ 0, i };
+            const til::point expectedPos{ 0, panOffset + i };
             TestUtils::VerifyExpectedString(tb, expectedString, expectedPos);
         }
 
         // For the second to last row, verify that it is blank.
         {
             const std::wstring expectedBlankLine(initialTermView.Width(), L' ');
-            const til::point blankLinePos{ 0, rowsToWrite - 1 };
+            const til::point blankLinePos{ 0, panOffset + rowsToWrite - 1 };
             TestUtils::VerifyExpectedString(tb, expectedBlankLine, blankLinePos);
         }
 
         // For the last row, verify we have an entire row of asterisks for the mode line.
         {
             const std::wstring expectedModeLine(initialTermView.Width() - 1, L'*');
-            const til::point modeLinePos{ 0, rowsToWrite };
+            const til::point modeLinePos{ 0, panOffset + rowsToWrite };
             TestUtils::VerifyExpectedString(tb, expectedModeLine, modeLinePos);
         }
     };
 
     // This will verify the text emitted from the PTY.
 
-    expectedOutput.push_back("\x1b[H"); // cursor returns to top left corner.
-    for (auto i = 0; i < rowsToWrite - 1; ++i)
+    expectedOutput.push_back("\r\n"); // cursor moved to bottom left corner
+    expectedOutput.push_back("\n"); // linefeed pans the viewport down
     {
-        const std::string expectedString(4, static_cast<char>('B' + i));
-        expectedOutput.push_back(expectedString);
-        expectedOutput.push_back("\x1b[K"); // erase the rest of the line.
-        expectedOutput.push_back("\r\n");
+        // Cursor gets reset into second line from bottom, left most column
+        std::stringstream ss;
+        ss << "\x1b[" << initialTermView.Height() - 1 << ";1H";
+        expectedOutput.push_back(ss.str());
     }
     {
-        expectedOutput.push_back(""); // nothing for the empty line
-        expectedOutput.push_back("\x1b[K"); // erase the rest of the line.
-        expectedOutput.push_back("\r\n");
+        // Bottom of the scroll region is replaced with a blank line
+        const std::string expectedString(initialTermView.Width(), ' ');
+        expectedOutput.push_back(expectedString);
     }
+    expectedOutput.push_back("\r\n"); // cursor moved to bottom left corner
     {
+        // Mode line is redrawn at the bottom of the viewport
         const std::string expectedString(initialTermView.Width() - 1, '*');
-        // There will be one extra blank space at the end of the line, to prevent delayed EOL wrapping
-        expectedOutput.push_back(expectedString + " ");
+        expectedOutput.push_back(expectedString);
+        expectedOutput.push_back(" ");
     }
     {
         // Cursor gets reset into second line from bottom, left most column
@@ -1761,15 +1764,15 @@ void ConptyRoundtripTests::ScrollWithMargins()
 
     Log::Comment(L"Verify host buffer contains pattern moved up one and mode line still in place.");
     // Verify the host side.
-    verifyBufferAfter(hostTb);
+    verifyBufferAfter(hostTb, 0);
 
     Log::Comment(L"Emit PTY frame and validate it transmits the right data.");
     // Paint the frame
     VERIFY_SUCCEEDED(renderer.PaintFrame());
 
     Log::Comment(L"Verify terminal buffer contains pattern moved up one and mode line still in place.");
-    // Verify the terminal side.
-    verifyBufferAfter(termTb);
+    // Verify the terminal side. Note the viewport has panned down a line.
+    verifyBufferAfter(termTb, 1);
 }
 
 void ConptyRoundtripTests::DontWrapMoveCursorInSingleFrame()