Fix semantic drag selection over-running non-word ranges into the next word#12985
Conversation
…t word Semantic (double-click) + drag selection in rich-text surfaces (agent conversations, markdown, code blocks, table cells) over-extended when the drag tail landed on a non-word character: the word-boundary iterators start "outside a word", so they skipped the whole boundary run AND continued to the end of the next word. Add a shared `TextBuffer::semantic_expansion_target` that snaps to the maximal contiguous run of the same character class as the char under the tail (a word, or a run of boundary chars), matching the terminal block list's grid behavior (`semantic_search_left/right`). Route both `FormattedTextElement::expand_selection` and `Text::expand_selection` through it, and expose a reusable `word_boundaries::is_word_boundary`. Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR routes semantic drag expansion for formatted and plain rich text through a shared helper so boundary-character tails stop before the next word, and adds unit coverage for non-word tails. I found no diff-line code correctness, security, or spec-context issues in the provided artifacts.
Concerns
- This is a user-facing selection behavior change, but the PR context only says a screenshot was posted in Slack; no screenshot or screen recording is attached or embedded in the PR description. For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
… is_word_boundary method - semantic_expansion_target now faithfully mirrors the terminal grid's semantic_search_left/right instead of grouping a contiguous boundary run. This fixes the reviewer-reported divergence: dragging the tail onto a non-word char no longer includes trailing whitespace (e.g. "alpha," and "foo...", not "alpha, " / "foo... "). - Make is_word_boundary a method on WordBoundariesPolicy instead of a free function. - Add direct unit tests for FormattedTextElement::expand_selection, backed by a new TextFrame::mock_with_positions that lays out glyphs/carets with real positions so geometry is assertable. - Update the buffer-level test to assert block-list-accurate values, including explicit "no trailing whitespace" cases. Co-Authored-By: Oz <oz-agent@warp.dev>
Human Remarks (Andy)
double-click-and-drag works differently in the block list vs rich text views and the block list behavior is the correct one. This PR resolves that difference and makes rich text behave the same. Added some more tests for this case.
Description
Semantic (double-click) + drag selection in agent conversations over-extended: when the drag tail landed on a non-word character (e.g.
,or...), the selection jumped to the end of the next word instead of stopping at the end of the non-word run. The terminal block list already behaved correctly.Root cause: rich-text semantic expansion used word-boundary iterators that start outside a word —
word_ends_from_offset_exclusive(forward) andword_starts_backward_from_offset_exclusive(backward) incrates/warpui_core/src/text/word_boundaries.rs. When the tail sits on a boundary char, the iterator skips the whole boundary run and continues into the adjacent word. The buggy pattern was duplicated in two call sites:FormattedTextElement::expand_selection(the AI/agent/markdown/code-block path) andText::expand_selection(plainText, e.g. table cells). The terminal block list uses a separate, correct path (grid handlersemantic_search_left/right) that stops at the first boundary char.Fix: add a shared
TextBuffer::semantic_expansion_target(position, direction, policy)that snaps to the maximal contiguous run of the same character class as the char under the tail:Both
expand_selectioncall sites now route through this helper (kept in sync), andword_boundaries::is_word_boundaryis exposed for reuse. The terminal block list is unaffected (separate path).Intended behavior change: double-clicking directly on punctuation now selects the punctuation run rather than
prevWord punct nextWord, consistent with the grid.Linked Issue
Reported via the factory-client bug-triage Slack thread (linked below).
ready-to-specorready-to-implement.Testing
Added regression unit test
crates/warpui_core/src/text/word_boundaries_tests.rs::test_semantic_expansion_stops_at_boundary_run(fails before / passes after): forfoo... bar baz, forward target from offsets 3/4/5 == col 7 and backward from 3/4/5 == col 3 (buggy values were 10 and 0); word-char cases unchanged../script/formatclean;cargo clippy --workspace --all-targets --all-features --tests -- -D warningsclean; fullwarpui_coretest suite passes (293 tests).Manual UI verification on the running app (agent conversation): double-click
foo, drag onto...→ highlightsfoo...(no longerfoo... bar); double-clickalpha, drag onto the comma →alpha,(no longeralpha, bravo); dragging onto a word still selects the whole word (alpha, bravo).I have manually tested my changes locally
Screenshots / Videos
Verified on the agent-conversation surface (screenshot posted in the Slack thread).
Agent Mode
Slack thread: https://warpdev.slack.com/archives/C0BCE7AELJ2/p1782281505938299?thread_ts=1782281505.938299&cid=C0BCE7AELJ2
Conversation: https://staging.warp.dev/conversation/a72ed78e-011f-4eb4-9072-e7dede99c407
Run: https://oz.staging.warp.dev/runs/019ef841-92c2-739f-bb00-7e91a9dd20eb
This PR was generated with Oz.