Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
9 commits merged into from
Feb 11, 2020
15 changes: 15 additions & 0 deletions src/renderer/vt/XtermEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -298,6 +312,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
_newBottomLine = false;
}
_deferredCursorPos = INVALID_COORDS;
_delayedEolWrap = false;
return hr;
}

Expand Down
18 changes: 16 additions & 2 deletions src/renderer/vt/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
// probably in the "delayed EOL wrap" state. Different terminals (conhost,
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
// gnome-terminal, wt) all behave differently with how the cursor behaves at
// an end of line. Marke that we're in the delayed EOL wrap state - we don't
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/vt/vtrenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down