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

## Summary of the Pull Request

This is a fix that technically was caused by #357, though we didn't have the Terminal at the time, so I only fixed conhost then. When a client app prints the very last column in the buffer, the cursor is often not _actually_ moved to the next row quite yet. The cursor usually just "floats" on the last character of the row, until something happens. This could be a printable character, which will print it on the next line, or a newline, which will move the cursor to the next line manually, or it could be a backspace, which might take the cursor back a character. 

Conhost and gnome-terminal behave slightly differently here, and wt behaves differently all together. Heck, conhost behaves differently depending on what output mode you're in. 

The scenario in question is typing a full row of text, then hitting backspace to erase the last char of the row.

What we were emitting before in this case was definitely wrong - we'd emit a space at that last row, but then not increment our internal tracker of where the cursor is, so the cursor in conpty and the terminal would be misaligned. The easy fix for this is to make sure to always update the `_lastText` member appropriately. This is the `RightExclusive` change.

The second part of this change is to not be so tricksy immediately following a "delayed eol wrap". When we have just printed the last char like that, always use the VT sequence CUP the next time the cursor moves. Depending on the terminal emulator and it's flags, performing a BS in this state might not bring the cursor to the correct position. 

## References

#405, #780, #357

## PR Checklist
* [x] Closes #1245
* [x] I work here
* [ ] Tests added/passed 
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

With the impending #405 PR I have, this still works, but the sequences that are emitted change, so I didn't write a test for this currently.

## Validation Steps Performed

Tried the scenario for both #357 and #1245 in inception, `gnome-temrinal` and `wt` all, and they all display the cursor correctly.

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;