Skip to content

Comments

Simplify word expansion functions#19882

Open
carlos-zamora wants to merge 2 commits intomainfrom
dev/cazamor/refactor-word-nav
Open

Simplify word expansion functions#19882
carlos-zamora wants to merge 2 commits intomainfrom
dev/cazamor/refactor-word-nav

Conversation

@carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Feb 18, 2026

Summary of the Pull Request

Simplifies the word expansion functions in TextBuffer by removing the following functions:

  • GetWordStart2()
  • GetWordEnd2()
  • MoveToNextWord()
  • MoveToPreviousWord()
  • _GetWordStartForAccessibility()
  • _GetWordStartForSelection()
  • _GetWordEndForAccessibility()
  • _GetWordEndForSelection()

In favor of a simple:

  • GetWordStart()
  • GetWordEnd()
  • _GetDelimiterClassRunStart()`
  • _GetDelimiterClassRunEnd()

Tests were used to help ensure a regression doesn't occur. That said, there were a few modifications there too:

  • Removed MoveByWord test
    • It directly called MoveToNextWord() and MoveToPreviousWord(), which no longer exist. These were helper functions for UiaTextRangeBase, and any special logic has been moved into _moveEndpointByUnitWord using the unified GetWordStart()/GetWordEnd() APIs.
    • The tested behavior is still covered by MoveToPreviousWord, MovementAtExclusiveEnd, and the generated word movement tests.
  • updated GetWordBoundaries tests
    • Inclusive --> exclusive end positions for GetWordEnd(): The old _GetWordEndForSelection() returned inclusive end positions, whereas the new GetWordEnd() returns exclusive end positions. This is what GetWordEnd2() already used, so every old expected value shifted +1 in the x direction (or to {RightExclusive, y} at row boundaries) to account for that.
    • ControlChar wrap-crossing behavior: The old _GetWordStartForSelection() had a special check at the left boundary that prevented whitespace runs from crossing wrapped row boundaries. The new _GetDelimiterClassRunStart() doesn't have this special case (it treats ControlChar the same as other delimiter classes when the row was wrap-forced). This changed one test case: GetWordStart({1, 4}) in selection mode went from {0, 4} to {6, 3} (the whitespace run now crosses the wrap boundary to row 3). This matches the behavior TerminalSelection was already getting from GetWordStart2().

Validation Steps Performed

Tests passed:
✅ Conhost.Interactivity.Win32.Unit.Tests.dll
✅ UnitTests_TerminalCore\Terminal.Core.Unit.Tests.dll

Word navigation feels good for...
✅ Narrator
✅ NVDA
✅ Mouse selection
✅ Mark mode

Closes #4423

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Product-Terminal The new Windows Terminal. labels Feb 18, 2026
// - pos - the buffer position being within the current delimiter class
// - wordDelimiters - what characters are we considering for the separation of words
til::point TextBuffer::_GetDelimiterClassRunEnd(til::point pos, const std::wstring_view wordDelimiters) const
// - accessibilityMode - when true, cross non-wrapped row boundaries freely
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, in essence, this is "if false, add CRLF/newlines to the wordDelemiters list", right?

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to test conhost, expressly, with this change. I'm afraid of the test values changing--because my mental model is that a refactor shouldn't also require us to change the tests (how would we ensure the refactor was correct if the original tests didn't pass?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor Word Expansion in TextBuffer

3 participants