Move cursor in conpty correctly after a backspace when we've delayed an EOL wrap

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -128,6 +128,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
         g.EnableConptyModeForTests();
 
         expectedOutput.clear();
+        _checkConptyOutput = true;
+        _logConpty = false;
 
         return true;
     }
@@ -152,6 +154,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
     TEST_METHOD(WriteAFewSimpleLines);
     TEST_METHOD(PassthroughClearScrollback);
 
+    TEST_METHOD(MoveCursorAtEOL);
+
 private:
     bool _writeCallback(const char* const pch, size_t const cch);
     void _flushFirstFrame();
@@ -344,6 +348,70 @@ void ConptyRoundtripTests::WriteAFewSimpleLines()
     verifyData(termTb);
 }
 
+void ConptyRoundtripTests::MoveCursorAtEOL()
+{
+    // This is a test for GH#1245
+    VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
+
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& renderer = *g.pRender;
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& hostSm = si.GetStateMachine();
+    auto& hostTb = si.GetTextBuffer();
+    auto& termTb = *term->_buffer;
+
+    _flushFirstFrame();
+
+    Log::Comment(NoThrowString().Format(
+        L"Write exactly a full line of text"));
+    hostSm.ProcessString(std::wstring(TerminalViewWidth, L'A'));
+
+    auto verifyData0 = [](TextBuffer& tb) {
+        auto iter = tb.GetCellDataAt({ 0, 0 });
+        TestUtils::VerifySpanOfText(L"A", iter, 0, TerminalViewWidth);
+        TestUtils::VerifySpanOfText(L" ", iter, 0, TerminalViewWidth);
+    };
+
+    verifyData0(hostTb);
+
+    // TODO: GH#405/#4415 - Before #405 merges, the VT sequences conpty emits
+    // might change, but the buffer contents shouldn't.
+    // If they do change and these tests break, that's to be expected.
+    expectedOutput.push_back(std::string(TerminalViewWidth, 'A'));
+    expectedOutput.push_back("\x1b[1;80H");
+
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    verifyData0(termTb);
+
+    Log::Comment(NoThrowString().Format(
+        L"Emulate backspacing at a bash prompt when the previous line wrapped.\n"
+        L"We'll move the cursor up to the last char of the prev line, and erase it."));
+    hostSm.ProcessString(L"\x1b[1;80H");
+    hostSm.ProcessString(L"\x1b[K");
+
+    auto verifyData1 = [](TextBuffer& tb) {
+        auto iter = tb.GetCellDataAt({ 0, 0 });
+        // There should be 79 'A's, followed by a space, and the following line should be blank.
+        TestUtils::VerifySpanOfText(L"A", iter, 0, TerminalViewWidth - 1);
+        TestUtils::VerifySpanOfText(L" ", iter, 0, 1);
+        TestUtils::VerifySpanOfText(L" ", iter, 0, TerminalViewWidth);
+
+        auto& cursor = tb.GetCursor();
+        VERIFY_ARE_EQUAL(TerminalViewWidth - 1, cursor.GetPosition().X);
+        VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y);
+    };
+
+    verifyData1(hostTb);
+
+    expectedOutput.push_back(" ");
+    expectedOutput.push_back("\x1b[1;80H");
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    verifyData1(termTb);
+}
+
 void ConptyRoundtripTests::PassthroughClearScrollback()
 {
     Log::Comment(NoThrowString().Format(

src/renderer/vt/XtermEngine.cpp

@@ -258,6 +258,20 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
                 hr = _Write(seq);
             }
         }
+        else if (_delayedEolWrap)
+        {
+            // GH#1245, GH#357 - If we were in the delayed EOL wrap state, make
+            // sure to _manually_ position the cursor now, with a full CUP
+            // sequence, don't try and be clever with \b or \r or other control
+            // sequences. Different terminals (conhost, gnome-terminal, wt) all
+            // behave differently with how the cursor behaves at an end of line.
+            // This is the only solution that works in all of them, and also
+            // works wrapped lines emitted by conpty.
+            //
+            // Make sure to do this _after_ the possible \r\n branch above,
+            // otherwise we might accidentally break wrapped lines (GH#405)
+            hr = _CursorPosition(coord);
+        }
         else if (coord.X == 0 && coord.Y == _lastText.Y)
         {
             // Start of this line
@@ -298,6 +312,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
         _newBottomLine = false;
     }
     _deferredCursorPos = INVALID_COORDS;
+    _delayedEolWrap = false;
     return hr;
 }
 

src/renderer/vt/paint.cpp

@@ -450,7 +450,7 @@ using namespace Microsoft::Console::Types;
     RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8(wstr));
 
     // Update our internal tracker of the cursor's position.
-    // See MSFT:20266233
+    // See MSFT:20266233 (which is also GH#357)
     // If the cursor is at the rightmost column of the terminal, and we write a
     //      space, the cursor won't actually move to the next cell (which would
     //      be {0, _lastText.Y++}). The cursor will stay visibly in that last
@@ -461,10 +461,24 @@ using namespace Microsoft::Console::Types;
     //      we'll determine that we need to emit a \b to put the cursor in the
     //      right position. This is wrong, and will cause us to move the cursor
     //      back one character more than we wanted.
-    if (_lastText.X < _lastViewport.RightInclusive())
+    //
+    // GH#1245: This needs to be RightExclusive, _not_ inclusive. Otherwise, we
+    // won't update our internal cursor position tracker correctly at the last
+    // character of the row.
+    if (_lastText.X < _lastViewport.RightExclusive())
     {
         _lastText.X += static_cast<short>(columnsActual);
     }
+    // GH#1245: If we wrote the exactly last char of the row, then we're in the
+    // "delayed EOL wrap" state. Different terminals (conhost, gnome-terminal,
+    // wt) all behave differently with how the cursor behaves at an end of line.
+    // Mark that we're in the delayed EOL wrap state - we don't want to be
+    // clever about how we move the cursor in this state, since different
+    // terminals will handle a backspace differently in this state.
+    if (_lastText.X >= _lastViewport.RightInclusive())
+    {
+        _delayedEolWrap = true;
+    }
 
     short sNumSpaces;
     try

src/renderer/vt/vtrenderer.hpp

@@ -144,6 +144,8 @@ namespace Microsoft::Console::Render
         Microsoft::Console::VirtualTerminal::RenderTracing _trace;
         bool _inResizeRequest{ false };
 
+        bool _delayedEolWrap{ false };
+
         [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept;
         [[nodiscard]] HRESULT _WriteFormattedString(const std::string* const pFormat, ...) noexcept;
         [[nodiscard]] HRESULT _Flush() noexcept;