From 9596b06bffe65a2941f6d7a4fcc0dd9ed4d40eb0 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 7 Jun 2022 16:20:05 -0700 Subject: [PATCH 1/6] Remove most uses of `CompareInBounds` --- src/buffer/out/textBuffer.cpp | 28 ++++++----- .../TerminalCore/TerminalSelection.cpp | 6 +-- src/types/UiaTextRangeBase.cpp | 46 ++++++++----------- src/types/inc/viewport.hpp | 4 +- src/types/viewport.cpp | 15 ++---- 5 files changed, 47 insertions(+), 52 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 859bc5b231f..80d8852961d 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1138,7 +1138,7 @@ til::point TextBuffer::GetWordStart(const til::point target, const std::wstring_ // that it actually points to a space in the buffer copy = bufferSize.BottomRightInclusive(); } - else if (bufferSize.CompareInBounds(target, limit, true) >= 0) + else if (target >= limit) { // if at/past the limit --> clamp to limit copy = limitOptional.value_or(bufferSize.BottomRightInclusive()); @@ -1254,7 +1254,7 @@ til::point TextBuffer::GetWordEnd(const til::point target, const std::wstring_vi // Already at/past the limit. Can't move forward. const auto bufferSize{ GetSize() }; const auto limit{ limitOptional.value_or(bufferSize.EndExclusive()) }; - if (bufferSize.CompareInBounds(target, limit, true) >= 0) + if (target >= limit) { return target; } @@ -1282,7 +1282,7 @@ til::point TextBuffer::_GetWordEndForAccessibility(const til::point target, cons const auto bufferSize{ GetSize() }; auto result{ target }; - if (bufferSize.CompareInBounds(target, limit, true) >= 0) + if (target >= limit) { // if we're already on/past the last RegularChar, // clamp result to that position @@ -1419,7 +1419,7 @@ bool TextBuffer::MoveToNextWord(til::point& pos, const std::wstring_view wordDel const auto limit{ limitOptional.value_or(bufferSize.EndExclusive()) }; const auto copy{ _GetWordEndForAccessibility(pos, wordDelimiters, limit) }; - if (bufferSize.CompareInBounds(copy, limit, true) >= 0) + if (copy >= limit) { return false; } @@ -1466,7 +1466,7 @@ til::point TextBuffer::GetGlyphStart(const til::point pos, std::optional 0) + if (resultPos > limit) { resultPos = limit; } @@ -1494,7 +1494,7 @@ til::point TextBuffer::GetGlyphEnd(const til::point pos, bool accessibilityMode, const auto limit{ limitOptional.value_or(bufferSize.EndExclusive()) }; // Clamp pos to limit - if (bufferSize.CompareInBounds(resultPos, limit, true) > 0) + if (resultPos > limit) { resultPos = limit; } @@ -1524,9 +1524,15 @@ til::point TextBuffer::GetGlyphEnd(const til::point pos, bool accessibilityMode, bool TextBuffer::MoveToNextGlyph(til::point& pos, bool allowExclusiveEnd, std::optional limitOptional) const { const auto bufferSize = GetSize(); - const auto limit{ limitOptional.value_or(bufferSize.EndExclusive()) }; - - const auto distanceToLimit{ bufferSize.CompareInBounds(pos, limit, true) }; + bool pastEndExclusive = false; + const auto limit = [bufferSize, limitOptional](bool& pastEndExclusive) { + const auto endExclusive{ bufferSize.EndExclusive() }; + const auto val = limitOptional.value_or(endExclusive); + pastEndExclusive = val > endExclusive; + return pastEndExclusive ? endExclusive : val; + }(pastEndExclusive); + + const auto distanceToLimit{ bufferSize.CompareInBounds(pos, limit) + (pastEndExclusive ? 1 : 0) }; if (distanceToLimit >= 0) { // Corner Case: we're on/past the limit @@ -1569,7 +1575,7 @@ bool TextBuffer::MoveToPreviousGlyph(til::point& pos, std::optional const auto bufferSize = GetSize(); const auto limit{ limitOptional.value_or(bufferSize.EndExclusive()) }; - if (bufferSize.CompareInBounds(pos, limit, true) > 0) + if (pos > limit) { // we're past the end // clamp us to the limit @@ -1611,7 +1617,7 @@ const std::vector TextBuffer::GetTextRects(til::point start // (0,0) is the top-left of the screen // the physically "higher" coordinate is closer to the top-left // the physically "lower" coordinate is closer to the bottom-right - const auto [higherCoord, lowerCoord] = bufferSize.CompareInBounds(start, end) <= 0 ? + const auto [higherCoord, lowerCoord] = start <= end ? std::make_tuple(start, end) : std::make_tuple(end, start); diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 135dd39422b..c4a58a2dfd3 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -182,7 +182,7 @@ void Terminal::SetSelectionEnd(const til::point viewportPos, std::optional Terminal::_PivotSelection(const til::point targetPos, bool& targetStart) const { - if (targetStart = _activeBuffer().GetSize().CompareInBounds(targetPos, _selection->pivot) <= 0) + if (targetStart = targetPos <= _selection->pivot) { // target is before pivot // treat target as start @@ -424,7 +424,7 @@ void Terminal::_MoveByWord(SelectionDirection direction, til::point& pos) case SelectionDirection::Left: { const auto wordStartPos{ _activeBuffer().GetWordStart(pos, _wordDelimiters) }; - if (_activeBuffer().GetSize().CompareInBounds(_selection->pivot, pos) < 0) + if (_selection->pivot < pos) { // If we're moving towards the pivot, move one more cell pos = wordStartPos; @@ -447,7 +447,7 @@ void Terminal::_MoveByWord(SelectionDirection direction, til::point& pos) case SelectionDirection::Right: { const auto wordEndPos{ _activeBuffer().GetWordEnd(pos, _wordDelimiters) }; - if (_activeBuffer().GetSize().CompareInBounds(pos, _selection->pivot) < 0) + if (pos < _selection->pivot) { // If we're moving towards the pivot, move one more cell pos = _activeBuffer().GetWordEnd(pos, _wordDelimiters); diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 1202b7a34da..78407e663cb 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -70,7 +70,7 @@ try RETURN_IF_FAILED(RuntimeClassInitialize(pData, pProvider, wordDelimiters)); // start is before/at end, so this is valid - if (_pData->GetTextBuffer().GetSize().CompareInBounds(start, end, true) <= 0) + if (start <= end) { _start = start; _end = end; @@ -166,7 +166,7 @@ bool UiaTextRangeBase::SetEndpoint(TextPatternRangeEndpoint endpoint, const til: case TextPatternRangeEndpoint_End: _end = val; // if end is before start... - if (bufferSize.CompareInBounds(_end, _start, true) < 0) + if (_end < _start) { // make this range degenerate at end _start = _end; @@ -175,7 +175,7 @@ bool UiaTextRangeBase::SetEndpoint(TextPatternRangeEndpoint endpoint, const til: case TextPatternRangeEndpoint_Start: _start = val; // if start is after end... - if (bufferSize.CompareInBounds(_start, _end, true) > 0) + if (_start > _end) { // make this range degenerate at start _end = _start; @@ -249,10 +249,10 @@ try // This is a temporary solution to comparing two UTRs from different TextBuffers // Ensure both endpoints fit in the current buffer. const auto bufferSize = _pData->GetTextBuffer().GetSize(); - RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true)); + RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(mine) || !bufferSize.IsInBounds(other)); // compare them - *pRetVal = bufferSize.CompareInBounds(mine, other, true); + *pRetVal = bufferSize.CompareInBounds(mine, other); UiaTracing::TextRange::CompareEndpoints(*this, endpoint, *range, targetEndpoint, *pRetVal); return S_OK; @@ -293,7 +293,7 @@ void UiaTextRangeBase::_expandToEnclosingUnit(TextUnit unit) // If we're past document end, // set us to ONE BEFORE the document end. // This allows us to expand properly. - if (bufferSize.CompareInBounds(_start, documentEnd, true) >= 0) + if (_start >= documentEnd) { _start = documentEnd; bufferSize.DecrementInBounds(_start, true); @@ -653,8 +653,8 @@ try bufferSize.IncrementInBounds(end, true); // make sure what was found is within the bounds of the current range - if ((searchDirection == Search::Direction::Forward && bufferSize.CompareInBounds(end, _end, true) < 0) || - (searchDirection == Search::Direction::Backward && bufferSize.CompareInBounds(start, _start) > 0)) + if ((searchDirection == Search::Direction::Forward && end < _end) || + (searchDirection == Search::Direction::Backward && start > _start)) { RETURN_IF_FAILED(Clone(ppRetVal)); auto& range = static_cast(**ppRetVal); @@ -872,7 +872,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_ // startAnchor: the earliest til::point we will get a bounding rect for auto startAnchor = GetEndpoint(TextPatternRangeEndpoint_Start); - if (bufferSize.CompareInBounds(startAnchor, viewportOrigin, true) < 0) + if (startAnchor < viewportOrigin) { // earliest we can be is the origin startAnchor = viewportOrigin; @@ -880,7 +880,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_ // endAnchor: the latest til::point we will get a bounding rect for auto endAnchor = GetEndpoint(TextPatternRangeEndpoint_End); - if (bufferSize.CompareInBounds(endAnchor, viewportEnd, true) > 0) + if (endAnchor > viewportEnd) { // latest we can be is the viewport end endAnchor = viewportEnd; @@ -889,7 +889,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_ // _end is exclusive, let's be inclusive so we don't have to think about it anymore for bounding rects bufferSize.DecrementInBounds(endAnchor, true); - if (IsDegenerate() || bufferSize.CompareInBounds(_start, viewportEnd, true) > 0 || bufferSize.CompareInBounds(_end, viewportOrigin, true) < 0) + if (IsDegenerate() || _start > viewportEnd || _end < viewportOrigin) { // An empty array is returned for a degenerate (empty) text range. // reference: https://docs.microsoft.com/en-us/windows/win32/api/uiautomationclient/nf-uiautomationclient-iuiautomationtextrange-getboundingrectangles @@ -994,10 +994,7 @@ std::wstring UiaTextRangeBase::_getTextValue(til::CoordType maxLength) const // TODO GH#5406: create a different UIA parent object for each TextBuffer // nvaccess/nvda#11428: Ensure our endpoints are in bounds // otherwise, we'll FailFast catastrophically - if (!bufferSize.IsInBounds(_start, true) || !bufferSize.IsInBounds(_end, true)) - { - THROW_HR(E_FAIL); - } + THROW_HR_IF(E_FAIL, !bufferSize.IsInBounds(_start) || !bufferSize.IsInBounds(_end)); // convert _end to be inclusive auto inclusiveEnd = _end; @@ -1045,11 +1042,11 @@ try constexpr auto endpoint = TextPatternRangeEndpoint::TextPatternRangeEndpoint_Start; const auto bufferSize{ _pData->GetTextBuffer().GetSize() }; const auto documentEnd = _getDocumentEnd(); - if (bufferSize.CompareInBounds(_start, documentEnd, true) > 0) + if (_start > documentEnd) { _start = documentEnd; } - if (bufferSize.CompareInBounds(_end, documentEnd, true) > 0) + if (_end > documentEnd) { _end = documentEnd; } @@ -1119,11 +1116,11 @@ IFACEMETHODIMP UiaTextRangeBase::MoveEndpointByUnit(_In_ TextPatternRangeEndpoin } CATCH_LOG(); - if (bufferSize.CompareInBounds(_start, documentEnd, true) > 0) + if (_start > documentEnd) { _start = documentEnd; } - if (bufferSize.CompareInBounds(_end, documentEnd, true) > 0) + if (_end > documentEnd) { _end = documentEnd; } @@ -1173,7 +1170,7 @@ try const auto bufferSize = _pData->GetTextBuffer().GetSize(); const auto mine = GetEndpoint(endpoint); const auto other = range->GetEndpoint(targetEndpoint); - RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true)); + RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(mine) || !bufferSize.IsInBounds(other)); SetEndpoint(endpoint, range->GetEndpoint(targetEndpoint)); @@ -1199,10 +1196,7 @@ try else { const auto bufferSize = _pData->GetTextBuffer().GetSize(); - if (!bufferSize.IsInBounds(_start, true) || !bufferSize.IsInBounds(_end, true)) - { - return E_FAIL; - } + RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(_start) || !bufferSize.IsInBounds(_end)); auto inclusiveEnd = _end; bufferSize.DecrementInBounds(inclusiveEnd); _pData->SelectNewRegion(_start, inclusiveEnd); @@ -1516,7 +1510,7 @@ void UiaTextRangeBase::_moveEndpointByUnitWord(_In_ const int moveCount, { case MovementDirection::Forward: { - if (bufferSize.CompareInBounds(nextPos, documentEnd, true) >= 0) + if (nextPos >= documentEnd) { success = false; } @@ -1737,7 +1731,7 @@ void UiaTextRangeBase::_moveEndpointByUnitDocument(_In_ const int moveCount, } CATCH_LOG(); - if (preventBoundary || bufferSize.CompareInBounds(target, documentEnd, true) >= 0) + if (preventBoundary || target >= documentEnd) { return; } diff --git a/src/types/inc/viewport.hpp b/src/types/inc/viewport.hpp index d762f64348f..0772d8fe8cb 100644 --- a/src/types/inc/viewport.hpp +++ b/src/types/inc/viewport.hpp @@ -55,7 +55,7 @@ namespace Microsoft::Console::Types til::size Dimensions() const noexcept; bool IsInBounds(const Viewport& other) const noexcept; - bool IsInBounds(const til::point pos, bool allowEndExclusive = false) const noexcept; + bool IsInBounds(const til::point pos) const noexcept; void Clamp(til::point& pos) const; Viewport Clamp(const Viewport& other) const noexcept; @@ -65,7 +65,7 @@ namespace Microsoft::Console::Types bool IncrementInBoundsCircular(til::point& pos) const noexcept; bool DecrementInBounds(til::point& pos, bool allowEndExclusive = false) const noexcept; bool DecrementInBoundsCircular(til::point& pos) const noexcept; - int CompareInBounds(const til::point first, const til::point second, bool allowEndExclusive = false) const noexcept; + int CompareInBounds(const til::point first, const til::point second) const noexcept; enum class XWalk { diff --git a/src/types/viewport.cpp b/src/types/viewport.cpp index 45e1728887b..5b73a716246 100644 --- a/src/types/viewport.cpp +++ b/src/types/viewport.cpp @@ -208,13 +208,8 @@ bool Viewport::IsInBounds(const Viewport& other) const noexcept // includes the last til::point in a given viewport. // Return Value: // - True if it lies inside the viewport. False otherwise. -bool Viewport::IsInBounds(const til::point pos, bool allowEndExclusive) const noexcept +bool Viewport::IsInBounds(const til::point pos) const noexcept { - if (allowEndExclusive && pos == EndExclusive()) - { - return true; - } - return pos.X >= Left() && pos.X < RightExclusive() && pos.Y >= Top() && pos.Y < BottomExclusive(); } @@ -365,11 +360,11 @@ bool Viewport::DecrementInBoundsCircular(til::point& pos) const noexcept // - This is so you can do s_CompareCoords(first, second) <= 0 for "first is left or the same as second". // (the < looks like a left arrow :D) // - The magnitude of the result is the distance between the two coordinates when typing characters into the buffer (left to right, top to bottom) -int Viewport::CompareInBounds(const til::point first, const til::point second, bool allowEndExclusive) const noexcept +int Viewport::CompareInBounds(const til::point first, const til::point second) const noexcept { // Assert that our coordinates are within the expected boundaries - FAIL_FAST_IF(!IsInBounds(first, allowEndExclusive)); - FAIL_FAST_IF(!IsInBounds(second, allowEndExclusive)); + FAIL_FAST_IF(!IsInBounds(first)); + FAIL_FAST_IF(!IsInBounds(second)); // First set the distance vertically // If first is on row 4 and second is on row 6, first will be -2 rows behind second * an 80 character row would be -160. @@ -432,7 +427,7 @@ bool Viewport::WalkInBounds(til::point& pos, const WalkDir dir, bool allowEndExc bool Viewport::WalkInBoundsCircular(til::point& pos, const WalkDir dir, bool allowEndExclusive) const noexcept { // Assert that the position given fits inside this viewport. - FAIL_FAST_IF(!IsInBounds(pos, allowEndExclusive)); + assert((allowEndExclusive && pos == EndExclusive()) || IsInBounds(pos)); if (dir.x == XWalk::LeftToRight) { From 4ce925ede855923ca78fb88f66d56efe315545ea Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 9 Jun 2022 08:57:41 -0700 Subject: [PATCH 2/6] address PR comments --- src/types/UiaTextRangeBase.cpp | 7 +++---- src/types/viewport.cpp | 6 ------ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 78407e663cb..9e045523834 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -248,11 +248,10 @@ try // TODO GH#5406: create a different UIA parent object for each TextBuffer // This is a temporary solution to comparing two UTRs from different TextBuffers // Ensure both endpoints fit in the current buffer. - const auto bufferSize = _pData->GetTextBuffer().GetSize(); - RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(mine) || !bufferSize.IsInBounds(other)); - // compare them - *pRetVal = bufferSize.CompareInBounds(mine, other); + // directly calculate the distance between the two endpoints + *pRetVal = (mine.Y - other.Y) * _pData->GetTextBuffer().GetSize().Width(); + *pRetVal += (mine.X - other.X); UiaTracing::TextRange::CompareEndpoints(*this, endpoint, *range, targetEndpoint, *pRetVal); return S_OK; diff --git a/src/types/viewport.cpp b/src/types/viewport.cpp index 5b73a716246..6af518a5e8e 100644 --- a/src/types/viewport.cpp +++ b/src/types/viewport.cpp @@ -203,9 +203,6 @@ bool Viewport::IsInBounds(const Viewport& other) const noexcept // - Determines if the given coordinate position lies within this viewport. // Arguments: // - pos - Coordinate position -// - allowEndExclusive - if true, allow the EndExclusive til::point as a valid position. -// Used in accessibility to signify that the exclusive end -// includes the last til::point in a given viewport. // Return Value: // - True if it lies inside the viewport. False otherwise. bool Viewport::IsInBounds(const til::point pos) const noexcept @@ -350,9 +347,6 @@ bool Viewport::DecrementInBoundsCircular(til::point& pos) const noexcept // Arguments: // - first- The first coordinate position // - second - The second coordinate position -// - allowEndExclusive - if true, allow the EndExclusive til::point as a valid position. -// Used in accessibility to signify that the exclusive end -// includes the last til::point in a given viewport. // Return Value: // - Negative if First is to the left of the Second. // - 0 if First and Second are the same coordinate. From b105d68a26193058f985be903cd8a884fa8fcc49 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 9 Jun 2022 11:55:34 -0700 Subject: [PATCH 3/6] clamp to inclusive pos & compare returns 0/1 --- src/buffer/out/textBuffer.cpp | 18 +++++++++--------- src/types/UiaTextRangeBase.cpp | 11 ++++++----- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 80d8852961d..e99add10e4f 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1524,15 +1524,15 @@ til::point TextBuffer::GetGlyphEnd(const til::point pos, bool accessibilityMode, bool TextBuffer::MoveToNextGlyph(til::point& pos, bool allowExclusiveEnd, std::optional limitOptional) const { const auto bufferSize = GetSize(); - bool pastEndExclusive = false; - const auto limit = [bufferSize, limitOptional](bool& pastEndExclusive) { - const auto endExclusive{ bufferSize.EndExclusive() }; - const auto val = limitOptional.value_or(endExclusive); - pastEndExclusive = val > endExclusive; - return pastEndExclusive ? endExclusive : val; - }(pastEndExclusive); - - const auto distanceToLimit{ bufferSize.CompareInBounds(pos, limit) + (pastEndExclusive ? 1 : 0) }; + bool pastEndInclusive = false; + const auto limit = [bufferSize, limitOptional](bool& pastEndInclusive) { + const auto endInclusive{ bufferSize.BottomRightInclusive() }; + const auto val = limitOptional.value_or(endInclusive); + pastEndInclusive = val > endInclusive; + return pastEndInclusive ? endInclusive : val; + }(pastEndInclusive); + + const auto distanceToLimit{ bufferSize.CompareInBounds(pos, limit) + (pastEndInclusive ? 1 : 0) }; if (distanceToLimit >= 0) { // Corner Case: we're on/past the limit diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 9e045523834..d9ef72bda44 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -246,12 +246,13 @@ try const auto mine = GetEndpoint(endpoint); // TODO GH#5406: create a different UIA parent object for each TextBuffer - // This is a temporary solution to comparing two UTRs from different TextBuffers - // Ensure both endpoints fit in the current buffer. + // We should return E_FAIL if we detect that the endpoints are from two different TextBuffer objects. + // For now, we're fine to not do that because we're not using any code that can FAIL_FAST anymore. - // directly calculate the distance between the two endpoints - *pRetVal = (mine.Y - other.Y) * _pData->GetTextBuffer().GetSize().Width(); - *pRetVal += (mine.X - other.X); + // compare them + *pRetVal = mine < other ? -1 : + mine > other ? 1 : + 0; UiaTracing::TextRange::CompareEndpoints(*this, endpoint, *range, targetEndpoint, *pRetVal); return S_OK; From e9b2ffc4554688a4d5dca9995d27801c09a4b8fe Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 9 Jun 2022 13:47:59 -0700 Subject: [PATCH 4/6] fix test --- .../win32/ut_interactivity_win32/UiaTextRangeTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index 90e0302861a..d73379d548e 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -484,9 +484,9 @@ class UiaTextRangeTests Log::Comment(L"_start and end should be 2 units apart. Sign depends on order of comparison."); THROW_IF_FAILED(utr1->CompareEndpoints(TextPatternRangeEndpoint_End, utr2.Get(), TextPatternRangeEndpoint_End, &comparison)); - VERIFY_IS_TRUE(comparison == -2); + VERIFY_IS_TRUE(comparison == -1); THROW_IF_FAILED(utr2->CompareEndpoints(TextPatternRangeEndpoint_End, utr1.Get(), TextPatternRangeEndpoint_End, &comparison)); - VERIFY_IS_TRUE(comparison == 2); + VERIFY_IS_TRUE(comparison == 1); } TEST_METHOD(ExpandToEnclosingUnit) From e0695ec1404cb1a3f1185cf7bdb625a5a5b32d19 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 10 Jun 2022 09:13:35 -0700 Subject: [PATCH 5/6] make 'MoveToNextGlyph' prettier --- src/buffer/out/textBuffer.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index e99add10e4f..ac699ec4351 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1524,13 +1524,17 @@ til::point TextBuffer::GetGlyphEnd(const til::point pos, bool accessibilityMode, bool TextBuffer::MoveToNextGlyph(til::point& pos, bool allowExclusiveEnd, std::optional limitOptional) const { const auto bufferSize = GetSize(); - bool pastEndInclusive = false; - const auto limit = [bufferSize, limitOptional](bool& pastEndInclusive) { + bool pastEndInclusive; + til::point limit; + { + // if the limit is past the end of the buffer, + // 1) clamp limit to end of buffer + // 2) set pastEndInclusive const auto endInclusive{ bufferSize.BottomRightInclusive() }; const auto val = limitOptional.value_or(endInclusive); pastEndInclusive = val > endInclusive; - return pastEndInclusive ? endInclusive : val; - }(pastEndInclusive); + limit = pastEndInclusive ? endInclusive : val; + } const auto distanceToLimit{ bufferSize.CompareInBounds(pos, limit) + (pastEndInclusive ? 1 : 0) }; if (distanceToLimit >= 0) From c98940a95b256e3955877f66eb2891f7070da663 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 30 Jun 2022 16:31:41 -0700 Subject: [PATCH 6/6] backwards --- src/types/viewport.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/types/viewport.cpp b/src/types/viewport.cpp index 23d93de10ac..f0eb50cf33e 100644 --- a/src/types/viewport.cpp +++ b/src/types/viewport.cpp @@ -357,8 +357,8 @@ bool Viewport::DecrementInBoundsCircular(til::point& pos) const noexcept int Viewport::CompareInBounds(const til::point first, const til::point second) const noexcept { // Assert that our coordinates are within the expected boundaries - assert(!IsInBounds(first)); - assert(!IsInBounds(second)); + assert(IsInBounds(first)); + assert(IsInBounds(second)); // First set the distance vertically // If first is on row 4 and second is on row 6, first will be -2 rows behind second * an 80 character row would be -160.