Skip to content

Commit

Permalink
Use gsl::narrow for narrowing casts, and document the inclusiveness o…
Browse files Browse the repository at this point in the history
…f the scroll rects and the reason for using SHORT_MAX as a right boundary.
  • Loading branch information
j4james committed Sep 9, 2019
1 parent a917d0f commit f370a91
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
6 changes: 4 additions & 2 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1356,7 +1356,8 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool
if (screenInfo.IsCursorInMargins(oldCursorPosition))
{
// Cursor is at the top of the viewport
// Rectangle to cut out of the existing buffer
// Rectangle to cut out of the existing buffer. This is inclusive.
// It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
SMALL_RECT srScroll;
srScroll.Left = 0;
srScroll.Right = SHORT_MAX;
Expand Down Expand Up @@ -2030,7 +2031,8 @@ void DoSrvPrivateModifyLinesImpl(const unsigned int count, const bool insert)
const auto cursorPosition = textBuffer.GetCursor().GetPosition();
if (screenInfo.IsCursorInMargins(cursorPosition))
{
// Rectangle to cut out of the existing buffer
// Rectangle to cut out of the existing buffer. This is inclusive.
// It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
SMALL_RECT srScroll;
srScroll.Left = 0;
srScroll.Right = SHORT_MAX;
Expand Down
8 changes: 4 additions & 4 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3073,7 +3073,7 @@ void _FillLine(COORD position, T fillContent, TextAttribute fillAttr)
template<class T>
void _FillLine(int line, T fillContent, TextAttribute fillAttr)
{
_FillLine({ 0, SHORT(line) }, fillContent, fillAttr);
_FillLine({ 0, gsl::narrow<SHORT>(line) }, fillContent, fillAttr);
}

template<class T>
Expand Down Expand Up @@ -3107,7 +3107,7 @@ bool _ValidateLineContains(COORD position, T expectedContent, TextAttribute expe
template<class T>
bool _ValidateLineContains(int line, T expectedContent, TextAttribute expectedAttr)
{
return _ValidateLineContains({ 0, SHORT(line) }, expectedContent, expectedAttr);
return _ValidateLineContains({ 0, gsl::narrow<SHORT>(line) }, expectedContent, expectedAttr);
}

template<class T>
Expand Down Expand Up @@ -3255,8 +3255,8 @@ void ScreenBufferTests::ScrollOperations()
}

Log::Comment(L"Scrolled area should have moved up/down by given magnitude.");
viewportChar += wchar_t(deletedLines); // Characters dropped when deleting
viewportLine += SHORT(insertedLines); // Lines skipped when inserting
viewportChar += gsl::narrow<wchar_t>(deletedLines); // Characters dropped when deleting
viewportLine += gsl::narrow<SHORT>(insertedLines); // Lines skipped when inserting
while (viewportLine < viewportEnd - deletedLines)
{
VERIFY_IS_TRUE(_ValidateLineContains(viewportLine++, viewportChar++, viewportAttr));
Expand Down
7 changes: 5 additions & 2 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,8 @@ bool AdaptDispatch::_InsertDeleteHelper(_In_ unsigned int const uiCount, const b
RETURN_IF_FALSE(_conApi->GetConsoleScreenBufferInfoEx(&csbiex));

const auto cursor = csbiex.dwCursorPosition;
// Rectangle to cut out of the existing buffer
// Rectangle to cut out of the existing buffer. This is inclusive.
// It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
SMALL_RECT srScroll;
srScroll.Left = cursor.X;
srScroll.Right = SHORT_MAX;
Expand Down Expand Up @@ -955,11 +956,13 @@ bool AdaptDispatch::_ScrollMovement(const ScrollDirection sdDirection, _In_ unsi

if (fSuccess)
{
// Rectangle to cut out of the existing buffer. This is inclusive.
// It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
SMALL_RECT srScreen;
srScreen.Left = 0;
srScreen.Right = SHORT_MAX;
srScreen.Top = csbiex.srWindow.Top;
srScreen.Bottom = csbiex.srWindow.Bottom - 1;
srScreen.Bottom = csbiex.srWindow.Bottom - 1; // srWindow is exclusive, hence the - 1

// Paste coordinate for cut text above
COORD coordDestination;
Expand Down

0 comments on commit f370a91

Please sign in to comment.