Fix clearing marks

src/buffer/out/textBuffer.cpp

@@ -1176,36 +1176,40 @@ void TextBuffer::Reset() noexcept
     _initialAttributes = _currentAttributes;
 }
 
-void TextBuffer::ClearScrollback(const til::CoordType start, const til::CoordType height)
+// Arguments:
+// - newFirstRow: The current y-position of the viewport. We'll clear up until here.
+// - rowsToKeep: the number of rows to keep in the buffer.
+void TextBuffer::ClearScrollback(const til::CoordType newFirstRow, const til::CoordType rowsToKeep)
 {
-    if (start <= 0)
+    // We're already at the top? don't clear anything. There's no scrollback.
+    if (newFirstRow <= 0)
     {
         return;
     }
-
-    if (height <= 0)
+    // The new viewport should keep 0 rows? Then just reset everything.
+    if (rowsToKeep <= 0)
     {
         _decommit();
         return;
     }
 
+    ClearMarksInRange(til::point{ 0, 0 }, til::point{ _width, std::max(0, newFirstRow - 1) });
+
     // Our goal is to move the viewport to the absolute start of the underlying memory buffer so that we can
     // MEM_DECOMMIT the remaining memory. _firstRow is used to make the TextBuffer behave like a circular buffer.
-    // The start parameter is relative to the _firstRow. The trick to get the content to the absolute start
+    // The newFirstRow parameter is relative to the _firstRow. The trick to get the content to the absolute start
     // is to simply add _firstRow ourselves and then reset it to 0. This causes ScrollRows() to write into
     // the absolute start while reading from relative coordinates. This works because GetRowByOffset()
     // operates modulo the buffer height and so the possibly-too-large startAbsolute won't be an issue.
-    const auto startAbsolute = _firstRow + start;
+    const auto startAbsolute = _firstRow + newFirstRow;
     _firstRow = 0;
-    ScrollRows(startAbsolute, height, -startAbsolute);
+    ScrollRows(startAbsolute, rowsToKeep, -startAbsolute);
 
     const auto end = _estimateOffsetOfLastCommittedRow();
-    for (auto y = height; y <= end; ++y)
+    for (auto y = rowsToKeep; y <= end; ++y)
     {
         GetMutableRowByOffset(y).Reset(_initialAttributes);
     }
-
-    ClearMarksInRange(til::point{ 0, height }, til::point{ _width, _height });
 }
 
 // Routine Description:

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -239,12 +239,14 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
     TEST_METHOD(MultilinePromptRegions);
     TEST_METHOD(ManyMultilinePromptsWithTrailingSpaces);
     TEST_METHOD(ReflowPromptRegions);
+    TEST_METHOD(ClearMarksTest);
 
 private:
     bool _writeCallback(const char* const pch, const size_t cch);
     void _flushFirstFrame();
     void _resizeConpty(const til::CoordType sx, const til::CoordType sy);
     void _clearConpty();
+    void _clear(int clearBufferMethod, SCREEN_INFORMATION& si);
 
     [[nodiscard]] std::tuple<TextBuffer*, TextBuffer*> _performResize(const til::size newSize);
 
@@ -2720,9 +2722,6 @@ void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest()
     BEGIN_TEST_METHOD_PROPERTIES()
         TEST_METHOD_PROPERTY(L"Data:clearBufferMethod", L"{0, 1, 2}")
     END_TEST_METHOD_PROPERTIES();
-    constexpr auto ClearLikeCls = 0;
-    constexpr auto ClearLikeClearHost = 1;
-    constexpr auto ClearWithVT = 2;
     INIT_TEST_PROPERTY(int, clearBufferMethod, L"Controls whether we clear the buffer like cmd or like powershell");
 
     Log::Comment(L"This test checks the shims for cmd.exe and powershell.exe. "
@@ -2789,6 +2788,31 @@ void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest()
     VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
     VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());
 
+    _clear(clearBufferMethod, si);
+
+    VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
+    VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());
+
+    Log::Comment(L"Painting the frame");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    Log::Comment(L"========== Checking the host buffer state (after) ==========");
+    verifyBuffer(*hostTb, si.GetViewport().ToExclusive(), true);
+
+    Log::Comment(L"Painting the frame");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+    Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
+    verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true);
+}
+
+void ConptyRoundtripTests::_clear(int clearBufferMethod, SCREEN_INFORMATION& si)
+{
+    constexpr auto ClearLikeCls = 0;
+    constexpr auto ClearLikeClearHost = 1;
+    constexpr auto ClearWithVT = 2;
+
+    auto& sm = si.GetStateMachine();
+
     if (clearBufferMethod == ClearLikeCls)
     {
         // Execute the cls, EXACTLY LIKE CMD.
@@ -2840,20 +2864,6 @@ void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest()
 
         sm.ProcessString(L"\x1b[3J");
     }
-
-    VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
-    VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());
-
-    Log::Comment(L"Painting the frame");
-    VERIFY_SUCCEEDED(renderer.PaintFrame());
-
-    Log::Comment(L"========== Checking the host buffer state (after) ==========");
-    verifyBuffer(*hostTb, si.GetViewport().ToExclusive(), true);
-
-    Log::Comment(L"Painting the frame");
-    VERIFY_SUCCEEDED(renderer.PaintFrame());
-    Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
-    verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true);
 }
 
 void ConptyRoundtripTests::TestResizeWithCookedRead()
@@ -4945,3 +4955,98 @@ void ConptyRoundtripTests::ReflowPromptRegions()
     Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
     verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true, true);
 }
+
+void ConptyRoundtripTests::ClearMarksTest()
+{
+    BEGIN_TEST_METHOD_PROPERTIES()
+        TEST_METHOD_PROPERTY(L"Data:clearBufferMethod", L"{0, 1, 2}")
+    END_TEST_METHOD_PROPERTIES();
+
+    INIT_TEST_PROPERTY(int, clearBufferMethod, L"Controls whether we clear the buffer like cmd or like powershell");
+
+    Log::Comment(L"This test checks the shims for cmd.exe and powershell.exe. "
+                 L"Their build in commands for clearing the console buffer "
+                 L"should work to clear the terminal buffer, not just the "
+                 L"terminal viewport.");
+
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& renderer = *g.pRender;
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto* hostTb = &si.GetTextBuffer();
+    auto* termTb = term->_mainBuffer.get();
+
+    auto& sm = si.GetStateMachine();
+
+    _flushFirstFrame();
+
+    _checkConptyOutput = false;
+    _logConpty = false;
+
+    const auto hostView = si.GetViewport();
+    const auto end = 2 * hostView.Height();
+
+    auto writePrompt = [](StateMachine& stateMachine, const auto& path) {
+        // A prompt looks like:
+        // `PWSH C:\> `
+        //
+        // which is 10 characters for "C:\"
+        stateMachine.ProcessString(FTCS_D);
+        stateMachine.ProcessString(FTCS_A);
+        stateMachine.ProcessString(L"\x1b]9;9;");
+        stateMachine.ProcessString(path);
+        stateMachine.ProcessString(L"\x7");
+        stateMachine.ProcessString(L"PWSH ");
+        stateMachine.ProcessString(path);
+        stateMachine.ProcessString(L"> ");
+        stateMachine.ProcessString(FTCS_B);
+    };
+    auto writeCommand = [](StateMachine& stateMachine, const auto& cmd) {
+        stateMachine.ProcessString(cmd);
+        stateMachine.ProcessString(FTCS_C);
+        stateMachine.ProcessString(L"\r\n");
+    };
+
+    for (auto i = 0; i < end; i++)
+    {
+        writePrompt(sm, L"C:\\");
+        writeCommand(sm, L"Foo-bar");
+        sm.ProcessString(L"This is some text     \r\n");
+    }
+    writePrompt(sm, L"C:\\");
+
+    auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& /*viewport*/, const bool afterClear = false) {
+        const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope;
+        const auto marks = tb.GetMarkExtents();
+        if (afterClear)
+        {
+            VERIFY_ARE_EQUAL(0u, marks.size());
+        }
+        else
+        {
+            VERIFY_IS_GREATER_THAN(marks.size(), 1u, L"There should be at least one mark");
+        }
+    };
+
+    Log::Comment(L"========== Checking the host buffer state (before) ==========");
+    verifyBuffer(*hostTb, si.GetViewport().ToExclusive());
+
+    Log::Comment(L"Painting the frame");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    Log::Comment(L"========== Checking the terminal buffer state (before) ==========");
+    verifyBuffer(*termTb, term->_mutableViewport.ToExclusive());
+
+    VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
+    VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());
+
+    _clear(clearBufferMethod, si);
+
+    VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
+    VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());
+
+    Log::Comment(L"Painting the frame");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+    Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
+    verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true);
+}

src/terminal/adapter/adaptDispatch.cpp

@@ -3294,10 +3294,6 @@ bool AdaptDispatch::_EraseAll()
     // Also reset the line rendition for the erased rows.
     textBuffer.ResetLineRenditionRange(newViewportTop, newViewportBottom);
 
-    // Clear any marks that remain below the start of the
-    textBuffer.ClearMarksInRange(til::point{ 0, newViewportTop },
-                                 til::point{ bufferSize.Width(), bufferSize.Height() });
-
     // GH#5683 - If this succeeded, but we're in a conpty, return `false` to
     // make the state machine propagate this ED sequence to the connected
     // terminal application. While we're in conpty mode, when the client