From fb56e64537de5f93786ed121f703da2a4105badc Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Mon, 8 Jun 2026 01:36:51 +0300 Subject: [PATCH] fix: improve windows pane behavior --- AGENTS.md | 1 + src/app/state.rs | 47 +++--- src/client/mod.rs | 56 ++++++- src/config/io.rs | 38 +++-- src/pane.rs | 40 ++--- src/pane/terminal.rs | 16 ++ src/platform/windows.rs | 124 +++++++++++++++- src/protocol/render_ansi.rs | 284 +++++++++++++++++++++++++++++++++--- src/server/headless.rs | 64 ++++++++ src/server/render_stream.rs | 87 ++++++++++- src/sound.rs | 90 ++++++++++-- src/terminal/runtime.rs | 4 + 12 files changed, 743 insertions(+), 108 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c83a1d6d..7658a051 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,6 +90,7 @@ Do not use GitHub closing keywords like `fixes #`, `closes # bool { - let has_effective_agent_label = self - .workspaces - .get(ws_idx) - .and_then(|ws| ws.terminal_id(pane_id)) - .and_then(|terminal_id| self.terminals.get(terminal_id)) - .and_then(crate::terminal::TerminalState::effective_agent_label) - .is_some(); - - pane_exposes_host_cursor_for_target(has_effective_agent_label, cfg!(windows)) + true } pub(crate) fn integration_updates_available(&self) -> bool { @@ -1750,28 +1742,31 @@ impl AppState { } } -fn pane_exposes_host_cursor_for_target( - has_effective_agent_label: bool, - target_is_windows: bool, -) -> bool { - !target_is_windows || !has_effective_agent_label -} - #[cfg(test)] mod tests { use super::*; use crossterm::event::KeyEvent; #[test] - fn windows_agent_panes_do_not_expose_host_cursor() { - assert!(!pane_exposes_host_cursor_for_target(true, true)); - assert!(pane_exposes_host_cursor_for_target(false, true)); - } + fn agent_terminal_keeps_final_child_cursor_exposed() { + let mut state = AppState::test_new(); + let ws = crate::workspace::Workspace::test_new("test"); + let pane_id = ws.tabs[0].root_pane; + state.terminals.insert( + ws.tabs[0].panes[&pane_id].attached_terminal_id.clone(), + crate::terminal::TerminalState::new( + ws.tabs[0].panes[&pane_id].attached_terminal_id.clone(), + std::path::PathBuf::from("/tmp"), + ), + ); + state + .terminals + .get_mut(&ws.tabs[0].panes[&pane_id].attached_terminal_id) + .expect("terminal state") + .launch_argv = Some(vec!["codex".to_string()]); + state.workspaces = vec![ws]; - #[test] - fn non_windows_agent_panes_keep_existing_host_cursor_behavior() { - assert!(pane_exposes_host_cursor_for_target(true, false)); - assert!(pane_exposes_host_cursor_for_target(false, false)); + assert!(state.pane_exposes_host_cursor(0, pane_id)); } #[test] diff --git a/src/client/mod.rs b/src/client/mod.rs index 8236bb44..1e46f190 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -19,6 +19,8 @@ use std::io::{self, Write as _}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, OnceLock}; use std::time::Duration; +#[cfg(windows)] +use std::time::Instant; use crossterm::event::{ DisableBracketedPaste, DisableFocusChange, DisableMouseCapture, EnableBracketedPaste, @@ -68,12 +70,21 @@ struct ClientState { mouse_scroll_lines: usize, /// Whether outer focus gain should force a full host-terminal redraw. redraw_on_focus_gained: bool, + #[cfg(windows)] + pending_cursor_reveal: Option, } #[derive(Debug, Default)] #[cfg(windows)] struct AttachEscapeState; +#[derive(Debug)] +#[cfg(windows)] +struct PendingCursorReveal { + due_at: Instant, + bytes: Vec, +} + #[derive(Debug, Default)] #[cfg(unix)] struct AttachEscapeState { @@ -201,6 +212,37 @@ fn attach_scroll_action( impl ClientState { fn request_full_redraw(&mut self) { self.blit_encoder = render_ansi::BlitEncoder::new(); + #[cfg(windows)] + { + self.pending_cursor_reveal = None; + } + } + + #[cfg(windows)] + fn update_pending_cursor_reveal(&mut self, reveal: Option>) { + const CURSOR_REVEAL_DEBOUNCE: Duration = Duration::from_millis(90); + self.pending_cursor_reveal = reveal.map(|bytes| PendingCursorReveal { + due_at: Instant::now() + CURSOR_REVEAL_DEBOUNCE, + bytes, + }); + } + + #[cfg(windows)] + fn flush_pending_cursor_reveal_if_due(&mut self) { + let Some(pending) = &self.pending_cursor_reveal else { + return; + }; + if pending.due_at > Instant::now() { + return; + } + + let pending = self + .pending_cursor_reveal + .take() + .expect("pending cursor reveal"); + let mut stdout = io::stdout(); + let _ = stdout.write_all(&pending.bytes); + let _ = stdout.flush(); } } @@ -771,6 +813,8 @@ async fn run_client_loop( #[cfg(unix)] mouse_scroll_lines, redraw_on_focus_gained, + #[cfg(windows)] + pending_cursor_reveal: None, }; debug!(?negotiated_encoding, "client render encoding active"); @@ -942,7 +986,14 @@ async fn run_client_loop( } ClientLoopEvent::ServerMessage(msg) => match msg { ServerMessage::Frame(frame_data) => { + #[cfg(windows)] + let encoded = state + .blit_encoder + .encode_with_deferred_cursor_reveal(&frame_data, false); + #[cfg(not(windows))] let encoded = state.blit_encoder.encode(&frame_data, false); + #[cfg(windows)] + let deferred_cursor_reveal = encoded.deferred_cursor_reveal.clone(); let mut stdout = io::stdout(); let graphics = if state.kitty_graphics_enabled { frame_data.graphics.as_slice() @@ -953,6 +1004,8 @@ async fn run_client_loop( write_encoded_frame_with_graphics(&mut stdout, &encoded.bytes, graphics); let _ = stdout.flush(); state.blit_encoder.commit(frame_data, encoded); + #[cfg(windows)] + state.update_pending_cursor_reveal(deferred_cursor_reveal); } ServerMessage::Terminal(frame) => { if state.kitty_graphics_enabled && contains_kitty_graphics_bytes(&frame.bytes) { @@ -1008,7 +1061,8 @@ async fn run_client_loop( ))); } ClientLoopEvent::Timer => { - // Check if we should quit. + #[cfg(windows)] + state.flush_pending_cursor_reveal_if_due(); } } } diff --git a/src/config/io.rs b/src/config/io.rs index 1bfabf96..984f25e0 100644 --- a/src/config/io.rs +++ b/src/config/io.rs @@ -14,30 +14,20 @@ pub fn app_dir_name() -> &'static str { pub fn config_dir() -> PathBuf { if let Ok(dir) = std::env::var("XDG_CONFIG_HOME") { - PathBuf::from(dir).join(app_dir_name()) - } else if cfg!(windows) { - windows_config_dir() - } else if let Ok(home) = std::env::var("HOME") { - PathBuf::from(home).join(format!(".config/{}", app_dir_name())) - } else { - std::env::temp_dir().join(app_dir_name()) + return PathBuf::from(dir).join(app_dir_name()); } + platform_config_dir() } pub fn state_dir() -> PathBuf { if let Ok(dir) = std::env::var("XDG_STATE_HOME") { - PathBuf::from(dir).join(app_dir_name()) - } else if cfg!(windows) { - windows_state_dir() - } else if let Ok(home) = std::env::var("HOME") { - PathBuf::from(home).join(format!(".local/state/{}", app_dir_name())) - } else { - std::env::temp_dir().join(format!("{}-state", app_dir_name())) + return PathBuf::from(dir).join(app_dir_name()); } + platform_state_dir() } #[cfg(windows)] -fn windows_config_dir() -> PathBuf { +fn platform_config_dir() -> PathBuf { if let Ok(dir) = std::env::var("APPDATA") { return PathBuf::from(dir).join(app_dir_name()); } @@ -54,12 +44,16 @@ fn windows_config_dir() -> PathBuf { } #[cfg(not(windows))] -fn windows_config_dir() -> PathBuf { - unreachable!("windows_config_dir is only called on Windows") +fn platform_config_dir() -> PathBuf { + if let Ok(home) = std::env::var("HOME") { + PathBuf::from(home).join(format!(".config/{}", app_dir_name())) + } else { + std::env::temp_dir().join(app_dir_name()) + } } #[cfg(windows)] -fn windows_state_dir() -> PathBuf { +fn platform_state_dir() -> PathBuf { if let Ok(dir) = std::env::var("LOCALAPPDATA") { return PathBuf::from(dir).join(app_dir_name()); } @@ -76,8 +70,12 @@ fn windows_state_dir() -> PathBuf { } #[cfg(not(windows))] -fn windows_state_dir() -> PathBuf { - unreachable!("windows_state_dir is only called on Windows") +fn platform_state_dir() -> PathBuf { + if let Ok(home) = std::env::var("HOME") { + PathBuf::from(home).join(format!(".local/state/{}", app_dir_name())) + } else { + std::env::temp_dir().join(format!("{}-state", app_dir_name())) + } } impl Config { diff --git a/src/pane.rs b/src/pane.rs index 32c8c181..b7a1f6c8 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -1047,6 +1047,7 @@ fn pane_shell_command_builder(shell_config: PaneShellConfig<'_>) -> io::Result bool { - #[cfg(windows)] - { - let name = Path::new(shell) - .file_name() - .and_then(std::ffi::OsStr::to_str) - .unwrap_or(shell) - .to_ascii_lowercase(); - matches!( - name.as_str(), - "powershell" | "powershell.exe" | "pwsh" | "pwsh.exe" - ) - } - #[cfg(not(windows))] - { - let _ = shell; - false - } +#[cfg(not(windows))] +fn apply_windows_powershell_cwd_reporting(cmd: &mut CommandBuilder, shell: &str) { + let _ = (cmd, shell); } +#[cfg(windows)] +fn is_windows_powershell_shell(shell: &str) -> bool { + let name = Path::new(shell) + .file_name() + .and_then(std::ffi::OsStr::to_str) + .unwrap_or(shell) + .to_ascii_lowercase(); + matches!( + name.as_str(), + "powershell" | "powershell.exe" | "pwsh" | "pwsh.exe" + ) +} + +#[cfg(windows)] fn windows_powershell_cwd_prompt_wrapper() -> &'static str { r#"$global:__HERDR_ORIGINAL_PROMPT = if (Test-Path Function:\prompt) { (Get-Command prompt -CommandType Function).ScriptBlock } else { { "PS $($executionContext.SessionState.Path.CurrentLocation)$('>' * ($nestedPromptLevel + 1)) " } }; function global:prompt { try { if ($PWD.Provider.Name -eq 'FileSystem') { $uri = ([System.Uri]$PWD.ProviderPath).AbsoluteUri; [Console]::Write("$([char]27)]7;$uri$([char]7)") } } catch {}; & $global:__HERDR_ORIGINAL_PROMPT }"# } @@ -2004,6 +2004,10 @@ impl PaneRuntime { }) } + pub fn synchronized_output_active(&self) -> bool { + self.terminal.synchronized_output_active() + } + pub fn visible_text(&self) -> String { self.terminal.visible_text() } diff --git a/src/pane/terminal.rs b/src/pane/terminal.rs index 5f44854d..fa281553 100644 --- a/src/pane/terminal.rs +++ b/src/pane/terminal.rs @@ -187,6 +187,10 @@ impl PaneTerminal { self.ghostty.cursor_state() } + pub fn synchronized_output_active(&self) -> bool { + self.ghostty.synchronized_output_active() + } + pub fn visible_text(&self) -> String { self.ghostty.visible_text() } @@ -889,6 +893,18 @@ impl GhosttyPaneTerminal { }) } + pub fn synchronized_output_active(&self) -> bool { + self.core + .lock() + .ok() + .and_then(|core| { + core.terminal + .mode_get(crate::ghostty::MODE_SYNCHRONIZED_OUTPUT) + .ok() + }) + .unwrap_or(false) + } + pub fn encode_terminal_key( &self, key: crate::input::TerminalKey, diff --git a/src/platform/windows.rs b/src/platform/windows.rs index 269b3ab2..d92095a1 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -84,23 +84,75 @@ fn select_pane_foreground_job( let descendants = descendant_entries(shell_pid, entries); let mut candidates = Vec::new(); - for entry in descendants { + for entry in &descendants { let process = foreground_process_from_entry(entry); let job = ForegroundJob { process_group_id: entry.pid, processes: vec![process], }; - if crate::detect::identify_agent_in_job(&job).is_some() { - candidates.push(job); + if let Some((agent, _)) = crate::detect::identify_agent_in_job(&job) { + candidates.push((*entry, agent)); } } match candidates.len() { - 1 => candidates.pop(), - _ => Some(shell_job()), + 1 => candidates + .pop() + .map(|(entry, _)| foreground_job_from_entry(entry)), + _ => select_single_agent_chain_candidate(&candidates, entries).map_or_else( + || Some(shell_job()), + |entry| Some(foreground_job_from_entry(entry)), + ), } } +fn foreground_job_from_entry(entry: &WindowsProcessEntry) -> ForegroundJob { + ForegroundJob { + process_group_id: entry.pid, + processes: vec![foreground_process_from_entry(entry)], + } +} + +fn select_single_agent_chain_candidate<'a>( + candidates: &[(&'a WindowsProcessEntry, crate::detect::Agent)], + entries: &[WindowsProcessEntry], +) -> Option<&'a WindowsProcessEntry> { + let (_, first_agent) = candidates.first()?; + if !candidates.iter().all(|(_, agent)| agent == first_agent) { + return None; + } + + let parent_by_pid: HashMap = entries + .iter() + .map(|entry| (entry.pid, entry.parent_pid)) + .collect(); + + candidates.iter().map(|(entry, _)| *entry).find(|entry| { + candidates.iter().all(|(other, _)| { + entry.pid == other.pid || process_is_ancestor(entry.pid, other.pid, &parent_by_pid) + }) + }) +} + +fn process_is_ancestor( + ancestor_pid: u32, + descendant_pid: u32, + parent_by_pid: &HashMap, +) -> bool { + let mut current = descendant_pid; + while let Some(parent) = parent_by_pid.get(¤t).copied() { + if parent == ancestor_pid { + return true; + } + if parent == 0 || parent == current { + return false; + } + current = parent; + } + + false +} + fn descendant_entries(root_pid: u32, entries: &[WindowsProcessEntry]) -> Vec<&WindowsProcessEntry> { let mut children: HashMap> = HashMap::new(); for entry in entries { @@ -532,6 +584,68 @@ mod tests { assert_eq!(job.processes[0].name, "cmd.exe"); } + #[test] + fn windows_process_tree_selects_topmost_codex_process_in_single_agent_chain() { + let entries = vec![ + test_entry(10, 1, "powershell.exe", &["powershell.exe"]), + test_entry( + 20, + 10, + "node.exe", + &[ + "node.exe", + "C:\\Users\\herdr\\AppData\\Roaming\\npm\\node_modules\\@openai\\codex\\bin\\codex.js", + ], + ), + test_entry( + 30, + 20, + "codex.exe", + &["C:\\Users\\herdr\\AppData\\Roaming\\npm\\node_modules\\@openai\\codex\\node_modules\\@openai\\codex-win32-x64\\vendor\\x86_64-pc-windows-msvc\\bin\\codex.exe"], + ), + test_entry(40, 30, "node_repl.exe", &["node_repl.exe"]), + test_entry( + 50, + 40, + "codex.exe", + &["codex.exe", "app-server", "--listen", "stdio://"], + ), + ]; + + let job = super::select_pane_foreground_job(10, &entries).unwrap(); + + assert_eq!(job.process_group_id, 20); + assert_eq!(job.processes[0].name, "node.exe"); + } + + #[test] + fn windows_process_tree_selects_topmost_claude_process_in_single_agent_chain() { + let entries = vec![ + test_entry(10, 1, "powershell.exe", &["powershell.exe"]), + test_entry(20, 10, "claude.exe", &["claude.exe"]), + test_entry(30, 20, "claude.exe", &["claude.exe", "mcp-server"]), + ]; + + let job = super::select_pane_foreground_job(10, &entries).unwrap(); + + assert_eq!(job.process_group_id, 20); + assert_eq!(job.processes[0].name, "claude.exe"); + } + + #[test] + fn windows_process_tree_returns_shell_for_same_agent_siblings() { + let entries = vec![ + test_entry(10, 1, "powershell.exe", &["powershell.exe"]), + test_entry(20, 10, "codex.exe", &["codex.exe"]), + test_entry(30, 10, "codex.exe", &["codex.exe"]), + ]; + + let job = super::select_pane_foreground_job(10, &entries).unwrap(); + + assert_eq!(job.process_group_id, 10); + assert_eq!(job.processes[0].name, "powershell.exe"); + } + #[test] fn windows_process_tree_returns_shell_for_plain_descendant() { let entries = vec![ diff --git a/src/protocol/render_ansi.rs b/src/protocol/render_ansi.rs index 1c28b00b..d8c92795 100644 --- a/src/protocol/render_ansi.rs +++ b/src/protocol/render_ansi.rs @@ -11,8 +11,10 @@ //! `CUP` positions during the frame stream. //! 5. After writing all changed cells, restore the final cursor visibility //! and position from `frame.cursor`. -//! 6. After ending synchronized output, repeat the final cursor anchor so -//! external IMEs can place candidate windows at the real input position. +//! 6. On platforms that need it, repeat the final cursor anchor after ending +//! synchronized output so external IMEs can place candidate windows at the +//! real input position. Windows Terminal exposes that repeat as visible +//! cursor movement during active TUI repaints, so Windows skips it. //! //! Escape sequences used: //! - `CSI H` (CUP) — move cursor to (row, col) @@ -37,6 +39,9 @@ pub(crate) struct EncodedBlit { pub(crate) bytes: Vec, /// Whether this frame was encoded as a full redraw. pub(crate) full: bool, + /// Cursor reveal bytes to write after a short quiet period. + #[cfg(any(windows, test))] + pub(crate) deferred_cursor_reveal: Option>, next_last_visible_cursor: Option<(u16, u16)>, next_last_cursor_shape: u8, } @@ -55,6 +60,24 @@ impl BlitEncoder { } pub(crate) fn encode(&self, frame: &FrameData, force_full: bool) -> EncodedBlit { + self.encode_inner(frame, force_full, false) + } + + #[cfg(any(windows, test))] + pub(crate) fn encode_with_deferred_cursor_reveal( + &self, + frame: &FrameData, + force_full: bool, + ) -> EncodedBlit { + self.encode_inner(frame, force_full, true) + } + + fn encode_inner( + &self, + frame: &FrameData, + force_full: bool, + defer_visible_cursor_during_content_changes: bool, + ) -> EncodedBlit { let prev = if force_full { None } else { @@ -69,12 +92,19 @@ impl BlitEncoder { let mut bytes = Vec::new(); let mut next_last_visible_cursor = self.last_visible_cursor; let mut next_last_cursor_shape = self.last_cursor_shape; + let suppress_visible_cursor = defer_visible_cursor_during_content_changes + && should_defer_visible_cursor(frame, prev, full); + #[cfg(any(windows, test))] + let deferred_cursor_reveal = suppress_visible_cursor + .then(|| encode_visible_cursor_reveal(frame)) + .flatten(); blit_frame_to_with_cursor_memory( &mut bytes, frame, prev, &mut next_last_visible_cursor, &mut next_last_cursor_shape, + suppress_visible_cursor, ); if let Some(stats) = prof_stats { crate::render_prof::duration_since("ansi_encode.total", prof_started); @@ -91,6 +121,8 @@ impl BlitEncoder { EncodedBlit { bytes, full, + #[cfg(any(windows, test))] + deferred_cursor_reveal, next_last_visible_cursor, next_last_cursor_shape, } @@ -351,6 +383,7 @@ fn blit_frame_to(writer: impl Write, frame: &FrameData, prev: Option<&FrameData> prev, &mut last_visible_cursor, &mut last_cursor_shape, + false, ); } @@ -360,20 +393,43 @@ fn blit_frame_to_with_cursor_memory( prev: Option<&FrameData>, last_visible_cursor: &mut Option<(u16, u16)>, last_cursor_shape: &mut u8, + suppress_visible_cursor: bool, +) { + blit_frame_to_with_cursor_memory_and_policy( + &mut writer, + frame, + prev, + last_visible_cursor, + last_cursor_shape, + repeat_ime_anchor_after_sync(), + suppress_visible_cursor, + ); +} + +fn blit_frame_to_with_cursor_memory_and_policy( + mut writer: impl Write, + frame: &FrameData, + prev: Option<&FrameData>, + last_visible_cursor: &mut Option<(u16, u16)>, + last_cursor_shape: &mut u8, + repeat_ime_anchor: bool, + suppress_visible_cursor: bool, ) { // On first frame or size change, do a full redraw. let full_redraw = prev.is_none() || prev.is_some_and(|p| p.width != frame.width || p.height != frame.height); + // Hide cursor before any cell writes to avoid stray cursor artifacts + // on terminals that render the hardware cursor at intermediate CUP positions. + // Keep this outside synchronized output so terminals that defer sync-block + // side effects still hide the cursor before frame painting begins. + let _ = writer.write_all(b"\x1b[?25l"); + // Ask terminals that support synchronized output to apply the whole frame // atomically. This keeps IMEs and cursor trackers from observing the // intermediate CUP positions used while painting changed cells. let _ = writer.write_all(b"\x1b[?2026h"); - // Hide cursor before any cell writes to avoid stray cursor artifacts - // on terminals that render the hardware cursor at intermediate CUP positions. - let _ = writer.write_all(b"\x1b[?25l"); - // Start each frame from a known OSC 8 state. If a previous write was // interrupted or the outer terminal had an active hyperlink, unlinked cells // must not inherit it. @@ -395,7 +451,10 @@ fn blit_frame_to_with_cursor_memory( // cell rather than the focused pane's input position. When the focused pane // hides its cursor, still park the host cursor intentionally so IMEs do not // anchor to whichever cell happened to be painted last. - let host_cursor = resolve_host_cursor_state(frame, last_visible_cursor); + let mut host_cursor = resolve_host_cursor_state(frame, last_visible_cursor); + if suppress_visible_cursor && host_cursor.visible { + host_cursor.visible = false; + } write_host_cursor_state(&mut writer, host_cursor, last_cursor_shape); // End the synchronized output block immediately after the final cursor @@ -404,13 +463,40 @@ fn blit_frame_to_with_cursor_memory( // Some native IMEs track candidate-window placement from normal terminal // cursor updates and may not observe cursor moves emitted inside synchronized - // output. Re-emit only the resolved final cursor anchor after the sync block; - // intermediate paint cursor positions remain hidden and the focused pane's - // requested cursor visibility is preserved. - write_ime_anchor_cursor_state(&mut writer, host_cursor); + // output. Re-emit only the resolved final cursor anchor after the sync block + // on targets that need it; Windows Terminal exposes that repeat as cursor + // movement during active TUI repaints. + if repeat_ime_anchor { + write_ime_anchor_cursor_state(&mut writer, host_cursor); + } let _ = writer.flush(); } +#[cfg(windows)] +fn repeat_ime_anchor_after_sync() -> bool { + false +} + +#[cfg(not(windows))] +fn repeat_ime_anchor_after_sync() -> bool { + true +} + +fn should_defer_visible_cursor(frame: &FrameData, prev: Option<&FrameData>, full: bool) -> bool { + let cursor_changed = prev.is_some_and(|prev| prev.cursor != frame.cursor); + frame_has_cell_changes(frame, prev, full) || cursor_changed +} + +fn frame_has_cell_changes(frame: &FrameData, prev: Option<&FrameData>, full: bool) -> bool { + if full { + return true; + } + let Some(prev) = prev else { + return true; + }; + compute_prof_blit_stats(frame, Some(prev), false).changed_cells > 0 +} + /// Writes all cells in the frame (full redraw). fn cell_width(cell: &CellData) -> usize { cell.symbol.width() @@ -507,6 +593,22 @@ fn write_ime_anchor_cursor_state(writer: &mut impl Write, cursor: HostCursorStat } } +#[cfg(any(windows, test))] +fn encode_visible_cursor_reveal(frame: &FrameData) -> Option> { + let cursor = frame.cursor.as_ref()?; + if !cursor.visible { + return None; + } + + let mut bytes = Vec::new(); + let position = clamp_cursor_position(frame, cursor.x, cursor.y); + write_cursor_position(&mut bytes, position); + let shape = normalize_cursor_shape(cursor.shape); + let _ = write!(bytes, "\x1b[{} q", shape); + let _ = bytes.write_all(b"\x1b[?25h"); + Some(bytes) +} + fn write_all_cells(writer: &mut impl Write, frame: &FrameData) { let mut active_hyperlink = None; for row in 0..frame.height { @@ -835,8 +937,8 @@ mod tests { let output_str = String::from_utf8(output).unwrap(); assert!( - output_str.starts_with("\x1b[?2026h\x1b[?25l"), - "should begin synchronized output and hide cursor before any cell writes during full redraw" + output_str.starts_with("\x1b[?25l\x1b[?2026h"), + "should hide cursor before synchronized frame painting during full redraw" ); } @@ -869,8 +971,8 @@ mod tests { let output_str = String::from_utf8(output).unwrap(); assert!( - output_str.starts_with("\x1b[?2026h\x1b[?25l"), - "should begin synchronized output and hide cursor before any cell writes during diff" + output_str.starts_with("\x1b[?25l\x1b[?2026h"), + "should hide cursor before synchronized frame painting during diff" ); } @@ -883,20 +985,20 @@ mod tests { let output_str = String::from_utf8(output).unwrap(); assert!( - output_str.starts_with("\x1b[?2026h"), + output_str.starts_with("\x1b[?25l\x1b[?2026h"), "should begin synchronized output before frame writes" ); let sync_end = output_str .find("\x1b[?2026l") .expect("should end synchronized output after frame writes"); assert!( - sync_end + "\x1b[?2026l".len() < output_str.len(), - "should end synchronized output before trailing IME cursor update" + sync_end > 0, + "should end synchronized output after frame writes" ); } #[test] - fn blit_frame_repeats_final_cursor_state_after_synchronized_output() { + fn blit_frame_can_repeat_final_cursor_state_after_synchronized_output() { let frame = FrameData { cells: vec![make_cell("A", 0, 0, 0); 9], width: 3, @@ -911,8 +1013,18 @@ mod tests { graphics: Vec::new(), }; + let mut last_visible_cursor = None; + let mut last_cursor_shape = 0; let mut output = Vec::new(); - blit_frame_to(&mut output, &frame, None); + blit_frame_to_with_cursor_memory_and_policy( + &mut output, + &frame, + None, + &mut last_visible_cursor, + &mut last_cursor_shape, + true, + false, + ); let output_str = String::from_utf8(output).unwrap(); let sync_end = output_str @@ -925,6 +1037,115 @@ mod tests { ); } + #[test] + fn blit_frame_can_skip_final_cursor_state_after_synchronized_output() { + let frame = FrameData { + cells: vec![make_cell("A", 0, 0, 0); 9], + width: 3, + height: 3, + cursor: Some(CursorState { + x: 2, + y: 1, + visible: true, + shape: 0, + }), + hyperlinks: Vec::new(), + graphics: Vec::new(), + }; + + let mut last_visible_cursor = None; + let mut last_cursor_shape = 0; + let mut output = Vec::new(); + blit_frame_to_with_cursor_memory_and_policy( + &mut output, + &frame, + None, + &mut last_visible_cursor, + &mut last_cursor_shape, + false, + false, + ); + + let output_str = String::from_utf8(output).unwrap(); + let sync_end = output_str + .find("\x1b[?2026l") + .expect("should end synchronized output"); + let trailing_cursor = &output_str[sync_end + "\x1b[?2026l".len()..]; + assert_eq!( + trailing_cursor, "", + "should not expose a post-sync cursor repeat when the target terminal flickers on it" + ); + } + + #[test] + fn blit_encoder_can_defer_visible_cursor_reveal_for_content_changes() { + let frame = FrameData { + cells: vec![make_cell("A", 0, 0, 0); 9], + width: 3, + height: 3, + cursor: Some(CursorState { + x: 2, + y: 1, + visible: true, + shape: 6, + }), + hyperlinks: Vec::new(), + graphics: Vec::new(), + }; + let encoder = BlitEncoder::new(); + + let encoded = encoder.encode_with_deferred_cursor_reveal(&frame, false); + + let output_str = String::from_utf8(encoded.bytes).unwrap(); + assert!( + !output_str.contains("\x1b[?25h"), + "active repaint should keep the native cursor hidden" + ); + assert_eq!( + encoded.deferred_cursor_reveal.as_deref(), + Some(&b"\x1b[2;3H\x1b[6 q\x1b[?25h"[..]) + ); + } + + #[test] + fn blit_encoder_can_defer_visible_cursor_reveal_for_cursor_only_changes() { + let prev = FrameData { + cells: vec![make_cell("A", 0, 0, 0); 9], + width: 3, + height: 3, + cursor: Some(CursorState { + x: 0, + y: 0, + visible: true, + shape: 0, + }), + hyperlinks: Vec::new(), + graphics: Vec::new(), + }; + let mut curr = prev.clone(); + curr.cursor = Some(CursorState { + x: 2, + y: 1, + visible: true, + shape: 0, + }); + let mut encoder = BlitEncoder::new(); + let first = encoder.encode(&prev, false); + encoder.commit(prev, first); + + let encoded = encoder.encode_with_deferred_cursor_reveal(&curr, false); + + let output_str = String::from_utf8(encoded.bytes).unwrap(); + assert!( + !output_str.contains("\x1b[?25h"), + "cursor-only movement should remain hidden until the debounce fires" + ); + assert_eq!( + encoded.deferred_cursor_reveal.as_deref(), + Some(&b"\x1b[2;3H\x1b[0 q\x1b[?25h"[..]) + ); + } + #[test] fn blit_frame_emits_cursor_shape_before_visibility_without_touching_ime_anchor() { let frame = FrameData { @@ -941,8 +1162,18 @@ mod tests { graphics: Vec::new(), }; + let mut last_visible_cursor = None; + let mut last_cursor_shape = 0; let mut output = Vec::new(); - blit_frame_to(&mut output, &frame, None); + blit_frame_to_with_cursor_memory_and_policy( + &mut output, + &frame, + None, + &mut last_visible_cursor, + &mut last_cursor_shape, + true, + false, + ); let output_str = String::from_utf8(output).unwrap(); let final_cursor = output_str @@ -994,20 +1225,24 @@ mod tests { let mut last_cursor_shape = 0; let mut output = Vec::new(); - blit_frame_to_with_cursor_memory( + blit_frame_to_with_cursor_memory_and_policy( &mut output, &visible, None, &mut last_visible_cursor, &mut last_cursor_shape, + true, + false, ); output.clear(); - blit_frame_to_with_cursor_memory( + blit_frame_to_with_cursor_memory_and_policy( &mut output, &hidden, Some(&visible), &mut last_visible_cursor, &mut last_cursor_shape, + true, + false, ); let output_str = String::from_utf8(output).unwrap(); @@ -1337,6 +1572,7 @@ mod tests { None, &mut last_visible_cursor, &mut last_cursor_shape, + false, ); output.clear(); blit_frame_to_with_cursor_memory( @@ -1345,6 +1581,7 @@ mod tests { Some(&visible), &mut last_visible_cursor, &mut last_cursor_shape, + false, ); let output_str = String::from_utf8(output).unwrap(); @@ -1377,6 +1614,7 @@ mod tests { None, &mut last_visible_cursor, &mut last_cursor_shape, + false, ); let output_str = String::from_utf8(output).unwrap(); diff --git a/src/server/headless.rs b/src/server/headless.rs index f313161f..0f9f3bb6 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -4868,6 +4868,70 @@ next_tab = "" ); } + #[tokio::test] + async fn virtual_render_hides_focused_pane_cursor_during_synchronized_output() { + let mut state = AppState::test_new(); + state.reveal_hidden_cursor_for_cjk_ime = true; + let mut ws = crate::workspace::Workspace::test_new("test"); + let pane_id = ws.tabs[0].root_pane; + let runtime = crate::terminal::TerminalRuntime::test_with_screen_bytes(20, 5, b"left"); + ws.insert_test_runtime(pane_id, runtime); + + state.workspaces = vec![ws]; + state.active = Some(0); + state.selected = 0; + state.mode = crate::app::Mode::Terminal; + + let area = Rect::new(0, 0, 80, 24); + let _ = crate::server::render_stream::render_virtual(&mut state, area, true); + let terminal_runtimes = crate::terminal::TerminalRuntimeRegistry::new(); + let runtime = state + .runtime_for_pane(&terminal_runtimes, pane_id) + .expect("pane runtime after initial render"); + runtime.test_process_pty_bytes(b"\x1b[?2026h\x1b[2;3H"); + assert!(runtime.synchronized_output_active()); + + let (_buffer, cursor) = + crate::server::render_stream::render_virtual(&mut state, area, false); + + assert_eq!( + cursor, None, + "child cursor positions are unstable while synchronized output is active" + ); + } + + #[tokio::test] + async fn virtual_render_hides_focused_pane_cursor_during_synchronized_output_resize() { + let mut state = AppState::test_new(); + let mut ws = crate::workspace::Workspace::test_new("test"); + let pane_id = ws.tabs[0].root_pane; + let runtime = crate::terminal::TerminalRuntime::test_with_screen_bytes(20, 5, b"left"); + ws.insert_test_runtime(pane_id, runtime); + + state.workspaces = vec![ws]; + state.active = Some(0); + state.selected = 0; + state.mode = crate::app::Mode::Terminal; + + let initial_area = Rect::new(0, 0, 80, 24); + let _ = crate::server::render_stream::render_virtual(&mut state, initial_area, true); + let terminal_runtimes = crate::terminal::TerminalRuntimeRegistry::new(); + let runtime = state + .runtime_for_pane(&terminal_runtimes, pane_id) + .expect("pane runtime after initial render"); + runtime.test_process_pty_bytes(b"\x1b[?2026h\x1b[2;3H"); + assert!(runtime.synchronized_output_active()); + + let resized_area = Rect::new(0, 0, 100, 30); + let (_buffer, cursor) = + crate::server::render_stream::render_virtual(&mut state, resized_area, true); + + assert_eq!( + cursor, None, + "pre-resize synchronized output should suppress the cursor even if resize clears the mode" + ); + } + #[tokio::test] async fn virtual_render_exposes_hidden_pane_cursor_when_reveal_hidden_for_cjk_ime() { let mut state = AppState::test_new(); diff --git a/src/server/render_stream.rs b/src/server/render_stream.rs index e158a6d4..6699298d 100644 --- a/src/server/render_stream.rs +++ b/src/server/render_stream.rs @@ -260,11 +260,15 @@ pub(crate) fn render_virtual_with_runtime_registry( resize_panes: bool, cell_size: crate::kitty_graphics::HostCellSize, ) -> (ratatui::buffer::Buffer, Option) { + let pre_compute_suppresses_focused_terminal_cursor = + focused_terminal_suppresses_host_cursor(app_state, terminal_runtimes); if resize_panes { crate::ui::compute_view_with_cell_size(app_state, terminal_runtimes, area, cell_size); } else { crate::ui::compute_view_without_resizing_panes(app_state, terminal_runtimes, area); } + let suppress_focused_terminal_cursor = pre_compute_suppresses_focused_terminal_cursor + || focused_terminal_suppresses_host_cursor(app_state, terminal_runtimes); let backend = CursorTrackingBackend::new(area.width, area.height); let mut terminal = ratatui::Terminal::new(backend).expect("TestBackend::new should never fail"); @@ -276,8 +280,15 @@ pub(crate) fn render_virtual_with_runtime_registry( .expect("render to TestBackend should never fail"); let buffer = terminal.backend().buffer().clone(); - let cursor = focused_terminal_cursor(app_state, terminal_runtimes) - .or_else(|| terminal.backend().rendered_cursor()); + let cursor = if suppress_focused_terminal_cursor { + None + } else { + focused_terminal_cursor(app_state, terminal_runtimes).or_else(|| { + (!focused_terminal_owns_host_cursor(app_state, terminal_runtimes)) + .then(|| terminal.backend().rendered_cursor()) + .flatten() + }) + }; (buffer, cursor) } @@ -287,6 +298,7 @@ pub(crate) fn render_terminal_virtual( runtime: &crate::terminal::TerminalRuntime, area: Rect, ) -> (ratatui::buffer::Buffer, Option) { + let suppress_cursor = runtime.synchronized_output_active(); let backend = CursorTrackingBackend::new(area.width, area.height); let mut terminal = ratatui::Terminal::new(backend).expect("TestBackend::new should never fail"); @@ -297,15 +309,20 @@ pub(crate) fn render_terminal_virtual( .expect("render to TestBackend should never fail"); let buffer = terminal.backend().buffer().clone(); - let cursor = runtime - .cursor_state(area, true) + let cursor = (!suppress_cursor) + .then(|| runtime.cursor_state(area, true)) + .flatten() .map(|cursor| CursorState { x: cursor.x, y: cursor.y, visible: cursor.visible && !crate::ui::pane_is_scrolled_back(runtime), shape: cursor.shape, }) - .or_else(|| terminal.backend().rendered_cursor()); + .or_else(|| { + (!suppress_cursor) + .then(|| terminal.backend().rendered_cursor()) + .flatten() + }); (buffer, cursor) } @@ -355,8 +372,10 @@ pub(crate) fn focused_terminal_cursor( return None; } let rt = app_state.runtime_for_pane_in_workspace(terminal_runtimes, ws_idx, info.id)?; + if rt.synchronized_output_active() { + return None; + } let scrolled_back = crate::ui::pane_is_scrolled_back(rt); - // Determine whether the IME-anchor reveal applies to this focused pane. // The master switch must be on, and either no agent filter is configured // (apply to any pane) or the focused pane's detected agent matches the @@ -406,3 +425,59 @@ pub(crate) fn focused_terminal_cursor( None } } + +fn focused_terminal_owns_host_cursor( + app_state: &AppState, + terminal_runtimes: &TerminalRuntimeRegistry, +) -> bool { + if app_state.mode != Mode::Terminal { + return false; + } + + let Some(ws_idx) = app_state.active else { + return false; + }; + let Some(info) = app_state + .view + .pane_infos + .iter() + .find(|info| info.is_focused) + else { + return false; + }; + if !app_state.pane_exposes_host_cursor(ws_idx, info.id) { + return false; + } + + app_state + .runtime_for_pane_in_workspace(terminal_runtimes, ws_idx, info.id) + .is_some() +} + +fn focused_terminal_suppresses_host_cursor( + app_state: &AppState, + terminal_runtimes: &TerminalRuntimeRegistry, +) -> bool { + if app_state.mode != Mode::Terminal { + return false; + } + + let Some(ws_idx) = app_state.active else { + return false; + }; + let Some(info) = app_state + .view + .pane_infos + .iter() + .find(|info| info.is_focused) + else { + return false; + }; + if !app_state.pane_exposes_host_cursor(ws_idx, info.id) { + return false; + } + + app_state + .runtime_for_pane_in_workspace(terminal_runtimes, ws_idx, info.id) + .is_some_and(crate::terminal::TerminalRuntime::synchronized_output_active) +} diff --git a/src/sound.rs b/src/sound.rs index ab6c11c5..ef45ccc7 100644 --- a/src/sound.rs +++ b/src/sound.rs @@ -1,7 +1,8 @@ //! Sound notifications for agent state changes. //! //! Embeds mp3 files in the binary and plays them via system audio tools. -//! Uses afplay (macOS) or decoder-capable Linux audio players — no Rust audio dependencies. +//! Uses afplay (macOS), Windows MediaPlayer, or decoder-capable Linux audio +//! players — no Rust audio dependencies. use std::io::Write; use std::path::{Path, PathBuf}; @@ -89,23 +90,81 @@ fn temp_sound_path() -> PathBuf { std::env::temp_dir().join(format!("herdr-sound-{}-{id}.mp3", std::process::id())) } +#[cfg(windows)] fn run_player(path: &Path) -> Result { - if cfg!(target_os = "macos") { - Command::new("afplay") - .arg(path) - .output() - .map_err(|e| format!("no audio player available: {e}")) - } else { - run_linux_player(path) - } + run_windows_player(path) } +#[cfg(target_os = "macos")] +fn run_player(path: &Path) -> Result { + Command::new("afplay") + .arg(path) + .output() + .map_err(|e| format!("no audio player available: {e}")) +} + +#[cfg(not(any(windows, target_os = "macos")))] +fn run_player(path: &Path) -> Result { + run_linux_player(path) +} + +#[cfg(any(windows, test))] +fn windows_media_player_script() -> &'static str { + r#" +param([string]$Path) +$ErrorActionPreference = 'Stop' +Add-Type -AssemblyName PresentationCore +$resolved = (Resolve-Path -LiteralPath $Path).ProviderPath +$player = [System.Windows.Media.MediaPlayer]::new() +$script:done = $false +$script:failed = $null +$player.add_MediaEnded({ $script:done = $true }) +$player.add_MediaFailed({ + param($sender, $eventArgs) + $script:failed = $eventArgs.ErrorException + $script:done = $true +}) +$player.Open([Uri]::new($resolved)) +$deadline = [DateTime]::UtcNow.AddSeconds(15) +while (-not $script:done -and -not $player.NaturalDuration.HasTimeSpan -and [DateTime]::UtcNow -lt $deadline) { + Start-Sleep -Milliseconds 25 +} +if ($script:failed) { throw $script:failed } +$player.Play() +while (-not $script:done -and [DateTime]::UtcNow -lt $deadline) { + Start-Sleep -Milliseconds 50 +} +$player.Close() +if ($script:failed) { throw $script:failed } +if (-not $script:done) { throw 'sound playback timed out' } +"# +} + +#[cfg(windows)] +fn run_windows_player(path: &Path) -> Result { + Command::new("powershell.exe") + .args([ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-ExecutionPolicy", + "Bypass", + "-Command", + windows_media_player_script(), + ]) + .arg(path) + .output() + .map_err(|e| format!("Windows MediaPlayer playback failed: {e}")) +} + +#[cfg(not(any(windows, target_os = "macos")))] #[derive(Debug, Clone, Copy)] struct AudioPlayer { program: &'static str, args: &'static [&'static str], } +#[cfg(not(any(windows, target_os = "macos")))] impl AudioPlayer { fn output(self, path: &Path) -> std::io::Result { Command::new(self.program) @@ -115,6 +174,7 @@ impl AudioPlayer { } } +#[cfg(not(any(windows, target_os = "macos")))] fn linux_audio_players() -> &'static [AudioPlayer] { // Do not add bare aplay here. It does not decode MP3 and plays MP3 bytes as raw PCM. &[ @@ -141,6 +201,7 @@ fn linux_audio_players() -> &'static [AudioPlayer] { ] } +#[cfg(not(any(windows, target_os = "macos")))] fn run_linux_player(path: &Path) -> Result { let mut errors = Vec::new(); @@ -158,6 +219,7 @@ fn run_linux_player(path: &Path) -> Result { )) } +#[cfg(not(any(windows, target_os = "macos")))] fn player_error(player: AudioPlayer, output: &Output) -> String { let stderr = String::from_utf8_lossy(&output.stderr); let stderr = stderr.trim(); @@ -178,6 +240,7 @@ mod tests { assert_ne!(temp_sound_path(), temp_sound_path()); } + #[cfg(not(any(windows, target_os = "macos")))] #[test] fn linux_audio_players_are_mp3_capable() { let programs: Vec<&str> = linux_audio_players() @@ -188,4 +251,13 @@ mod tests { assert_eq!(programs, ["paplay", "pw-play", "ffplay", "mpg123", "mpv"]); assert!(!programs.contains(&"aplay")); } + + #[test] + fn windows_media_player_script_accepts_literal_path_argument() { + let script = windows_media_player_script(); + + assert!(script.contains("param([string]$Path)")); + assert!(script.contains("Resolve-Path -LiteralPath $Path")); + assert!(script.contains("System.Windows.Media.MediaPlayer")); + } } diff --git a/src/terminal/runtime.rs b/src/terminal/runtime.rs index c74301ad..5d1ce1d7 100644 --- a/src/terminal/runtime.rs +++ b/src/terminal/runtime.rs @@ -238,6 +238,10 @@ impl TerminalRuntime { self.0.cursor_state(area, show_cursor) } + pub fn synchronized_output_active(&self) -> bool { + self.0.synchronized_output_active() + } + pub fn visible_text(&self) -> String { self.0.visible_text() }