Route editor visual-selection drag through quadraui DragTarget::TextSelection (#508 part 3)

[JDonaghy]

Context

Follow-up to #508 ("Adopt quadraui TUI text-selection + OSC52 clipboard").
Part 1 (clipboard delegation) landed on
issue-508-adopt-quadraui-tui-text-selection-osc52. Part 3 (this issue) was
scoped out during that work — smoke-tested and confirmed out of scope for
that branch.

Context resolved from #508

The open question on #508 is resolved: quadraui::DragTarget::TextSelection
has no explicit payload field, but region is a WidgetId (String-like), so
window/line context can be encoded the same way existing scrollbar widget ids
already are (e.g. tui:editor:<window_id>:vsb, see
mouse.rs::apply_scrollbar_drag). No quadraui-side change is needed —
this is implementable entirely on the vimcode side through the existing API.

Why deferred despite being unblocked

The editor's visual-selection drag is document-model, not just a widget
drag: window_zone_hit_test → WindowZone::TextArea { buf_line, col } → engine.mouse_drag(window, line, col) (src/tui_main/mouse.rs:1004). That
document mapping must stay in the engine regardless.

Folding the drag-origin arbitration under DragState/DragTarget only
replaces a bool flag (mouse_text_drag) with a DragTarget match — a
marginal LOC win — for real risk to vim visual-selection correctness across
split windows (multiple editor windows/panes each with their own drag
origin). This needs a written design first, not a rushed swap.

Work

• Design how DragTarget::TextSelection's region: WidgetId encodes
  (window_id, origin_line, origin_col) or equivalent, following the
  tui:editor:<window_id>:vsb-style convention already used for scrollbars.
• Move the drag-origin arbitration + gesture lifecycle
  (mouse_text_drag bookkeeping in src/tui_main/mouse.rs) onto quadraui's
  DragTarget dispatch, mirroring how scrollbar drag already flows through
  quadraui::dispatch_mouse_drag (src/tui_main/mouse.rs:51).
• Keep the actual document-model hit-testing
  (window_zone_hit_test → buf_line/col → engine.mouse_drag) unchanged in
  the engine — only the drag-origin arbitration moves.
• Verify behavior explicitly across split windows: starting a visual-mode
  drag in one split must not be hijacked by, or leak into, another split's
  drag state.

Acceptance

• Editor mouse-drag visual selection dispatches through quadraui
  DragTarget::TextSelection for drag-origin arbitration.
• src/tui_main/mouse.rs's bespoke mouse_text_drag bookkeeping shrinks or
  is removed.
• Vim visual-selection semantics (character-wise drag selection, extending
  across split windows correctly, no cross-split drag leakage) are
  unchanged from current behavior — needs explicit multi-split manual
  verification, not just single-window testing.

Blocked on: none (quadraui#269/#283 already landed; payload-encoding question
already resolved above). Depends on: #508 Part 1 merged first for shared
context, but not a hard code dependency.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Implemented #565: routed TUI editor visual-selection drag-origin arbitration through quadraui::DragTarget::TextSelection in src/tui_main/mouse.rs, replacing the bespoke mouse_text_drag bool. Region encodes origin window id (tui:editor::text convention). Engine's own mouse_drag_origin_window guard left untouched as defense-in-depth. Added 3 unit tests for the new encode/decode helpers. Full build/clippy/fmt/test suite (default + --no-default-features) all clean. Committed and pushed to issue-565-route-editor-visual-selection-drag-throu, no PR opened yet per workflow.

[JDonaghy]

Coordinator: Assignment Complete

Machine: elitebook
Status: done
Exit code: 0
Duration: —

Summary

PASS: #565 TUI drag-origin arbitration via DragTarget::TextSelection confirmed (basic drag, cross-split isolation, dblclick-drag, clean release). Unrelated pre-existing GTK cross-split leak found, out of scope for this branch (no gtk/ files touched) -- to be filed separately.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Small, well-scoped drag-origin arbitration swap; no blocking issues, includes tests

Verdict: approve

Review findings

Review: #565 — Route editor visual-selection drag through quadraui DragTarget::TextSelection

Branch: issue-565-route-editor-visual-selection-drag-throu
Diff: git diff origin/develop...origin/issue-565-route-editor-visual-selection-drag-throu — 2 files changed, +120/-18 (src/tui_main/mod.rs, src/tui_main/mouse.rs)

Summary

Small, well-scoped change. Replaces the bespoke mouse_text_drag: bool with arming
quadraui::DragTarget::TextSelection at mouse-down (single-click and double-click
branches), encoding the origin window id in the region: WidgetId as
tui:editor:<window_id>:text — following the existing tui:editor:<id>:vsb/hsb
scrollbar convention exactly as the issue prescribed. The document-model hit-testing
path (window_zone_hit_test → buf_line/col → engine.mouse_drag) is untouched, as
required. This matches the issue's explicit scope-limiting instruction ("only the
drag-origin arbitration moves").

Verification performed

• Read full diff against origin/develop (fetched fresh).
• Read the surrounding (unchanged) context in src/tui_main/mouse.rs (mouse-down,
  Drag(Left), Up(Left) handlers) and src/core/engine/keys.rs
  (Engine::mouse_click/mouse_drag) to check for behavior drift.
• Diffed old vs. new mouse.rs/mod.rs in full (not just the hunk) to confirm no
  other reference to mouse_text_drag was left dangling — confirmed clean, only
  comments mention the old name now.
• Checked quadraui::DragTarget/WidgetId/dispatch_mouse_drag source
  (~/src/quadraui/quadraui/src/dispatch.rs, types.rs) to confirm:
  • WidgetId/WindowId derive PartialEq/Eq so the new tests and the
    !=/is_some_and comparisons compile and behave as expected.
  • dispatch_mouse_drag on a TextSelection target only produces
    UiEvent::TextSelectionChanged, which apply_scrollbar_drag's match doesn't
    handle (falls to _ => {}) — so routing the drag through
    apply_scrollbar_drag/dispatch_mouse_drag first (existing, unrelated code) is a
    harmless no-op for text drags, not a regression.
• Manually traced the cross-split behavior this issue was explicitly worried
  about: Engine::mouse_drag (src/core/engine/keys.rs:7524) already has its own
  mouse_drag_origin_window guard that ignores drag calls targeting a different
  window than the one that started the gesture, with no side effects before that
  early return. The new cross_split check in mouse.rs (mouse.rs:1043) is
  therefore a defense-in-depth mirror of an already-enforced invariant, exactly as
  the code comment states — calling or skipping engine.mouse_drag in the
  cross-split case is behaviorally identical either way.
• Compared old vs. new timing of when the "armed" flag becomes true: old
  mouse_text_drag was only set true inside the Drag handler after the first
  successful TextArea hit-test; the new drag_state.begin(TextSelection..) is set
  immediately at mouse-down. Traced one edge case this actually fixes: if the
  first Drag event after mouse-down in the editor jumps straight into the
  terminal panel (before ever re-hitting a TextArea zone), the old code left
  mouse_text_drag == false and fell through into the terminal drag-select branch,
  which could hijack an unrelated pre-existing terminal selection. The new
  text_drag_armed (true immediately on arming) closes that gap. Net effect is a
  small correctness improvement, not a regression.
• Confirmed the ctrl-click "add cursor" branch behavior is unchanged: neither old
  nor new code arms a text-drag/flag there, so no drag-origin arbitration kicks in
  for that gesture in either version.
• Confirmed other click sites in the file using is_double/drag patterns (e.g. the
  find/replace panel around mouse.rs:355-410) are unrelated widgets, untouched by
  this diff, and don't need the same treatment.
• Confirmed platform-neutrality: only src/tui_main/ files changed; no GTK files
  touched. This matches the issue's explicit "GTK backend was out of scope per issue
  text (TUI-only)" pinned note, and CLAUDE.md's platform-neutrality rule doesn't
  apply here since this isn't new backend-specific feature code — it's swapping
  one already-backend-local bookkeeping mechanism (bool) for another
  (quadraui::DragState), both already TUI-only constructs, consistent with the
  issue's own scoping rationale (this is document-model, not shared quadraui
  widget behavior).

Checklist

• Tests added/updated: Yes — 3 new unit tests for text_drag_widget_id /
  text_drag_origin_window (round-trip, distinct windows, rejecting unrelated
  widget-id formats like the scrollbar's :vsb suffix or a foreign explorer:sb
  id). These are appropriately scoped to the new pure encode/decode helpers.
  Gap: no integration-level test in mouse.rs drives handle_mouse end-to-end
  across two split windows to assert no drag leakage — but the issue itself calls
  for this as manual/explicit multi-split verification ("needs explicit
  multi-split manual verification, not just single-window testing"), not an
  automated test, and the engine-level guard this relies on
  (mouse_drag_origin_window) already has its own existing test per the pinned
  context. Not blocking, but worth a callout to the human doing the multi-split
  smoke pass called for in the issue's acceptance criteria.
• Stayed within scope: Yes. Only touches the drag-origin arbitration
  bookkeeping; hit-testing/engine document-model logic is untouched, matching the
  issue's explicit instruction not to move that.
• Security: None applicable — no I/O, no paths, no shell/process invocation in
  this diff.
• Follows existing patterns: Yes — the text_drag_widget_id/
  text_drag_origin_window pair mirrors the existing tui:editor:<id>:vsb/hsb
  scrollbar widget-id convention (apply_scrollbar_drag) both in format and in
  doc-comment style, and reuses the existing drag_state.begin()/.end()/
  .target() API already used for scrollbar drags — no new abstractions
  introduced.
• Platform-neutral: Yes, TUI-only as scoped; no GTK changes.

Build/test

Per reviewer sandbox policy I did not run cargo build/cargo test/cargo clippy
(build/test validation is the separate pre-merge smoke gate's job) — verification
above was done by reading the diff plus the quadraui source it depends on.

Verdict

No blocking issues found. The change is small, honors the issue's explicit
scope-limiting instructions, doesn't regress cross-split drag safety (and arguably
tightens one edge case), and includes tests for the new pure logic it introduces.
Recommend the human do the multi-split manual drag check the issue calls for before
merging, but that's a smoke-test/acceptance step, not a code defect.

Approve.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Rebase was a no-op (branch already based on current origin/develop tip, develop-ahead=0). Full test suite passed clean (all 44 binaries, 0 failed; pre-existing 67 warnings are unrelated to the #565 diff - only src/tui_main/mouse.rs and mod.rs changed). Pushed (no-op, already up to date). verify-merge confirmed clean single-commit history. coord merge succeeded: opened and merged PR #569 into develop. NOTE: automated 'gh issue close #565' failed with a GraphQL error (closeIssue) - issue #565 needs to be closed manually.