Skip to content

Commit

Permalink
Add support for setting the cursor visibility in Terminal (#4902)
Browse files Browse the repository at this point in the history
Adds support for setting the cursor visibility in Terminal. Visibility
is a property entirely independent from whether the cursor is "on" or
not. The cursor blinker _should_ change the "IsOn" property. It was
actually changing the "Visible" property, which was incorrect. This PR
additionally corrects the naming of the method used by the cursor
blinker, and makes it do the right thing.

I added a pair of tests, one taken straight from conhost. In
copy-pasting that test, I took it a step further and implemented
`^[[?12h`, `^[[?12l`, which enables/disables cursor blinking, for the
`TerminalCore`. THIS DOES NOT ADD SUPPORT FOR DISABLING BLINKING IN THE
APP. Conpty doesn't emit the blinking on/off sequences quite yet, but
when it _does_, the Terminal will be ready.

## References
* I'd bet this conflicts with #2892
* This isn't a solution for #1379
* There shockingly isn't an issue for cursor blink state via conpty...?

## PR Checklist
* [x] Closes #3093
* [x] Closes #3499
* [x] Closes #4644
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
  • Loading branch information
zadjii-msft authored Mar 13, 2020
1 parent c530d2a commit 38058a7
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 91 deletions.
4 changes: 2 additions & 2 deletions src/cascadia/PublicTerminalCore/HwndTerminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,13 +527,13 @@ void _stdcall TerminalBlinkCursor(void* terminal)
return;
}

publicTerminal->_terminal->SetCursorVisible(!publicTerminal->_terminal->IsCursorVisible());
publicTerminal->_terminal->SetCursorOn(!publicTerminal->_terminal->IsCursorOn());
}

void _stdcall TerminalSetCursorVisible(void* terminal, const bool visible)
{
const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
publicTerminal->_terminal->SetCursorVisible(visible);
publicTerminal->_terminal->SetCursorOn(visible);
}

// Routine Description:
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
// Manually show the cursor when a key is pressed. Restarting
// the timer prevents flickering.
_terminal->SetCursorVisible(true);
_terminal->SetCursorOn(true);
_cursorTimer.value().Start();
}

Expand Down Expand Up @@ -1447,7 +1447,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
if (_cursorTimer.has_value())
{
// When the terminal focuses, show the cursor immediately
_terminal->SetCursorVisible(true);
_terminal->SetCursorOn(true);
_cursorTimer.value().Start();
}
_rowsToScroll = _settings.RowsToScroll();
Expand Down Expand Up @@ -1479,7 +1479,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
if (_cursorTimer.has_value())
{
_cursorTimer.value().Stop();
_terminal->SetCursorVisible(false);
_terminal->SetCursorOn(false);
}
}

Expand Down Expand Up @@ -1619,7 +1619,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
return;
}
_terminal->SetCursorVisible(!_terminal->IsCursorVisible());
_terminal->SetCursorOn(!_terminal->IsCursorOn());
}

// Method Description:
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalCore/ITerminalApi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ namespace Microsoft::Terminal::Core

virtual bool SetCursorPosition(short x, short y) noexcept = 0;
virtual COORD GetCursorPosition() noexcept = 0;
virtual bool SetCursorVisibility(const bool visible) noexcept = 0;
virtual bool CursorLineFeed(const bool withReturn) noexcept = 0;
virtual bool EnableCursorBlinking(const bool enable) noexcept = 0;

virtual bool DeleteCharacter(const size_t count) noexcept = 0;
virtual bool InsertCharacter(const size_t count) noexcept = 0;
Expand Down
11 changes: 7 additions & 4 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -740,13 +740,16 @@ try
CATCH_LOG()

// Method Description:
// - Sets the visibility of the text cursor.
// - Sets the cursor to be currently on. On/Off is tracked independently of
// cursor visibility (hidden/visible). On/off is controlled by the cursor
// blinker. Visibility is usually controlled by the client application. If the
// cursor is hidden, then the cursor will remain hidden. If the cursor is
// Visible, then it will immediately become visible.
// Arguments:
// - isVisible: whether the cursor should be visible
void Terminal::SetCursorVisible(const bool isVisible) noexcept
void Terminal::SetCursorOn(const bool isOn) noexcept
{
auto& cursor = _buffer->GetCursor();
cursor.SetIsVisible(isVisible);
_buffer->GetCursor().SetIsOn(isOn);
}

bool Terminal::IsCursorBlinkingAllowed() const noexcept
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ namespace Microsoft::Terminal::Core
namespace TerminalCoreUnitTests
{
class TerminalBufferTests;
class TerminalApiTest;
class ConptyRoundtripTests;
};
#endif
Expand Down Expand Up @@ -84,6 +85,8 @@ class Microsoft::Terminal::Core::Terminal final :
bool ReverseText(bool reversed) noexcept override;
bool SetCursorPosition(short x, short y) noexcept override;
COORD GetCursorPosition() noexcept override;
bool SetCursorVisibility(const bool visible) noexcept override;
bool EnableCursorBlinking(const bool enable) noexcept override;
bool CursorLineFeed(const bool withReturn) noexcept override;
bool DeleteCharacter(const size_t count) noexcept override;
bool InsertCharacter(const size_t count) noexcept override;
Expand Down Expand Up @@ -166,7 +169,7 @@ class Microsoft::Terminal::Core::Terminal final :
void SetScrollPositionChangedCallback(std::function<void(const int, const int, const int)> pfn) noexcept;
void SetBackgroundCallback(std::function<void(const uint32_t)> pfn) noexcept;

void SetCursorVisible(const bool isVisible) noexcept;
void SetCursorOn(const bool isOn) noexcept;
bool IsCursorBlinkingAllowed() const noexcept;

#pragma region TextSelection
Expand Down Expand Up @@ -276,6 +279,7 @@ class Microsoft::Terminal::Core::Terminal final :

#ifdef UNIT_TESTING
friend class TerminalCoreUnitTests::TerminalBufferTests;
friend class TerminalCoreUnitTests::TerminalApiTest;
friend class TerminalCoreUnitTests::ConptyRoundtripTests;
#endif
};
20 changes: 20 additions & 0 deletions src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -573,3 +573,23 @@ bool Terminal::IsVtInputEnabled() const noexcept
// We should never be getting this call in Terminal.
FAIL_FAST();
}

bool Terminal::SetCursorVisibility(const bool visible) noexcept
{
_buffer->GetCursor().SetIsVisible(visible);
return true;
}

bool Terminal::EnableCursorBlinking(const bool enable) noexcept
{
_buffer->GetCursor().SetBlinkingAllowed(enable);

// 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)
_buffer->GetCursor().SetIsOn(true);
return true;
}
16 changes: 16 additions & 0 deletions src/cascadia/TerminalCore/TerminalDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ try
}
CATCH_LOG_RETURN_FALSE()

bool TerminalDispatch::CursorVisibility(const bool isVisible) noexcept
{
return _terminalApi.SetCursorVisibility(isVisible);
}

bool TerminalDispatch::EnableCursorBlinking(const bool enable) noexcept
{
return _terminalApi.EnableCursorBlinking(enable);
}

bool TerminalDispatch::CursorForward(const size_t distance) noexcept
try
{
Expand Down Expand Up @@ -378,6 +388,12 @@ bool TerminalDispatch::_PrivateModeParamsHelper(const DispatchTypes::PrivateMode
case DispatchTypes::PrivateModeParams::ALTERNATE_SCROLL:
success = EnableAlternateScroll(enable);
break;
case DispatchTypes::PrivateModeParams::DECTCEM_TextCursorEnableMode:
success = CursorVisibility(enable);
break;
case DispatchTypes::PrivateModeParams::ATT610_StartCursorBlink:
success = EnableCursorBlinking(enable);
break;
default:
// If no functions to call, overall dispatch was a failure.
success = false;
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalCore/TerminalDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ class TerminalDispatch : public Microsoft::Console::VirtualTerminal::TermDispatc
bool CursorPosition(const size_t line,
const size_t column) noexcept override; // CUP

bool CursorVisibility(const bool isVisible) noexcept override; // DECTCEM
bool EnableCursorBlinking(const bool enable) noexcept override; // ATT610

bool CursorForward(const size_t distance) noexcept override;
bool CursorBackward(const size_t distance) noexcept override;
bool CursorUp(const size_t distance) noexcept override;
Expand Down
Loading

0 comments on commit 38058a7

Please sign in to comment.