Skip to content

Commit

Permalink
Get rid of the ConGetSet::MoveCursorVertically API which isn't used a…
Browse files Browse the repository at this point in the history 
…nymore.
  • Loading branch information
@j4james
j4james committed Dec 22, 2019
1 parent f68dfae commit 4a9c6dd
Show file tree
Hide file tree
Showing 6 changed files with 0 additions and 128 deletions.
61 changes: 0 additions & 61 deletions src/host/getset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1407,67 +1407,6 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool
return Status;
}

// Routine Description:
// - A private API call for moving the cursor vertically in the buffer. This is
// because the vertical cursor movements in VT are constrained by the
// scroll margins, while the absolute positioning is not.
// Parameters:
// - screenInfo - a reference to the screen buffer we should move the cursor for
// - lines - The number of lines to move the cursor. Up is negative, down positive.
// Return value:
// - S_OK if handled successfully. Otherwise an appropriate HRESULT for failing to clamp.
[[nodiscard]] HRESULT DoSrvMoveCursorVertically(SCREEN_INFORMATION& screenInfo, const short lines)
{
auto& cursor = screenInfo.GetActiveBuffer().GetTextBuffer().GetCursor();
COORD clampedPos = { cursor.GetPosition().X, cursor.GetPosition().Y + lines };

// Make sure the cursor doesn't move outside the viewport.
screenInfo.GetViewport().Clamp(clampedPos);

// Make sure the cursor stays inside the margins
if (screenInfo.AreMarginsSet())
{
const auto margins = screenInfo.GetAbsoluteScrollMargins().ToInclusive();

const auto cursorY = cursor.GetPosition().Y;

const auto lo = margins.Top;
const auto hi = margins.Bottom;

// See microsoft/terminal#2929 - If the cursor is _below_ the top
// margin, it should stay below the top margin. If it's _above_ the
// bottom, it should stay above the bottom. Cursor movements that stay
// outside the margins shouldn't necessarily be affected. For example,
// moving up while below the bottom margin shouldn't just jump straight
// to the bottom margin. See
// ScreenBufferTests::CursorUpDownOutsideMargins for a test of that
// behavior.
const bool cursorBelowTop = cursorY >= lo;
const bool cursorAboveBottom = cursorY <= hi;

if (cursorBelowTop)
{
try
{
clampedPos.Y = std::max(clampedPos.Y, lo);
}
CATCH_RETURN();
}

if (cursorAboveBottom)
{
try
{
clampedPos.Y = std::min(clampedPos.Y, hi);
}
CATCH_RETURN();
}
}
cursor.SetPosition(clampedPos);

return S_OK;
}

// Routine Description:
// - A private API call for swapping to the alternate screen buffer. In virtual terminals, there exists both a "main"
// screen buffer and an alternate. ASBSET creates a new alternate, and switches to it. If there is an already
Expand Down
1 change: 0 additions & 1 deletion src/host/getset.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool

[[nodiscard]] NTSTATUS DoSrvPrivateSetScrollingRegion(SCREEN_INFORMATION& screenInfo, const SMALL_RECT& scrollMargins);
[[nodiscard]] NTSTATUS DoSrvPrivateReverseLineFeed(SCREEN_INFORMATION& screenInfo);
[[nodiscard]] HRESULT DoSrvMoveCursorVertically(SCREEN_INFORMATION& screenInfo, const short lines);

[[nodiscard]] NTSTATUS DoSrvPrivateUseAlternateScreenBuffer(SCREEN_INFORMATION& screenInfo);
void DoSrvPrivateUseMainScreenBuffer(SCREEN_INFORMATION& screenInfo);
Expand Down
17 changes: 0 additions & 17 deletions src/host/outputStream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,23 +397,6 @@ bool ConhostInternalGetSet::PrivateReverseLineFeed()
return NT_SUCCESS(DoSrvPrivateReverseLineFeed(_io.GetActiveOutputBuffer()));
}

// Routine Description:
// - Connects the MoveCursorVertically call directly into our Driver Message servicing call inside Conhost.exe
// MoveCursorVertically is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on out public API surface.
// Return Value:
// - true if successful (see DoSrvMoveCursorVertically). false otherwise.
bool ConhostInternalGetSet::MoveCursorVertically(const ptrdiff_t lines)
{
SHORT l;
if (FAILED(PtrdiffTToShort(lines, &l)))
{
return false;
}

return SUCCEEDED(DoSrvMoveCursorVertically(_io.GetActiveOutputBuffer(), l));
}

// Routine Description:
// - Connects the SetConsoleTitleW API call directly into our Driver Message servicing call inside Conhost.exe
// Arguments:
Expand Down
2 changes: 0 additions & 2 deletions src/host/outputStream.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::

bool PrivateReverseLineFeed() override;

bool MoveCursorVertically(const ptrdiff_t lines) override;

bool SetConsoleTitleW(const std::wstring_view title) override;

bool PrivateUseAlternateScreenBuffer() override;
Expand Down
2 changes: 0 additions & 2 deletions src/terminal/adapter/conGetSet.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ namespace Microsoft::Console::VirtualTerminal
virtual bool PrivateSuppressResizeRepaint() = 0;
virtual bool IsConsolePty(bool& isPty) const = 0;

virtual bool MoveCursorVertically(const ptrdiff_t lines) = 0;

virtual bool DeleteLines(const size_t count) = 0;
virtual bool InsertLines(const size_t count) = 0;

Expand Down
45 changes: 0 additions & 45 deletions src/terminal/adapter/ut_adapter/adapterTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,19 +376,6 @@ class TestGetSet final : public ConGetSet
return TRUE;
}

bool MoveCursorVertically(const ptrdiff_t lines) override
{
Log::Comment(L"MoveCursorVertically MOCK called...");
short l;
VERIFY_SUCCEEDED(PtrdiffTToShort(lines, &l));
if (_moveCursorVerticallyResult)
{
VERIFY_ARE_EQUAL(_expectedLines, l);
_cursorPos = { _cursorPos.X, _cursorPos.Y + l };
}
return !!_moveCursorVerticallyResult;
}

bool SetConsoleTitleW(const std::wstring_view title)
{
Log::Comment(L"SetConsoleTitleW MOCK called...");
Expand Down Expand Up @@ -745,8 +732,6 @@ class TestGetSet final : public ConGetSet
// Attribute default is gray on black.
_attribute = FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED;
_expectedAttribute = _attribute;

_expectedLines = 0;
}

void PrepCursor(CursorX xact, CursorY yact)
Expand Down Expand Up @@ -862,7 +847,6 @@ class TestGetSet final : public ConGetSet
bool _expectedMeta = false;
unsigned int _expectedOutputCP = 0;
bool _isPty = false;
short _expectedLines = 0;
bool _privateBoldTextResult = false;
bool _expectedIsBold = false;
bool _isBold = false;
Expand Down Expand Up @@ -921,7 +905,6 @@ class TestGetSet final : public ConGetSet
COLORREF _expectedCursorColor = 0;
bool _getConsoleOutputCPResult = false;
bool _isConsolePtyResult = false;
bool _moveCursorVerticallyResult = false;
bool _privateSetDefaultAttributesResult = false;
bool _moveToBottomResult = false;

Expand Down Expand Up @@ -1041,24 +1024,6 @@ class AdapterTest
Log::Comment(L"Test 1: Cursor doesn't move when placed in corner of viewport.");
_testGetSet->PrepData(direction);

switch (direction)
{
case CursorDirection::UP:
Log::Comment(L"Testing up direction.");
_testGetSet->_expectedLines = -1;
_testGetSet->_moveCursorVerticallyResult = true;
break;
case CursorDirection::DOWN:
Log::Comment(L"Testing down direction.");
_testGetSet->_expectedLines = 1;
_testGetSet->_moveCursorVerticallyResult = true;
break;
default:
_testGetSet->_expectedLines = 0;
_testGetSet->_moveCursorVerticallyResult = false;
break;
}

VERIFY_IS_TRUE((_pDispatch.get()->*(moveFunc))(1));

Log::Comment(L"Test 1b: Cursor moves to left of line with next/prev line command when cursor can't move higher/lower.");
Expand Down Expand Up @@ -1095,13 +1060,9 @@ class AdapterTest
{
case CursorDirection::UP:
_testGetSet->_expectedCursorPos.Y--;
_testGetSet->_expectedLines = -1;
_testGetSet->_moveCursorVerticallyResult = true;
break;
case CursorDirection::DOWN:
_testGetSet->_expectedCursorPos.Y++;
_testGetSet->_expectedLines = 1;
_testGetSet->_moveCursorVerticallyResult = true;
break;
case CursorDirection::RIGHT:
_testGetSet->_expectedCursorPos.X++;
Expand Down Expand Up @@ -1131,13 +1092,9 @@ class AdapterTest
{
case CursorDirection::UP:
_testGetSet->_expectedCursorPos.Y = _testGetSet->_viewport.Top;
_testGetSet->_expectedLines = -100;
_testGetSet->_moveCursorVerticallyResult = true;
break;
case CursorDirection::DOWN:
_testGetSet->_expectedCursorPos.Y = _testGetSet->_viewport.Bottom - 1;
_testGetSet->_expectedLines = 100;
_testGetSet->_moveCursorVerticallyResult = true;
break;
case CursorDirection::RIGHT:
_testGetSet->_expectedCursorPos.X = _testGetSet->_viewport.Right - 1;
Expand Down Expand Up @@ -1196,7 +1153,6 @@ class AdapterTest
Log::Comment(L"Test 6: When SetConsoleCursorPosition throws a failure, call fails and cursor doesn't move.");
_testGetSet->PrepData(direction);
_testGetSet->_setConsoleCursorPositionResult = FALSE;
_testGetSet->_moveCursorVerticallyResult = false;

VERIFY_IS_FALSE((_pDispatch.get()->*(moveFunc))(0));
VERIFY_ARE_EQUAL(_testGetSet->_expectedCursorPos, _testGetSet->_cursorPos);
Expand All @@ -1205,7 +1161,6 @@ class AdapterTest
Log::Comment(L"Test 7: When GetConsoleScreenBufferInfo throws a failure, call fails and cursor doesn't move.");
_testGetSet->PrepData(CursorX::LEFT, CursorY::TOP);
_testGetSet->_getConsoleScreenBufferInfoExResult = FALSE;
_testGetSet->_moveCursorVerticallyResult = true;
Log::Comment(NoThrowString().Format(
L"Cursor Up and Down don't need GetConsoleScreenBufferInfoEx, so they will succeed"));
if (direction == CursorDirection::UP || direction == CursorDirection::DOWN)
Expand Down

0 comments on commit 4a9c6dd

Please sign in to comment.