diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 294813a3f55..9bd34412fd5 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -996,19 +996,29 @@ const COORD TextBuffer::GetWordStart(const COORD target, const std::wstring_view // so the words in the example include ["word ", "other "] // NOTE: the start anchor (this one) is inclusive, whereas the end anchor (GetWordEnd) is exclusive - // can't expand left - if (target.X == GetSize().Left()) - { +#pragma warning(suppress : 26496) + // GH#7664: Treat EndExclusive as EndInclusive so + // that it actually points to a space in the buffer + auto copy{ target }; + const auto bufferSize{ GetSize() }; + if (target == bufferSize.Origin()) + { + // can't expand left return target; } + else if (target == bufferSize.EndExclusive()) + { + // treat EndExclusive as EndInclusive + copy = { bufferSize.RightInclusive(), bufferSize.BottomInclusive() }; + } if (accessibilityMode) { - return _GetWordStartForAccessibility(target, wordDelimiters); + return _GetWordStartForAccessibility(copy, wordDelimiters); } else { - return _GetWordStartForSelection(target, wordDelimiters); + return _GetWordStartForSelection(copy, wordDelimiters); } } @@ -1108,6 +1118,12 @@ const COORD TextBuffer::GetWordEnd(const COORD target, const std::wstring_view w // so the words in the example include ["word ", "other "] // NOTE: the end anchor (this one) is exclusive, whereas the start anchor (GetWordStart) is inclusive + // Already at the end. Can't move forward. + if (target == GetSize().EndExclusive()) + { + return target; + } + if (accessibilityMode) { return _GetWordEndForAccessibility(target, wordDelimiters); @@ -1243,6 +1259,12 @@ bool TextBuffer::MoveToNextWord(COORD& pos, const std::wstring_view wordDelimite auto copy = pos; const auto bufferSize = GetSize(); + // Already at the end. Can't move forward. + if (pos == bufferSize.EndExclusive()) + { + return false; + } + // started on a word, continue until the end of the word while (_GetDelimiterClassAt(copy, wordDelimiters) == DelimiterClass::RegularChar) { @@ -1287,7 +1309,14 @@ bool TextBuffer::MoveToNextWord(COORD& pos, const std::wstring_view wordDelimite bool TextBuffer::MoveToPreviousWord(COORD& pos, std::wstring_view wordDelimiters) const { auto copy = pos; - auto bufferSize = GetSize(); + const auto bufferSize{ GetSize() }; + + // GH#7663: Treat EndExclusive as EndInclusive so + // that it actually points to a space in the buffer + if (pos == bufferSize.EndExclusive()) + { + copy = { bufferSize.RightInclusive(), bufferSize.BottomInclusive() }; + } // started on whitespace/delimiter, continue until the end of the previous word while (_GetDelimiterClassAt(copy, wordDelimiters) != DelimiterClass::RegularChar) diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 1ee79447b08..638e15c1fe4 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -2065,7 +2065,7 @@ void TextBufferTests::GetWordBoundaries() { { 79, 0 }, {{ 10, 0 }, { 5, 0 }} }, // tests for second line of text - { { 0, 1 }, {{ 0, 1 }, { 0, 1 }} }, + { { 0, 1 }, {{ 0, 1 }, { 5, 0 }} }, { { 1, 1 }, {{ 0, 1 }, { 5, 0 }} }, { { 2, 1 }, {{ 2, 1 }, { 2, 1 }} }, { { 3, 1 }, {{ 2, 1 }, { 2, 1 }} }, diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index 414d3f52e68..50e36e66f5c 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -106,6 +106,26 @@ class UiaTextRangeTests ExpectedResult expected; }; + static constexpr wchar_t* toString(TextUnit unit) noexcept + { + // if a format is not supported, it goes to the next largest text unit + switch (unit) + { + case TextUnit_Character: + return L"Character"; + case TextUnit_Format: + case TextUnit_Word: + return L"Word"; + case TextUnit_Line: + return L"Line"; + case TextUnit_Paragraph: + case TextUnit_Page: + case TextUnit_Document: + default: + return L"Document"; + } + }; + TEST_METHOD_SETUP(MethodSetup) { CONSOLE_INFORMATION& gci = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals().getConsoleInformation(); @@ -304,26 +324,6 @@ class UiaTextRangeTests // based on the text unit we are testing constexpr TextUnit supportedUnits[] = { TextUnit_Character, TextUnit_Word, TextUnit_Line, TextUnit_Document }; - auto toString = [&](TextUnit unit) { - // if a format is not supported, it goes to the next largest text unit - switch (unit) - { - case TextUnit_Character: - return L"Character"; - case TextUnit_Format: - case TextUnit_Word: - return L"Word"; - case TextUnit_Line: - return L"Line"; - case TextUnit_Paragraph: - case TextUnit_Page: - case TextUnit_Document: - return L"Document"; - default: - throw E_INVALIDARG; - } - }; - struct TextUnitBoundaries { COORD start; @@ -1103,4 +1103,91 @@ class UiaTextRangeTests VERIFY_ARE_EQUAL(test.expected.end, utr->_end); } } + + TEST_METHOD(ExpansionAtExclusiveEnd) + { + // GH#7664: When attempting to expand to an enclosing unit + // at the end exclusive, the UTR should refuse to move past + // the end. + + const auto bufferSize{ _pTextBuffer->GetSize() }; + const til::point endInclusive{ bufferSize.RightInclusive(), bufferSize.BottomInclusive() }; + const auto endExclusive{ bufferSize.EndExclusive() }; + + // Iterate over each TextUnit. If the we don't support + // the given TextUnit, we're supposed to fallback + // to the last one that was defined anyways. + Microsoft::WRL::ComPtr utr; + for (int unit = TextUnit::TextUnit_Character; unit != TextUnit::TextUnit_Document; ++unit) + { + Log::Comment(NoThrowString().Format(L"%s", toString(static_cast(unit)))); + + // Create a degenerate UTR at EndExclusive + THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider, endInclusive, endExclusive)); + THROW_IF_FAILED(utr->ExpandToEnclosingUnit(static_cast(unit))); + + VERIFY_ARE_EQUAL(endExclusive, utr->_end); + } + } + + TEST_METHOD(MovementAtExclusiveEnd) + { + // GH#7663: When attempting to move from end exclusive, + // the UTR should refuse to move past the end. + + const auto bufferSize{ _pTextBuffer->GetSize() }; + const til::point endInclusive = { bufferSize.RightInclusive(), bufferSize.BottomInclusive() }; + const auto endExclusive{ bufferSize.EndExclusive() }; + + // Iterate over each TextUnit. If the we don't support + // the given TextUnit, we're supposed to fallback + // to the last one that was defined anyways. + Microsoft::WRL::ComPtr utr; + int moveAmt; + for (int unit = TextUnit::TextUnit_Character; unit != TextUnit::TextUnit_Document; ++unit) + { + Log::Comment(NoThrowString().Format(L"Forward by %s", toString(static_cast(unit)))); + + // Create a degenerate UTR at EndExclusive + THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider, endInclusive, endExclusive)); + THROW_IF_FAILED(utr->Move(static_cast(unit), 1, &moveAmt)); + + VERIFY_ARE_EQUAL(endExclusive, utr->_end); + VERIFY_ARE_EQUAL(0, moveAmt); + } + + // Verify that moving backwards still works properly + const COORD writeTarget{ 2, 2 }; + _pTextBuffer->Write({ L"temp" }, writeTarget); + for (int unit = TextUnit::TextUnit_Character; unit != TextUnit::TextUnit_Document; ++unit) + { + COORD expectedEnd; + switch (static_cast(unit)) + { + case TextUnit::TextUnit_Character: + expectedEnd = endInclusive; + break; + case TextUnit::TextUnit_Word: + expectedEnd = writeTarget; + break; + case TextUnit::TextUnit_Line: + expectedEnd = endExclusive; + break; + case TextUnit::TextUnit_Document: + expectedEnd = bufferSize.Origin(); + break; + default: + continue; + } + + Log::Comment(NoThrowString().Format(L"Backwards by %s", toString(static_cast(unit)))); + + // Create a degenerate UTR at EndExclusive + THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider, endInclusive, endExclusive)); + THROW_IF_FAILED(utr->Move(static_cast(unit), -1, &moveAmt)); + + VERIFY_ARE_EQUAL(expectedEnd, utr->_end); + VERIFY_ARE_EQUAL(-1, moveAmt); + } + } };