Repository: microsoft/terminal (language: cpp)

Issue:
COOKED_READ: Writing more text than fit into the ConPTY viewport behaves weirdly
### Steps to reproduce

* Make Windows Terminal small
* Launch `cmd.exe`
* Write lots of text, more than fit into the viewport

### Expected Behavior

No flickering and everything works as if there's infinite scrollback.

### Actual Behavior

To bring my feelings across I wrote a haiku:

The window flickering was hell.
The scrolling drained me of life's happiness.
Cooked read drives me nuts.


Discussion / hints:

Fix patch:
diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
index 3c5136b1910..99c2bd02ec9 100644
--- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
+++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
@@ -3401,7 +3401,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS()
         }
         else if (writingMethod == PrintWithWriteCharsLegacy)
         {
-            WriteCharsLegacy(si, str, true, nullptr);
+            WriteCharsLegacy(si, str, false, nullptr);
         }
     };
 
diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp
index 5ff5c68f5db..d9858c7e003 100644
--- a/src/host/_stream.cpp
+++ b/src/host/_stream.cpp
@@ -70,13 +70,50 @@ static void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point
 
     if (coordCursor.y >= bufferSize.height)
     {
-        // At the end of the buffer. Scroll contents of screen buffer so new position is visible.
-        StreamScrollRegion(screenInfo);
+        const auto vtIo = ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo();
+        const auto renderer = ServiceLocator::LocateGlobals().pRender;
+        const auto needsConPTYWorkaround = interactive && vtIo->IsUsingVt();
+        auto& buffer = screenInfo.GetTextBuffer();
+        const auto isActiveBuffer = buffer.IsActiveBuffer();
+
+        // ConPTY translates scrolling into newlines. We don't want that during cooked reads (= "cmd.exe prompts")
+        // because the entire prompt is supposed to fit into the VT viewport, with no scrollback. If we didn't do that,
+        // any prompt larger than the viewport will cause >1 lines to be added to scrollback, even if typing backspaces.
+        // You can test this by removing this branch, launch Windows Terminal, and fill the entire viewport with text in cmd.exe.
+        if (needsConPTYWorkaround)
+        {
+            buffer.SetAsActiveBuffer(false);
+            buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
+            buffer.SetAsActiveBuffer(isActiveBuffer);
+
+            if (isActiveBuffer && renderer)
+            {
+                renderer->TriggerRedrawAll();
+            }
+        }
+        else
+        {
+            buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
+
+            if (isActiveBuffer)
+            {
+                if (const auto notifier = ServiceLocator::LocateAccessibilityNotifier())
+                {
+                    notifier->NotifyConsoleUpdateScrollEvent(0, -1);
+                }
+                if (renderer)
+                {
+                    static constexpr til::point delta{ 0, -1 };
+                    renderer->TriggerScroll(&delta);
+                }
+            }
+        }
 
-        if (nullptr != psScrollY)
+        if (psScrollY)
         {
-            *psScrollY += bufferSize.height - coordCursor.y - 1;
+            *psScrollY += 1;
         }
+
         coordCursor.y = bufferSize.height - 1;
     }
 
diff --git a/src/host/output.cpp b/src/host/output.cpp
index beae8a8ef98..bd168fce9cd 100644
--- a/src/host/output.cpp
+++ b/src/host/output.cpp
@@ -293,38 +293,6 @@ static void _ScrollScreen(SCREEN_INFORMATION& screenInfo, const Viewport& source
     textBuffer.TriggerRedraw(fill);
 }
 
-// Routine Description:
-// - This routine is a special-purpose scroll for use by AdjustCursorPosition.
-// Arguments:
-// - screenInfo - reference to screen buffer info.
-// Return Value:
-// - true if we succeeded in scrolling the buffer, otherwise false (if we're out of memory)
-void StreamScrollRegion(SCREEN_INFORMATION& screenInfo)
-{
-    // Rotate the circular buffer around and wipe out the previous final line.
-    auto& buffer = screenInfo.GetTextBuffer();
-    buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
-
-    // Trigger a graphical update if we're active.
-    if (screenInfo.IsActiveScreenBuffer())
-    {
-        til::point coordDelta;
-        coordDelta.y = -1;
-
-        auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
-        if (pNotifier)
-        {
-            // Notify accessibility that a scroll has occurred.
-            pNotifier->NotifyConsoleUpdateScrollEvent(coordDelta.x, coordDelta.y);
-        }
-
-        if (ServiceLocator::LocateGlobals().pRender != nullptr)
-        {
-            ServiceLocator::LocateGlobals().pRender->TriggerScroll(&coordDelta);
-        }
-    }
-}
-
 // Routine Description:
 // - This routine copies ScrollRectangle to DestinationOrigin then fills in ScrollRectangle with Fill.
 // - The scroll region is copied to a third buffer, the scroll region is filled, then the original contents of the scroll region are copied to the destination.
diff --git a/src/host/output.h b/src/host/output.h
index fff6c001c25..ffbe75b3ad6 100644
--- a/src/host/output.h
+++ b/src/host/output.h
@@ -47,7 +47,5 @@ void ScrollRegion(SCREEN_INFORMATION& screenInfo,
 
 VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pProcessData);
 
-void StreamScrollRegion(SCREEN_INFORMATION& screenInfo);
-
 // For handling process handle state, not the window state itself.
 void CloseConsoleProcessState();
diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp
index 83d2f21b24a..a5859d6766f 100644
--- a/src/host/readDataCooked.cpp
+++ b/src/host/readDataCooked.cpp
@@ -752,7 +752,7 @@ void COOKED_READ_DATA::_flushBuffer()
         const auto distanceBeforeCursor = _writeChars(view.substr(0, _bufferCursor));
         const auto distanceAfterCursor = _writeChars(view.substr(_bufferCursor));
         const auto distanceEnd = distanceBeforeCursor + distanceAfterCursor;
-        const auto eraseDistance = std::max(0, _distanceEnd - distanceEnd);
+        const auto eraseDistance = std::max<ptrdiff_t>(0, _distanceEnd - distanceEnd);
 
         // If the contents of _buffer became shorter we'll have to erase the previously printed contents.
         _erase(eraseDistance);
@@ -766,7 +766,7 @@ void COOKED_READ_DATA::_flushBuffer()
 }
 
 // This is just a small helper to fill the next N cells starting at the current cursor position with whitespace.
-void COOKED_READ_DATA::_erase(const til::CoordType distance) const
+void COOKED_READ_DATA::_erase(ptrdiff_t distance) const
 {
     if (distance <= 0)
     {
@@ -793,7 +793,7 @@ void COOKED_READ_DATA::_erase(const til::CoordType distance) const
 // A helper to write text and calculate the number of cells we've written.
 // _unwindCursorPosition then allows us to go that many cells back. Tracking cells instead of explicit
 // buffer positions allows us to pay no further mind to whether the buffer scrolled up or not.
-til::CoordType COOKED_READ_DATA::_writeChars(const std::wstring_view& text) const
+ptrdiff_t COOKED_READ_DATA::_writeChars(const std::wstring_view& text) const
 {
     if (text.empty())
     {
@@ -802,18 +802,20 @@ til::CoordType COOKED_READ_DATA::_writeChars(const std::wstring_view& text) cons
 
     const auto& textBuffer = _screenInfo.GetTextBuffer();
     const auto& cursor = textBuffer.GetCursor();
-    const auto width = textBuffer.GetSize().Width();
+    const auto width = static_cast<ptrdiff_t>(textBuffer.GetSize().Width());
     const auto initialCursorPos = cursor.GetPosition();
     til::CoordType scrollY = 0;
 
     WriteCharsLegacy(_screenInfo, text, true, &scrollY);
 
     const auto finalCursorPos = cursor.GetPosition();
-    return (finalCursorPos.y - initialCursorPos.y + scrollY) * width + finalCursorPos.x - initialCursorPos.x;
+    const auto distance = (finalCursorPos.y - initialCursorPos.y + scrollY) * width + finalCursorPos.x - initialCursorPos.x;
+    assert(distance >= 0);
+    return distance;
 }
 
 // Moves the given point by the given distance inside the text buffer, as if moving a cursor with the left/right arrow keys.
-til::point COOKED_READ_DATA::_offsetPosition(til::point pos, til::CoordType distance) const
+til::point COOKED_READ_DATA::_offsetPosition(til::point pos, ptrdiff_t distance) const
 {
     const auto size = _screenInfo.GetTextBuffer().GetSize().Dimensions();
     const auto w = static_cast<ptrdiff_t>(size.width);
@@ -832,7 +834,7 @@ til::point COOKED_READ_DATA::_offsetPosition(til::point pos, til::CoordType dist
 
 // This moves the cursor `distance`-many cells back up in the buffer.
 // It's intended to be used in combination with _writeChars.
-void COOKED_READ_DATA::_unwindCursorPosition(til::CoordType distance) const
+void COOKED_READ_DATA::_unwindCursorPosition(ptrdiff_t distance) const
 {
     if (distance <= 0)
     {
diff --git a/src/host/readDataCooked.hpp b/src/host/readDataCooked.hpp
index 49f6c06bc9b..8f1e5849e81 100644
--- a/src/host/readDataCooked.hpp
+++ b/src/host/readDataCooked.hpp
@@ -113,10 +113,10 @@ class COOKED_READ_DATA final : public ReadData
     void _handlePostCharInputLoop(bool isUnicode, size_t& numBytes, ULONG& controlKeyState);
     void _markAsDirty();
     void _flushBuffer();
-    void _erase(til::CoordType distance) const;
-    til::CoordType _writeChars(const std::wstring_view& text) const;
-    til::point _offsetPosition(til::point pos, til::CoordType distance) const;
-    void _unwindCursorPosition(til::CoordType distance) const;
+    void _erase(ptrdiff_t distance) const;
+    ptrdiff_t _writeChars(const std::wstring_view& text) const;
+    til::point _offsetPosition(til::point pos, ptrdiff_t distance) const;
+    void _unwindCursorPosition(ptrdiff_t distance) const;
     void _replaceBuffer(const std::wstring_view& str);
 
     void _popupPush(PopupKind kind);
@@ -140,8 +140,8 @@ class COOKED_READ_DATA final : public ReadData
 
     std::wstring _buffer;
     size_t _bufferCursor = 0;
-    til::CoordType _distanceCursor;
-    til::CoordType _distanceEnd;
+    ptrdiff_t _distanceCursor = 0;
+    ptrdiff_t _distanceEnd = 0;
     bool _bufferDirty = false;
     bool _insertMode = false;
 


Test patch: