diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index e6b4be530b7..112cf396432 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/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( diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 2c1d8a08ed1..dfbe434f2d2 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/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; } diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 10642d158cc..1c20a85a1a9 100644 --- a/src/renderer/vt/paint.cpp +++ b/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(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 diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 27658e7f9c8..5e9cdb1e716 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/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;