From 9285c2e9340098ef6a6482e4419b9d1b8442e1a2 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Wed, 2 Sep 2026 16:36:26 -0500 Subject: [PATCH 1/2] =?UTF-8?q?#756:=20converge=20the=20drag-follow-throug?= =?UTF-8?q?h=20rung=20=E2=80=94=20one=20router,=20both=20backends?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 6 of the #733 mouse ladder. Both backends carried a complete, independently-ordered copy of the "pointer moved with the left button held" ladder — `MouseEventKind::Drag(Left)` in `src/tui_main/mouse.rs` and `App::handle_mouse_drag_msg` in `src/gtk/mod.rs` — and they had drifted three ways: 1. TUI had no minimap drag arm at all. Its arm matched `Down | Drag` but sat below a `Down`-only gate, so press-and-hold on a TUI minimap seeked once and froze. GTK's has worked since #35. 2. The armed-scrollbar widget-id tables were two disjoint half-tables: each backend's switch was missing every id the other had, and the three shared ids were written out twice. 3. The two orders disagreed on which gesture wins. `render::route_mouse_drag` + `MouseDragState`/`MouseDragRoute` state the order ONCE; `render::apply_scroll_offset` is the union of the two widget-id tables; `render::apply_terminal_content_drag` and `render::in_terminal_pane_content` resolve terminal drags through the same `route_bottom_panel_click` the press path already used (GTK's drag path still did a bare `x / char_width` against a window-absolute x — bug 2 of the #754 banner, fixed on the press side and left here). Both bespoke ladders are deleted in this commit. What stays per backend is stated at each call site: the terminal resize/split clamps (unit conversions), GTK's Pango column inverse, and the absent GTK sidebar separator / command-line selection / explorer DnD. Tests, all verified RED against the unfixed tree: - `both_backends_resolve_the_same_layout_and_point_to_the_same_rung` — one engine painted twice (TUI cells, GTK pixels), same logical points through both conventions, same rung. RED when the router's editor-zone hit test is hardcoded to the TUI's cell metrics. - `armed_gestures_resolve_in_a_fixed_order` pins the ladder itself. - `tui_minimap_drag_keeps_seeking_while_the_button_is_held` (TuiDriver, asserts on painted `line N content` markers) — RED without the new `MouseDragRoute::Minimap` arm. - `an_editor_text_drag_paints_a_selection_through_the_shared_drag_router` (GtkDriver, asserts on probed pixels) — RED with the `EditorText` arm disabled. Entry-point line count: 3,874 -> 3,710 (measured at pickup on this branch; 4,861 when #756 was filed). The <800 target is NOT met — see the PR body. Co-Authored-By: Claude Opus 5 --- src/gtk/click.rs | 27 +- src/gtk/mod.rs | 459 ++++++++++++------------ src/gtk/testing.rs | 73 ++++ src/render.rs | 725 ++++++++++++++++++++++++++++++++++++++ src/tui_main/mouse.rs | 718 +++++++++++++++++-------------------- src/tui_main/shell_app.rs | 72 ++++ 6 files changed, 1433 insertions(+), 641 deletions(-) diff --git a/src/gtk/click.rs b/src/gtk/click.rs index 228742b8..4b4de963 100644 --- a/src/gtk/click.rs +++ b/src/gtk/click.rs @@ -638,8 +638,8 @@ pub(super) fn handle_mouse_double_click( } } -/// Handle mouse drag — extend visual selection, or keep seeking the -/// minimap while a minimap drag is held. +/// Apply a [`render::MouseDragRoute::EditorText`] drag — extend the visual +/// selection to the glyph under the cursor. /// /// #568: this only ever fires while a mouse button is held (drag /// continuation), so text-selection resolution goes through @@ -649,15 +649,10 @@ pub(super) fn handle_mouse_double_click( /// origin-window lock then keeps the selection itself pinned to the split /// the drag started in. /// -/// The minimap check (#35) is deliberately *not* routed through that -/// `mutate_focus`-gated call: it isn't a text-selection drag at all (a -/// mouse-down that lands on the minimap resolves to `ClickTarget::Minimap`, -/// never `BufferPos`, so no selection-drag ever originates there), and the -/// gate exists solely to stop a text-selection drag stealing focus/actions -/// elsewhere — a concern that doesn't apply to continuing to scroll the -/// strip the drag started on. Checked first and unconditionally, mirroring -/// `src/tui_main/mouse.rs`, which resolves `Down` and `Drag` through -/// `apply_minimap_click` identically. +/// #756: the minimap check that used to open this function is gone — the +/// strip is now [`render::MouseDragRoute::Minimap`], arbitrated above the +/// editor text area by the shared drag router, so this function is only +/// reached once that router has already ruled the point out. #[allow(clippy::too_many_arguments)] pub(super) fn handle_mouse_drag( engine: &mut Engine, @@ -675,16 +670,6 @@ pub(super) fn handle_mouse_drag( frame_hit_map: Option<&quadraui::FrameHitMap>, tab_bar_zones: &HashMap, ) { - // ── Minimap drag-to-scroll (#35) ──────────────────────────────────────── - // Checked first, before the mutate_focus-gated resolver below: see the - // doc comment on this function for why this is not the "focus-stealing - // text-selection drag" case that gate exists to stop. If the pointer is - // over the strip, keep seeking and skip buffer-selection resolution - // entirely for this event. - if render_mod::apply_minimap_click(engine, cached_layout, x, y).is_some() { - return; - } - if let ClickTarget::BufferPos(wid, line, col) = pixel_to_click_target( engine, backend, diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index db12bc55..d1495cfc 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3416,274 +3416,256 @@ impl App { ) } + // ── Drag-follow-through rung (#756, mouse-ladder slice 6) ──────────────── + // + // Which gesture owns a move-with-the-button-held is + // `render::route_mouse_drag`, sequenced ONCE and shared verbatim with TUI's + // `handle_mouse`. This backend used to state its own order here — armed + // scrollbar → hover popup → modal swallow → tab drag → divider → split → + // resize → terminal → editor — while TUI stated a different one, and each + // knew scrollbar widget ids the other did not. See the rung's banner in + // `render.rs`. fn handle_mouse_drag_msg(&mut self, x: f64, y: f64, width: f64, height: f64) { - // Phase B.4 drag dispatch: feed the move through quadraui's - // dispatcher so an active drag (scrollbar, handle, etc.) - // translates into primitive-specific events, then guard - // against drag-events-inside-modal leaking through to the - // base layer (#192). - // - // Keep the stack fresh: if the picker is open, ensure its - // current bounds are recorded (popup size depends on - // has_preview which can change mid-picker). + // Keep the picker's modal-stack entry fresh before anything hit-tests + // the stack: the popup's size depends on `has_preview`, which can change + // mid-picker. + let picker_open = self.engine.borrow().picker_open; { - let engine = self.engine.borrow(); - let picker_open = engine.picker_open; - drop(engine); let picker_id = quadraui::WidgetId::new("picker"); + let stack_rc = self.backend.borrow().modal_stack_handle(); + let mut stack = stack_rc.borrow_mut(); if picker_open { let (px, py, pw, ph) = self.compute_picker_popup_bounds(width, height); - self.backend - .borrow() - .modal_stack_handle() - .borrow_mut() - .push( - picker_id.clone(), - quadraui::Rect { - x: px as f32, - y: py as f32, - width: pw as f32, - height: ph as f32, - }, - ); + stack.push( + picker_id, + quadraui::Rect { + x: px as f32, + y: py as f32, + width: pw as f32, + height: ph as f32, + }, + ); } else { - self.backend - .borrow() - .modal_stack_handle() - .borrow_mut() - .pop(&picker_id); + stack.pop(&picker_id); } + } - let drag_rc = self.backend.borrow().drag_state_handle(); - let drag = drag_rc.borrow(); - let drag_active = drag.is_active(); - if drag_active { - // Run dispatch_mouse_drag: emits MouseMoved + any - // primitive-specific drag-update events. + let bottom_metrics = render::BottomPanelMetrics { + panel_left: self.painted_bottom_panel_left(), + col_width: self.cached_char_width.max(1.0), + }; + let drag_rc = self.backend.borrow().drag_state_handle(); + let stack_rc = self.backend.borrow().modal_stack_handle(); + let route = { + let engine = self.engine.borrow(); + let layout_ref = self.cached_screen_layout.borrow(); + let state = render::MouseDragState { + layout: layout_ref.as_ref(), + armed_target: drag_rc.borrow().is_active(), + hover_popup_selecting: engine.editor_hover_has_focus + && engine + .editor_hover + .as_ref() + .is_some_and(|h| h.selection.is_some()) + && self.editor_hover_popup_rect.get().is_some(), + modal_hit: stack_rc + .borrow() + .hit_test(quadraui::Point { + x: x as f32, + y: y as f32, + }) + .is_some(), + // GTK has no canvas sidebar separator, command-line selection or + // explorer drag-and-drop: the separator is a `gtk::Paned`, the + // command line paints through `Surface::CommandLine` (which + // exposes no character hit test — the quadraui gap #752 + // recorded), and the file tree is a native widget with its own + // DnD. Stated here rather than omitted so the asymmetry is + // visible at the call site. + sidebar_resizing: false, + sidebar_dnd: false, + sidebar_body: None, + command_line_selecting: false, + tab_dragging: self.tab_drag.is_armed_or_dragging(), + divider_grabbed: self.divider_grab.is_some(), + terminal_split_dragging: self.terminal_split_dragging, + terminal_panel_resizing: self.terminal_resize_dragging, + in_terminal_content: render::in_terminal_pane_content( + &engine, + x, + y, + bottom_metrics, + ), + cell: ( + self.cached_char_width.max(1.0), + self.cached_line_height.max(1.0), + ), + }; + render::route_mouse_drag(&state, x, y) + }; + + match route { + render::MouseDragRoute::ArmedTarget => { let events = quadraui::dispatch_mouse_drag( - &drag, + &drag_rc.borrow(), quadraui::Point { x: x as f32, y: y as f32, }, Default::default(), ); - drop(drag); - // Apply each scroll event by widget id. + let picker_visible_rows = if picker_open { + let lh = self.cached_line_height.max(1.0); + let has_preview = self.engine.borrow().picker_preview.is_some(); + render::PickerGeometry::compute( + width as f32, + height as f32, + has_preview, + &render::gtk_picker_sizing(lh as f32), + ) + .visible_rows + } else { + 0 + }; for ev in &events { if let quadraui::UiEvent::ScrollOffsetChanged { widget, new_offset } = ev { - match widget.as_str() { - "picker" => { - let lh = self.cached_line_height.max(1.0); - let has_preview = self.engine.borrow().picker_preview.is_some(); - let geo = render::PickerGeometry::compute( - width as f32, - height as f32, - has_preview, - &render::gtk_picker_sizing(lh as f32), - ); - let vis = geo.visible_rows; - let mut engine = self.engine.borrow_mut(); - engine.picker_scroll_top = *new_offset; - if engine.picker_selected < *new_offset { - engine.picker_selected = *new_offset; - } else if vis > 0 && engine.picker_selected >= *new_offset + vis { - engine.picker_selected = *new_offset + vis - 1; - } - engine.picker_load_preview(); - self.draw_needed.set(true); - } - "editor_hover" => { - self.engine - .borrow_mut() - .editor_hover_set_scroll(*new_offset); - self.draw_needed.set(true); - } - "terminal_scrollback" => { - if let Some(term) = self.engine.borrow_mut().active_terminal_mut() { - term.set_scroll_offset(*new_offset); - } - self.draw_needed.set(true); - } - "debug_output" => { - let mut engine = self.engine.borrow_mut(); - engine.debug_output_scroll = *new_offset; - engine.debug_output_auto_scroll = false; - self.draw_needed.set(true); - } - w if w.starts_with("editor:h_sb:") => { - if let Some(id_str) = w.strip_prefix("editor:h_sb:") { - if let Ok(id) = id_str.parse::() { - let win_id = core::WindowId(id); - self.engine - .borrow_mut() - .set_scroll_left_for_window(win_id, *new_offset); - self.draw_needed.set(true); - } - } - } - _ => {} - } + // #756: the widget-id → scroll-state table is + // `render::apply_scroll_offset`, shared with TUI. The + // copy this replaced knew `picker` and `editor:h_sb:N` + // and nothing else — see the rung's banner in + // `render.rs`, point 2, for why two half-tables is a + // silent trap rather than a live bug. + render::apply_scroll_offset( + &mut self.engine.borrow_mut(), + widget.as_str(), + *new_offset, + render::ScrollApplyContext { + picker_visible_rows, + }, + ); } } - return; } - drop(drag); - - // Editor hover popup text selection drag (#216). Must run - // BEFORE the modal-stack-swallow guard below so an - // in-progress selection drag inside the popup (which is - // now on the modal stack) doesn't get short-circuited. - { - let engine = self.engine.borrow(); - if engine.editor_hover_has_focus - && engine + render::MouseDragRoute::HoverPopupSelection => { + if let Some((px, py, _pw, _ph)) = self.editor_hover_popup_rect.get() { + let padding = 4.0; + let lh = self.cached_line_height.max(1.0); + let scroll = self + .engine + .borrow() .editor_hover .as_ref() - .is_some_and(|h| h.selection.is_some()) - { - if let Some((px, py, _pw, _ph)) = self.editor_hover_popup_rect.get() { - let padding = 4.0; - let lh = self.cached_line_height.max(1.0); - let scroll = engine - .editor_hover - .as_ref() - .map(|h| h.scroll_top) - .unwrap_or(0); - drop(engine); - let rel_x = x - px - padding; - let rel_y = y - py - padding; - let content_line = (rel_y / lh).max(0.0) as usize + scroll; - let content_col = self.pixel_to_editor_hover_col(rel_x, content_line); - self.engine - .borrow_mut() - .editor_hover_extend_selection(content_line, content_col); - self.draw_needed.set(true); - return; + .map(|h| h.scroll_top) + .unwrap_or(0); + let rel_x = x - px - padding; + let rel_y = y - py - padding; + let content_line = (rel_y / lh).max(0.0) as usize + scroll; + let content_col = self.pixel_to_editor_hover_col(rel_x, content_line); + self.engine + .borrow_mut() + .editor_hover_extend_selection(content_line, content_col); + } + } + render::MouseDragRoute::TabDrag => { + // `64.0` is the squared 8-device-pixel threshold. + match self.tab_drag.handle_move(x, y, 64.0) { + render::TabDragMove::Tracking => { + // Cursor and the cached per-group bounds are both in + // absolute surface coordinates, so the hit-test matches + // what the overlay draws (#515). + let groups = self.cached_drop_groups.borrow(); + let zone = render::compute_tab_drop_zone( + x as f32, + y as f32, + &groups, + self.cached_drop_tbh.get(), + ); + drop(groups); + self.tab_drag.track(zone); + } + render::TabDragMove::Crossed { press_x, press_y } => { + // Unlike TUI, this backend's arm fires for the whole + // tab-bar band, so the press has to be re-resolved to + // confirm it was on a tab. If it was not, disarm and + // re-route the same event with the machine idle — the + // one rung that can decline after being asked. + if let Some(source) = self.tab_drag_source_at(press_x, press_y) { + self.tab_drag.begin(source, x, y); + } else { + self.tab_drag.disarm(); + self.draw_needed.set(true); + self.handle_mouse_drag_msg(x, y, width, height); + return; + } } + render::TabDragMove::Pending | render::TabDragMove::Idle => {} } } - - let stack_rc = self.backend.borrow().modal_stack_handle(); - let stack = stack_rc.borrow(); - let hit_point = quadraui::Point { - x: x as f32, - y: y as f32, - }; - if stack.hit_test(hit_point).is_some() { - // Drag landed inside an open modal but there's no - // active drag target — swallow so it doesn't leak to - // the editor (#192). Active modal drags have already - // been handled above. - return; + render::MouseDragRoute::Divider => { + if let (Some(grab), Some((group_dividers, window_dividers, _))) = + (self.divider_grab, self.painted_divider_geometry(x, y)) + { + render::apply_divider_drag( + &mut self.engine.borrow_mut(), + grab, + &group_dividers, + &window_dividers, + x, + y, + ); + } } - } - // Tab drag-and-drop: the arm → threshold → track machine is shared - // with TUI (`render::TabDragState`, #753). `64.0` is the squared - // 8-device-pixel threshold. - match self.tab_drag.handle_move(x, y, 64.0) { - render::TabDragMove::Tracking => { - // Compute the drop zone from the per-group bounds cached by - // render_content. Cursor (x, y) and those bounds are both in - // absolute surface coordinates, so the hit-test matches what - // the overlay draws. (#515 — previously used relative 0-based - // bounds vs an absolute cursor, which misclassified the zone - // after a split.) - let groups = self.cached_drop_groups.borrow(); - let zone = render::compute_tab_drop_zone( - x as f32, - y as f32, - &groups, - self.cached_drop_tbh.get(), - ); - drop(groups); - self.tab_drag.track(zone); - self.draw_needed.set(true); - return; + render::MouseDragRoute::TerminalSplitDivider => { + if self.cached_char_width > 0.0 { + const SB_W: f64 = 6.0; + let min_x = self.cached_char_width * 5.0; + let max_x = (width - SB_W - self.cached_char_width * 5.0).max(min_x); + let clamped_x = x.clamp(min_x, max_x); + let left_cols = (clamped_x / self.cached_char_width) as u16; + self.engine + .borrow_mut() + .terminal_split_set_drag_cols(left_cols); + } } - render::TabDragMove::Crossed { press_x, press_y } => { - // Unlike TUI, GTK's arm fires for the whole tab-bar band, so - // the press has to be re-resolved to confirm it was on a tab. - if let Some(source) = self.tab_drag_source_at(press_x, press_y) { - self.tab_drag.begin(source, x, y); - self.draw_needed.set(true); - return; + render::MouseDragRoute::TerminalPanelResize => { + if self.cached_line_height > 0.0 { + let global_status_rows = if self.engine.borrow().settings.window_status_line { + 0.0 + } else { + 1.0 + }; + let status_h = (1.0 + global_status_rows) * self.cached_line_height; + let available = (height - y - status_h).max(0.0); + // Leave at least 4 editor lines visible (+ tab bar chrome) + let min_editor_lines = 4.0 + 1.0; + let max_rows = + ((height - status_h - min_editor_lines * self.cached_line_height) + / self.cached_line_height) as u16; + let max_rows = max_rows.saturating_sub(2).max(5); + let new_rows = ((available / self.cached_line_height) as u16) + .saturating_sub(2) + .clamp(5, max_rows); + self.engine.borrow_mut().session.terminal_panel_rows = new_rows; } - // Not a tab — the press is disarmed and we fall through. - self.tab_drag.disarm(); } - // Haven't moved enough yet, don't start any drag. - render::TabDragMove::Pending => return, - render::TabDragMove::Idle => {} - } - // Divider drag — group boundary or `:split` boundary, both through the - // shared applier (#753). - if let Some(grab) = self.divider_grab { - if let Some((group_dividers, window_dividers, _)) = self.painted_divider_geometry(x, y) - { - render::apply_divider_drag( + render::MouseDragRoute::Minimap => { + let layout_ref = self.cached_screen_layout.borrow(); + if let Some(ref layout) = *layout_ref { + let mut engine = self.engine.borrow_mut(); + render::apply_minimap_click(&mut engine, layout, x, y); + } + } + render::MouseDragRoute::TerminalContent => { + // #533: shared drag handler — tries forward_mouse(Move) when the + // child has mouse reporting, falls back to local selection. + render::apply_terminal_content_drag( &mut self.engine.borrow_mut(), - grab, - &group_dividers, - &window_dividers, x, y, + bottom_metrics, ); } - self.draw_needed.set(true); - return; - } - // Terminal split divider drag — update visual position (no PTY resize yet). - if self.terminal_split_dragging { - if self.cached_char_width > 0.0 { - const SB_W: f64 = 6.0; - let min_x = self.cached_char_width * 5.0; - let max_x = (width - SB_W - self.cached_char_width * 5.0).max(min_x); - let clamped_x = x.clamp(min_x, max_x); - let left_cols = (clamped_x / self.cached_char_width) as u16; - self.engine - .borrow_mut() - .terminal_split_set_drag_cols(left_cols); - self.draw_needed.set(true); - } - // Terminal panel resize drag. - } else if self.terminal_resize_dragging { - if self.cached_line_height > 0.0 { - let global_status_rows = if self.engine.borrow().settings.window_status_line { - 0.0 - } else { - 1.0 - }; - let status_h = (1.0 + global_status_rows) * self.cached_line_height; - let available = (height - y - status_h).max(0.0); - // Leave at least 4 editor lines visible (+ tab bar chrome) - let min_editor_lines = 4.0 + 1.0; // 4 lines + tab bar - let max_rows = ((height - status_h - min_editor_lines * self.cached_line_height) - / self.cached_line_height) as u16; - let max_rows = max_rows.saturating_sub(2).max(5); - let new_rows = ((available / self.cached_line_height) as u16) - .saturating_sub(2) - .clamp(5, max_rows); - self.engine.borrow_mut().session.terminal_panel_rows = new_rows; - self.draw_needed.set(true); - } - } else { - // Drag in the terminal content area (text selection). Geometry is - // cached at paint time on engine.bottom_panel_geometry (#418). - let content_row = match self.engine.borrow().resolve_bottom_panel_zone(y) { - Some(crate::core::engine::BottomPanelZone::Content { row_offset }) => { - Some(row_offset) - } - _ => None, - }; - if let Some(row) = content_row { - let col = (x / self.cached_char_width.max(1.0)) as u16; - // #533: shared drag handler — tries forward_mouse(Move) - // when the child has mouse reporting, falls back to local - // selection update. - self.engine.borrow_mut().handle_terminal_pane_drag(col, row); - self.draw_needed.set(true); - } else { + render::MouseDragRoute::EditorText => { let layout_ref = self.cached_screen_layout.borrow(); if let Some(ref layout) = *layout_ref { let mut engine = self.engine.borrow_mut(); @@ -3704,9 +3686,16 @@ impl App { &self.cached_tab_bar_zones.borrow(), ); } - self.draw_needed.set(true); } + // #192: a drag inside an open modal with nothing armed is swallowed + // so it cannot leak to the editor underneath. + render::MouseDragRoute::ModalSwallow + | render::MouseDragRoute::SidebarResize + | render::MouseDragRoute::SidebarBody + | render::MouseDragRoute::CommandLine + | render::MouseDragRoute::None => {} } + self.draw_needed.set(true); } fn handle_mouse_up_msg(&mut self) { diff --git a/src/gtk/testing.rs b/src/gtk/testing.rs index 5b85868a..f07f9104 100644 --- a/src/gtk/testing.rs +++ b/src/gtk/testing.rs @@ -2954,6 +2954,79 @@ second line here ); } + /// #756 acceptance, GTK half of the drag rung: an editor text-selection + /// drag must still paint a selection now that + /// `render::route_mouse_drag` — not `handle_mouse_drag_msg`'s own + /// hand-ordered ladder — decides that the point belongs to the editor. + /// + /// Asserted on painted pixels (`CLAUDE.md` testing rule 1): the cell the + /// drag sweeps over must change background between before and after, which + /// is only true if the `MouseDragRoute::EditorText` arm actually reached + /// `handle_mouse_drag`. Confirmed RED by disabling that arm + /// (`EditorText if false =>`, so the route falls to the no-op group): the + /// two probes then both read `(39, 39, 43)`. + #[test] + fn an_editor_text_drag_paints_a_selection_through_the_shared_drag_router() { + let mut engine = Engine::new(); + engine.settings.use_nerd_fonts = false; + let mut text = String::new(); + for _ in 0..60 { + text.push_str("alpha beta gamma delta epsilon\n"); + } + engine.buffer_mut().insert(0, &text); + let mut h = harness(engine, 1400, 900); + let win = h.engine.borrow().active_window_id(); + h.window_center(win).expect("editor pane must paint"); + let cw = h.painted_char_width(); + let lh = h + .painted_line_height() + .expect("the frame must publish the line height it painted with"); + assert!( + cw > 0.0, + "the frame must publish the char width it painted with" + ); + + // Anchor inside the *text*, not at the pane's centre: every line in + // the fixture is 30 characters, so a pane-centre press would land past + // end-of-line on both ends of the sweep and select nothing. + let (text_x, row_y) = { + let layout = h.screen_layout.borrow(); + let rw = layout + .as_ref() + .expect("a frame must have painted") + .windows + .iter() + .find(|w| w.window_id == win) + .expect("the active pane must be in the layout"); + ( + rw.rect.x + rw.gutter_char_width as f64 * cw, + rw.rect.y + lh * 2.5, + ) + }; + let probe = ((text_x + cw * 3.5) as i32, row_y as i32); + // Park the caret on the probe's row *first*, so the `before` sample + // already includes the cursor-line highlight and the only thing left + // for the gesture below to change is the selection itself. + h.driver.click((text_x + cw * 0.5) as f32, row_y as f32); + let before = h.driver.pixel(probe.0, probe.1); + + // Press to the left of the probe and sweep past it while held — the + // press anchors the selection, the *move* is the rung under test. + h.driver + .mouse_down((text_x + cw * 0.5) as f32, row_y as f32); + h.driver + .mouse_move((text_x + cw * 8.5) as f32, row_y as f32); + h.driver.mouse_up((text_x + cw * 8.5) as f32, row_y as f32); + + let after = h.driver.pixel(probe.0, probe.1); + assert_ne!( + before, after, + "a held drag across the editor text must repaint the swept cell \ + with the selection background; both probes read {before:?} at \ + {probe:?}" + ); + } + /// The mirror image of the test above: a drag that *starts* inside the /// sidebar must keep its grab for the rest of the gesture even once the /// pointer wanders out over the editor — a panel scrollbar-thumb or tree diff --git a/src/render.rs b/src/render.rs index a229edbf..1624f0a7 100644 --- a/src/render.rs +++ b/src/render.rs @@ -3678,6 +3678,17 @@ impl TabDragState { self.dragging } + /// `true` while the machine would consume a move — either a live drag or an + /// armed press still under the travel threshold. + /// + /// [`route_mouse_drag`] asks this rather than `is_dragging` because a + /// still-armed press must also win the event: it is the move that promotes + /// it, and letting a lower rung claim that move first is how an + /// almost-a-drag turns into a stray text selection. + pub fn is_armed_or_dragging(&self) -> bool { + self.dragging || self.press.is_some() + } + /// Latest pointer position during a live drag, for the ghost label. pub fn cursor(&self) -> Option<(f64, f64)> { self.cursor @@ -3759,6 +3770,415 @@ impl TabDragState { } } +// ═══ Drag-follow-through rung (#756, mouse-ladder slice 6) ═══════════════════ +// +// Everything above arbitrates a *press*. This rung arbitrates the events that +// come after one: pointer moves with the left button still held. Both backends +// had a complete, independently-ordered copy of that ladder — +// `MouseEventKind::Drag(Left)` in `src/tui_main/mouse.rs` and +// `App::handle_mouse_drag_msg` in `src/gtk/mod.rs` — and they had drifted in +// three ways that no amount of reading either one in isolation would reveal: +// +// 1. **The minimap had no TUI drag arm at all.** GTK checks +// `apply_minimap_click` first thing in `handle_mouse_drag`, so holding the +// button on the strip keeps seeking. TUI's minimap arm sits *below* an +// `if ev.kind != Down(Left) { return }` gate, so its `Down | Drag` match was +// unreachable for `Drag`: press-and-hold on a TUI minimap scrolled once and +// then froze. The arm was written to handle both; the ladder order silently +// took the drag half away. That is exactly the "one backend arbitrates a +// rung and the other does not" failure #756 exists to end. +// +// 2. **The armed-scrollbar table was two disjoint half-tables.** Both run +// `quadraui::dispatch_mouse_drag` and then switch on the emitted +// `ScrollOffsetChanged { widget, .. }`. TUI's switch knew `explorer:sb`, +// `ext_panel:sb`, `tui:search_results`, `debug_sidebar:*` and +// `tui:editor:N:vsb|hsb`; GTK's knew `picker` and `editor:h_sb:N`; each was +// missing every id the other had, and the three ids they *did* share +// (`editor_hover`, `terminal_scrollback`, `debug_output`) were written out +// twice. Today each missing id names a surface only the other backend +// registers, so nothing is visibly broken — but the failure mode is a +// silent one: register an existing surface on the second backend and its +// thumb tracks (quadraui does that) over content that never scrolls, +// because nothing on that side applies the offset. [`apply_scroll_offset`] +// is the union, once, so registering the surface is the whole of the work. +// +// 3. **The orders disagreed on which gesture wins.** TUI ran +// sidebar-resize → hover-popup-selection → sidebar-body → explorer-DnD → +// tab-drag → command-line → armed-scrollbar → …; GTK ran +// armed-scrollbar → hover-popup-selection → modal-swallow → tab-drag → +// divider → …. Most pairs are spatially disjoint so the divergence was +// invisible until a surface overlapped — which is precisely how it will +// re-fork the next time someone adds one. [`route_mouse_drag`] states the +// order once; `render::mouse_drag_router_tests` pins it, and the parity test +// in that module drives the same `ScreenLayout` and point through both +// backends' unit conventions and asserts they land on the same rung. +// +// **Deliberately still per backend**: the *apply* half of the rungs whose +// arithmetic is genuinely in the backend's own units — the terminal panel's +// resize clamp (TUI counts rows off the bottom chrome, GTK divides pixels by +// `cached_line_height`) and the split divider's column clamp. Those are unit +// conversions, not policy, and the policy — which gesture owns the event — is +// what this rung moved. + +/// Which rung of the pointer-drag ladder owns a move-with-button-held event. +/// +/// Resolved by [`route_mouse_drag`] from geometry and drag bookkeeping alone — +/// no engine mutation — so a test can ask "who would win here?" on either +/// backend without driving a real gesture. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MouseDragRoute { + /// A `quadraui::DragState` target is armed — a scrollbar or picker thumb. + /// The caller runs `dispatch_mouse_drag` and feeds the resulting + /// `ScrollOffsetChanged` ids to [`apply_scroll_offset`]. + ArmedTarget, + /// The editor hover popup is mid text-selection (#216). + HoverPopupSelection, + /// The point is inside an open modal and nothing is armed — swallow it so + /// the drag cannot leak to the editor underneath (#192). + ModalSwallow, + /// The sidebar separator is being dragged to resize the sidebar. + SidebarResize, + /// The sidebar body owns the gesture: the search / settings form + /// controllers' own drag handling, or an explorer drag-and-drop in flight. + SidebarBody, + /// The tab drag machine ([`TabDragState`]) is armed or already tracking. + TabDrag, + /// Command-line / message-line text selection. + CommandLine, + /// A group or `:split` divider is grabbed. + Divider, + /// The terminal split's vertical divider is being dragged. + TerminalSplitDivider, + /// The bottom panel's top edge is being dragged (panel resize). + TerminalPanelResize, + /// The pointer is over a minimap strip — keep seeking. + Minimap, + /// The pointer is inside the terminal's content rows — extend the + /// terminal's own selection (or forward the move to the child). + TerminalContent, + /// The editor text area — extend the visual selection. + EditorText, + /// Nothing owns it. + None, +} + +/// Everything [`route_mouse_drag`] needs, in the caller's own units. +/// +/// The booleans are the caller's drag bookkeeping (its own `dragging_sidebar` / +/// `divider_grab` / … flags); the rects are what the last frame actually +/// painted. Nothing here is recomputed by the router. +#[derive(Debug, Clone, Copy)] +pub struct MouseDragState<'a> { + /// The layout the last frame painted, for the minimap and editor hit tests. + pub layout: Option<&'a ScreenLayout>, + /// `quadraui::DragState::is_active()`. + pub armed_target: bool, + /// A text selection is in progress inside the editor hover popup. + pub hover_popup_selecting: bool, + /// `ModalStack::hit_test(point).is_some()`. + pub modal_hit: bool, + /// The sidebar separator is being dragged. + pub sidebar_resizing: bool, + /// An explorer drag-and-drop is in flight. Armed rather than geometric: + /// once a row is picked up the gesture belongs to the tree even while the + /// pointer is outside the sidebar (that is how "dragged away, no target" + /// is expressed), so a geometric test alone would hand the move to the + /// editor the moment the user left the panel. + pub sidebar_dnd: bool, + /// The sidebar body's painted bounds, when that body wants drag events + /// (search / settings form controllers, or an explorer DnD in flight). + /// `None` when the sidebar is hidden or its panel has no drag behaviour. + pub sidebar_body: Option, + /// [`TabDragState`] is armed or dragging. + pub tab_dragging: bool, + /// A command-line / message-line selection is in progress. + pub command_line_selecting: bool, + /// A divider is grabbed. + pub divider_grabbed: bool, + /// The terminal split divider is being dragged. + pub terminal_split_dragging: bool, + /// The bottom panel is being resized. + pub terminal_panel_resizing: bool, + /// `true` when the point is inside a painted terminal pane's content cells + /// — ask [`in_terminal_pane_content`], which both backends call so the + /// question cannot be answered differently on each. + pub in_terminal_content: bool, + /// One text cell in the caller's units: `(char_width, line_height)`. + /// `(1.0, 1.0)` on TUI, the measured font advance/line height on GTK. + pub cell: (f64, f64), +} + +impl Default for MouseDragState<'_> { + fn default() -> Self { + Self { + layout: None, + armed_target: false, + hover_popup_selecting: false, + modal_hit: false, + sidebar_resizing: false, + sidebar_dnd: false, + sidebar_body: None, + tab_dragging: false, + command_line_selecting: false, + divider_grabbed: false, + terminal_split_dragging: false, + terminal_panel_resizing: false, + in_terminal_content: false, + // Cell metrics default to the TUI's whole-cell grid; GTK always + // states its measured font metrics explicitly. + cell: (1.0, 1.0), + } + } +} + +/// Arbitrate a pointer move with the left button held. +/// +/// The order below is the converged one; see this section's banner for what the +/// two per-backend orders it replaced each got wrong. Two properties are load +/// bearing and pinned by tests: +/// +/// * **Armed gestures beat geometry.** Every flag-driven rung (an armed drag +/// target, a live divider grab, a tab drag, …) is tested before any hit test, +/// because a gesture that has already started must not be stolen by whatever +/// the pointer happens to fly over mid-drag. +/// * **The minimap beats the editor text area.** The strip is carved off the +/// active window's right edge, so `find_window_at` matches there too; testing +/// the editor first would make press-and-hold on the strip select text +/// instead of seeking, which is the GTK behaviour TUI was missing. +pub fn route_mouse_drag(state: &MouseDragState<'_>, x: f64, y: f64) -> MouseDragRoute { + // ── Armed gestures, in the order they can be armed ────────────────────── + if state.armed_target { + return MouseDragRoute::ArmedTarget; + } + if state.hover_popup_selecting { + return MouseDragRoute::HoverPopupSelection; + } + if state.sidebar_resizing { + return MouseDragRoute::SidebarResize; + } + if state.sidebar_dnd { + return MouseDragRoute::SidebarBody; + } + if state.tab_dragging { + return MouseDragRoute::TabDrag; + } + if state.command_line_selecting { + return MouseDragRoute::CommandLine; + } + if state.divider_grabbed { + return MouseDragRoute::Divider; + } + if state.terminal_split_dragging { + return MouseDragRoute::TerminalSplitDivider; + } + if state.terminal_panel_resizing { + return MouseDragRoute::TerminalPanelResize; + } + + // ── Modal swallow ─────────────────────────────────────────────────────── + // Below the armed rungs (a scrollbar thumb *inside* a modal is an armed + // target and must keep dragging) and above every geometric rung (nothing + // painted under a modal may see the event). + if state.modal_hit { + return MouseDragRoute::ModalSwallow; + } + + // ── Geometry ──────────────────────────────────────────────────────────── + if state + .sidebar_body + .is_some_and(|r| x >= r.x as f64 && x < (r.x + r.width) as f64 && y >= r.y as f64) + { + return MouseDragRoute::SidebarBody; + } + if let Some(layout) = state.layout { + if minimap_click_line(layout, x, y).is_some() { + return MouseDragRoute::Minimap; + } + } + if state.in_terminal_content { + return MouseDragRoute::TerminalContent; + } + // The editor area needs no left-edge gate: `layout.windows` holds only the + // painted editor windows, so a point over the activity bar or sidebar + // simply matches none of them. (TUI used to carry an `editor_left` guard + // here; it was a second, hand-maintained statement of the same fact.) + if let Some(layout) = state.layout { + if let Some(idx) = find_window_at(layout, x, y) { + let rw = &layout.windows[idx]; + let zone = window_zone_hit_test( + rw, + x - rw.rect.x, + y - rw.rect.y, + state.cell.1.max(f64::MIN_POSITIVE), + state.cell.0.max(f64::MIN_POSITIVE), + ); + if matches!(zone, WindowZone::TextArea { .. }) { + return MouseDragRoute::EditorText; + } + } + } + MouseDragRoute::None +} + +/// `true` when `(x, y)` is inside a painted terminal pane's *content* cells — +/// the [`MouseDragRoute::TerminalContent`] gate, asked the same way on both +/// backends so the two cannot disagree about where the terminal starts. +/// +/// Deliberately the same resolver the press path uses +/// ([`route_bottom_panel_click`]), so a press that anchored a terminal +/// selection and the drags that extend it can never land in different spaces. +pub fn in_terminal_pane_content( + engine: &Engine, + x: f64, + y: f64, + metrics: BottomPanelMetrics, +) -> bool { + matches!( + route_bottom_panel_click(engine, x, y, metrics), + Some(BottomPanelRoute::Pane { .. }) + | Some(BottomPanelRoute::Split( + quadraui::TerminalSplitHit::LeftPane { .. } + | quadraui::TerminalSplitHit::RightPane { .. } + )) + ) +} + +/// Apply a pointer *drag* inside the bottom panel's content rows. +/// +/// Resolves through the same [`route_bottom_panel_click`] the press path uses, +/// so a terminal selection drag reads the same pane-local cell the press +/// anchored it to. This replaces two hand-rolled translations: TUI reconstructed +/// the strip's top row and the active pane's x from `terminal_split_layout` by +/// hand, and GTK did a bare `x / char_width` against a window-absolute `x` +/// (bug 2 of the `#754` banner, which the press path fixed and the drag path +/// kept). Returns `true` when the drag was consumed. +pub fn apply_terminal_content_drag( + engine: &mut Engine, + x: f64, + y: f64, + metrics: BottomPanelMetrics, +) -> bool { + match route_bottom_panel_click(engine, x, y, metrics) { + Some(BottomPanelRoute::Pane { col, row_offset }) => { + engine.handle_terminal_pane_drag(col, row_offset); + true + } + Some(BottomPanelRoute::Split( + quadraui::TerminalSplitHit::LeftPane { col, row } + | quadraui::TerminalSplitHit::RightPane { col, row }, + )) => { + engine.handle_terminal_pane_drag(col, row); + true + } + _ => false, + } +} + +/// Per-frame context [`apply_scroll_offset`] needs for the widgets whose apply +/// depends on more than the offset itself. +#[derive(Debug, Clone, Copy, Default)] +pub struct ScrollApplyContext { + /// Rows the unified picker's result list can show, for the selection clamp + /// [`apply_picker_scroll_offset`] applies. `0` when no picker is open. + pub picker_visible_rows: usize, +} + +/// Apply one `quadraui::UiEvent::ScrollOffsetChanged` to the scroll state the +/// `widget` id names. Returns `true` when the id was recognised. +/// +/// This is the union of the two tables described in point 2 of this section's +/// banner, so a widget that scrolls on one backend now scrolls on both. Ids +/// that a backend never emits simply never match there — the cost of the union +/// is a few dead arms, and the cost of *not* unioning it was a silently +/// non-scrolling scrollbar. +/// +/// Some ids are handled entirely inside quadraui's `SidebarSystem` +/// (`tui:search_results`, `debug_sidebar:*`); they return `true` so the caller +/// still consumes the event rather than letting it fall through to the editor. +pub fn apply_scroll_offset( + engine: &mut Engine, + widget: &str, + new_offset: usize, + ctx: ScrollApplyContext, +) -> bool { + match widget { + "picker" => { + apply_picker_scroll_offset(engine, new_offset, ctx.picker_visible_rows); + true + } + "explorer:sb" => { + engine + .explorer_tree + .borrow_mut() + .set_scroll_offset(new_offset); + true + } + "ext_panel:sb" => { + engine.ext_panel_scroll_top = new_offset; + true + } + "editor_hover" => { + engine.editor_hover_set_scroll(new_offset); + true + } + // Inverted scrollbars: `Terminal::set_scroll_offset` and + // `debug_output_scroll` both mean "lines from the bottom", and + // `dispatch_mouse_drag` already reports the offset in that space. + "terminal_scrollback" => { + if let Some(term) = engine.active_terminal_mut() { + term.set_scroll_offset(new_offset); + } + true + } + // TUI's drag path emits the `tui:`-prefixed id, its click path and GTK + // emit the bare one. Both name the same surface. + "debug_output" | "tui:debug_output" => { + engine.debug_output_scroll = new_offset; + engine.debug_output_auto_scroll = false; + true + } + "tui:settings" => { + engine.settings_scroll_top = new_offset; + true + } + // Owned by quadraui's `SidebarSystem` — consume without applying. + "tui:search_results" => true, + other if other.starts_with("debug_sidebar:") => true, + // Editor window scrollbars. TUI encodes both axes as + // `tui:editor::`; GTK only paints a horizontal one + // and encodes it as `editor:h_sb:`. + other if other.starts_with("tui:editor:") => { + let Some((wid_str, axis)) = other["tui:editor:".len()..].split_once(':') else { + return false; + }; + let Ok(wid) = wid_str.parse::() else { + return false; + }; + let window_id = crate::core::WindowId(wid); + match axis { + "vsb" => { + engine.set_scroll_top_for_window(window_id, new_offset); + engine.sync_scroll_binds(); + true + } + "hsb" => { + engine.set_scroll_left_for_window(window_id, new_offset); + true + } + _ => false, + } + } + other if other.starts_with("editor:h_sb:") => { + let Ok(wid) = other["editor:h_sb:".len()..].parse::() else { + return false; + }; + engine.set_scroll_left_for_window(crate::core::WindowId(wid), new_offset); + true + } + _ => false, + } +} + // ═══ Panels rung (#754, mouse-ladder slice 4) ════════════════════════════════ // // The rung beneath the divider rung above: once no modal, no chrome band and no @@ -23118,3 +23538,308 @@ mod tests { ); } } + +// ═══ Drag-rung tests (#756, mouse-ladder slice 6) ════════════════════════════ + +#[cfg(test)] +mod mouse_drag_router_tests { + use super::*; + use crate::core::Mode; + + /// A buffer long and deep enough that `build_screen_layout` paints a + /// minimap strip — the rung the two backends disagreed about. + fn drag_engine() -> Engine { + crate::core::session::suppress_disk_saves(); + let mut e = Engine::new_for_test(); + e.mode = Mode::Normal; + let mut text = String::new(); + for i in 0..400 { + text.push_str(&format!("line {i} content that is reasonably long\n")); + } + e.buffer_mut().insert(0, &text); + e.settings.minimap = true; + e + } + + /// Paint one frame at the caller's cell metrics. + /// + /// `cell = (1.0, 1.0)` is the TUI's whole-cell grid; `(9.0, 18.0)` stands in + /// for a GTK font's advance/line height. The *same* logical 80×24 screen is + /// described in both, which is what makes the parity assertion below mean + /// "the two backends see one screen", not "two screens happen to agree". + fn frame(engine: &Engine, cell: (f64, f64)) -> ScreenLayout { + let (cw, ch) = cell; + let bounds = WindowRect::new(0.0, 0.0, 80.0 * cw, 24.0 * ch); + let (rects, _) = engine.calculate_group_window_rects(bounds, ch); + let theme = Theme::onedark(); + build_screen_layout(engine, &theme, &rects, ch, cw, true) + } + + /// Every rung, in the order [`route_mouse_drag`] must resolve them, paired + /// with the one state field that selects it. + /// + /// Not a tautology: this is the only place the order is written down, so a + /// reviewer reads it here, and reordering the router fails this first with + /// a diff that names both rungs. + #[test] + fn armed_gestures_resolve_in_a_fixed_order() { + type Arm = (&'static str, fn(&mut MouseDragState<'_>), MouseDragRoute); + let ladder: &[Arm] = &[ + ( + "armed drag target", + |s| s.armed_target = true, + MouseDragRoute::ArmedTarget, + ), + ( + "hover popup selection", + |s| s.hover_popup_selecting = true, + MouseDragRoute::HoverPopupSelection, + ), + ( + "sidebar resize", + |s| s.sidebar_resizing = true, + MouseDragRoute::SidebarResize, + ), + ( + "sidebar drag-and-drop", + |s| s.sidebar_dnd = true, + MouseDragRoute::SidebarBody, + ), + ( + "tab drag", + |s| s.tab_dragging = true, + MouseDragRoute::TabDrag, + ), + ( + "command line selection", + |s| s.command_line_selecting = true, + MouseDragRoute::CommandLine, + ), + ( + "divider grab", + |s| s.divider_grabbed = true, + MouseDragRoute::Divider, + ), + ( + "terminal split divider", + |s| s.terminal_split_dragging = true, + MouseDragRoute::TerminalSplitDivider, + ), + ( + "terminal panel resize", + |s| s.terminal_panel_resizing = true, + MouseDragRoute::TerminalPanelResize, + ), + ( + "modal swallow", + |s| s.modal_hit = true, + MouseDragRoute::ModalSwallow, + ), + ]; + + // Turning every flag on at once must yield the *first* rung; dropping + // that rung must reveal the next, all the way down. One pass pins both + // membership and order. + for skip in 0..ladder.len() { + let mut state = MouseDragState::default(); + for (_, set, _) in &ladder[skip..] { + set(&mut state); + } + let (name, _, expected) = &ladder[skip]; + assert_eq!( + route_mouse_drag(&state, 10.0, 10.0), + *expected, + "with every rung from `{name}` down asserted, `{name}` must win" + ); + } + } + + /// A held drag over the minimap strip belongs to the minimap. + /// + /// Red against unfixed `develop`: there was no shared drag router at all, + /// and TUI's own minimap arm sat below a `Down`-only gate, so a TUI + /// press-and-hold on the strip resolved to nothing after the first cell. + /// + /// `build_screen_layout` shrinks each window rect by exactly the reserved + /// width, so the strip and the text area are spatially disjoint — this + /// pins that the strip is *claimed*, and the editor assertion beside it + /// pins that claiming it did not swallow the text area next door. + #[test] + fn a_held_drag_over_the_minimap_strip_keeps_seeking() { + let engine = drag_engine(); + let layout = frame(&engine, (1.0, 1.0)); + let mm = layout + .minimap + .first() + .expect("the fixture must paint a minimap strip"); + let strip = minimap_strip_rect(mm); + let state = MouseDragState { + layout: Some(&layout), + ..Default::default() + }; + + assert_eq!( + route_mouse_drag( + &state, + (strip.x + strip.width / 2.0) as f64, + (strip.y + strip.height / 2.0) as f64, + ), + MouseDragRoute::Minimap, + "a held drag over the minimap must keep seeking" + ); + + let text_x = strip.x as f64 - 2.0; + assert_eq!( + route_mouse_drag(&state, text_x, (strip.y + strip.height / 2.0) as f64), + MouseDragRoute::EditorText, + "the text area immediately left of the strip must still select text" + ); + } + + /// **The parity test #756 asks for.** One engine, painted twice — once in + /// the TUI's whole-cell units and once in GTK-shaped pixel units — then the + /// *same logical points* driven through [`route_mouse_drag`] in each + /// backend's own convention. Every point must land on the same rung. + /// + /// Verified RED by re-forking the ladder the way the two backends were + /// forked before this slice: hardcoding `route_mouse_drag`'s editor-zone + /// hit test to `1.0, 1.0` (the TUI's cell metrics, i.e. writing the shared + /// router from one backend's side and letting the other inherit its + /// units) fails here — `cell (0.5, 2.5): TUI routed a held drag to None + /// but GTK routed the same point on the same screen to EditorText` — with + /// a diff that names both rungs. That is the class of divergence — one + /// backend's + /// convention silently baked into a "shared" rung — that the two + /// independently-ordered ladders kept producing. + #[test] + fn both_backends_resolve_the_same_layout_and_point_to_the_same_rung() { + const GTK_CELL: (f64, f64) = (9.0, 18.0); + let engine = drag_engine(); + let tui = frame(&engine, (1.0, 1.0)); + let gtk = frame(&engine, GTK_CELL); + + assert_eq!( + tui.windows.len(), + gtk.windows.len(), + "the two frames must describe the same screen" + ); + assert_eq!( + tui.minimap.len(), + gtk.minimap.len(), + "the two frames must describe the same screen" + ); + + // Logical cell coordinates walked across the whole screen, plus the + // centre of the minimap strip so the rung that actually differed is + // always sampled regardless of grid alignment. + let mut points: Vec<(f64, f64)> = Vec::new(); + for col in (0..80).step_by(3) { + for row in (0..24).step_by(2) { + points.push((col as f64 + 0.5, row as f64 + 0.5)); + } + } + let strip = minimap_strip_rect(&tui.minimap[0]); + points.push(( + (strip.x + strip.width / 2.0) as f64, + (strip.y + strip.height / 2.0) as f64, + )); + + let mut saw_minimap = false; + let mut saw_editor = false; + for (cx, cy) in points { + let tui_route = route_mouse_drag( + &MouseDragState { + layout: Some(&tui), + cell: (1.0, 1.0), + ..Default::default() + }, + cx, + cy, + ); + let gtk_route = route_mouse_drag( + &MouseDragState { + layout: Some(>k), + cell: GTK_CELL, + ..Default::default() + }, + cx * GTK_CELL.0, + cy * GTK_CELL.1, + ); + assert_eq!( + tui_route, gtk_route, + "cell ({cx}, {cy}): TUI routed a held drag to {tui_route:?} but \ + GTK routed the same point on the same screen to {gtk_route:?} \ + — the ladder has re-forked" + ); + saw_minimap |= tui_route == MouseDragRoute::Minimap; + saw_editor |= tui_route == MouseDragRoute::EditorText; + } + assert!( + saw_minimap && saw_editor, + "the sampled points must actually exercise both the minimap and the \ + editor text area (minimap={saw_minimap}, editor={saw_editor}), or \ + agreement is vacuous" + ); + } + + /// The union table from point 2 of the rung's banner: every widget id + /// *either* backend emits must apply on *both*. The pre-#756 copies each + /// knew roughly half of this list, so a scrollbar that tracked on one + /// backend silently scrolled nothing on the other. + #[test] + fn the_scroll_offset_table_is_the_union_of_both_backends() { + let ids = [ + "picker", + "explorer:sb", + "ext_panel:sb", + "editor_hover", + "terminal_scrollback", + "debug_output", + "tui:debug_output", + "tui:settings", + "tui:search_results", + "debug_sidebar:0", + "tui:editor:0:vsb", + "tui:editor:0:hsb", + "editor:h_sb:0", + ]; + let mut engine = drag_engine(); + for id in ids { + assert!( + apply_scroll_offset(&mut engine, id, 1, ScrollApplyContext::default()), + "`{id}` is emitted by at least one backend and must apply on both" + ); + } + assert!( + !apply_scroll_offset( + &mut engine, + "not:a:widget", + 1, + ScrollApplyContext::default() + ), + "an unknown id must report unhandled so the caller can fall through" + ); + } + + /// The union is not just an arm-count: an id only *one* backend registers + /// today must still actually move the state it names when the other + /// backend starts registering it. Asserted on the tree's own scroll + /// offset, not on the table having an arm for the id. + #[test] + fn an_explorer_scrollbar_offset_moves_the_tree_on_either_backend() { + let mut engine = drag_engine(); + engine.explorer_tree.borrow_mut().set_scroll_offset(0); + assert!(apply_scroll_offset( + &mut engine, + "explorer:sb", + 7, + ScrollApplyContext::default() + )); + assert_eq!( + engine.explorer_tree.borrow().scroll_offset(), + 7, + "applying an `explorer:sb` offset must move the tree, whichever \ + backend's drag emitted it" + ); + } +} diff --git a/src/tui_main/mouse.rs b/src/tui_main/mouse.rs index 72016eb8..1d19eb68 100644 --- a/src/tui_main/mouse.rs +++ b/src/tui_main/mouse.rs @@ -60,70 +60,18 @@ fn apply_scrollbar_drag( let mut handled = false; for ev in &events { if let quadraui::UiEvent::ScrollOffsetChanged { widget, new_offset } = ev { - let key = widget.as_str(); - match key { - "explorer:sb" => { - engine - .explorer_tree - .borrow_mut() - .set_scroll_offset(*new_offset); - handled = true; - } - "ext_panel:sb" => { - engine.ext_panel_scroll_top = *new_offset; - handled = true; - } - "editor_hover" => { - engine.editor_hover_set_scroll(*new_offset); - handled = true; - } - "tui:search_results" => { - handled = true; - } - // Inverted scrollbars: top of track = max offset (oldest - // content), bottom = 0 (newest). dispatch_mouse_drag - // reports the raw forward offset; flip it here. - "terminal_scrollback" => { - if let Some(term) = engine.active_terminal_mut() { - term.set_scroll_offset(*new_offset); - } - handled = true; - } - "tui:debug_output" => { - engine.debug_output_scroll = *new_offset; - engine.debug_output_auto_scroll = false; - handled = true; - } - // debug_sidebar:N — no longer needed, SidebarSystem handles scrollbar internally - other if other.starts_with("debug_sidebar:") => { - handled = true; - } - // Editor window scrollbars — widget id format - // `tui:editor::`. Apply-side parses the - // window id and routes to the per-window scroll setters. - other if other.starts_with("tui:editor:") => { - if let Some(rest) = other.strip_prefix("tui:editor:") { - if let Some((wid_str, axis)) = rest.split_once(':') { - if let Ok(wid) = wid_str.parse::() { - let window_id = crate::core::WindowId(wid); - match axis { - "vsb" => { - engine.set_scroll_top_for_window(window_id, *new_offset); - engine.sync_scroll_binds(); - handled = true; - } - "hsb" => { - engine.set_scroll_left_for_window(window_id, *new_offset); - handled = true; - } - _ => {} - } - } - } - } - } - _ => {} - } + // #756: the widget-id → scroll-state table is + // `render::apply_scroll_offset`, shared with GTK. The two + // per-backend copies this replaced each knew ids the other did + // not — see the rung's banner in `render.rs`, point 2. + handled |= render::apply_scroll_offset( + engine, + widget.as_str(), + *new_offset, + render::ScrollApplyContext { + picker_visible_rows: 0, + }, + ); } } handled @@ -151,6 +99,150 @@ fn text_drag_origin_window(region: &quadraui::WidgetId) -> Option, + explorer_drag_active: &mut Option<(usize, Option)>, +) { + let in_sidebar_cols = + geo.sb_visible && col >= geo.ab_width && col < geo.ab_width + geo.sidebar_width; + + // Explorer drag-and-drop: activate or update the target row. + if explorer_drag_src.is_some() || explorer_drag_active.is_some() { + if in_sidebar_cols && engine.active_panel_is(PANEL_EXPLORER) { + let sidebar_row = row.saturating_sub(geo.menu_rows); + if sidebar_row >= 1 { + let tree_row = (sidebar_row as usize).saturating_sub(1) + + engine.explorer_tree.borrow().scroll_offset(); + if tree_row < engine.explorer_rows.len() { + if let Some(src_row) = *explorer_drag_src { + // Only activate the drag if the target differs from the source. + if tree_row != src_row { + *explorer_drag_active = Some((src_row, Some(tree_row))); + *explorer_drag_src = None; + } + } else if let Some((src, _)) = explorer_drag_active { + *explorer_drag_active = Some((*src, Some(tree_row))); + } + } + } + } else if let Some((src, _)) = explorer_drag_active { + // Dragged outside the sidebar — clear the target but keep the drag active. + *explorer_drag_active = Some((*src, None)); + } + if explorer_drag_active.is_some() { + return; + } + } + + if !in_sidebar_cols { + return; + } + let move_ev = quadraui::UiEvent::MouseMoved { + position: quadraui::Point::new(col as f32, row as f32), + buttons: quadraui::ButtonMask { + left: true, + right: false, + middle: false, + }, + }; + if engine.active_panel_is(PANEL_SEARCH) { + engine.handle_search_sidebar_ui_event(move_ev); + } else if engine.active_panel_is(PANEL_SETTINGS) { + let content_start = 2_u16; + let content_height = geo.term_height.saturating_sub(4); + let q_rect = quadraui::Rect::new( + geo.ab_width as f32, + content_start as f32, + geo.sidebar_width as f32, + content_height as f32, + ); + render::populate_settings_form_controller(engine); + let result = engine + .settings_form_controller + .borrow_mut() + .handle_cached(&move_ev, q_rect); + if !matches!(result, quadraui::FormControllerEvent::Ignored) { + engine.settings_scroll_top = engine.settings_form_controller.borrow().scroll_offset(); + } + } +} + +/// Apply a [`render::MouseDragRoute::EditorText`] drag: extend the visual +/// selection to the cell under the cursor. +/// +/// #565: which window the gesture "belongs to" flows through the +/// `DragTarget::TextSelection` armed at mouse-down, mirroring how scrollbar +/// drags carry their owning widget id, so a drag can neither leak into nor be +/// hijacked by another split. +/// +/// Still per backend because the column inverse is: TUI divides by a whole +/// cell, GTK asks Pango per glyph (`quadraui::gtk::editor_col_at_x`, see #560). +/// The *routing* above it is shared. +fn apply_tui_editor_text_drag( + engine: &mut Engine, + drag_state: &quadraui::DragState, + last_layout: Option<&render::ScreenLayout>, + col: u16, + row: u16, +) { + let text_drag_origin = match drag_state.target() { + Some(quadraui::DragTarget::TextSelection { region, .. }) => text_drag_origin_window(region), + _ => None, + }; + let Some(layout) = last_layout else { return }; + // #550: `rw.rect` is already absolute terminal-screen space, so the raw + // event `col`/`row` are used directly — no editor-area-relative translation. + let Some(idx) = render::find_window_at(layout, col as f64, row as f64) else { + return; + }; + let rw = &layout.windows[idx]; + let zone = render::window_zone_hit_test( + rw, + (col as f64) - rw.rect.x, + (row as f64) - rw.rect.y, + 1.0, + 1.0, + ); + let render::WindowZone::TextArea { + view_row, buf_line, .. + } = zone + else { + return; + }; + // Cross-split guard: if this drag started in a different window, ignore it + // here — matches the engine's own `mouse_drag_origin_window` guard (kept as + // defence in depth) but decided earlier, from the arbitrated drag target. + if text_drag_origin.is_some_and(|w| w != rw.window_id) { + return; + } + // #560: resolve via the shared quadraui text-layout inverse + // (`EditorLayout::col_at_x`) instead of hand-rolled cell math, so TUI and + // GTK column resolution can never diverge. + let (editor, editor_layout) = render::editor_text_layout(rw, 1.0, 1.0); + let col_in_text = editor_layout.col_at_x(&editor, view_row, col as f32); + engine.mouse_drag(rw.window_id, buf_line, col_in_text); +} + // ─── Mouse handling ─────────────────────────────────────────────────────────── #[allow(clippy::too_many_arguments)] @@ -698,334 +790,190 @@ pub(super) fn handle_mouse( *dragging_sidebar = true; return sidebar_width; } - MouseEventKind::Drag(MouseButton::Left) if *dragging_sidebar => { - let new_w = col.saturating_sub(ab_width); - return new_w.clamp(15, 150); - } - MouseEventKind::Drag(MouseButton::Left) if *hover_selecting => { - // Extend text selection in the editor hover popup - if let Some((px, py, _pw, _ph)) = editor_hover_popup_rect { - let scroll = engine - .editor_hover - .as_ref() - .map(|h| h.scroll_top) - .unwrap_or(0); - let content_line = (row.saturating_sub(py + 1)) as usize + scroll; - let content_col = col.saturating_sub(px + 2) as usize; - engine.editor_hover_extend_selection(content_line, content_col); - } - return sidebar_width; - } - MouseEventKind::Drag(MouseButton::Left) - if sb_visible - && col >= ab_width - && col < ab_width + sidebar_width - && engine.active_panel_is(PANEL_SEARCH) => - { - let move_ev = quadraui::UiEvent::MouseMoved { - position: quadraui::Point::new(col as f32, row as f32), - buttons: quadraui::ButtonMask { - left: true, - right: false, - middle: false, - }, + // ── Drag-follow-through rung (#756, mouse-ladder slice 6) ──────────── + // + // Which gesture owns a move-with-the-button-held is + // `render::route_mouse_drag`, sequenced ONCE and shared verbatim with + // GTK's `handle_mouse_drag_msg`. Five separately-guarded match arms and + // a ~260-line catch-all used to state that order here, in an order GTK + // did not share — and with a minimap arm that this backend wrote for + // `Down | Drag` but placed below a `Down`-only gate, so press-and-hold + // on a TUI minimap seeked once and then froze. See the rung's banner in + // `render.rs` for all three drifts it collapses. + MouseEventKind::Drag(MouseButton::Left) => { + let point = quadraui::Point { + x: col as f32, + y: row as f32, }; - engine.handle_search_sidebar_ui_event(move_ev); - return sidebar_width; - } - MouseEventKind::Drag(MouseButton::Left) - if sb_visible - && engine.active_panel_is(PANEL_SETTINGS) - && col >= ab_width - && col < ab_width + sidebar_width => - { - let content_start = 2_u16; - let content_height = term_height.saturating_sub(4); - let q_rect = quadraui::Rect::new( - ab_width as f32, - content_start as f32, - sidebar_width as f32, - content_height as f32, - ); - let move_ev = quadraui::UiEvent::MouseMoved { - position: quadraui::Point::new(col as f32, row as f32), - buttons: quadraui::ButtonMask { - left: true, - middle: false, - right: false, - }, + let bottom_metrics = render::BottomPanelMetrics { + panel_left: editor_left as f64, + col_width: 1.0, + }; + let sidebar_panel_drags = sb_visible + && (engine.active_panel_is(PANEL_SEARCH) || engine.active_panel_is(PANEL_SETTINGS)); + let state = render::MouseDragState { + layout: last_layout, + armed_target: drag_state.is_active(), + hover_popup_selecting: *hover_selecting, + // Modal drags never reach here: the modal-overlay rung at the + // top of this function returns first. GTK has no equivalent + // early return, so the flag is the backend's to set. + modal_hit: false, + sidebar_resizing: *dragging_sidebar, + sidebar_dnd: explorer_drag_src.is_some() || explorer_drag_active.is_some(), + sidebar_body: sidebar_panel_drags.then(|| { + quadraui::Rect::new( + ab_width as f32, + 0.0, + sidebar_width as f32, + term_height as f32, + ) + }), + tab_dragging: tab_drag.is_armed_or_dragging(), + command_line_selecting: *cmd_dragging, + divider_grabbed: divider_grab.is_some(), + terminal_split_dragging: *dragging_terminal_split, + terminal_panel_resizing: *dragging_terminal_resize, + // #756: the painted geometry the press path already reads, + // instead of the hand-rolled `term_height - bottom_chrome - + // quickfix - strip` walk this arm used to redo — the + // arithmetic that #754 found wrong in four places on the press + // side and left untouched here. + in_terminal_content: render::in_terminal_pane_content( + engine, + col as f64, + row as f64, + bottom_metrics, + ), + cell: (1.0, 1.0), }; - render::populate_settings_form_controller(engine); - let result = engine - .settings_form_controller - .borrow_mut() - .handle_cached(&move_ev, q_rect); - if !matches!(result, quadraui::FormControllerEvent::Ignored) { - engine.settings_scroll_top = - engine.settings_form_controller.borrow().scroll_offset(); + let route = render::route_mouse_drag(&state, col as f64, row as f64); + if route == render::MouseDragRoute::SidebarResize { + return col.saturating_sub(ab_width).clamp(15, 150); } - return sidebar_width; - } - MouseEventKind::Drag(MouseButton::Left) => { - // Explorer drag-and-drop: activate or update target row. - if explorer_drag_src.is_some() || explorer_drag_active.is_some() { - if sb_visible - && engine.active_panel_is(PANEL_EXPLORER) - && col >= ab_width - && col < ab_width + sidebar_width - { - let sidebar_row = row.saturating_sub(menu_rows); - if sidebar_row >= 1 { - let tree_row = (sidebar_row as usize).saturating_sub(1) - + engine.explorer_tree.borrow().scroll_offset(); - if tree_row < engine.explorer_rows.len() { - if let Some(src_row) = *explorer_drag_src { - // Only activate drag if target differs from source. - if tree_row != src_row { - *explorer_drag_active = Some((src_row, Some(tree_row))); - *explorer_drag_src = None; - } - } else if let Some((src, _)) = explorer_drag_active { - *explorer_drag_active = Some((*src, Some(tree_row))); - } - } - } - } else if let Some((src, _)) = explorer_drag_active { - // Mouse dragged outside sidebar — clear target but keep active. - *explorer_drag_active = Some((*src, None)); + match route { + render::MouseDragRoute::ArmedTarget => { + apply_scrollbar_drag(drag_state, point, engine, sidebar); } - if explorer_drag_active.is_some() { - return sidebar_width; + render::MouseDragRoute::HoverPopupSelection => { + if let Some((px, py, _pw, _ph)) = editor_hover_popup_rect { + let scroll = engine + .editor_hover + .as_ref() + .map(|h| h.scroll_top) + .unwrap_or(0); + let content_line = (row.saturating_sub(py + 1)) as usize + scroll; + let content_col = col.saturating_sub(px + 2) as usize; + engine.editor_hover_extend_selection(content_line, content_col); + } } - } - // Tab drag-and-drop: the arm → threshold → track machine is - // shared with GTK (`render::TabDragState`, #753). `2.0` is the - // squared cell threshold — see `handle_move`'s doc for why it is - // exactly the Manhattan `dx + dy >= 2` this replaced. - match tab_drag.handle_move(col as f64, row as f64, 2.0) { - render::TabDragMove::Tracking => { - tab_drag.track(compute_tui_tab_drop_zone( + render::MouseDragRoute::SidebarBody => { + apply_tui_sidebar_body_drag( engine, col, row, - editor_left, - last_layout, - *terminal_size, - )); - return sidebar_width; + SidebarBodyDragGeometry { + ab_width, + sidebar_width, + term_height, + menu_rows, + sb_visible, + }, + explorer_drag_src, + explorer_drag_active, + ); } - render::TabDragMove::Crossed { .. } => { - // Only the tab-bar arm arms the drag here, so the press - // point is known to have been on a tab: use the active - // group + active tab as the source (GTK has to re-resolve - // the press because its arm covers the whole band). - let gid = engine.active_group; - let tidx = engine - .editor_groups - .get(&gid) - .map(|g| g.active_tab) - .unwrap_or(0); - tab_drag.begin((gid, tidx), col as f64, row as f64); - return sidebar_width; + render::MouseDragRoute::TabDrag => { + match tab_drag.handle_move(col as f64, row as f64, 2.0) { + render::TabDragMove::Tracking => { + tab_drag.track(compute_tui_tab_drop_zone( + engine, + col, + row, + editor_left, + last_layout, + *terminal_size, + )); + } + render::TabDragMove::Crossed { .. } => { + // Only the tab-bar arm arms the drag here, so the press + // point is known to have been on a tab: use the active + // group + active tab as the source (GTK has to + // re-resolve the press because its arm covers the whole + // band). + let gid = engine.active_group; + let tidx = engine + .editor_groups + .get(&gid) + .map(|g| g.active_tab) + .unwrap_or(0); + tab_drag.begin((gid, tidx), col as f64, row as f64); + } + render::TabDragMove::Pending | render::TabDragMove::Idle => {} + } } - // Haven't moved enough yet — don't start any drag. - render::TabDragMove::Pending => return sidebar_width, - render::TabDragMove::Idle => {} - } - // Command-line text selection drag - if *cmd_dragging { - if let Some(ref mut sel) = *cmd_sel { - sel.1 = col as usize; + render::MouseDragRoute::CommandLine => { + if let Some(ref mut sel) = *cmd_sel { + sel.1 = col as usize; + } } - return sidebar_width; - } - // Phase B.4 Stage 5c: every scrollbar drag flows through the - // shared `quadraui::DragState::ScrollbarY` + `dispatch_mouse_drag`. - // Widget id routes the resulting `ScrollOffsetChanged` to the - // matching scroll-state field. Sites covered: - // - `explorer:sb`, `ext_panel:sb`, `editor_hover` (Stage 5a) - // - `tui:search_results`, `tui:debug_sidebar:N` (5c) - // - `terminal_scrollback`, `tui:debug_output` (5c, inverted) - if drag_state.is_active() { - let point = quadraui::Point { - x: col as f32, - y: row as f32, - }; - if apply_scrollbar_drag(drag_state, point, engine, sidebar) { - return sidebar_width; + render::MouseDragRoute::Divider => { + // #550: `div.axis_start`/`.axis_size` are already absolute + // terminal-screen coordinates, so `col`/`row` compare + // directly with no editor-origin subtraction. + if let (Some(grab), Some(layout)) = (*divider_grab, last_layout) { + render::apply_divider_drag( + engine, + grab, + &layout.group_dividers, + &layout.window_dividers, + col as f64, + row as f64, + ); + } } - } - // Terminal panel resize drag - if *dragging_terminal_resize { - let qf_h: u16 = render::quickfix_panel_rows(engine); - let available = term_height.saturating_sub(row + bottom_chrome + qf_h); - // Leave at least 4 editor lines visible (+ menu/tab bar chrome) - let min_editor_chrome = 4 + menu_rows + 1; // 4 lines + menu + tab bar - let max_rows = term_height - .saturating_sub(bottom_chrome + qf_h + min_editor_chrome + 2) // +2 for terminal tab bar + header - .max(5); - let new_rows = available.saturating_sub(1).clamp(5, max_rows); - engine.session.terminal_panel_rows = new_rows; - return sidebar_width; - } - // Divider drag — group boundary or `:split` boundary, both - // through the shared applier (#753). - // - // #550: `div.axis_start`/`.axis_size` are already absolute - // terminal-screen coordinates, so `col`/`row` compare directly - // with no editor-origin subtraction. - if let Some(grab) = *divider_grab { - if let Some(layout) = last_layout { - render::apply_divider_drag( - engine, - grab, - &layout.group_dividers, - &layout.window_dividers, - col as f64, - row as f64, - ); + render::MouseDragRoute::TerminalSplitDivider => { + let panel_col = col.saturating_sub(editor_left); + let screen_w = terminal_size.map(|s| s.width).unwrap_or(80); + let panel_w = screen_w.saturating_sub(editor_left); + let left_cols = panel_col.clamp(5, panel_w.saturating_sub(6)); + engine.terminal_split_set_drag_cols(left_cols); } - return sidebar_width; - } - // Terminal split divider drag — update visual column position (no PTY resize yet). - if *dragging_terminal_split { - let panel_col = col.saturating_sub(editor_left); - let screen_w = terminal_size.map(|s| s.width).unwrap_or(80); - let panel_w = screen_w.saturating_sub(editor_left); - let left_cols = panel_col.clamp(5, panel_w.saturating_sub(6)); - engine.terminal_split_set_drag_cols(left_cols); - return sidebar_width; - } - // Phase B.4 Stage 5c: terminal scrollback + debug output - // scrollbars are inverted (top of track = oldest content, - // bottom = live view). Their drag math now lives in the - // shared `if drag_state.is_active()` block above; the - // receive site for `terminal_scrollback` / - // `tui:debug_output` flips the offset with `max - new_offset` - // so `term.set_scroll_offset` / `engine.debug_output_scroll` - // continue to mean "lines from the bottom". - - // Phase B.4 Stage 5d: editor-window scrollbar drag math now - // lives in the shared `if drag_state.is_active()` block above - // via `tui:editor:N:vsb` / `tui:editor:N:hsb` widget ids. The - // legacy `dragging_scrollbar` local + `ScrollDragState` are - // gone. - // Text drag-to-select — find window under cursor and extend visual - // selection. #565: the drag-origin arbitration (which window this - // gesture "belongs to", so a drag can't leak into or be hijacked - // by another split) now flows through the `DragTarget::TextSelection` - // armed at mouse-down, mirroring how scrollbar drags carry their - // owning widget id — replacing the old bespoke `mouse_text_drag` - // bool. The document-model hit-testing below - // (`window_zone_hit_test` → `buf_line`/`col` → `engine.mouse_drag`) - // is unchanged. - let text_drag_origin = match drag_state.target() { - Some(quadraui::DragTarget::TextSelection { region, .. }) => { - text_drag_origin_window(region) + render::MouseDragRoute::TerminalPanelResize => { + let qf_h: u16 = render::quickfix_panel_rows(engine); + let available = term_height.saturating_sub(row + bottom_chrome + qf_h); + // Leave at least 4 editor lines visible (+ menu/tab bar chrome) + let min_editor_chrome = 4 + menu_rows + 1; // 4 lines + menu + tab bar + let max_rows = term_height + .saturating_sub(bottom_chrome + qf_h + min_editor_chrome + 2) // +2 for terminal tab bar + header + .max(5); + engine.session.terminal_panel_rows = + available.saturating_sub(1).clamp(5, max_rows); } - _ => None, - }; - let text_drag_armed = matches!( - drag_state.target(), - Some(quadraui::DragTarget::TextSelection { .. }) - ); - if col >= editor_left { - if let Some(layout) = last_layout { - // #550: `rw.rect` (and everything `find_window_at`/ - // `window_zone_hit_test` compare it against) is already - // absolute terminal-screen space, so the raw event - // `col`/`row` are used directly — no editor-area-relative - // translation. - if let Some(idx) = render::find_window_at(layout, col as f64, row as f64) { - let rw = &layout.windows[idx]; - let zone = render::window_zone_hit_test( - rw, - (col as f64) - rw.rect.x, - (row as f64) - rw.rect.y, - 1.0, - 1.0, - ); - if let render::WindowZone::TextArea { - view_row, buf_line, .. - } = zone - { - // Cross-split guard: if this drag started in a - // different window, ignore it here — matches the - // engine's own `mouse_drag_origin_window` guard - // (kept as defense-in-depth) but decided earlier, - // from the arbitrated drag target. - let cross_split = text_drag_origin.is_some_and(|w| w != rw.window_id); - if !cross_split { - // #560: resolve via the shared quadraui - // text-layout inverse (`EditorLayout::col_at_x`) - // instead of hand-rolled cell math, so TUI and - // GTK column resolution can never diverge. - // `col_at_x` takes an absolute x matching - // `editor.rect`'s space (mirrors GTK's - // `editor_col_at_x` call, see gtk/click.rs). - let (editor, editor_layout) = - render::editor_text_layout(rw, 1.0, 1.0); - let col_in_text = - editor_layout.col_at_x(&editor, view_row, col as f32); - engine.mouse_drag(rw.window_id, buf_line, col_in_text); - } - return sidebar_width; - } + render::MouseDragRoute::Minimap => { + if let Some(layout) = last_layout { + render::apply_minimap_click(engine, layout, col as f64, row as f64); } } - // Editor drag moved outside all windows (e.g. into terminal area) — - // stop processing so it doesn't bleed into other panels. - if text_drag_armed { - return sidebar_width; - } - } - // Terminal drag-to-select in content rows. - // Only activate if the drag originated in the terminal (selection exists) - // and the mouse is within the terminal panel bounds. - { - let qf_rows: u16 = render::quickfix_panel_rows(engine); - let strip_rows: u16 = if engine.terminal_open { - super::effective_terminal_panel_rows_tui(engine, term_height) + 1 - } else { - 0 - }; - let term_strip_top = - term_height.saturating_sub(bottom_chrome + qf_rows + strip_rows); - if engine.terminal_open - && strip_rows > 0 - && col >= editor_left - && row > term_strip_top - && row < term_strip_top + strip_rows - && engine - .active_terminal() - .is_some_and(|t| t.selection.is_some()) - { - let term_row = row - term_strip_top - 1; - // #444: pane-relative col. Click uses - // TerminalSplitLayout::hit_test which returns - // 0-based col from the active pane's left edge. - // Drag must match, otherwise right-pane drag - // overshoots by left_pane_cols. Left pane is - // unaffected because left.x == editor_left. - let split_layout = engine.terminal_split_layout.borrow(); - let active_pane_x = if let Some(ref sl) = *split_layout { - if engine.terminal_active == 1 { - sl.right.x as u16 - } else { - sl.left.x as u16 - } - } else { - editor_left - }; - drop(split_layout); - let term_col = col.saturating_sub(active_pane_x); + render::MouseDragRoute::TerminalContent => { // #533: shared drag handler — tries forward_mouse(Move) - // when the child has mouse reporting, falls back to - // local selection update. - engine.handle_terminal_pane_drag(term_col, term_row); - return sidebar_width; + // when the child has mouse reporting, falls back to local + // selection update. + render::apply_terminal_content_drag( + engine, + col as f64, + row as f64, + bottom_metrics, + ); } + render::MouseDragRoute::EditorText => { + apply_tui_editor_text_drag(engine, drag_state, last_layout, col, row); + } + render::MouseDragRoute::ModalSwallow + | render::MouseDragRoute::SidebarResize + | render::MouseDragRoute::None => {} } + return sidebar_width; } MouseEventKind::Up(MouseButton::Left) if sb_visible && engine.active_panel_is(PANEL_SEARCH) => @@ -2444,23 +2392,23 @@ pub(super) fn handle_mouse( } } - // ── Minimap click / drag (#35) ────────────────────────────────────────── - // Pure rect plumbing: hand the click's cell coordinates to the shared - // resolver, which owns the hit-test and the scroll. Drag keeps seeking - // while the button is held, matching the GTK side. + // ── Minimap press (#35) ───────────────────────────────────────────────── + // Pure rect plumbing: hand the press's cell coordinates to the shared + // resolver, which owns the hit-test and the scroll. // // Deliberately *after* both divider hit-tests: the strip is carved off // the active window's right edge, so in a `:vsplit` it abuts (and would // otherwise swallow) the divider's grab column — and a divider drag is // the more destructive gesture to lose. - if matches!( - ev.kind, - MouseEventKind::Down(MouseButton::Left) | MouseEventKind::Drag(MouseButton::Left) - ) { - if let Some(layout) = last_layout { - if render::apply_minimap_click(engine, layout, col as f64, row as f64).is_some() { - return sidebar_width; - } + // + // #756: this arm used to match `Down | Drag`, but it sits below the + // `ev.kind != Down(Left) → return` gate above, so the `Drag` half was + // unreachable and press-and-hold on a TUI minimap seeked exactly once. The + // drag half is now `render::MouseDragRoute::Minimap`, shared with GTK, + // which had the arm working all along. + if let Some(layout) = last_layout { + if render::apply_minimap_click(engine, layout, col as f64, row as f64).is_some() { + return sidebar_width; } } diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index 9c974354..e5ed10e3 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -5580,6 +5580,78 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// #756 acceptance, TUI half of the drag rung: pressing and holding on the + /// minimap and dragging must keep seeking — the GTK twin of this + /// (`minimap_drag_keeps_seeking_while_the_button_is_held` in + /// `src/gtk/testing.rs`) has passed since #35, and this backend had no + /// equivalent behaviour at all. + /// + /// **RED against unfixed `develop`.** `mouse.rs`'s minimap arm matched + /// `Down(Left) | Drag(Left)`, but it sits below an + /// `if ev.kind != Down(Left) { return }` gate, so the `Drag` half was + /// unreachable: a press seeked once and every subsequent held move fell + /// through to the text-selection arm. Reverting `handle_mouse`'s + /// `MouseDragRoute::Minimap` arm leaves the second assertion below + /// comparing two identical screens — it fails. + /// + /// Asserted on the *painted* buffer lines (`CLAUDE.md` testing rule 1), not + /// on `scroll_top`: the lowest `line N content` marker visible on screen + /// must move further down the file after the held move than it did after + /// the press alone. + #[test] + fn tui_minimap_drag_keeps_seeking_while_the_button_is_held() { + /// Lowest `line N content` marker painted on screen — i.e. the first + /// buffer line the viewport is showing. + fn top_painted_line(screen: &str) -> Option { + screen + .match_indices("line ") + .filter_map(|(i, _)| { + let rest = &screen[i + "line ".len()..]; + let digits: String = rest.chars().take_while(|c| c.is_ascii_digit()).collect(); + if digits.is_empty() || !rest[digits.len()..].starts_with(" content") { + return None; + } + digits.parse::().ok() + }) + .min() + } + + let mut app = TuiShellApp::new(None); + let mut text = String::new(); + for i in 0..400 { + text.push_str(&format!("line {i} content\n")); + } + app.engine.buffer_mut().insert(0, &text); + assert!( + app.engine.settings.minimap, + "setup sanity: the minimap must default on, or this test proves nothing" + ); + let mut driver = driver_with_shell(app, config(), 100, 24); + + // `minimap_reserved_width` gives a 100-column pane a 15-column strip + // (`min(MINIMAP_TARGET_COLS, 100 * MINIMAP_WIDTH_FRACTION)`, clamped + // into `[MINIMAP_MIN_COLS, MINIMAP_MAX_COLS]`), painted flush against + // the pane's right edge — so column 97 is inside the strip with room + // to spare on either side of the exact width. + let strip_col = 97.0; + + driver.mouse_down(strip_col, 6.0); + let after_down = top_painted_line(&driver.screen()) + .expect("the editor must paint `line N content` markers after the press"); + + driver.mouse_move(strip_col, 17.0); + let after_drag = top_painted_line(&driver.screen()) + .expect("the editor must still paint `line N content` markers after the drag"); + + assert!( + after_drag > after_down, + "a held drag further down the minimap must scroll further into the \ + file, but the painted viewport still starts at line {after_down} \ + (press) vs {after_drag} (drag); screen:\n{}", + driver.screen() + ); + } + /// #609: `render_content` must also paint the tab-hover tooltip — /// `render_tab_hover_tooltip`, ported from `draw_frame`'s raw-`Buffer` /// write to `Backend::draw_status_bar` (see that function's doc From c895136af044cab2d97b8e21dc063d95bf105cce Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Wed, 2 Sep 2026 17:01:38 -0500 Subject: [PATCH 2/2] #756 review fix iteration 1: restore TUI drag-to-select text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocking regression: `armed_target: drag_state.is_active()` made every TUI editor text-selection drag resolve to `MouseDragRoute::ArmedTarget` instead of `EditorText`, because the same click path arms the shared `quadraui::DragState` with `DragTarget::TextSelection` (#565) — not just scrollbars/picker thumbs, which is what `ArmedTarget`'s handler (`apply_scrollbar_drag`) actually reacts to. `EditorText`, the only arm that calls `Engine::mouse_drag` to extend the selection, was unreachable for the whole gesture. Fixed with a new shared `render::drag_state_arms_scrollbar`, called from both backends, that excludes `TextSelection` from `armed_target` — a no-op on GTK, which never arms `TextSelection` on its shared `DragState`. Added `tui_editor_text_drag_paints_a_selection_through_the_shared_drag_router` (`src/tui_main/shell_app.rs`), the TUI twin of the GTK black-box test the review noted was missing. Verified RED against the unfixed router. Non-blocking concern also fixed: once `armed_target` no longer swallows a TextSelection drag, the drag falls through to pure geometry again, so a selection drag whose pointer strays over the minimap strip or into the terminal panel could get hijacked mid-gesture. Added `MouseDragState::text_selection_active` (driven by the pre-existing, already-shared `Engine::mouse_drag_active`) as an armed rung ahead of that geometry, plus a router-level regression test. Not addressed: the <800-line entry-point target. Measured after this fix: handle_mouse 2470, handle_mouse_event 368, handle_mouse_click_msg 430, handle_mouse_drag_msg 278, try_route_sidebar_mouse_event 174 = 3720 total (vs 3710 before this fix; the delta is doc comments on the two touched call sites). Converging the remaining click/hover/scroll ladder in `handle_mouse` (~1,200+ of its 2,470 lines are still bespoke Down / Moved / right-click handling, not thin `render.rs` delegation) is a multi-session-scale rewrite across both backends, not something this fix iteration can responsibly attempt without either running out of budget mid-rewrite or shipping it unverified. Flagging for the coordinator per the review's own framing: either re-scope/re-title this PR to the drag rung it actually converges and file a tracked follow-up for the rest of the closing slice, or plan further fix iterations specifically for that conversion. cargo build, cargo clippy -- -D warnings, cargo fmt --check all clean. Scoped test run (render::mouse_drag_router_tests, the new/existing TUI shell_app drag tests, tui_main::mouse::tests, gtk::testing::*): 259/259 passing. Co-Authored-By: Claude Sonnet 5 --- src/gtk/mod.rs | 8 ++- src/render.rs | 103 +++++++++++++++++++++++++++++++++++++- src/tui_main/mouse.rs | 6 ++- src/tui_main/shell_app.rs | 62 +++++++++++++++++++++++ 4 files changed, 176 insertions(+), 3 deletions(-) diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index d1495cfc..3c2c0030 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -3461,7 +3461,7 @@ impl App { let layout_ref = self.cached_screen_layout.borrow(); let state = render::MouseDragState { layout: layout_ref.as_ref(), - armed_target: drag_rc.borrow().is_active(), + armed_target: render::drag_state_arms_scrollbar(&drag_rc.borrow()), hover_popup_selecting: engine.editor_hover_has_focus && engine .editor_hover @@ -3490,6 +3490,12 @@ impl App { divider_grabbed: self.divider_grab.is_some(), terminal_split_dragging: self.terminal_split_dragging, terminal_panel_resizing: self.terminal_resize_dragging, + // #756 review: mirrors TUI's guard — see the field's doc + // comment in `render.rs`. GTK's `EditorText` arm doesn't run + // through the shared `DragState`, but it drives the same + // `Engine::mouse_drag`, so `mouse_drag_active` is just as + // valid a "already extending" signal here. + text_selection_active: engine.mouse_drag_active, in_terminal_content: render::in_terminal_pane_content( &engine, x, diff --git a/src/render.rs b/src/render.rs index 1624f0a7..9a408321 100644 --- a/src/render.rs +++ b/src/render.rs @@ -3862,6 +3862,31 @@ pub enum MouseDragRoute { None, } +/// Whether `drag_state` is armed with the rung [`MouseDragRoute::ArmedTarget`] +/// means: a scrollbar or picker thumb. +/// +/// **Not** the same as `quadraui::DragState::is_active()`. TUI's editor click +/// path (#565) arms the *same* shared `DragState` with +/// `DragTarget::TextSelection` so a later `Drag` event can recover which +/// window the selection started in — but that gesture belongs to the +/// [`MouseDragRoute::EditorText`] rung, not `ArmedTarget`, whose handler +/// (`apply_scrollbar_drag` / GTK's `dispatch_mouse_drag` arm) only reacts to +/// `UiEvent::ScrollOffsetChanged` and silently drops `TextSelectionChanged`. +/// Feeding `is_active()` straight into `armed_target` therefore made +/// `route_mouse_drag` resolve *every* text-selection drag to `ArmedTarget`, +/// making the `EditorText` rung unreachable for the whole gesture (#756 +/// review). GTK never arms `TextSelection` on its shared `DragState` (it +/// drives `EditorText` through its own `handle_mouse_drag`), so this is a +/// no-op there today — called from both backends anyway so the exclusion is +/// stated once, not re-derived if that ever changes. +pub fn drag_state_arms_scrollbar(drag_state: &quadraui::DragState) -> bool { + drag_state.is_active() + && !matches!( + drag_state.target(), + Some(quadraui::DragTarget::TextSelection { .. }) + ) +} + /// Everything [`route_mouse_drag`] needs, in the caller's own units. /// /// The booleans are the caller's drag bookkeeping (its own `dragging_sidebar` / @@ -3871,7 +3896,10 @@ pub enum MouseDragRoute { pub struct MouseDragState<'a> { /// The layout the last frame painted, for the minimap and editor hit tests. pub layout: Option<&'a ScreenLayout>, - /// `quadraui::DragState::is_active()`. + /// Whether an `ArmedTarget`-rung gesture (scrollbar/picker thumb) is in + /// progress. Compute with [`drag_state_arms_scrollbar`] — **not** + /// `quadraui::DragState::is_active()` directly; see that function's doc + /// comment for why the two differ on TUI. pub armed_target: bool, /// A text selection is in progress inside the editor hover popup. pub hover_popup_selecting: bool, @@ -3899,6 +3927,23 @@ pub struct MouseDragState<'a> { pub terminal_split_dragging: bool, /// The bottom panel is being resized. pub terminal_panel_resizing: bool, + /// `Engine::mouse_drag_active` — a previous move in this same gesture + /// already extended the editor's visual selection. + /// + /// Armed rather than geometric, for the same reason `sidebar_dnd` is: + /// once a selection has started extending, a later move that strays over + /// the minimap strip or the terminal-panel rect must not get stolen by + /// that geometry — it must keep extending the selection, exactly like + /// dragging a native text selection past a window's edge. Set from the + /// engine's own state (shared by both backends) rather than from + /// `armed_target`/`quadraui::DragState` because [`drag_state_arms_scrollbar`] + /// deliberately excludes `DragTarget::TextSelection` from `armed_target` — + /// this field is what keeps that exclusion from re-opening the same + /// "geometry steals an in-progress gesture" bug one rung down (#756 + /// review: an editor-text drag whose pointer strays into the terminal + /// panel used to get re-routed to [`MouseDragRoute::TerminalContent`] + /// once `armed_target` no longer swallowed it). + pub text_selection_active: bool, /// `true` when the point is inside a painted terminal pane's content cells /// — ask [`in_terminal_pane_content`], which both backends call so the /// question cannot be answered differently on each. @@ -3923,6 +3968,7 @@ impl Default for MouseDragState<'_> { divider_grabbed: false, terminal_split_dragging: false, terminal_panel_resizing: false, + text_selection_active: false, in_terminal_content: false, // Cell metrics default to the TUI's whole-cell grid; GTK always // states its measured font metrics explicitly. @@ -3974,6 +4020,9 @@ pub fn route_mouse_drag(state: &MouseDragState<'_>, x: f64, y: f64) -> MouseDrag if state.terminal_panel_resizing { return MouseDragRoute::TerminalPanelResize; } + if state.text_selection_active { + return MouseDragRoute::EditorText; + } // ── Modal swallow ─────────────────────────────────────────────────────── // Below the armed rungs (a scrollbar thumb *inside* a modal is an armed @@ -23630,6 +23679,11 @@ mod mouse_drag_router_tests { |s| s.terminal_panel_resizing = true, MouseDragRoute::TerminalPanelResize, ), + ( + "text selection already extending", + |s| s.text_selection_active = true, + MouseDragRoute::EditorText, + ), ( "modal swallow", |s| s.modal_hit = true, @@ -23696,6 +23750,53 @@ mod mouse_drag_router_tests { ); } + /// #756 review (non-blocking concern): once `armed_target` correctly + /// excludes `DragTarget::TextSelection` (see [`drag_state_arms_scrollbar`]), + /// an in-progress editor text-selection drag is arbitrated purely by + /// geometry again — which means a pointer that strays over the minimap + /// strip or into the terminal panel mid-selection would get hijacked + /// into [`MouseDragRoute::Minimap`] / [`MouseDragRoute::TerminalContent`] + /// instead of continuing to extend the editor selection. `text_selection_active` + /// is the guard: pins that once a selection has started extending + /// (`Engine::mouse_drag_active`), it keeps winning over both geometric + /// rungs, the same way `sidebar_dnd` keeps winning over the editor once an + /// explorer drag has been picked up. + #[test] + fn a_selection_already_extending_beats_the_minimap_and_terminal_geometry() { + let engine = drag_engine(); + let layout = frame(&engine, (1.0, 1.0)); + let mm = layout + .minimap + .first() + .expect("the fixture must paint a minimap strip"); + let strip = minimap_strip_rect(mm); + let state = MouseDragState { + layout: Some(&layout), + text_selection_active: true, + in_terminal_content: true, + ..Default::default() + }; + + assert_eq!( + route_mouse_drag( + &state, + (strip.x + strip.width / 2.0) as f64, + (strip.y + strip.height / 2.0) as f64, + ), + MouseDragRoute::EditorText, + "an in-progress text selection must not be stolen by the minimap \ + strip it happens to be dragged over" + ); + + let text_x = strip.x as f64 - 2.0; + assert_eq!( + route_mouse_drag(&state, text_x, (strip.y + strip.height / 2.0) as f64), + MouseDragRoute::EditorText, + "nor by `in_terminal_content` geometry, even though it is asserted \ + true here" + ); + } + /// **The parity test #756 asks for.** One engine, painted twice — once in /// the TUI's whole-cell units and once in GTK-shaped pixel units — then the /// *same logical points* driven through [`route_mouse_drag`] in each diff --git a/src/tui_main/mouse.rs b/src/tui_main/mouse.rs index 1d19eb68..2028bd2c 100644 --- a/src/tui_main/mouse.rs +++ b/src/tui_main/mouse.rs @@ -813,7 +813,7 @@ pub(super) fn handle_mouse( && (engine.active_panel_is(PANEL_SEARCH) || engine.active_panel_is(PANEL_SETTINGS)); let state = render::MouseDragState { layout: last_layout, - armed_target: drag_state.is_active(), + armed_target: render::drag_state_arms_scrollbar(drag_state), hover_popup_selecting: *hover_selecting, // Modal drags never reach here: the modal-overlay rung at the // top of this function returns first. GTK has no equivalent @@ -834,6 +834,10 @@ pub(super) fn handle_mouse( divider_grabbed: divider_grab.is_some(), terminal_split_dragging: *dragging_terminal_split, terminal_panel_resizing: *dragging_terminal_resize, + // #756 review: keeps a straying pointer from getting + // re-routed to the minimap or terminal-content rungs mid + // selection — see the field's doc comment in `render.rs`. + text_selection_active: engine.mouse_drag_active, // #756: the painted geometry the press path already reads, // instead of the hand-rolled `term_height - bottom_chrome - // quickfix - strip` walk this arm used to redo — the diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index e5ed10e3..100a362b 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -5652,6 +5652,68 @@ mod tests { ); } + /// #756 review fix: click-then-drag text selection was broken on TUI. + /// TUI's own editor-click path (#565) arms the *same* shared + /// `quadraui::DragState` this rung's `ArmedTarget` route checks first — + /// with `DragTarget::TextSelection`, so `armed_target: drag_state.is_active()` + /// made every text-selection drag resolve to `ArmedTarget` (whose handler, + /// `apply_scrollbar_drag`, only reacts to `ScrollOffsetChanged` and drops + /// `TextSelectionChanged` on the floor), leaving the `EditorText` rung — + /// the only place that calls `Engine::mouse_drag` to actually extend the + /// selection — unreachable for the whole gesture. The GTK twin of this + /// (`an_editor_text_drag_paints_a_selection_through_the_shared_drag_router` + /// in `src/gtk/testing.rs`) already covered GTK, which never arms + /// `TextSelection` on its `DragState`; this is the missing TUI half. + /// + /// **RED against the unfixed router** (`armed_target: drag_state.is_active()` + /// instead of `render::drag_state_arms_scrollbar(drag_state)`): the click + /// arms `TextSelection` at mouse-down, so the `Drag` event right after it + /// resolves to `ArmedTarget`, `apply_scrollbar_drag` finds no + /// `ScrollOffsetChanged` to apply, and the probed cell's style never + /// changes — `before == after`, and this test fails. + /// + /// Same probe-a-swept-cell's-*style* technique as the GTK twin (`CLAUDE.md` + /// testing rule 1: assert on rendered output, not on state). + #[test] + fn tui_editor_text_drag_paints_a_selection_through_the_shared_drag_router() { + let mut app = TuiShellApp::new(None); + let mut text = String::new(); + for i in 0..40 { + text.push_str(&format!("line {i} content that is reasonably long\n")); + } + app.engine.buffer_mut().insert(0, &text); + let mut driver = driver_with_shell(app, config(), 100, 24); + + let bounds = driver + .find_bounds("line 5 content") + .expect("the fixture line should be painted"); + let row_y = bounds.y + bounds.height / 2.0; + let start_x = bounds.x + 1.0; + let probe_x = (bounds.x + 6.0) as u16; + let end_x = bounds.x + 12.0; + + // Park the cursor on the row first (a plain click, not a drag) so the + // "before" sample already includes any cursor-line highlight — the + // only thing left for the gesture below to change is the selection. + driver.click(start_x, row_y); + let before = driver.style_at(probe_x, row_y as u16); + + // Press to the left of the probe and drag past it while held — the + // press arms `DragTarget::TextSelection` on the shared `DragState`, + // and the very next `Drag` event is the one the regression swallowed. + driver.mouse_down(start_x, row_y); + driver.mouse_move(end_x, row_y); + driver.mouse_up(end_x, row_y); + + let after = driver.style_at(probe_x, row_y as u16); + assert_ne!( + before, after, + "a held drag across the editor text must repaint the swept cell \ + with the selection style; both probes read {before:?} at \ + column {probe_x}, row {row_y}" + ); + } + /// #609: `render_content` must also paint the tab-hover tooltip — /// `render_tab_hover_tooltip`, ported from `draw_frame`'s raw-`Buffer` /// write to `Backend::draw_status_bar` (see that function's doc