Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ gui = ["gtk4", "pangocairo", "gio", "quadraui/gtk"]
# that file and CLAUDE.md's "Wrong quadraui checkout" section) — this
# replaces the old `VIMCODE_QUADRAUI_UNPINNED=1` escape hatch, which could
# not build backwards drift (a checkout older than a rev vimcode requires).
quadraui = { git = "https://github.com/JDonaghy/quadraui.git", rev = "bde2f4481009fa156b20a76debd16c23468a5537", features = ["tui", "terminal"] }
quadraui = { git = "https://github.com/JDonaghy/quadraui.git", rev = "69fd9cdd2107be401306bf64d1fd9553127d7226", features = ["tui", "terminal"] }
gtk4 = { version = "0.7", features = ["v4_10"], optional = true }
pangocairo = { version = "0.18", optional = true }
ropey = "1.6.1"
Expand Down Expand Up @@ -121,7 +121,7 @@ cc = "1.0"
# do not remove this without reading both first. #691: same rev as the
# `quadraui` dep above, resolved via git instead of a sibling path.
[patch.crates-io]
vt100 = { git = "https://github.com/JDonaghy/quadraui.git", rev = "bde2f4481009fa156b20a76debd16c23468a5537" }
vt100 = { git = "https://github.com/JDonaghy/quadraui.git", rev = "69fd9cdd2107be401306bf64d1fd9553127d7226" }

# Optimize dependencies in dev builds — GTK/Relm4/Cairo/Pango FFI calls and
# Tree-sitter parsing are 10-50× slower without optimization, causing visible
Expand Down
18 changes: 11 additions & 7 deletions src/gtk/click.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,18 @@ pub(super) fn pixel_to_click_target(
}
}

// ── Minimap click / drag (#35) ──────────────────────────────────────────
// Pure rect plumbing: the shared resolver owns the hit-test and the scroll.
// Checked before the zone walk because the strip was carved out of the
// active window's rect, so a `ScreenZone::Window` hit would otherwise
// swallow it. Gated on `mutate_focus` so a hover query never scrolls.
// ── Minimap click / drag (#35, #722) ────────────────────────────────────
// Pure rect plumbing: the shared resolver owns the hit-test and the
// scroll. Checked before the zone walk because every window's strip is
// carved out of that window's own rect, so a `ScreenZone::Window` hit
// would otherwise swallow it. Gated on `mutate_focus` so a hover query
// never scrolls. `apply_minimap_click` resolves against *every* pane's
// strip and reports which one it hit — never assumed to be the active
// window, since a split can have a strip on an inactive pane too.
if mutate_focus {
if let Some(line) = render_mod::apply_minimap_click(engine, cached_layout, x, y) {
let window_id = engine.active_window_id();
if let Some((window_id, line)) =
render_mod::apply_minimap_click(engine, cached_layout, x, y)
{
return ClickTarget::Minimap(window_id, line);
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/gtk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8498,8 +8498,9 @@ impl quadraui::ShellApp for App {
frame.draw(backend);
}

// #35: minimap strip on the active window's right edge — one call, the
// font-scaling rasteriser is quadraui's.
// #35/#722: minimap strips on every window's right edge (one entry
// per `WindowId` in `screen.minimap`, not just the active window's)
// — one call, the font-scaling rasteriser is quadraui's.
render::draw_minimap_strip(backend, screen);

// ── Recover a FrameHitMap for Editor/TabBar zone detection (#449) ──────
Expand Down
185 changes: 171 additions & 14 deletions src/gtk/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3811,8 +3811,9 @@ mod minimap {
let l = layout.as_ref().unwrap();
let mm = l
.minimap
.as_ref()
.expect("the layout must carry a minimap when the setting is on");
.iter()
.find(|m| m.window_id == win_on)
.expect("the layout must carry a minimap for the pane when the setting is on");
let rw = l.windows.iter().find(|w| w.window_id == win_on).unwrap();
(mm.rect, rw.rect.width, rw.text_viewport_cols)
};
Expand All @@ -3829,21 +3830,23 @@ mod minimap {
let layout = h_off.screen_layout.borrow();
let l = layout.as_ref().unwrap();
assert!(
l.minimap.is_none(),
l.minimap.is_empty(),
"`minimap: false` must remove the minimap from the layout"
);
let rw = l.windows.iter().find(|w| w.window_id == win_off).unwrap();
(rw.rect.width, rw.text_viewport_cols)
};

// `reserved_width` is defined in character cells; assert the strip in
// that unit too, without this test having to re-derive GTK's
// char_width (which is not the same number the harness reports).
assert_eq!(
off_cols - on_cols,
crate::render::MINIMAP_COLS,
"the editor must regain exactly MINIMAP_COLS text columns when \
the minimap is off (on={on_cols}, off={off_cols})"
// #722: the reserved width is now a proportion of the pane's own
// width rather than a fixed `MINIMAP_COLS` count, so the column
// delta isn't a pinned constant any more. The pixel-exact assertion
// right below is the real acceptance check (`build_screen_layout`
// narrows/widens the rect by exactly `strip.width`); this is just a
// column-domain sanity check that *some* text columns came back.
assert!(
off_cols > on_cols,
"the editor must regain text columns when the minimap is off \
(on={on_cols}, off={off_cols})"
);
assert_eq!(
on_w + strip.width,
Expand Down Expand Up @@ -3912,7 +3915,11 @@ mod minimap {
let (strip, total, rect, gutter_px, char_w, lh) = {
let layout = h.screen_layout.borrow();
let l = layout.as_ref().unwrap();
let mm = l.minimap.as_ref().expect("minimap must be present");
let mm = l
.minimap
.iter()
.find(|m| m.window_id == win)
.expect("minimap must be present for the active pane");
let rw = l
.windows
.iter()
Expand Down Expand Up @@ -4032,8 +4039,9 @@ mod minimap {
.as_ref()
.unwrap()
.minimap
.as_ref()
.expect("minimap must be present");
.iter()
.find(|m| m.window_id == win)
.expect("minimap must be present for the active pane");
(mm.rect, mm.minimap.total_buffer_lines)
};
assert_eq!(
Expand Down Expand Up @@ -4079,6 +4087,155 @@ mod minimap {
the drag continuation never reached the minimap"
);
}

/// `engine_with_shaped_buffer` split into two panes (`:vsplit`) — the
/// fixture the split-minimap black-box tests below share.
fn engine_with_split_shaped_buffer() -> Engine {
use crate::core::window::SplitDirection;
let mut engine = engine_with_shaped_buffer();
engine.split_window(SplitDirection::Vertical, None);
engine
}

/// #722 acceptance, painted-output tier: a `:vsplit` must paint **two**
/// independent minimap strips, one over each pane's own buffer — not a
/// single strip pinned to whichever pane happens to be active.
///
/// GTK twin of TUI's
/// `split_paints_two_independent_minimap_strips_via_shell_app`
/// (`tui_main/shell_app.rs`) — same acceptance criterion, driven
/// through the real headless `GtkDriver` paint path. Every #722 GTK
/// test before this one (including
/// `minimap_paints_a_strip_whose_width_matches_reserved_width` above)
/// only ever painted a single, unsplit window; the review that
/// reopened #722 flagged the missing split/pixel coverage by name.
///
/// RED against the pre-#722 code (single `Option<RenderedMinimap>`
/// gated on `active_window_id`): `screen.minimap` would carry no entry
/// for the inactive pane at all, so its strip band would sample as a
/// uniform block (plain editor background) instead of the varied
/// braille/syntax-colour content asserted below — confirmed by hand by
/// reverting `build_screen_layout`'s minimap map to
/// `.find(|(id, _)| *id == active_window_id)` before restoring the fix.
#[test]
fn split_paints_two_independent_minimap_strips() {
let mut h = harness(engine_with_split_shaped_buffer(), 1400, 900);

let win_ids: Vec<_> = h.engine.borrow().windows.keys().copied().collect();
assert_eq!(win_ids.len(), 2, "`:vsplit` must produce two windows");

// Paint a first frame — also primes `screen_layout`/`painted_line_height`.
for id in &win_ids {
h.window_center(*id)
.unwrap_or_else(|| panic!("pane {id:?} must paint"));
}
let lh = h
.painted_line_height()
.expect("frame must publish the line height it painted with");

for id in &win_ids {
let strip = {
let layout = h.screen_layout.borrow();
layout
.as_ref()
.unwrap()
.minimap
.iter()
.find(|m| m.window_id == *id)
.unwrap_or_else(|| panic!("pane {id:?} must carry its own minimap"))
.rect
};
let x0 = (strip.x + 2.0) as i32;
let x1 = (strip.x + strip.width - 2.0) as i32;
let y0 = (strip.y + lh) as i32;
let y1 = (strip.y + strip.height - lh) as i32;
let mut seen = std::collections::HashSet::new();
for y in (y0..y1).step_by(3) {
for x in x0..x1 {
seen.insert(h.driver.pixel(x, y));
}
}
assert!(
seen.len() > 1,
"pane {id:?}'s minimap strip must paint content, not a \
uniform block: sampled x in {x0}..{x1}, y in {y0}..{y1}, \
found only {:?}",
seen
);
}
}

/// #722 acceptance, painted-output tier: switching focus between panes
/// of a `:vsplit` must not move either pane's text — GTK twin of TUI's
/// `focus_change_does_not_move_either_panes_text_via_shell_app`.
/// Coverage for the "migrates on focus change, reflowing both panes"
/// symptom the issue called out as *worse* than the missing strip (the
/// width reclaim was gated on the same `is_active` flag as the strip
/// itself, so both panes reflowed on every focus change).
///
/// Mutates focus directly through `Engine::focus_next_window` on the
/// harness's shared `engine: Rc<RefCell<Engine>>` — the same "assert on
/// engine state after an event" escape hatch the harness's own doc
/// comment describes, used here only to *drive* the focus change
/// (GTK's own Ctrl-W accelerator wiring is out of scope for this test)
/// — then forces a real repaint (`h.driver.render()`, the same pattern
/// `menu_bar_visible`'s test elsewhere in this file uses) and diffs the
/// two *painted* window rects, which is the acceptance claim under
/// test.
///
/// RED against the pre-#722 code: focusing the right pane would widen
/// it (reclaiming the now-inactive left pane's minimap width) and
/// narrow the left pane by the same amount, moving both panes' painted
/// rects — confirmed by hand by reverting the `minimap_w` reclaim to
/// the old `is_active`-gated single value before restoring this fix.
#[test]
fn focus_change_does_not_move_either_panes_text() {
let mut h = harness(engine_with_split_shaped_buffer(), 1400, 900);

let win_ids: Vec<_> = h.engine.borrow().windows.keys().copied().collect();
assert_eq!(win_ids.len(), 2, "`:vsplit` must produce two windows");
for id in &win_ids {
h.window_center(*id)
.unwrap_or_else(|| panic!("pane {id:?} must paint"));
}

fn painted_rects(
h: &Harness<impl AppLogic>,
win_ids: &[crate::core::WindowId],
) -> Vec<(f64, f64, f64, f64)> {
let layout = h.screen_layout.borrow();
let l = layout.as_ref().unwrap();
win_ids
.iter()
.map(|id| {
let r = l.windows.iter().find(|w| w.window_id == *id).unwrap().rect;
(r.x, r.y, r.width, r.height)
})
.collect()
}

let before = painted_rects(&h, &win_ids);

let active_before = h.engine.borrow().active_window_id();
h.engine.borrow_mut().focus_next_window();
let active_after = h.engine.borrow().active_window_id();
assert_ne!(
active_before, active_after,
"test setup sanity: focus_next_window must actually move focus \
to the other pane, or this test isn't exercising a focus \
change at all"
);
h.driver.render();

let after = painted_rects(&h, &win_ids);

assert_eq!(
before, after,
"cycling focus between panes of a `:vsplit` must not move \
either pane's painted rect (i.e. must not reflow either \
pane's text width); before={before:?}, after={after:?}"
);
}
}

/// Black-box coverage for the VimCode app icon painted left of the `File`
Expand Down
Loading
Loading