diff --git a/src/tui_main/mod.rs b/src/tui_main/mod.rs index 7db7b029..fbfe1fbd 100644 --- a/src/tui_main/mod.rs +++ b/src/tui_main/mod.rs @@ -89,7 +89,7 @@ use ratatui::style::{Color as RColor, Modifier}; use ratatui::Terminal; use crate::core::engine::EngineAction; -use crate::core::window::{GroupId, SplitDirection}; +use crate::core::window::{GroupDivider, GroupId, SplitDirection}; use crate::core::{Engine, Mode, OpenMode, WindowRect}; use crate::icons; use crate::render::{self, build_screen_layout, Color, RenderedWindow, Theme}; diff --git a/src/tui_main/render_impl.rs b/src/tui_main/render_impl.rs index f4dc2ba0..d1939bd6 100644 --- a/src/tui_main/render_impl.rs +++ b/src/tui_main/render_impl.rs @@ -541,42 +541,18 @@ pub(super) fn draw_frame( let layout = draw_breadcrumb_bar(backend, bc_rect, t.bar, theme); *t.draw_layout.borrow_mut() = Some(layout); } - // Draw divider lines (vertical only — horizontal splits use the tab bar as divider). - // `div.position`/`.cross_start` are already absolute terminal-screen - // coordinates (#550), matching `editor_area`'s own coordinate space — - // no offset addition needed. - let sep_fg = rc(theme.separator); - let sep_bg = rc(theme.background); - for div in &split.dividers { - if div.direction == SplitDirection::Vertical { - let div_x = div.position as u16; - let y_start = div.cross_start as u16; - let y_end = y_start + div.cross_size as u16; - for y in y_start..y_end { - if div_x < editor_area.x + editor_area.width { - // #481: the window immediately to the left already - // renders its own separator in the column right before - // the divider (`div_x - 1`) — either its vertical - // scrollbar (via `quadraui::tui::draw_editor`, glyphs - // `█`/`░`) when it overflows, or a plain divider line - // (`│`, painted by `render_separators`) when it does - // not. Either way that column already visually - // separates the two groups, so painting a second - // divider glyph beside it produces a phantom - // "duplicate scrollbar"/double-line bar in multi-tab- - // group layouts. Skip the divider on rows where such a - // separator already occupies `div_x - 1`. - if div_x > editor_area.x { - let left = frame.buffer_mut()[(div_x - 1, y)].symbol(); - if left == "█" || left == "░" || left == "│" { - continue; - } - } - set_cell(frame.buffer_mut(), div_x, y, '│', sep_fg, sep_bg); - } - } - } - } + // Draw divider lines between editor groups. `div.position`/ + // `.cross_start` are already absolute terminal-screen coordinates + // (#550), matching `editor_area`'s own coordinate space — no offset + // addition needed. #609: routed through `Backend::draw_status_bar` + // (via `render_group_dividers`/`group_divider_cells`) instead of a + // raw `Buffer` write, so `TuiShellApp::render_content` — which has + // no `Buffer`/`Frame` to write into — can paint the exact same + // dividers; see `group_divider_cells`'s doc comment for how the old + // `frame.buffer_mut()[(div_x - 1, y)]` read-back (the #481 + // phantom-divider-beside-scrollbar guard) became a pure data + // computation both call sites now share. + render_group_dividers(backend, &split.dividers, &screen.windows, editor_area, theme); } else { // Single group: tab bar at row 0 of editor_area, windows at row 1+. for target in &tab_bar_targets { @@ -627,7 +603,6 @@ pub(super) fn draw_frame( // ── Tab drag overlay ──────────────────────────────────────────────────── if tab_drag_source.is_some() { render_tab_drag_overlay( - frame, backend, engine, editor_area, @@ -642,17 +617,14 @@ pub(super) fn draw_frame( // ── Tab hover tooltip (rendered on top of editor, below tab bar) ────── if let Some(ref tooltip_text) = screen.tab_tooltip { let menu_rows: u16 = if engine.menu_bar_visible { 1 } else { 0 }; - let tooltip_row = menu_rows + 1; // just below the tab bar row - let len = tooltip_text.chars().count() as u16; - // Position at the right edge of the editor area, or where the tooltip fits. - let tooltip_x = editor_area.x; - let tooltip_w = len.min(editor_area.width); - let fg = rc(theme.hover_fg); - let bg = rc(theme.hover_bg); - for dx in 0..tooltip_w { - let ch = tooltip_text.chars().nth(dx as usize).unwrap_or(' '); - set_cell(frame.buffer_mut(), tooltip_x + dx, tooltip_row, ch, fg, bg); - } + render_tab_hover_tooltip( + backend, + editor_area.x, + menu_rows + 1, // just below the tab bar row + editor_area.width, + tooltip_text, + theme, + ); } // ── Editor popups: completion / hover / editor-hover / diff-peek / @@ -1403,8 +1375,7 @@ fn build_tui_tab_slots( /// `tab_drop_zone` is the most recently computed drop zone. #[allow(clippy::too_many_arguments)] pub(super) fn render_tab_drag_overlay( - frame: &mut ratatui::Frame, - backend: &mut super::backend::TuiBackend, + backend: &mut dyn quadraui::Backend, engine: &Engine, editor_area: Rect, screen: &render::ScreenLayout, @@ -1448,13 +1419,12 @@ pub(super) fn render_tab_drag_overlay( }; { - use quadraui::Backend; let q_overlay = quadraui::DropOverlay { highlight: overlay.highlight, insertion_bar: overlay.insertion_bar, ghost_position: Some(overlay.ghost_position), }; - backend.set_current_theme(super::quadraui_tui::q_theme(theme)); + backend.set_theme(super::quadraui_tui::q_theme(theme)); backend.draw_drop_overlay(&q_overlay); } @@ -1476,23 +1446,53 @@ pub(super) fn render_tab_drag_overlay( if tab_drag_cursor.is_some() && !drag_label.is_empty() { let label = &drag_label; - if !label.is_empty() { - let gx = overlay.ghost_position.0 as u16; - let gy = overlay.ghost_position.1 as u16; - let ghost_fg = RColor::White; - let ghost_bg = RColor::Indexed(238); - let buf = frame.buffer_mut(); - for (i, ch) in label.chars().enumerate() { - let cx = gx + i as u16; - let area = buf.area; - if cx < area.x + area.width && gy < area.y + area.height { - buf[(cx, gy)].set_char(ch).set_fg(ghost_fg).set_bg(ghost_bg); - } - } - } + let gx = overlay.ghost_position.0 as u16; + let gy = overlay.ghost_position.1 as u16; + // #609: was a raw `Buffer` write (`frame.buffer_mut()`); routed + // through `draw_rule_row` (the `Backend::draw_status_bar` trick — + // see `draw_rule_cell_themed`'s doc comment) so this reaches the screen + // from `&mut dyn Backend`, which `TuiShellApp::render_content` has + // but no `Frame`/`Buffer` for. `RColor::White` / + // `RColor::Indexed(238)` (an xterm-256 palette index with no + // meaningful `quadraui::Color` RGB equivalent) become plain + // truecolor RGB — `Color::from_rgb(255, 255, 255)` and the + // `Indexed(238)` grayscale-ramp equivalent `Color::from_rgb(68, 68, + // 68)` (xterm 256-color formula: `8 + (238 - 232) * 10`) — losing + // exact palette parity but matching every other `draw_status_bar` + // call site here, which are all already truecolor. + let ghost_fg = Color::from_rgb(255, 255, 255); + let ghost_bg = Color::from_rgb(68, 68, 68); + draw_rule_row(backend, gx, gy, label, ghost_fg, ghost_bg, theme); } } +/// Paint the tab-hover tooltip — the small popup shown when the mouse +/// hovers a tab and lingers, naming the buffer under the cursor. `screen +/// .tab_tooltip` (pre-computed by `render::build_screen_layout`) is the +/// only input; painting is a single-row `Backend::draw_status_bar` call +/// (the `draw_rule_row`/[`draw_rule_cell_themed`] trick, #609) instead of the raw +/// `Buffer` write this replaces, so both `draw_frame` and +/// `TuiShellApp::render_content` can call it — the two callers differ only +/// in `(x, y)`, since `render_content`'s `area` doesn't start at the +/// terminal's row 0 the way `draw_frame`'s `editor_area` implicitly did +/// (see call sites for the position math each uses). +pub(super) fn render_tab_hover_tooltip( + backend: &mut dyn quadraui::Backend, + x: u16, + y: u16, + max_width: u16, + tooltip_text: &str, + theme: &Theme, +) { + let len = tooltip_text.chars().count() as u16; + let w = len.min(max_width); + if w == 0 { + return; + } + let text: String = tooltip_text.chars().take(w as usize).collect(); + draw_rule_row(backend, x, y, &text, theme.hover_fg, theme.hover_bg, theme); +} + /// Compute the drop zone for a tab drag in TUI based on cursor cell position. pub(super) fn compute_tui_tab_drop_zone( engine: &Engine, @@ -1595,10 +1595,15 @@ pub(super) fn draw_breadcrumb_bar( // ─── Editor windows ─────────────────────────────────────────────────────────── /// `frame: None` (from `TuiShellApp::render_content`, #601) skips cursor -/// placement (see `render_window`'s doc comment) *and* skips -/// `render_separators`' window-divider lines — raw `Buffer` writes with no -/// `Backend::draw_*` trait equivalent yet, tracked as vimcode#609. The live -/// `draw_frame` path keeps passing `Some(frame)`, unchanged behavior. +/// placement only (see `render_window`'s doc comment) — cursor placement +/// needs `Frame::set_cursor_position`, which `render_content` still can't +/// reach directly, but #604 closed that gap a layer up (`tui/run.rs +/// ::render_frame` applies `TuiBackend`'s cached `cursor_position` after +/// `render_content` returns). `render_separators`'s window-divider lines +/// used to be a second, unrelated casualty of `frame: None` (raw `Buffer` +/// writes with no `Backend::draw_*` trait equivalent) — #609 ported it to +/// `Backend::draw_status_bar` (see that function's doc comment), so it now +/// runs unconditionally here regardless of `frame`. pub(super) fn render_all_windows( backend: &mut dyn quadraui::Backend, mut frame: Option<&mut ratatui::Frame>, @@ -1615,9 +1620,7 @@ pub(super) fn render_all_windows( }; render_window(backend, frame.as_deref_mut(), win_rect, window, theme); } - if let Some(frame) = frame { - render_separators(frame.buffer_mut(), windows, theme); - } + render_separators(backend, windows, theme); } /// Render the unified picker popup. Supports single-pane (no preview) and @@ -1756,26 +1759,39 @@ pub(super) fn char_col_to_visual(raw_text: &str, char_col: usize, tabstop: usize vis } -pub(super) fn render_separators( - buf: &mut ratatui::buffer::Buffer, - windows: &[RenderedWindow], - theme: &Theme, -) { - if windows.len() <= 1 { - return; - } - let sep_fg = rc(theme.separator); - let sep_bg = rc(theme.background); +/// True when `w`'s own `Backend::draw_editor` scrollbar occupies its last +/// column (i.e. its buffer content overflows the visible text rows). +/// Extracted from `render_separators`' vertical-separator branch (#481 +/// iter4's fix) so both `render_separators` and [`group_divider_cells`] +/// (#609) can apply the exact same "the scrollbar already doubles as the +/// separator" rule without duplicating the row-accounting math. +fn window_overflows_vertically(w: &RenderedWindow) -> bool { + let text_rows = (w.rect.height as usize) + .saturating_sub(if w.status_line.is_some() && w.rect.height > 1.0 { + 1 + } else { + 0 + }); + w.total_lines > text_rows +} +/// Absolute `(x, y)` terminal cells where [`render_separators`] paints a +/// vertical `'│'` window-divider glyph — the same geometry its own +/// painting loop below walks, factored out as a pure data computation (no +/// `Buffer`/`Backend` access) so [`group_divider_cells`] (#609) can ask +/// "would `render_separators` already put a divider-like glyph in this +/// cell?" without needing to read back whatever was actually painted. +fn vertical_separator_cells(windows: &[RenderedWindow]) -> std::collections::HashSet<(u16, u16)> { + let mut cells = std::collections::HashSet::new(); for i in 0..windows.len() { for j in (i + 1)..windows.len() { let a = &windows[i]; let b = &windows[j]; - // Vertical separator: window a is the left pane, b is the right pane. - // The boundary sits in the last column of a (`sep_x - 1`). - // Also require vertical overlap — windows from different groups may - // share an x edge but not overlap in y (e.g. 2×2 grid). + // Window a is the left pane, b is the right pane. The boundary + // sits in the last column of a (`sep_x - 1`). Also require + // vertical overlap — windows from different groups may share an + // x edge but not overlap in y (e.g. 2×2 grid). let v_overlap = a.rect.y.max(b.rect.y) < (a.rect.y + a.rect.height).min(b.rect.y + b.rect.height); if (a.rect.x + a.rect.width - b.rect.x).abs() < 1.0 && v_overlap { @@ -1799,26 +1815,139 @@ pub(super) fn render_separators( // boundaries. Let `draw_editor`'s scrollbar own the column; it // doubles as the visual separator. Only when the left window // has NO scrollbar do we draw a plain divider line. - // - // Match `draw_editor`'s overflow test exactly (it reserves the - // status-line row from the viewport) so we draw the '│' in - // precisely the cases where it drew no scrollbar. - let text_rows = (a.rect.height as usize).saturating_sub( - if a.status_line.is_some() && a.rect.height > 1.0 { - 1 - } else { - 0 - }, - ); - let has_scroll = a.total_lines > text_rows && y_end > y_start; + let has_scroll = window_overflows_vertically(a) && y_end > y_start; if !has_scroll { - for dy in 0..y_end.saturating_sub(y_start) { - let y = y_start + dy; - set_cell(buf, sep_x.saturating_sub(1), y, '│', sep_fg, sep_bg); + for y in y_start..y_end { + cells.insert((sep_x.saturating_sub(1), y)); } } } + } + } + cells +} + +/// Same trick as [`draw_rule_row`], for a horizontal run of `text` in one +/// row — used both for horizontal window separators (`'─'` repeated) and +/// the tab-drag ghost label / tab-hover tooltip (#609), which paint +/// multi-character text rather than a single rule glyph. +/// +/// Sets the backend theme on every call, which is correct (if slightly +/// redundant with the caller's own up-front `set_theme`) for the +/// single-shot call sites (drag ghost, hover tooltip) but wasteful for +/// per-cell loops — see [`draw_rule_cell_themed`]/[`draw_rule_row_themed`], +/// which [`render_separators`] and [`render_group_dividers`] use instead so +/// the ~50-field `quadraui::Theme` is rebuilt once per frame, not once per +/// divider cell. +fn draw_rule_row( + backend: &mut dyn quadraui::Backend, + x: u16, + y: u16, + text: &str, + fg: Color, + bg: Color, + theme: &Theme, +) { + backend.set_theme(super::quadraui_tui::q_theme(theme)); + draw_rule_row_themed(backend, x, y, text, fg, bg); +} + +/// Paint a single divider/rule glyph at `(x, y)` through +/// `Backend::draw_status_bar` — the same "a solid-colour `StatusBar` +/// segment stands in for a plain rule line" trick `AppShell::render`'s own +/// generic divider (quadraui `compose/app_shell.rs::render`'s +/// `divider_bounds` block) uses for the sidebar-resize divider, so no new +/// quadraui primitive is needed (#609). `tui/status_bar.rs::draw_status_bar` +/// paints segment text verbatim, one character per cell, in the segment's +/// `fg`/`bg` — a 1-cell-wide, 1-row `StatusBar` therefore renders exactly +/// like a raw `set_cell` write would, but reaches the screen through +/// `&mut dyn Backend`, which `set_cell`/`Buffer` writes cannot. +/// +/// Assumes the caller has already applied `backend.set_theme(...)` — see +/// [`draw_rule_row_themed`]'s doc comment for why callers that loop over +/// many cells use this instead of setting the theme per call. +fn draw_rule_cell_themed( + backend: &mut dyn quadraui::Backend, + x: u16, + y: u16, + ch: char, + fg: Color, + bg: Color, +) { + draw_rule_row_themed(backend, x, y, &ch.to_string(), fg, bg); +} + +/// Theme-less core of [`draw_rule_row`] — paints without touching the +/// backend's current theme. Callers that loop over many cells/rows in one +/// frame (`render_separators`, `render_group_dividers`) call +/// `backend.set_theme(...)` once up front and then use this (via +/// [`draw_rule_cell_themed`]) for every cell, instead of reconstructing the +/// theme on each of the dozens of divider cells a tall terminal can have. +fn draw_rule_row_themed( + backend: &mut dyn quadraui::Backend, + x: u16, + y: u16, + text: &str, + fg: Color, + bg: Color, +) { + if text.is_empty() { + return; + } + let bar = quadraui::StatusBar { + // Every divider/rule/ghost/tooltip draw shares this literal ID. + // That's intentionally inert today — `draw_status_bar`'s returned + // hit-region layout is discarded (`let _ = ...`) by every caller + // here, and TUI's `draw_status_bar` impl does no ID-keyed caching — + // but if a future caller wires hover/press state or click handling + // through this helper, every rule line sharing one ID will collide. + id: quadraui::WidgetId::new("tui:rule"), + left_segments: vec![quadraui::StatusBarSegment { + text: text.to_string(), + fg: render::to_quadraui_color(fg), + bg: render::to_quadraui_color(bg), + bold: false, + action_id: None, + }], + right_segments: vec![], + }; + let width = text.chars().count() as f32; + let q_rect = quadraui::Rect::new(x as f32, y as f32, width, 1.0); + let _ = backend.draw_status_bar(q_rect, &bar, None, None); +} + +/// Window/editor-group divider lines through `Backend::draw_status_bar` +/// (#609) — see [`draw_rule_cell_themed`]'s doc comment for the underlying trick. +/// Draws both the vertical dividers *between windows within a split group* +/// (e.g. `:vsplit`) and the horizontal ones (`:split`); the group-level +/// dividers *between* editor groups (`split.dividers`, drawn only when +/// `screen.editor_group_split.is_some()`) are a separate pass — +/// [`group_divider_cells`], called by both `draw_frame` and +/// `TuiShellApp::render_content`. +pub(super) fn render_separators( + backend: &mut dyn quadraui::Backend, + windows: &[RenderedWindow], + theme: &Theme, +) { + if windows.len() <= 1 { + return; + } + + // Set the backend theme once up front rather than per divider cell/row + // (see `draw_rule_row_themed`'s doc comment) — a tall terminal with + // several vertical dividers would otherwise rebuild the ~50-field + // `quadraui::Theme` dozens of times per frame. + backend.set_theme(super::quadraui_tui::q_theme(theme)); + + for (x, y) in vertical_separator_cells(windows) { + draw_rule_cell_themed(backend, x, y, '│', theme.separator, theme.background); + } + + for i in 0..windows.len() { + for j in (i + 1)..windows.len() { + let a = &windows[i]; + let b = &windows[j]; // Horizontal separator — also require horizontal overlap. // Skip when the upper window has a per-window status bar (it replaces the separator). @@ -1835,12 +1964,115 @@ pub(super) fn render_separators( let sep_y = (a.rect.y + a.rect.height) as u16; let x_start = a.rect.x.max(b.rect.x) as u16; let x_end = (a.rect.x + a.rect.width).min(b.rect.x + b.rect.width) as u16; - for x in x_start..x_end.max(x_start) { - set_cell(buf, x, sep_y.saturating_sub(1), '─', sep_fg, sep_bg); + if x_end > x_start { + let row: String = "─".repeat((x_end - x_start) as usize); + draw_rule_row_themed( + backend, + x_start, + sep_y.saturating_sub(1), + &row, + theme.separator, + theme.background, + ); + } + } + } + } +} + +/// Absolute `(x, y)` cells where the *group-level* divider (`split +/// .dividers` — the boundary between editor groups, e.g. `Ctrl+W v`, as +/// opposed to `render_separators`' within-group `:vsplit`/`:split` +/// dividers) should paint a vertical `'│'` glyph (#609). Filters out cells +/// where the window immediately to the left already shows a visual +/// separator of its own in that exact column — its own overflow scrollbar, +/// or a `render_separators` divider landing on the same cell (#481: two +/// adjacent divider-like columns read as a phantom "duplicate scrollbar"). +/// +/// Pure data computation over `windows`, no `Buffer` read: the pre-#609 +/// `draw_frame` loop this replaces read back +/// `frame.buffer_mut()[(div_x - 1, y)]`'s already-painted symbol to detect +/// the same condition, which only worked because it ran after +/// `render_all_windows`/`render_separators` had already painted into that +/// `Buffer`. `TuiShellApp::render_content` has no `Buffer` to read at all +/// (see this module's own doc comment), so this recomputes "does the left +/// window already separate the two groups here" directly from window +/// geometry (`window_overflows_vertically` for the scrollbar case, +/// `vertical_separator_cells` for the `render_separators` case) instead — +/// both `draw_frame` and `render_content` can now share one answer. +pub(super) fn group_divider_cells( + dividers: &[GroupDivider], + windows: &[RenderedWindow], + editor_area: Rect, +) -> Vec<(u16, u16)> { + let already_separated = vertical_separator_cells(windows); + let mut out = Vec::new(); + for div in dividers { + if div.direction != SplitDirection::Vertical { + continue; // horizontal splits use the tab bar as divider. + } + let div_x = div.position as u16; + if div_x >= editor_area.x + editor_area.width { + continue; + } + let y_start = div.cross_start as u16; + let y_end = y_start + div.cross_size as u16; + for y in y_start..y_end { + if div_x > editor_area.x { + let left_col = div_x - 1; + let left_has_scrollbar = windows.iter().any(|w| { + let last_col = (w.rect.x + w.rect.width) as u16; + // Bound the row range the same way `window_overflows_vertically` + // bounds `text_rows`: `draw_editor` only paints the scrollbar + // into the window's *text* rows, never the last row when that + // row is reserved for the per-window status line (see + // `render_window`). Without this the status-line row was + // wrongly treated as scrollbar-covered, leaving a 1-row gap in + // the group divider right at the neighbor's status bar. + let text_row_end = (w.rect.y + w.rect.height) as u16 + - if w.status_line.is_some() && w.rect.height > 1.0 { + 1 + } else { + 0 + }; + last_col.saturating_sub(1) == left_col + && (w.rect.y as u16..text_row_end).contains(&y) + && window_overflows_vertically(w) + }); + if left_has_scrollbar || already_separated.contains(&(left_col, y)) { + continue; } } + out.push((div_x, y)); } } + out +} + +/// Paint the group-level divider lines computed by [`group_divider_cells`] +/// through `Backend::draw_status_bar` (see [`draw_rule_cell_themed`]'s doc +/// comment for the underlying trick). Shared by `draw_frame` (the live +/// path) and `TuiShellApp::render_content` (#609) — see the latter's call +/// site for why it's the same call for both. +pub(super) fn render_group_dividers( + backend: &mut dyn quadraui::Backend, + dividers: &[GroupDivider], + windows: &[RenderedWindow], + editor_area: Rect, + theme: &Theme, +) { + let cells = group_divider_cells(dividers, windows, editor_area); + if cells.is_empty() { + return; + } + // Set the theme once for the whole batch — see `draw_rule_row_themed`'s + // doc comment for why the per-cell `draw_rule_cell_themed` (which + // assumes the theme is already set) is used here instead of + // `draw_rule_row` (which sets the theme on every call). + backend.set_theme(super::quadraui_tui::q_theme(theme)); + for (x, y) in cells { + draw_rule_cell_themed(backend, x, y, '│', theme.separator, theme.background); + } } // ─── Activity bar ───────────────────────────────────────────────────────────── @@ -1859,7 +2091,8 @@ pub(super) fn render_separators( #[cfg(test)] mod tests { use super::*; - use crate::core::window::GroupId; + use crate::core::window::{GroupId, WindowId}; + use crate::render::WindowStatusLine; use ratatui::backend::TestBackend; /// Create a hermetic engine for rendering tests. @@ -2608,4 +2841,105 @@ mod tests { // Render must not panic either. let _lines = render_tui(&e, 80, 24); } + + /// Minimal `RenderedWindow` fixture with every field defaulted except + /// what the caller overrides — mirrors `render.rs::build_rendered_window`'s + /// own `empty` closure, since `RenderedWindow` has no `Default` impl. + fn fixture_window( + window_id: WindowId, + rect: WindowRect, + total_lines: usize, + status_line: Option, + ) -> RenderedWindow { + RenderedWindow { + window_id, + rect, + lines: vec![], + cursor: None, + extra_cursors: vec![], + selection: None, + extra_selections: vec![], + yank_highlight: None, + scroll_top: 0, + scroll_left: 0, + total_lines, + gutter_char_width: 0, + text_viewport_cols: 0, + is_active: true, + show_active_bg: false, + has_git_diff: false, + has_breakpoints: false, + max_col: 0, + diagnostic_gutter: std::collections::HashMap::new(), + code_action_lines: std::collections::HashSet::new(), + bracket_match_positions: Vec::new(), + active_indent_col: None, + tabstop: 4, + cursorline: false, + status_line, + } + } + + /// #609 review fix: [`group_divider_cells`]'s `left_has_scrollbar` check + /// must exclude the neighbor window's per-window status-line row, the + /// same way `window_overflows_vertically` excludes it from `text_rows`. + /// Before the fix, the check spanned the window's *full* rect height, + /// so an overflowing left window with a status line wrongly looked + /// scrollbar-covered on its status-line row too — leaving a 1-row gap + /// in the group divider exactly at that row. Regresses the scenario the + /// higher-level `render_content_paints_group_divider_via_shell_app` + /// test deliberately sidesteps (per its own doc comment: short, + /// non-overflowing content). + #[test] + fn group_divider_cells_covers_neighbor_status_line_row() { + // Left window: overflows vertically (total_lines=20 >> the 9 text + // rows available after reserving 1 row for the status line out of + // height=10), and has a per-window status line on its last row (y=9). + let left = fixture_window( + WindowId(0), + WindowRect::new(0.0, 0.0, 10.0, 10.0), + 20, + Some(WindowStatusLine { + left_segments: vec![], + right_segments: vec![], + }), + ); + let divider = GroupDivider { + split_index: 0, + direction: SplitDirection::Vertical, + position: 10.0, + axis_start: 0.0, + axis_size: 20.0, + cross_start: 0.0, + cross_size: 10.0, + }; + let editor_area = Rect { + x: 0, + y: 0, + width: 20, + height: 10, + }; + + let cells = group_divider_cells(&[divider], &[left], editor_area); + + // The status-line row (y=9) is NOT one of the window's text rows — + // `draw_editor` never paints a scrollbar glyph there — so the group + // divider must still cover it. + assert!( + cells.contains(&(10, 9)), + "group divider should cover the neighbor's status-line row (y=9), \ + leaving no gap; got cells: {cells:?}" + ); + // Sanity check the other half of the rule: every actual text row + // (y=0..9) IS covered by the left window's own overflow scrollbar, + // so the group divider correctly stays out of those rows (letting + // the scrollbar double as the divider, per #481). + for y in 0..9u16 { + assert!( + !cells.contains(&(10, y)), + "row {y} is a text row with an overflow scrollbar; the group \ + divider should not double up there. got cells: {cells:?}" + ); + } + } } diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index 1e579a8b..c302aa60 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -71,8 +71,38 @@ //! (`render_ai_sidebar` takes `buf: &mut ratatui::buffer::Buffer` //! directly — no backend parameter at all, the most raw of the //! lot). -//! - Window/group divider lines + tab-drag overlay + tab-hover tooltip -//! (#609). +//! +//! #609 (Stage 2c, closed) additionally wires the window/editor-group +//! divider lines, the tab-drag ghost overlay, and the tab-hover tooltip +//! into `render_content`. All three were raw `Buffer`/`Frame` writes in +//! `draw_frame` with no `Backend::draw_*` trait equivalent at the time +//! #601 scoped them out — #609 found none was actually needed: a +//! 1-cell-wide (or 1-row) `StatusBar` with a single solid/text segment, +//! painted through `Backend::draw_status_bar`, reproduces a raw +//! `set_cell` write exactly (`render_impl.rs::draw_rule_cell`/ +//! `draw_rule_row`) — the same trick quadraui's own +//! `compose::app_shell::AppShell::render` already uses for its generic +//! sidebar-resize divider, confirming the issue's hunch that no new +//! primitive was required. `render_separators` (within-group +//! `:split`/`:vsplit` dividers) and the group-level divider loop +//! (`split.dividers`, between `Ctrl+W v`/`Ctrl+W s` groups) both moved +//! to this trick and now run unconditionally in `render_all_windows`/ +//! `draw_frame` — including on the *live* `draw_frame` path, which used +//! to read back `frame.buffer_mut()[(div_x - 1, y)]`'s painted symbol +//! to avoid a phantom double-divider beside an overflowing window's own +//! scrollbar (#481); that read-back is now `group_divider_cells`, a +//! pure data computation over `RenderedWindow` geometry shared by both +//! callers — see its doc comment. The tab-drag overlay reads +//! `TuiShellApp`'s `tui_drag_source`/`tui_drag_cursor`/ +//! `tui_tab_drop_zone` fields, which #602 already wires +//! `handle_mouse_event` to populate, so no further sequencing with #602 +//! was needed, just the paint call. The tab-hover tooltip sources +//! straight from `engine.tab_hover_tooltip`, a plain field with no +//! per-frame state of its own. Covered by +//! `render_content_paints_group_divider_via_shell_app` (`driver_with_shell`, +//! the required headless case per #609's acceptance bar), +//! `render_content_paints_tab_drag_ghost_via_shell_app`, and +//! `render_content_paints_tab_hover_tooltip_via_shell_app` below. //! //! #608 (Stage 2b, closed) additionally wires the quickfix panel and the //! bottom panel (terminal/debug output) into `render_content`, via @@ -805,9 +835,12 @@ impl ShellApp for TuiShellApp { // ── Tab bar(s) + breadcrumb bar(s) + editor windows ───────────── // Windows are painted first (matches `draw_frame`'s split-group // order — see its own comment) so window content can't overwrite - // an adjacent group's tab bar; divider lines between windows are - // skipped here (#609), same as passing `frame: None` skips them in - // `render_all_windows`. + // an adjacent group's tab bar. `render_all_windows` also paints the + // within-group (`:split`/`:vsplit`) divider lines unconditionally + // now (#609 routed `render_separators` through + // `Backend::draw_status_bar` — see its doc comment — so it no + // longer needs the raw `Frame` that `frame: None` used to skip it + // for). render_all_windows(backend, None, &screen.windows, &theme); let tui_tbh: f64 = if self.engine.settings.breadcrumbs && !self.engine.terminal_maximized { @@ -842,6 +875,59 @@ impl ShellApp for TuiShellApp { *t.draw_layout.borrow_mut() = Some(bc_layout); } + // ── Group divider lines (#609) ─────────────────────────────────── + // Between-*group* dividers (`Ctrl+W v`/`Ctrl+W s`, as opposed to + // `render_all_windows`'s within-group `render_separators` above) — + // only present when the editor is split into multiple groups. + // Mirrors `draw_frame`'s own divider block, ported to + // `Backend::draw_status_bar` via `render_group_dividers` (see its + // doc comment, and `group_divider_cells`'s for how the #481 + // phantom-divider-beside-scrollbar guard became a pure data + // computation instead of a `Buffer` read-back). + if let Some(ref split) = screen.editor_group_split { + render_group_dividers(backend, &split.dividers, &screen.windows, area, &theme); + } + + // ── Tab-drag ghost overlay (#609) ──────────────────────────────── + // Drag state (`tui_drag_source`/`tui_drag_cursor`/ + // `tui_tab_drop_zone`) is already live here — #602 wired + // `handle_mouse_event` to mutate these three fields via + // `mouse::handle_mouse` (see `handle()` below) — so painting from + // it needs no further sequencing with #602, just the paint call + // itself, which is this issue's scope. + if self.tui_drag_source.is_some() { + render_tab_drag_overlay( + backend, + &self.engine, + area, + &screen, + &theme, + self.tui_drag_source, + self.tui_drag_cursor, + &self.tui_tab_drop_zone, + ); + } + + // ── Tab-hover tooltip (#609) ───────────────────────────────────── + // Mirrors `draw_frame`'s own tooltip block, ported to + // `Backend::draw_status_bar` via `render_tab_hover_tooltip`. Unlike + // `draw_frame`'s `editor_area` (whose `y` is implicitly 0-based — + // it's the live terminal frame's own top-level split), `area` here + // is `layout.main_content_bounds`, already offset below whatever + // `AppShell::render` painted above it — see that function's doc + // comment for why the row math differs between the two callers. + if let Some(ref tooltip_text) = screen.tab_tooltip { + let menu_height: u16 = if self.engine.menu_bar_visible { 1 } else { 0 }; + render_tab_hover_tooltip( + backend, + area.x, + area.y + menu_height + 1, + area.width, + tooltip_text, + &theme, + ); + } + // ── Editor-anchored popups (completion/hover/editor-hover/ // diff-peek/signature-help) — same code `draw_frame` calls, all // already trait-only (#601's `paint_editor_popups` extraction). @@ -1764,6 +1850,131 @@ mod tests { ); } + /// #609: `render_content` must also paint the *group-level* divider + /// line between split editor groups — `render_group_dividers`, ported + /// from `draw_frame`'s raw-`Buffer`-read loop to + /// `Backend::draw_status_bar` (see that function's and + /// `group_divider_cells`'s doc comments). Content is short (well under + /// the viewport height) so neither pane overflows and shows a + /// scrollbar — the #481 guard that lets a pane's own scrollbar double + /// as the separator would otherwise mask the divider glyph itself from + /// this assertion. + /// + /// Rather than hard-code an expected column (fragile against + /// `AppShell`'s own activity-bar/sidebar layout constants), the + /// expected column is derived from the actual painted screen: both + /// panes' tab bars share row 0 (`"[No Name]"` once per pane), so the + /// divider must land strictly between the two tab labels' start + /// columns on every row of the editor body. + #[test] + fn render_content_paints_group_divider_via_shell_app() { + let mut app = TuiShellApp::new(None); + app.engine.buffer_mut().insert(0, "short\n"); + app.engine.open_editor_group(SplitDirection::Vertical); + let driver = driver_with_shell(app, config(), 80, 24); + let screen = driver.screen(); + let lines: Vec<&str> = screen.lines().collect(); + + let tab_row = lines[0]; + let starts: Vec = tab_row + .match_indices("[No Name]") + .map(|(i, _)| tab_row[..i].chars().count()) + .collect(); + assert_eq!( + starts.len(), + 2, + "expected two tab bars, one per pane; row:\n{tab_row}" + ); + let (left_tab_start, right_tab_start) = (starts[0], starts[1]); + + // Only scan columns to the right of the left pane's own tab label — + // the sidebar (a separate screen region, to the left of both panes) + // can paint its own unrelated '│' glyphs (e.g. explorer tree + // indent guides), which aren't the group divider under test. + let mut found_divider = false; + for (y, line) in lines.iter().enumerate().skip(1).take(15) { + let chars: Vec = line.chars().collect(); + let Some(col) = chars + .iter() + .enumerate() + .skip(left_tab_start) + .find(|(_, &c)| c == '│') + .map(|(i, _)| i) + else { + continue; + }; + found_divider = true; + assert!( + col > left_tab_start && col < right_tab_start, + "row {y}: divider at col {col} should land strictly between the \ + two panes' tab labels (cols {left_tab_start}..{right_tab_start}); \ + line:\n{line}" + ); + } + assert!( + found_divider, + "expected the group divider glyph '│' to paint via \ + TuiShellApp::render_content; screen:\n{screen}" + ); + } + + /// #609: `render_content` must also paint the tab-drag ghost overlay — + /// `render_tab_drag_overlay`, ported from a raw-`Frame`-write tail + /// (the ghost label) to `Backend::draw_status_bar` (see its doc + /// comment). Drives a real drag through `TuiDriver`'s mouse harness + /// (`mouse_down` + `mouse_move`, no `mouse_up`) so `tui_drag_source` + /// is genuinely live when `render_content` runs — #602 already wires + /// `handle_mouse_event` to populate it, so this is exercising the + /// paint side, not the input side. `mouse.rs`'s drag-start detection + /// requires the cursor to move `dx + dy >= 2` cells from the + /// mouse-down position before activating (distinguishing a drag from + /// a click), hence the `+4, +3` move. `driver_with_shell` wraps + /// `TuiShellApp` in an opaque `ShellAdapter` with no accessor back to + /// it (see this `mod tests`'s own doc comment), so this asserts on the + /// painted screen — a third `"[No Name]"` occurrence, the ghost label, + /// alongside the two static tab labels — rather than on + /// `tui_drag_source` directly. + #[test] + fn render_content_paints_tab_drag_ghost_via_shell_app() { + let mut app = TuiShellApp::new(None); + app.engine.new_tab(None); + let mut driver = driver_with_shell(app, config(), 80, 24); + let (tx, ty) = driver + .find("[No Name]") + .expect("tab label should be painted on screen"); + driver.mouse_down(tx, ty); + driver.mouse_move(tx + 4.0, ty + 3.0); + + let screen = driver.screen(); + let occurrences = screen.matches("[No Name]").count(); + assert!( + occurrences >= 3, + "expected the two static tab labels plus a drag-ghost label \ + (>= 3 occurrences of \"[No Name]\"), got {occurrences}; screen:\n{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 + /// comment). `screen.tab_tooltip` sources straight from + /// `engine.tab_hover_tooltip` (`render.rs`'s `ScreenLayout` builder), + /// a plain `Option` with no real-time hover-dwell timer behind + /// it — unlike the tab-drag overlay (real SGR mouse-drag sequencing, + /// left to `SMOKE_TESTS`), so this is fully reachable headlessly by + /// just setting the field directly before painting. + #[test] + fn render_content_paints_tab_hover_tooltip_via_shell_app() { + let mut app = TuiShellApp::new(None); + app.engine.tab_hover_tooltip = Some("ZQXW_609_TOOLTIP_MARKER".to_string()); + let driver = driver_with_shell(app, config(), 80, 24); + let screen = driver.screen(); + assert!( + screen.contains("ZQXW_609_TOOLTIP_MARKER"), + "tab-hover tooltip should paint via TuiShellApp::render_content; screen:\n{screen}" + ); + } + /// #607: `render_content` must also paint the *sidebar's* content — /// the explorer tree, since explorer is the default active panel — into /// `layout.sidebar_content_bounds`, via