Repository navigation
feat(ui): word-wise drag after double-click, line-wise extension after triple-click #8739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
5c99107
01404d5
99af051
24ae25c
797f345
a6c516e
7cf9520
b5cbd2f
4fd9868
a03e911
de9c6a1
133e7ff
b32dbbe
d5999ed
aaaae0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -100,6 +100,10 @@ export function TextSelectionController( | |||||||||||||||||||||||||
| const baselineFrameRef = useRef<ReadonlyFrame | null>(null); | ||||||||||||||||||||||||||
| const baselineViewportRectRef = useRef<ViewportRect | null>(null); | ||||||||||||||||||||||||||
| const lastClickRef = useRef<ClickRecord | null>(null); | ||||||||||||||||||||||||||
| const spanDragRef = useRef<{ | ||||||||||||||||||||||||||
| mode: 'word' | 'line'; | ||||||||||||||||||||||||||
| anchorSpan: { sx: number; sy: number; ex: number; ey: number }; | ||||||||||||||||||||||||||
| } | null>(null); | ||||||||||||||||||||||||||
| const bufferRef = useRef<ScreenBuffer | undefined>(undefined); | ||||||||||||||||||||||||||
| const propsRef = useRef(props); | ||||||||||||||||||||||||||
| propsRef.current = props; | ||||||||||||||||||||||||||
|
|
@@ -113,6 +117,7 @@ export function TextSelectionController( | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const clearSelection = useCallback(() => { | ||||||||||||||||||||||||||
| const selection = selectionRef.current; | ||||||||||||||||||||||||||
| spanDragRef.current = null; | ||||||||||||||||||||||||||
| if (selection.isEmpty) { | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
@@ -183,6 +188,42 @@ export function TextSelectionController( | |||||||||||||||||||||||||
| [getBuffer, stdout], | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Extend an active word/line drag so the range spans from the original | ||||||||||||||||||||||||||
| // multi-click span to the word/line boundary at the current point (issue | ||||||||||||||||||||||||||
| // #8738). Falls back to a single cell when the cursor is over whitespace. | ||||||||||||||||||||||||||
| const extendSpanDrag = useCallback( | ||||||||||||||||||||||||||
| (point: { x: number; y: number }) => { | ||||||||||||||||||||||||||
| const spanDrag = spanDragRef.current; | ||||||||||||||||||||||||||
| if (!spanDrag) return; | ||||||||||||||||||||||||||
| const selection = selectionRef.current; | ||||||||||||||||||||||||||
| const frame = getBuffer()?.frame ?? null; | ||||||||||||||||||||||||||
| const current = | ||||||||||||||||||||||||||
| spanDrag.mode === 'word' | ||||||||||||||||||||||||||
| ? (wordSpanAt(frame, point.x, point.y) ?? { | ||||||||||||||||||||||||||
| sx: point.x, | ||||||||||||||||||||||||||
| sy: point.y, | ||||||||||||||||||||||||||
| ex: point.x, | ||||||||||||||||||||||||||
| ey: point.y, | ||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The whitespace fallback collapses focus to the single whitespace cell, so stopping a word/line drag over a gap puts whitespace inside the copied range. Failure scenario: probe-verified — double-click 'foo' in 'foo bar baz' and drag to the space at grid x=3: selection becomes 0-3 and the clipboard gets 'foo ' with a trailing space; dragging left onto the space before the anchor word copies ' bar'. A natural gesture (stopping a word-drag on a gap) produces clipboard text the user must strip. The single-cell fallback is documented as deliberate, but the whitespace-in-clipboard harm is concrete. 中文说明(建议) 空白回退会把焦点坍缩到单个空白单元格,导致词/行拖动停在空隙处时复制文本带上空白。 触发场景(探针已验证):在 'foo bar baz' 中双击 'foo' 并拖到 grid x=3 的空格处——选区变为 0-3,剪贴板得到带尾随空格的 'foo ';向左拖到锚点词前的空格则会复制出带前导空格的 ' bar'。在空隙处停住词拖动是自然手势,却产生需要用户手工清理的剪贴板内容。单格回退在注释中是有意为之,但空白进入复制范围的实际损害是具体的。 — DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Declined. The single-cell fallback when the cursor is over whitespace is documented intentional behavior of this feature (see the comment above 中文说明已拒绝。光标位于空白单元格时回退到单格是本特性文档中明确记载的有意行为(见 |
||||||||||||||||||||||||||
| : (lineSpanAt(frame, point.y) ?? { | ||||||||||||||||||||||||||
| sx: point.x, | ||||||||||||||||||||||||||
| sy: point.y, | ||||||||||||||||||||||||||
| ex: point.x, | ||||||||||||||||||||||||||
| ey: point.y, | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| const a = spanDrag.anchorSpan; | ||||||||||||||||||||||||||
| const cursorAfter = | ||||||||||||||||||||||||||
| point.y > a.ey || (point.y === a.ey && point.x >= a.ex); | ||||||||||||||||||||||||||
| selection.anchor = cursorAfter | ||||||||||||||||||||||||||
| ? { x: a.sx, y: a.sy } | ||||||||||||||||||||||||||
| : { x: a.ex, y: a.ey }; | ||||||||||||||||||||||||||
| selection.focus = cursorAfter | ||||||||||||||||||||||||||
| ? { x: current.ex, y: current.ey } | ||||||||||||||||||||||||||
| : { x: current.sx, y: current.sy }; | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| [getBuffer], | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const handleMouse = useCallback( | ||||||||||||||||||||||||||
| (event: MouseEvent) => { | ||||||||||||||||||||||||||
| const selection = selectionRef.current; | ||||||||||||||||||||||||||
|
|
@@ -220,19 +261,29 @@ export function TextSelectionController( | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (count >= 2) { | ||||||||||||||||||||||||||
| const frame = getBuffer()?.frame ?? null; | ||||||||||||||||||||||||||
| const mode = count === 2 ? 'word' : 'line'; | ||||||||||||||||||||||||||
| const span = | ||||||||||||||||||||||||||
| count === 2 | ||||||||||||||||||||||||||
| ? wordSpanAt(frame, point.x, point.y) | ||||||||||||||||||||||||||
| : lineSpanAt(frame, point.y); | ||||||||||||||||||||||||||
| if (span) { | ||||||||||||||||||||||||||
| selection.selectSpan(span, count === 2 ? 'word' : 'line'); | ||||||||||||||||||||||||||
| // Enter a drag-capable word/line selection so a held double/triple | ||||||||||||||||||||||||||
| // click can extend by word/line on move (issue #8738). Copy happens | ||||||||||||||||||||||||||
| // on release, matching char drags. | ||||||||||||||||||||||||||
| selection.mode = mode; | ||||||||||||||||||||||||||
| selection.anchor = { x: span.sx, y: span.sy }; | ||||||||||||||||||||||||||
| selection.focus = { x: span.ex, y: span.ey }; | ||||||||||||||||||||||||||
| selection.dragging = true; | ||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The multi-click press now sets Failure scenario: under SGR button tracking a move fires whenever the pointer crosses a cell while the button is held. Probe-verified: press1→release1→press2→move(3,1)→release2→press3→release3 yields a null final selection — a 1-cell drift move between press2 and release2 kills the triple-click (press3 computes count=1, char mode). The same mechanism extends a jittery plain double-click's copy range (e.g. 'foo' → 'foo bar'). The old code ignored moves after a multi-click select, so both were impossible. 中文说明(建议) 多点点击按下时现在设置 触发场景:SGR 按钮追踪下,按住按键期间指针每跨过一个单元格就会触发 move。探针已验证:press1→release1→press2→move(3,1)→release2→press3→release3 最终得到空选区——press2 与 release2 之间 1 格的漂移就破坏了三击(press3 计算 count=1,进入字符模式)。同样的机制会让抖动的普通双击复制范围扩大(如 'foo' 变成 'foo bar')。旧代码会忽略多点点击之后的 move,这两种情况都不会发生。 — DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The multi-click path writes Failure scenario: two code paths now perform the same state transition differently; a future invariant added to
Suggested change
中文说明(建议) 多点点击路径直接写 影响:两条代码路径以不同方式执行同一状态转换;未来加在 — DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Word/line copy moved to release is silently dropped whenever the selection is invalidated mid-hold: the frame-subscriber Failure scenario: double-click a word (or triple-click a line) in the VP viewport while output is streaming. Press2 selects the word with 中文说明(建议) 词/行复制移到 release 后,若按住期间选区被失效清理(流式终端),复制会被静默丢弃:帧订阅者的 触发场景:输出流式滚动时在 VP 视口里双击选中一个词(或三击选中一行)。第 2 次按下以 — DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Declined. Copy-on-release is this PR's documented design ("Copy happens on release, matching char drags", commit 中文说明已拒绝。仅在释放时复制是本 PR 明确记载的设计("Copy happens on release, matching char drags",提交 |
||||||||||||||||||||||||||
| spanDragRef.current = { mode, anchorSpan: span }; | ||||||||||||||||||||||||||
| dragScrollTopRef.current = | ||||||||||||||||||||||||||
| propsRef.current.getScrollState().scrollTop; | ||||||||||||||||||||||||||
| recordBaseline(); | ||||||||||||||||||||||||||
| applyHighlight(); | ||||||||||||||||||||||||||
| copySelection(); | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
|
Comment on lines
283
to
286
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Declined, for the same reasons recorded on rc:3740996254: copy-on-release is this PR's documented design and makes interrupted word/line drags behave exactly like interrupted char drags (cleared, no copy) — the established B1 semantic for all drags. Restoring press-time copy conflicts with this round's rc:3741007226 finding, which pins release-only copy, and would write the clipboard even for gestures that never complete. The A/B-verified difference versus the merge base is real but is the documented, intentional convergence of multi-click and drag semantics; the behavior is now pinned by 中文说明已拒绝,理由与 rc:3740996254 上的记录相同:仅在释放时复制是本 PR 明确记载的设计,它使被打断的词/行拖动与被打断的字符拖动行为完全一致(清除选区、不复制)——这是 B1 范围内所有拖动的既定语义。恢复按下时复制与本轮 rc:3741007226 意见冲突(该意见要求钉住"仅释放时复制"),且会导致从未完成的手势也写入剪贴板。A/B 验证出的与 merge base 的差异确实存在,但那是多点点击与拖动语义有意收敛的、文档中记载的结果;该行为现已由 |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| spanDragRef.current = null; | ||||||||||||||||||||||||||
| selection.start(point); | ||||||||||||||||||||||||||
| dragScrollTopRef.current = propsRef.current.getScrollState().scrollTop; | ||||||||||||||||||||||||||
| recordBaseline(); | ||||||||||||||||||||||||||
|
|
@@ -257,22 +308,36 @@ export function TextSelectionController( | |||||||||||||||||||||||||
| if (!mapped) { | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| selection.extend(clampToViewport(mapped.point, mapped.rect)); | ||||||||||||||||||||||||||
| if (spanDragRef.current) { | ||||||||||||||||||||||||||
| extendSpanDrag(clampToViewport(mapped.point, mapped.rect)); | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| selection.extend(clampToViewport(mapped.point, mapped.rect)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| applyHighlight(); | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (event.name === 'left-release') { | ||||||||||||||||||||||||||
| // Word/line click-selects are not drags; leave them intact. | ||||||||||||||||||||||||||
| if (!selection.dragging) { | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| const spanDrag = spanDragRef.current; | ||||||||||||||||||||||||||
| spanDragRef.current = null; | ||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] Release-time word/line drag extension is dead code. In the Failure scenario: double/triple-click a word/line and drag, then release at a cell where the terminal emitted no final
Suggested change
中文说明(严重) release 时的词/行拖动扩展是死代码: 触发场景:双击/三击选中词/行后拖动,若终端在释放位置没有发出最后一个 — DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7) |
||||||||||||||||||||||||||
| const mapped = mapEvent(event); | ||||||||||||||||||||||||||
| if (mapped) { | ||||||||||||||||||||||||||
| selection.extend(clampToViewport(mapped.point, mapped.rect)); | ||||||||||||||||||||||||||
| if (spanDrag) { | ||||||||||||||||||||||||||
| extendSpanDrag(clampToViewport(mapped.point, mapped.rect)); | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| selection.extend(clampToViewport(mapped.point, mapped.rect)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| selection.finish(); | ||||||||||||||||||||||||||
| if (selection.isCollapsed || selection.isEmpty) { | ||||||||||||||||||||||||||
| // A collapsed range is a real single-cell span in word/line mode, | ||||||||||||||||||||||||||
| // but only a bare click in char mode. | ||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||
| selection.isEmpty || | ||||||||||||||||||||||||||
| (selection.isCollapsed && selection.mode === 'char') | ||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||
| clearSelection(); | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
@@ -288,6 +353,7 @@ export function TextSelectionController( | |||||||||||||||||||||||||
| recordBaseline, | ||||||||||||||||||||||||||
| mapEvent, | ||||||||||||||||||||||||||
| getBuffer, | ||||||||||||||||||||||||||
| extendSpanDrag, | ||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] Test gaps: the backward drag branch (
cursorAfter === false) has zero coverage, and the triple-click line-drag test cannot discriminate line-snapping from plain char extension.Failure scenario: the backward branch (anchor set to the span's end
a.ex/a.ey, focus to the current span's startcurrent.sx/current.sy) holds the most distinct logic in the diff; a regression there ships green — dragging a double-clicked 'baz' left to 'foo' would select 'foo' instead of 'foo bar baz'. Mutation-verified: de-wiring bothextendSpanDragcall sites toselection.extendleaves this line-drag test green, so line-mode snapping could degrade to per-cell extension and the suite would not catch it.中文说明
(建议) 测试缺口:向后拖动分支(
cursorAfter === false)零覆盖;三击行拖动测试无法区分行吸附与普通字符扩展。影响:向后分支(锚点取原 span 的终点
a.ex/a.ey、焦点取当前 span 的起点current.sx/current.sy)是本 diff 中最独特的逻辑;该分支一旦回归,套件依然全绿——把双击选中的 'baz' 向左拖到 'foo' 时只会选中 'foo' 而不是 'foo bar baz'。变异验证:把两处extendSpanDrag调用替换为selection.extend后,本行拖动测试仍然通过,说明行模式吸附退化为逐格扩展也不会被套件发现。— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)