diff --git a/crates/phux-client-core/src/multi_pane/mod.rs b/crates/phux-client-core/src/multi_pane/mod.rs index 235d88ff..0d28f4d0 100644 --- a/crates/phux-client-core/src/multi_pane/mod.rs +++ b/crates/phux-client-core/src/multi_pane/mod.rs @@ -391,6 +391,17 @@ mod tests { } } + /// A content rect spanning the whole viewport — the no-chrome case where + /// the hit-test layout equals the full viewport. + fn full_content(cols: u16, rows: u16) -> Rect { + Rect { + x: 0, + y: 0, + w: cols, + h: rows, + } + } + /// Clicking inside the focused pane forwards with focus unchanged /// and emits pane-local coordinates relative to the pane's `Rect`. #[test] @@ -401,7 +412,7 @@ mod tests { focus: Some(t(1)), }; // Click inside the left half (focused) at col 5, row 3. - let decision = route_mouse_event(&state, (80, 24), &mouse_at(5, 3)); + let decision = route_mouse_event(&state, full_content(80, 24), (80, 24), &mouse_at(5, 3)); match decision { RouteDecision::Pane { target, @@ -433,7 +444,12 @@ mod tests { // Click 2 cells into the right pane, on the second row. let click_x = right_rect.x + 2; let click_y = right_rect.y + 1; - let decision = route_mouse_event(&state, (80, 24), &mouse_at(click_x, click_y)); + let decision = route_mouse_event( + &state, + full_content(80, 24), + (80, 24), + &mouse_at(click_x, click_y), + ); match decision { RouteDecision::Pane { target, @@ -463,7 +479,12 @@ mod tests { // The divider sits at the column equal to the left pane's // width (its `Rect.w`). Any row on that column hits the gap. let left_w = layout.rects.get(&t(1)).copied().unwrap().w; - let decision = route_mouse_event(&state, (80, 24), &mouse_at(left_w, 10)); + let decision = route_mouse_event( + &state, + full_content(80, 24), + (80, 24), + &mouse_at(left_w, 10), + ); assert_eq!(decision, RouteDecision::DividerNoOp); } @@ -472,7 +493,7 @@ mod tests { #[test] fn route_mouse_single_pane_always_hits() { let state = LayoutState::single(t(1)); - let decision = route_mouse_event(&state, (80, 24), &mouse_at(40, 12)); + let decision = route_mouse_event(&state, full_content(80, 24), (80, 24), &mouse_at(40, 12)); match decision { RouteDecision::Pane { target, @@ -517,7 +538,12 @@ mod tests { // Zoom pane t(1): the render layout collapses to a single leaf, so // the same click lands on t(1) instead. let render = workspace.render_window(Some(&t(1))).unwrap(); - let decision = route_mouse_event(&render, (80, 24), &mouse_at(click_x, click_y)); + let decision = route_mouse_event( + &render, + full_content(80, 24), + (80, 24), + &mouse_at(click_x, click_y), + ); match decision { RouteDecision::Pane { target, @@ -536,7 +562,7 @@ mod tests { #[test] fn route_mouse_empty_layout_returns_no_focus() { let state = LayoutState::default(); - let decision = route_mouse_event(&state, (80, 24), &mouse_at(10, 5)); + let decision = route_mouse_event(&state, full_content(80, 24), (80, 24), &mouse_at(10, 5)); assert_eq!(decision, RouteDecision::NoFocus); } @@ -553,6 +579,7 @@ mod tests { }; let decision = route_mouse_event( &state, + full_content(80, 24), (80, 24), &MouseEvent { action: MouseAction::Press, @@ -573,6 +600,71 @@ mod tests { } } + /// Regression: the hit-test must tile into the same inset content rect the + /// renderer paints. With a bottom status bar reserving the last row, the + /// bottom-most viewport row is chrome, not pane. Hit-testing against the + /// full viewport (the bug) routes a click on that row to the bottom pane; + /// hit-testing against the inset content correctly drops it. A click that + /// is genuinely inside a painted pane still routes there. + #[test] + fn route_mouse_respects_status_bar_inset() { + let tree = split_at(&leaf(1), &t(1), &t(2), SplitDir::Vertical, 0.5).unwrap(); + let state = LayoutState { + tree: Some(tree), + focus: Some(t(1)), + }; + let viewport = (80, 24); + // Status bar reserves the last row: panes tile into rows 0..23. + let content = Rect { + x: 0, + y: 0, + w: 80, + h: 23, + }; + let status_row = 23; // the bottom row, owned by the status bar + + // The bug: hit-testing the status-bar row against the FULL viewport + // routes it to the bottom pane, which paints down to row 23 there. + let buggy = route_mouse_event( + &state, + full_content(80, 24), + viewport, + &mouse_at(10, status_row), + ); + assert!( + matches!(&buggy, RouteDecision::Pane { target, .. } if *target == t(2)), + "test premise: full-viewport tiling mis-routes the status-bar row to t(2), got {buggy:?}" + ); + + // The fix: hit-testing against the inset content drops the chrome click. + let fixed = route_mouse_event(&state, content, viewport, &mouse_at(10, status_row)); + assert_eq!( + fixed, + RouteDecision::DividerNoOp, + "a click on the reserved status-bar row must be dropped" + ); + + // A click genuinely inside the painted bottom pane still routes to t(2). + let bottom = compute_layout_in(&state, content, viewport) + .rects + .get(&t(2)) + .copied() + .unwrap(); + match route_mouse_event(&state, content, viewport, &mouse_at(10, bottom.y)) { + RouteDecision::Pane { + target, + pane_y, + focus_changed, + .. + } => { + assert_eq!(target, t(2), "click in painted bottom pane hits t(2)"); + assert!(focus_changed); + assert!((pane_y - 0.0).abs() < f64::EPSILON); + } + other => panic!("expected Pane decision, got {other:?}"), + } + } + // ------------------------------------------------------------------------- // Exact-tiling property test (phux-islu) // ------------------------------------------------------------------------- diff --git a/crates/phux-client-core/src/multi_pane/mouse.rs b/crates/phux-client-core/src/multi_pane/mouse.rs index 8243e2ee..aaa532bf 100644 --- a/crates/phux-client-core/src/multi_pane/mouse.rs +++ b/crates/phux-client-core/src/multi_pane/mouse.rs @@ -4,7 +4,7 @@ use phux_protocol::input::mouse::MouseEvent; use crate::layout::LayoutState; use crate::layout::Rect; -use super::layout::compute_layout; +use super::layout::compute_layout_in; // ----------------------------------------------------------------------------- // route_mouse_event — pure hit-test for INPUT_MOUSE routing (phux-4li.6) @@ -58,7 +58,15 @@ pub enum RouteDecision { /// coordinates per the parser in `attach::input` — the inbound SGR / /// X10 / urxvt-1015 dispatchers all emit `f64` pixels with "1 cell == /// 1 pixel" because the client does not know cell-size at parse time) -/// against the pane `Rect`s yielded by [`compute_layout`]. +/// against the pane `Rect`s yielded by [`compute_layout_in`]. +/// +/// `content` is the inset rectangle the renderer tiles panes into after +/// folding off the status bar row and any sidebar columns — it MUST be the +/// same `content_rect(viewport, has_bar, sidebar)` the paint path uses, or +/// the hit-test disagrees with what is on screen and clicks route to the +/// wrong pane (off by the status-bar row and/or the sidebar width). Clicks +/// that fall in the reserved chrome (status bar row, sidebar columns) miss +/// every inset rect and resolve to [`RouteDecision::DividerNoOp`]. /// /// Behavior matrix: /// @@ -66,15 +74,16 @@ pub enum RouteDecision { /// * Click inside a pane's `Rect` ⇒ [`RouteDecision::Pane`] with /// pane-local coords; `focus_changed = true` iff target ≠ /// `layout.focus`. -/// * Click on a divider cell or outside any pane (degenerate viewport, -/// undersized tree) ⇒ [`RouteDecision::DividerNoOp`]. +/// * Click on a divider cell or outside any pane (reserved chrome, +/// degenerate viewport, undersized tree) ⇒ [`RouteDecision::DividerNoOp`]. /// /// The function is pure (no allocation aside from the internal -/// [`compute_layout`] call's `HashMap`) and synchronous; the driver's +/// [`compute_layout_in`] call's `HashMap`) and synchronous; the driver's /// async input loop calls it once per `InputEvent::Mouse`. #[must_use] pub fn route_mouse_event( layout: &LayoutState, + content: Rect, viewport: (u16, u16), mouse: &MouseEvent, ) -> RouteDecision { @@ -85,10 +94,11 @@ pub fn route_mouse_event( return RouteDecision::NoFocus; } - // Single-pane fast path: skip the HashMap roundtrip. With no - // dividers there is exactly one rect covering the whole viewport - // and every click lands in it. - let multi = compute_layout(layout, viewport); + // Tile into the same inset content rect the renderer paints, so the + // hit-test rects match the on-screen pane rects cell-for-cell. Single + // pane: exactly one rect covering `content`, and every in-content click + // lands in it. + let multi = compute_layout_in(layout, content, viewport); if multi.rects.is_empty() { return RouteDecision::NoFocus; } diff --git a/crates/phux-client/src/attach/driver.rs b/crates/phux-client/src/attach/driver.rs index 6b35a6ee..1f2db30f 100644 --- a/crates/phux-client/src/attach/driver.rs +++ b/crates/phux-client/src/attach/driver.rs @@ -1389,6 +1389,7 @@ async fn main_loop( zoomed: &mut zoomed, sidebar, sidebar_enabled: &mut sidebar_enabled, + has_bar: status_bar.is_some(), }; let layout_changed = dispatch_input_events( out, @@ -1713,6 +1714,7 @@ async fn main_loop( zoomed: &mut zoomed, sidebar, sidebar_enabled: &mut sidebar_enabled, + has_bar: status_bar.is_some(), }; let layout_changed = dispatch_input_events( out, diff --git a/crates/phux-client/src/attach/input.rs b/crates/phux-client/src/attach/input.rs index 99888a6f..602441e8 100644 --- a/crates/phux-client/src/attach/input.rs +++ b/crates/phux-client/src/attach/input.rs @@ -578,14 +578,18 @@ fn c0_or_ascii_to_key(b: u8) -> Option { text: Some(char::from(b).to_string()), unshifted_codepoint: Some(u32::from(ascii_unshifted(b))), }), - // CR / LF → Enter. - 0x0D | 0x0A => Some(make_named_key(PhysicalKey::Enter, ModSet::empty())), + // CR → Enter. LF (0x0A) is Ctrl+J: it falls through to the Ctrl-letter + // arm below so the server encoder reproduces 0x0A rather than CR. In raw + // mode the host TTY sends CR for Return and LF only for Ctrl+J, so this + // split is unambiguous. + 0x0D => Some(make_named_key(PhysicalKey::Enter, ModSet::empty())), // BS / DEL → Backspace. 0x08 | 0x7F => Some(make_named_key(PhysicalKey::Backspace, ModSet::empty())), // HT → Tab. 0x09 => Some(make_named_key(PhysicalKey::Tab, ModSet::empty())), - // Ctrl-A..Ctrl-Z (skipping the dedicated mappings above and ESC). - 0x01..=0x1A if b != 0x08 && b != 0x09 && b != 0x0A && b != 0x0D => { + // Ctrl-A..Ctrl-Z (skipping the dedicated mappings above and ESC). LF + // (0x0A) lands here as Ctrl+J → letter 'J'. + 0x01..=0x1A if b != 0x08 && b != 0x09 && b != 0x0D => { let letter = b'A' + (b - 1); ascii_letter_to_key(letter).map(|key| KeyEvent { action: KeyAction::Press, @@ -1625,6 +1629,19 @@ mod tests { assert!(keys[0].mods.contains(ModSet::CTRL)); } + #[test] + fn lf_byte_becomes_ctrl_j_not_enter() { + // 0x0A (Ctrl+J) must not be laundered into Enter; the server encoder + // re-derives bytes from the KeyEvent, so an Enter event would emit CR + // and swallow the line feed. + let mut p = StdinParser::new(); + let evs = p.feed(&[0x0A]); + let keys = key_only(&evs); + assert_eq!(keys.len(), 1); + assert_eq!(keys[0].key, PhysicalKey::J); + assert_eq!(keys[0].mods, ModSet::CTRL); + } + // ---- UTF-8 ---------------------------------------------------------- #[test] diff --git a/crates/phux-client/src/attach/input_dispatch.rs b/crates/phux-client/src/attach/input_dispatch.rs index 8ae018d7..dd44c934 100644 --- a/crates/phux-client/src/attach/input_dispatch.rs +++ b/crates/phux-client/src/attach/input_dispatch.rs @@ -16,7 +16,7 @@ use phux_protocol::wire::frame::{FrameKind, SESSION_NAME_KEY, Scope}; use super::actions::{self, ActionError, PendingSplit, PendingWindow}; use super::connection::Connection; use super::driver::{AttachError, DEFAULT_GROUP_ID, PaneSlot, layout_key}; -use super::paint::SidebarReservation; +use super::paint::{SidebarReservation, content_rect}; use crate::layout::{Direction, SplitDir, Workspace}; use crate::predict::{Overlay, PredictionState}; use crate::render::Theme; @@ -111,6 +111,11 @@ pub(super) struct DispatchCtx<'a> { /// the per-frame `sidebar` reservation after dispatch so the toggle repaint /// reflects the new state. Owned by the driver like `zoomed`. pub sidebar_enabled: &'a mut bool, + /// Whether the status bar reserves its row this frame (`status_bar.is_some()` + /// in the driver). Mouse routing folds this into the same + /// `content_rect(viewport, has_bar, sidebar)` the paint path uses so a + /// click hit-tests against the rects actually on screen. + pub has_bar: bool, } /// Translate a batch of parser events into wire frames and ship them. @@ -262,14 +267,17 @@ pub(super) async fn dispatch_input_events( // event with pane-local coordinates substituted. if let InputEvent::Mouse(ref mouse) = ev { use super::multi_pane::{RouteDecision, route_mouse_event}; - // phux-4h5a P4 follow-up: mouse click-to-focus on the sidebar - // strip. This hit-test routes against the pane layout over the full - // viewport; when the sidebar is enabled it does not yet recognize a - // click in the strip's reserved columns as "focus window N" (the - // companion to the deferred `focus-window`-by-index action below). + // Hit-test against the SAME inset content rect the renderer tiles + // into — status-bar row and sidebar columns folded off the outer + // viewport. Routing against the full viewport instead disagrees with + // what is painted: a click near a divider lands one row off (the + // status bar) and, with a sidebar docked, one strip-width off in x, + // so it focuses/forwards to the wrong pane. Clicks in the reserved + // chrome miss every pane rect and become a DividerNoOp (dropped). + let content = content_rect(ctx.viewport, ctx.has_bar, ctx.sidebar); // phux-jow6: hit-test against the RENDER layout, not the real // tiled tree. When a pane is zoomed (phux-x2hm) the render layout - // is a single full-viewport leaf, so any click lands on the + // is a single full-content leaf, so any click lands on the // visible zoomed pane instead of whichever hidden tiled pane sits // under the cursor. Compute the decision in a scope that drops the // borrowing `Cow` before the click-to-focus `active_window_mut()` @@ -279,7 +287,7 @@ pub(super) async fn dispatch_input_events( tracing::debug!("dropping mouse event: no active window"); continue; }; - route_mouse_event(&render_ls, ctx.viewport, mouse) + route_mouse_event(&render_ls, content, ctx.viewport, mouse) }; match decision { RouteDecision::Pane { @@ -1476,6 +1484,7 @@ mod tests { zoomed: &mut zoomed, sidebar: None, sidebar_enabled: &mut sidebar_enabled, + has_bar: false, }; let focused = ctx.workspace.active_window().and_then(|w| w.focus.clone()); run_action(action, &mut ctx, focused.as_ref()) @@ -1672,6 +1681,7 @@ mod tests { zoomed: &mut zoomed, sidebar: None, sidebar_enabled: &mut sidebar_enabled, + has_bar: false, }; let effects = run_action(&bare_action("toggle-sidebar"), &mut ctx, None); let mut out: Vec = Vec::new(); @@ -1714,6 +1724,7 @@ mod tests { zoomed: &mut zoomed, sidebar: None, sidebar_enabled: &mut sidebar_enabled, + has_bar: false, }; let effects = run_action(&bare_action("toggle-sidebar"), &mut ctx, None); apply_action_effects( @@ -1788,6 +1799,7 @@ mod tests { zoomed: &mut zoomed, sidebar: None, sidebar_enabled: &mut sidebar_enabled, + has_bar: false, }; let focused = ctx.workspace.active_window().and_then(|w| w.focus.clone()); run_action(action, &mut ctx, focused.as_ref()) @@ -2057,6 +2069,7 @@ mod tests { zoomed: &mut zoomed, sidebar: None, sidebar_enabled: &mut sidebar_enabled, + has_bar: false, }; let action = phux_config::keybind::ResolvedAction { action: "detach".to_owned(), @@ -2122,6 +2135,7 @@ mod tests { zoomed: &mut zoomed, sidebar: None, sidebar_enabled: &mut sidebar_enabled, + has_bar: false, }; run_action(&bare_action("rename-session"), &mut ctx, None) }; @@ -2266,6 +2280,7 @@ mod tests { zoomed: &mut zoomed, sidebar: None, sidebar_enabled: &mut sidebar_enabled, + has_bar: false, }; dispatch_input_events( &mut out, diff --git a/crates/phux-server/src/input/key.rs b/crates/phux-server/src/input/key.rs index bd9e67d8..8ff5a0dd 100644 --- a/crates/phux-server/src/input/key.rs +++ b/crates/phux-server/src/input/key.rs @@ -138,6 +138,25 @@ mod tests { ); } + #[test] + fn encodes_ctrl_j_to_line_feed() { + // Ctrl+J must reach the PTY as LF (0x0A), distinct from Enter's CR. The + // client parser produces this event from a 0x0A input byte. + let terminal = make_terminal(); + let mut enc = PerTerminalKeyEncoder::new().expect("encoder"); + let ev = KeyEvent { + action: KeyAction::Press, + key: PhysicalKey::J, + mods: ModSet::CTRL, + consumed_mods: ModSet::CTRL, + composing: false, + text: None, + unshifted_codepoint: Some(u32::from('j')), + }; + let bytes = enc.encode(&ev, &terminal).expect("encode"); + assert_eq!(bytes, b"\n", "Ctrl+J must encode to LF, got {bytes:?}"); + } + /// ADR-0024: the wire atoms are phux-owned but share libghostty's /// discriminants; the `server`-gated conversions are lossless for known /// values, keeping the two in lockstep.