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

A pair of fixes related to cursor movement in conpty #4372

Merged
3 commits merged into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1341,7 +1341,14 @@ void DoSrvPrivateShowCursor(SCREEN_INFORMATION& screenInfo, const bool show) noe
void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool fEnable)
{
screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetBlinkingAllowed(fEnable);
screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetIsOn(!fEnable);
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

// GH#2642 - From what we've gathered from other terminals, when blinking is
// disabled, the cursor should remain On always, and have the visibility
// controlled by the IsVisible property. So when you do a printf "\e[?12l"
// to disable blinking, the cursor stays stuck On. At this point, only the
// cursor visibility property controls whether the user can see it or not.
// (Yes, the cursor can be On and NOT Visible)
screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor().SetIsOn(true);
}

// Routine Description:
Expand Down
16 changes: 14 additions & 2 deletions src/host/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,20 @@ void SetActiveScreenBuffer(SCREEN_INFORMATION& screenInfo)
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
gci.pCurrentScreenBuffer = &screenInfo;

// initialize cursor
screenInfo.GetTextBuffer().GetCursor().SetIsOn(false);
// initialize cursor GH#4102 - Typically, the cursor is set to on by the
// cursor blinker. Unfortunately, in conpty mode, there is no cursor
// blinker. So, in conpty mode, we need to leave the cursor on always. The
// cursor can still be set to hidden, and whether the cursor should be
// blinking will still be passed through to the terminal, but internally,
// the cursor should always be on.
//
// In particular, some applications make use of a calling
// `SetConsoleScreenBuffer` and `SetCursorPosition` without printing any
// text in between these calls. If we initialize the cursor to Off in conpty
// mode, then the cursor will remain off until they print text. This can
// lead to alignment problems in the terminal, because we won't move the
// terminal's cursor in this _exact_ scenario.
screenInfo.GetTextBuffer().GetCursor().SetIsOn(gci.IsInVtIoMode());

// set font
screenInfo.RefreshFontWithRenderer();
Expand Down
53 changes: 53 additions & 0 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ class ScreenBufferTests
TEST_METHOD(CursorSaveRestore);

TEST_METHOD(ScreenAlignmentPattern);

TEST_METHOD(TestCursorIsOn);
};

void ScreenBufferTests::SingleAlternateBufferCreationTest()
Expand Down Expand Up @@ -5730,3 +5732,54 @@ void ScreenBufferTests::ScreenAlignmentPattern()
expectedAttr.SetMetaAttributes(0);
VERIFY_ARE_EQUAL(expectedAttr, si.GetAttributes());
}

void ScreenBufferTests::TestCursorIsOn()
{
auto& g = ServiceLocator::LocateGlobals();
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& tbi = si.GetTextBuffer();
auto& stateMachine = si.GetStateMachine();
auto& cursor = tbi.GetCursor();

stateMachine.ProcessString(L"Hello World");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());

stateMachine.ProcessString(L"\x1b[?12l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_FALSE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());

stateMachine.ProcessString(L"\x1b[?12h");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());

cursor.SetIsOn(false);
stateMachine.ProcessString(L"\x1b[?12l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_FALSE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());

stateMachine.ProcessString(L"\x1b[?12h");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());

stateMachine.ProcessString(L"\x1b[?25l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_FALSE(cursor.IsVisible());

stateMachine.ProcessString(L"\x1b[?25h");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_TRUE(cursor.IsBlinkingAllowed());
VERIFY_IS_TRUE(cursor.IsVisible());

stateMachine.ProcessString(L"\x1b[?12;25l");
VERIFY_IS_TRUE(cursor.IsOn());
VERIFY_IS_FALSE(cursor.IsBlinkingAllowed());
VERIFY_IS_FALSE(cursor.IsVisible());
}