diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 9f5489cd..db771584 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -15,6 +15,7 @@ - Bumped the client/server protocol version to 15 for socket API placement mutation event and response compatibility. ### Fixed +- Plain PageUp/PageDown now reach primary-screen pager apps such as `less -X` and Git diff when they enter application cursor mode, while shell transcripts still use Herdr pane scrollback. (#953) - `prefix+e` scrollback editor panes now open on Windows without trying to run `/bin/sh`; Windows uses `VISUAL`, then `EDITOR`, then `notepad.exe` as the fallback editor. (#914) - `herdr pane split --current` now resolves to the calling Herdr pane instead of the UI-focused pane when run inside a pane. (#902) - Native Windows clients running inside Alacritty now preserve mouse reports and `ctrl+j` input instead of leaking mouse escape sequences into panes. `shift+enter` remains dependent on whether the outer terminal reports it as a distinct modified Enter key. (#792) diff --git a/src/app/input/terminal.rs b/src/app/input/terminal.rs index b7a23322..282fa263 100644 --- a/src/app/input/terminal.rs +++ b/src/app/input/terminal.rs @@ -88,17 +88,19 @@ impl App { self.state .runtime_for_pane_in_workspace(&self.terminal_runtimes, ws_idx, pane_id)?; - // Intercept plain PageUp/PageDown presses for pane scrollback when the - // focused pane doesn't handle its own scrolling (e.g., a plain shell - // with mouse off). Modified page keys are pane shortcuts, and release - // events should not produce a second host-scroll action. + // Intercept plain PageUp/PageDown presses for pane scrollback only + // when the focused pane looks like a shell transcript. Normal-screen + // pagers such as `less -X` keep the primary screen but enter + // application cursor mode while they own special keys. + // Modified page keys are pane shortcuts, and release events should not + // produce a second host-scroll action. // Only intercept when we know the pane state; if input_state is unknown, // fail-open and forward the key to the pane. if matches!(key_event.code, KeyCode::PageUp | KeyCode::PageDown) && key_event.modifiers.is_empty() { if let Some(input_state) = rt.input_state() { - if !input_state.alternate_screen && !input_state.mouse_reporting_enabled() { + if input_state.plain_page_keys_use_host_scrollback() { if key_event.kind == crossterm::event::KeyEventKind::Release { return None; } @@ -1191,4 +1193,48 @@ mod tests { // Forwarded to pane, so test runtime doesn't process it — scroll stays at bottom. assert_eq!(end_metrics.offset_from_bottom, 0); } + + #[tokio::test] + async fn page_up_forwarded_to_primary_screen_application_cursor_pane() { + let mut app = app_for_mouse_test(); + let mut ws = Workspace::test_new("test"); + let pane_id = ws.tabs[0].root_pane; + let pane_infos = ws.tabs[0].layout.panes(Rect::new(26, 2, 80, 18)); + let info = pane_infos[0].clone(); + let mut bytes = b"\x1b[?1h".to_vec(); + bytes.extend_from_slice(&numbered_lines_bytes(64)); + let (runtime, mut input_rx) = + crate::terminal::TerminalRuntime::test_with_channel_and_scrollback_bytes( + info.inner_rect.width, + info.inner_rect.height, + 16 * 1024, + &bytes, + 4, + ); + ws.tabs[0].runtimes.insert(pane_id, runtime); + + app.state.workspaces = vec![ws]; + app.state.active = Some(0); + app.state.selected = 0; + app.state.mode = Mode::Terminal; + app.state.view.pane_infos = pane_infos; + + let start_metrics = app + .state + .runtime_for_pane_in_workspace(&app.terminal_runtimes, 0, pane_id) + .and_then(crate::terminal::TerminalRuntime::scroll_metrics) + .expect("initial scroll metrics"); + assert_eq!(start_metrics.offset_from_bottom, 0); + + app.handle_terminal_key_headless(TerminalKey::new(KeyCode::PageUp, KeyModifiers::empty())); + + let forwarded = input_rx.try_recv().expect("forwarded PageUp"); + assert_eq!(forwarded.as_ref(), b"\x1b[5~"); + let end_metrics = app + .state + .runtime_for_pane_in_workspace(&app.terminal_runtimes, 0, pane_id) + .and_then(crate::terminal::TerminalRuntime::scroll_metrics) + .expect("scroll metrics after PageUp"); + assert_eq!(end_metrics.offset_from_bottom, 0); + } } diff --git a/src/pane/terminal.rs b/src/pane/terminal.rs index 79b4a19e..4876121f 100644 --- a/src/pane/terminal.rs +++ b/src/pane/terminal.rs @@ -97,6 +97,10 @@ impl InputState { pub fn mouse_reporting_enabled(self) -> bool { self.mouse_protocol_mode.reporting_enabled() } + + pub fn plain_page_keys_use_host_scrollback(self) -> bool { + !self.alternate_screen && !self.mouse_reporting_enabled() && !self.application_cursor + } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/src/server/headless.rs b/src/server/headless.rs index d0c90f6c..9ef8d1e3 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -247,9 +247,9 @@ fn apply_terminal_attach_scroll( AttachScrollDirection::Down => MouseEventKind::ScrollDown, }; if let AttachScrollSource::PageKey { input } = source { - let host_scroll = runtime.input_state().is_some_and(|input_state| { - !input_state.alternate_screen && !input_state.mouse_reporting_enabled() - }); + let host_scroll = runtime + .input_state() + .is_some_and(crate::pane::InputState::plain_page_keys_use_host_scrollback); if host_scroll { match direction { AttachScrollDirection::Up => runtime.scroll_up(lines.max(1) as usize), @@ -5275,14 +5275,17 @@ next_tab = "" rt.shutdown_timeout(Duration::from_millis(100)); } - #[test] - fn terminal_attach_page_key_host_scrolls_plain_terminal() { + fn with_terminal_attach_page_key_runtime( + initial_bytes: &[u8], + initial_scroll: usize, + test: impl FnOnce(&crate::terminal::TerminalRuntime, &mut mpsc::Receiver), + ) { let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() .expect("test runtime"); let _runtime_guard = rt.enter(); - let mut bytes = Vec::new(); + let mut bytes = initial_bytes.to_vec(); for line in 0..80 { bytes.extend_from_slice(format!("line {line:02}\r\n").as_bytes()); } @@ -5290,9 +5293,20 @@ next_tab = "" crate::terminal::TerminalRuntime::test_with_channel_and_scrollback_bytes( 20, 5, 4096, &bytes, 4, ); + if initial_scroll > 0 { + runtime.scroll_up(initial_scroll); + } + test(&runtime, &mut input_rx); + + drop(runtime); + drop(_runtime_guard); + rt.shutdown_timeout(Duration::from_millis(100)); + } + + fn apply_terminal_attach_page_up(runtime: &crate::terminal::TerminalRuntime) { apply_terminal_attach_scroll( - &runtime, + runtime, AttachScrollSource::PageKey { input: b"\x1b[5~".to_vec(), }, @@ -5302,111 +5316,80 @@ next_tab = "" None, 0, ) - .expect("page key scroll"); + .expect("page key"); + } - assert_eq!( - runtime - .scroll_metrics() - .expect("scroll metrics") - .offset_from_bottom, - 4 - ); - assert!(input_rx.try_recv().is_err()); - drop(runtime); - drop(_runtime_guard); - rt.shutdown_timeout(Duration::from_millis(100)); + #[test] + fn terminal_attach_page_key_host_scrolls_plain_terminal() { + with_terminal_attach_page_key_runtime(b"", 0, |runtime, input_rx| { + apply_terminal_attach_page_up(runtime); + + assert_eq!( + runtime + .scroll_metrics() + .expect("scroll metrics") + .offset_from_bottom, + 4 + ); + assert!(input_rx.try_recv().is_err()); + }); } #[test] fn terminal_attach_page_key_forwards_when_mouse_reporting() { - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("test runtime"); - let _runtime_guard = rt.enter(); - let mut bytes = b"\x1b[?1000h".to_vec(); - for line in 0..80 { - bytes.extend_from_slice(format!("line {line:02}\r\n").as_bytes()); - } - let (runtime, mut input_rx) = - crate::terminal::TerminalRuntime::test_with_channel_and_scrollback_bytes( - 20, 5, 4096, &bytes, 4, + with_terminal_attach_page_key_runtime(b"\x1b[?1000h", 3, |runtime, input_rx| { + apply_terminal_attach_page_up(runtime); + + assert_eq!( + runtime + .scroll_metrics() + .expect("scroll metrics") + .offset_from_bottom, + 0 ); - runtime.scroll_up(3); + assert_eq!( + input_rx.try_recv().expect("forwarded page key"), + Bytes::from_static(b"\x1b[5~") + ); + }); + } - apply_terminal_attach_scroll( - &runtime, - AttachScrollSource::PageKey { - input: b"\x1b[5~".to_vec(), - }, - AttachScrollDirection::Up, - 4, - None, - None, - 0, - ) - .expect("page key forward"); + #[test] + fn terminal_attach_page_key_forwards_when_application_cursor() { + with_terminal_attach_page_key_runtime(b"\x1b[?1h", 3, |runtime, input_rx| { + apply_terminal_attach_page_up(runtime); - assert_eq!( - runtime - .scroll_metrics() - .expect("scroll metrics") - .offset_from_bottom, - 0 - ); - assert_eq!( - input_rx.try_recv().expect("forwarded page key"), - Bytes::from_static(b"\x1b[5~") - ); - drop(runtime); - drop(_runtime_guard); - rt.shutdown_timeout(Duration::from_millis(100)); + assert_eq!( + runtime + .scroll_metrics() + .expect("scroll metrics") + .offset_from_bottom, + 0 + ); + assert_eq!( + input_rx.try_recv().expect("forwarded page key"), + Bytes::from_static(b"\x1b[5~") + ); + }); } #[test] fn terminal_attach_page_key_forwards_in_alternate_screen_without_mouse_reporting() { - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("test runtime"); - let _runtime_guard = rt.enter(); - let mut bytes = b"\x1b[?1049h".to_vec(); - for line in 0..80 { - bytes.extend_from_slice(format!("line {line:02}\r\n").as_bytes()); - } - let (runtime, mut input_rx) = - crate::terminal::TerminalRuntime::test_with_channel_and_scrollback_bytes( - 20, 5, 4096, &bytes, 4, + with_terminal_attach_page_key_runtime(b"\x1b[?1049h", 3, |runtime, input_rx| { + apply_terminal_attach_page_up(runtime); + + assert_eq!( + runtime + .scroll_metrics() + .expect("scroll metrics") + .offset_from_bottom, + 0 ); - runtime.scroll_up(3); - - apply_terminal_attach_scroll( - &runtime, - AttachScrollSource::PageKey { - input: b"\x1b[5~".to_vec(), - }, - AttachScrollDirection::Up, - 4, - None, - None, - 0, - ) - .expect("page key forward"); - - assert_eq!( - runtime - .scroll_metrics() - .expect("scroll metrics") - .offset_from_bottom, - 0 - ); - assert_eq!( - input_rx.try_recv().expect("forwarded page key"), - Bytes::from_static(b"\x1b[5~") - ); - drop(runtime); - drop(_runtime_guard); - rt.shutdown_timeout(Duration::from_millis(100)); + assert_eq!( + input_rx.try_recv().expect("forwarded page key"), + Bytes::from_static(b"\x1b[5~") + ); + }); } #[test]