From cfa482d7a8caa489c34ad9faec541d8a2f77faeb Mon Sep 17 00:00:00 2001 From: felipe Date: Thu, 17 Sep 2026 14:38:16 -0300 Subject: [PATCH 1/4] feat: add shell command prompt (default ':') to execute arbitrary commands safely --- src/config/keybindings.rs | 3 + src/gui/controller/custom_commands.rs | 61 +++++++++- src/gui/mod.rs | 77 ++++++++++++ src/gui/views.rs | 161 +++++++++++++++++++++++++- 4 files changed, 296 insertions(+), 6 deletions(-) diff --git a/src/config/keybindings.rs b/src/config/keybindings.rs index 0b992f6..5847e9f 100644 --- a/src/config/keybindings.rs +++ b/src/config/keybindings.rs @@ -126,6 +126,8 @@ pub struct UniversalKeybinding { pub reset_side_panel: String, #[serde(rename = "toggleDiffViewLayout")] pub toggle_diff_view_layout: String, + #[serde(rename = "customCommandPrompt")] + pub custom_command_prompt: String, } impl Default for UniversalKeybinding { @@ -181,6 +183,7 @@ impl Default for UniversalKeybinding { main_panel_full: "".into(), reset_side_panel: "".into(), toggle_diff_view_layout: "\\".into(), + custom_command_prompt: ":".into(), } } } diff --git a/src/gui/controller/custom_commands.rs b/src/gui/controller/custom_commands.rs index 30934eb..f456a95 100644 --- a/src/gui/controller/custom_commands.rs +++ b/src/gui/controller/custom_commands.rs @@ -68,10 +68,49 @@ fn execute_custom_command(gui: &mut Gui, cmd: &CustomCommand) -> Result<()> { } fn run_command(gui: &mut Gui, command: &str, show_output: bool) -> Result<()> { - let result = CmdBuilder::new("sh") - .args(&["-c", command]) + let shell = std::env::var("SHELL").unwrap_or_else(|_| "sh".to_string()); + let is_zsh = shell.ends_with("zsh"); + let is_bash = shell.ends_with("bash"); + + // Create a temporary script file to execute aliases/functions cleanly without TTY hang + let temp_dir = std::env::temp_dir(); + let temp_file = temp_dir.join(format!( + "lazygitrs-cmd-{}-{}.sh", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() + )); + + let mut script_content = String::new(); + if is_zsh { + script_content.push_str( + "#!/bin/zsh\nsetopt aliases\n[ -f ~/.zshrc ] && source ~/.zshrc >/dev/null 2>&1\n", + ); + } else if is_bash { + script_content.push_str( + "#!/bin/bash\nshopt -s expand_aliases\n[ -f ~/.bash_aliases ] && source ~/.bash_aliases >/dev/null 2>&1\n[ -f ~/.bashrc ] && source ~/.bashrc >/dev/null 2>&1\n", + ); + } else { + script_content.push_str("#!/bin/sh\n"); + } + script_content.push_str(command); + script_content.push('\n'); + + std::fs::write(&temp_file, &script_content)?; + + // Execute the temporary script with null stdin + let result = CmdBuilder::new(&shell) + .arg(&temp_file.to_string_lossy().to_string()) + .stdin(String::new()) .cwd_path(gui.git.repo_path()) - .run()?; + .run(); + + // Clean up temporary file + let _ = std::fs::remove_file(&temp_file); + + let result = result?; if let Ok(mut log) = gui.command_log.lock() { log.push(format!("$ {}", command)); @@ -168,3 +207,19 @@ fn context_id_to_name(ctx: ContextId) -> &'static str { ContextId::Staging => "staging", } } + +pub fn open_custom_command_prompt(gui: &mut Gui) -> Result<()> { + gui.popup = PopupState::Input { + title: "Execute Shell Command".to_string(), + textarea: crate::gui::popup::make_textarea(""), + on_confirm: Box::new(move |gui, input| { + if !input.trim().is_empty() { + run_command(gui, &input, true)?; + } + Ok(()) + }), + is_commit: false, + confirm_focused: false, + }; + Ok(()) +} diff --git a/src/gui/mod.rs b/src/gui/mod.rs index f642801..7a581fb 100644 --- a/src/gui/mod.rs +++ b/src/gui/mod.rs @@ -3429,6 +3429,10 @@ impl Gui { return self.handle_diff_focused_key(key); } + if matches_key(key, &keybindings.universal.custom_command_prompt) { + return controller::custom_commands::open_custom_command_prompt(self); + } + // Global keybindings if matches_key(key, &keybindings.universal.quit) || matches_key(key, &keybindings.universal.quit_alt1) @@ -5617,6 +5621,10 @@ impl Gui { CommandEntry::keybinding("I".into(), "Interactive rebase onto...".into()), CommandEntry::keybinding("G".into(), "Reset to...".into()), CommandEntry::keybinding("1-5".into(), "Jump to panel".into()), + CommandEntry::keybinding( + kb.universal.custom_command_prompt.clone(), + "Execute shell command".into(), + ), CommandEntry::keybinding("?".into(), "Show command palette".into()), CommandEntry::action( "".into(), @@ -8991,6 +8999,75 @@ mod terminal_mouse_tests { } assert_eq!(keys.len(), 8); } + + struct TempRepo { + path: PathBuf, + } + + impl TempRepo { + fn new(name: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "lazygitrs-shell-test-{}-{}-{}", + name, + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(), + std::process::id() + )); + std::fs::create_dir_all(&path).expect("create temp dir"); + let _ = std::process::Command::new("git") + .args(["init", "-b", "main"]) + .current_dir(&path) + .output(); + Self { path } + } + + fn path(&self) -> &std::path::Path { + &self.path + } + } + + impl Drop for TempRepo { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.path); + } + } + + #[test] + fn colon_key_opens_custom_command_prompt() { + let repo = TempRepo::new("colon-shell-prompt"); + let config = crate::config::AppConfig::load(false).unwrap_or_else(|_| { + let mut c = crate::config::AppConfig { + debug: false, + version: "test".into(), + user_config: crate::config::UserConfig::default(), + app_state: crate::config::AppState::default(), + config_dir: repo.path().to_path_buf(), + state_dir: repo.path().to_path_buf(), + state_path: repo.path().join("state.yml"), + }; + c + }); + let git = crate::git::GitCommands::new(repo.path()).unwrap(); + let mut gui = Gui::new(config, git, None).unwrap(); + + assert!(matches!(gui.popup, PopupState::None)); + + // Send ':' key + let colon = KeyEvent::new(KeyCode::Char(':'), KeyModifiers::NONE); + gui.handle_key(colon).unwrap(); + + match &gui.popup { + PopupState::Input { + title, is_commit, .. + } => { + assert_eq!(title, "Execute Shell Command"); + assert!(!is_commit); + } + _ => panic!("Expected PopupState::Input"), + } + } } fn setup_terminal() -> Result<(Term, bool)> { diff --git a/src/gui/views.rs b/src/gui/views.rs index eef8068..9e0d1f5 100644 --- a/src/gui/views.rs +++ b/src/gui/views.rs @@ -10,7 +10,7 @@ use std::collections::HashSet; use std::collections::HashMap; use std::sync::{Arc, Mutex}; -use crate::config::{AppConfig, Theme}; +use crate::config::{AppConfig, KeybindingConfig, Theme}; use crate::model::Model; use crate::model::commit::{Commit, CommitStat}; use crate::model::file_tree::{CommitFileTreeNode, FileTreeNode}; @@ -399,6 +399,7 @@ pub fn render( model, diff_focused, !cherry_pick_clipboard.is_empty(), + &config.user_config.keybinding, ); // Render text selection highlight overlay and tooltip (must be before popup) render_selection_overlay(frame, diff_view, fl.main_panel, theme); @@ -1079,6 +1080,7 @@ pub fn render( model, diff_focused, !cherry_pick_clipboard.is_empty(), + &config.user_config.keybinding, ); // Render text selection highlight overlay and tooltip @@ -1326,13 +1328,110 @@ pub fn checklist_item_at(popup: &PopupState, area: Rect, col: u16, row: u16) -> #[cfg(test)] mod tests { - use super::{checklist_item_at, command_log_geometry, menu_item_at, render_popup}; + use super::{ + checklist_item_at, command_log_geometry, format_key_hint, menu_item_at, render_popup, + }; use crate::config::Theme; use crate::gui::popup::{ChecklistItem, MenuItem, MessageKind, PopupState}; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::layout::Rect; + #[test] + fn format_key_hint_formats_modifiers_and_preserves_plain() { + assert_eq!(format_key_hint(""), "ctrl+s"); + assert_eq!(format_key_hint(""), "ctrl+s"); + assert_eq!(format_key_hint(""), "ctrl+s"); + assert_eq!(format_key_hint(""), "ctrl+s"); + assert_eq!(format_key_hint("Ctrl+S"), "ctrl+s"); + assert_eq!(format_key_hint("ctrl-s"), "ctrl+s"); + assert_eq!(format_key_hint(""), "ctrl+l"); + assert_eq!(format_key_hint(""), "ctrl+f"); + assert_eq!(format_key_hint(""), "alt+h"); + assert_eq!(format_key_hint(""), "alt+h"); + assert_eq!(format_key_hint(""), "shift+f"); + assert_eq!(format_key_hint(""), "shift+f"); + assert_eq!(format_key_hint(""), "shift+tab"); + assert_eq!(format_key_hint(""), "space"); + assert_eq!(format_key_hint(""), "space"); + assert_eq!(format_key_hint(""), "enter"); + assert_eq!(format_key_hint(""), "esc"); + assert_eq!(format_key_hint(":"), ":"); + assert_eq!(format_key_hint(""), ""); + } + + #[test] + fn status_bar_shows_shell_command_prompt_hint() { + use crate::config::KeybindingConfig; + use crate::gui::context::{ContextId, ContextManager}; + use crate::model::Model; + use crate::pager::side_by_side::DiffViewState; + + let backend = TestBackend::new(160, 1); + let mut terminal = Terminal::new(backend).expect("test terminal"); + let theme = Theme::default(); + let mut ctx_mgr = ContextManager::new(); + ctx_mgr.set_active(ContextId::Files); + let diff_view = DiffViewState::new(); + let model = Model::default(); + let keybindings = KeybindingConfig::default(); + + terminal + .draw(|f| { + super::render_status_bar( + f, + Rect::new(0, 0, 160, 1), + &ctx_mgr, + &diff_view, + &theme, + &model, + false, + false, + &keybindings, + ); + }) + .unwrap(); + + let buf = terminal.backend().buffer(); + let content: String = (0..160) + .map(|x| buf.cell((x, 0)).unwrap().symbol()) + .collect(); + assert!( + content.contains(": shell"), + "Expected ': shell' in status bar, got: {}", + content + ); + + // Test custom keybinding for custom_command_prompt (e.g. ;) + let mut custom_kb = KeybindingConfig::default(); + custom_kb.universal.custom_command_prompt = ";".into(); + terminal + .draw(|f| { + super::render_status_bar( + f, + Rect::new(0, 0, 160, 1), + &ctx_mgr, + &diff_view, + &theme, + &model, + false, + false, + &custom_kb, + ); + }) + .unwrap(); + + let buf = terminal.backend().buffer(); + let content: String = (0..160) + .map(|x| buf.cell((x, 0)).unwrap().symbol()) + .collect(); + assert!( + content.contains("; shell"), + "Expected '; shell' with custom keybinding, got: {}", + content + ); + } + #[test] fn command_log_is_hidden_when_main_panel_is_absent() { assert_eq!(command_log_geometry(Rect::default(), 1), None); @@ -2194,6 +2293,48 @@ fn get_info_content<'a>(model: &Model, ctx_mgr: &ContextManager) -> Vec } } +fn format_key_hint(key: &str) -> String { + let key = key.trim(); + let inner = if let Some(stripped) = key.strip_prefix('<').and_then(|k| k.strip_suffix('>')) { + stripped.trim() + } else { + key + }; + let inner_lower = inner.to_ascii_lowercase(); + if let Some(rest) = inner_lower + .strip_prefix("c-") + .or_else(|| inner_lower.strip_prefix("c+")) + .or_else(|| inner_lower.strip_prefix("ctrl-")) + .or_else(|| inner_lower.strip_prefix("ctrl+")) + { + format!("ctrl+{}", rest) + } else if let Some(rest) = inner_lower + .strip_prefix("a-") + .or_else(|| inner_lower.strip_prefix("a+")) + .or_else(|| inner_lower.strip_prefix("alt-")) + .or_else(|| inner_lower.strip_prefix("alt+")) + { + format!("alt+{}", rest) + } else if let Some(rest) = inner_lower + .strip_prefix("s-") + .or_else(|| inner_lower.strip_prefix("s+")) + .or_else(|| inner_lower.strip_prefix("shift-")) + .or_else(|| inner_lower.strip_prefix("shift+")) + { + format!("shift+{}", rest) + } else if inner_lower == "enter" || inner_lower == "return" { + "enter".to_string() + } else if inner_lower == "escape" || inner_lower == "esc" { + "esc".to_string() + } else if inner_lower == "space" { + "space".to_string() + } else if inner_lower == "backtab" || inner_lower == "shift-tab" || inner_lower == "shift+tab" { + "shift+tab".to_string() + } else { + key.to_string() + } +} + fn render_search_bar_or_status_bar( frame: &mut Frame, status_bar: Rect, @@ -2205,6 +2346,7 @@ fn render_search_bar_or_status_bar( model: &Model, diff_focused: bool, has_copied_commits: bool, + keybindings: &KeybindingConfig, ) { if let Some((query, match_count, current_match)) = search_state { let match_info = if match_count > 0 { @@ -2262,6 +2404,7 @@ fn render_search_bar_or_status_bar( model, diff_focused, has_copied_commits, + keybindings, ); } } @@ -2275,9 +2418,16 @@ fn render_status_bar( model: &Model, diff_focused: bool, has_copied_commits: bool, + keybindings: &KeybindingConfig, ) { let mut hints: Vec<(&str, &str)> = Vec::new(); let mut emphasized: Vec<&str> = Vec::new(); + let shell_key = format_key_hint(&keybindings.universal.custom_command_prompt); + let shell_hint = if !shell_key.is_empty() { + shell_key.as_str() + } else { + ":" + }; // When in a special state (rebasing/merging/cherry-picking), show those options prominently if model.is_rebasing { @@ -2426,7 +2576,12 @@ fn render_status_bar( } // Global hints (always last) - hints.extend([("q", "quit"), ("tab/1-5", "panels"), ("j/k", "nav")]); + hints.extend([ + ("q", "quit"), + (shell_hint, "shell"), + ("tab/1-5", "panels"), + ("j/k", "nav"), + ]); let key_style = Style::default() .fg(_theme.text) From 0a25695badc6e2b2ca08d17528277c1bc296ec68 Mon Sep 17 00:00:00 2001 From: Blankeos Date: Wed, 7 Oct 2026 02:51:22 +0800 Subject: [PATCH 2/4] Fix shell prompt execution, cancellation, and universal UI integration Run prompt and configured commands off the GUI thread with detached noninteractive processes, five-minute deadlines, process-group cancellation/reaping, and bounded stdout/stderr capture. Replace temporary scripts with safe native shell arguments and bash/zsh rc wrappers while preserving sh semantics for configured commands. Honor remapped and disabled prompt bindings in list, diff, and compare views without stealing search or popup input. Preserve literal pasted commands, report errors before stdout, expose scrollable/copyable results, bound command-log previews, and refresh repository/comparison data without losing filters or navigation. Add runner, UI, configuration, rendering, and output regressions; document limitations. --- README.md | 40 ++ src/config/keybindings.rs | 40 ++ src/gui/controller/custom_commands.rs | 323 +++++++-- src/gui/controller/diff_mode.rs | 193 +++++- src/gui/mod.rs | 293 +++++++- src/gui/popup.rs | 10 + src/gui/presentation/diff_mode.rs | 56 ++ src/gui/views.rs | 221 ++++++ src/os/mod.rs | 1 + src/os/shell.rs | 958 ++++++++++++++++++++++++++ 10 files changed, 2057 insertions(+), 78 deletions(-) create mode 100644 src/os/shell.rs diff --git a/README.md b/README.md index f92b0e0..12c1b8e 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,45 @@ only while a tree is active. The footer shows just the fold/unfold shortcut border; explicitly remapping a tree action to `'` will override it while the tree is active. +### Shell command prompt + +Press `:` in the normal view or compare mode to run a shell command from the +repository root without leaving the TUI. Remap the prompt shortcut in your config, +or set it to `""` to disable it: + +```yaml +keybinding: + universal: + customCommandPrompt: ":" # e.g. "" to remap; "" to disable +``` + +The prompt uses `$SHELL`, falling back to `sh` when it is unset or empty. Bash +loads `~/.bash_aliases` and `~/.bashrc` and enables alias expansion; zsh loads `${ZDOTDIR:-$HOME}/.zshrc`. +Aliases and functions defined there can be used, but a noninteractive guard in +your rc file may skip their definitions. Definitions that exist only in your +current interactive shell are not inherited. Fish and other shells receive the +command using their native `-c` invocation, so use that shell's syntax rather +than POSIX syntax. + +Commands run asynchronously and **noninteractively** with no input/TTY: editors, +password prompts, and other interactive programs are not supported here. Press +`Esc` while a command is running to cancel it; commands time out after five +minutes. Output includes the exit status, stdout, and stderr, with a 1 MiB capture +cap per stream (excess output is truncated). Background children are terminated +when the job ends, including on completion, cancellation, or timeout; this is +not a way to launch persistent background services. Processes that explicitly +create their own session can escape process-group cleanup; this prompt is not a +sandbox. The process runner currently requires Unix (macOS/Linux). + +Input preserves pasted newlines and quoted spacing; `Enter` executes the whole +command and `Esc` dismisses without running it. Scroll command results with +`j`/`k`, arrow keys, `PgUp`/`PgDn`, or `g`/`G`; `y` copies the result and +`Esc`/`Enter` closes it. The command log retains a bounded output preview. + +Configured `customCommands` still use `sh -c` rather than the prompt's `$SHELL` +and rc-file loading, but now run asynchronously with the same cancellation, +timeout, and output limits. + ### Upgrade Detects how you installed (brew / npm / bun / cargo / install.sh) and upgrades in place: @@ -145,6 +184,7 @@ Persisted State lives at `~/.local/state/lazygitrs/state.yml` and `~/.local/stat **New config properties:** - `git.commit.generateCommand` — shell command for AI-generated commit messages. See [What's different](#whats-different) for examples. +- `keybinding.universal.customCommandPrompt` — shell prompt shortcut (`":"` by default; `""` disables it). See [Shell command prompt](#shell-command-prompt). - `~/.config/lazygitrs/themes/*.json` — drop custom theme files here. See [Themes](#themes). ### Themes diff --git a/src/config/keybindings.rs b/src/config/keybindings.rs index 77fe68c..bacf318 100644 --- a/src/config/keybindings.rs +++ b/src/config/keybindings.rs @@ -78,6 +78,8 @@ pub struct UniversalKeybinding { pub start_search: String, #[serde(rename = "optionMenu")] pub option_menu: String, + #[serde(rename = "customCommandPrompt")] + pub custom_command_prompt: String, pub edit: String, #[serde(rename = "openFile")] pub open_file: String, @@ -167,6 +169,7 @@ impl Default for UniversalKeybinding { prev_match: "N".into(), start_search: "/".into(), option_menu: "x".into(), + custom_command_prompt: ":".into(), edit: "e".into(), open_file: "o".into(), scroll_up_main: "".into(), @@ -471,6 +474,43 @@ pub fn parse_key(s: &str) -> Option { mod tests { use super::*; + #[test] + fn custom_command_prompt_defaults_when_omitted() { + assert_eq!( + KeybindingConfig::default().universal.custom_command_prompt, + ":" + ); + for yaml in ["{}", "universal: {}", "universal:\n optionMenu: m"] { + let parsed: KeybindingConfig = serde_yaml::from_str(yaml).expect("valid yaml"); + assert_eq!(parsed.universal.custom_command_prompt, ":"); + assert_eq!( + parse_key(&parsed.universal.custom_command_prompt), + Some(KeyEvent::new(KeyCode::Char(':'), KeyModifiers::NONE)) + ); + } + } + + #[test] + fn custom_command_prompt_can_be_remapped() { + let parsed: KeybindingConfig = + serde_yaml::from_str("universal:\n customCommandPrompt: ''").expect("valid yaml"); + assert_eq!(parsed.universal.custom_command_prompt, ""); + assert_eq!( + parse_key(&parsed.universal.custom_command_prompt), + Some(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL)) + ); + assert_eq!(parsed.universal.start_search, "/"); + } + + #[test] + fn custom_command_prompt_can_be_disabled() { + let parsed: KeybindingConfig = + serde_yaml::from_str("universal:\n customCommandPrompt: ''").expect("valid yaml"); + assert!(parsed.universal.custom_command_prompt.is_empty()); + assert_eq!(parse_key(&parsed.universal.custom_command_prompt), None); + assert_eq!(parsed.universal.start_search, "/"); + } + #[test] fn test_tree_keybindings_defaults_and_deserialization() { let default_config = KeybindingConfig::default(); diff --git a/src/gui/controller/custom_commands.rs b/src/gui/controller/custom_commands.rs index f456a95..a5b487c 100644 --- a/src/gui/controller/custom_commands.rs +++ b/src/gui/controller/custom_commands.rs @@ -6,7 +6,12 @@ use crate::config::user_config::CustomCommand; use crate::gui::Gui; use crate::gui::context::ContextId; use crate::gui::popup::{MessageKind, PopupState}; -use crate::os::cmd::CmdBuilder; +use crate::os::shell::{ShellJob, ShellResult}; + +pub(in crate::gui) struct RunningCommand { + job: ShellJob, + show_output: bool, +} /// Try to handle a key as a custom command. Returns Ok(true) if handled. pub fn try_handle_key(gui: &mut Gui, key: KeyEvent) -> Result { @@ -68,69 +73,204 @@ fn execute_custom_command(gui: &mut Gui, cmd: &CustomCommand) -> Result<()> { } fn run_command(gui: &mut Gui, command: &str, show_output: bool) -> Result<()> { - let shell = std::env::var("SHELL").unwrap_or_else(|_| "sh".to_string()); - let is_zsh = shell.ends_with("zsh"); - let is_bash = shell.ends_with("bash"); - - // Create a temporary script file to execute aliases/functions cleanly without TTY hang - let temp_dir = std::env::temp_dir(); - let temp_file = temp_dir.join(format!( - "lazygitrs-cmd-{}-{}.sh", - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() - )); - - let mut script_content = String::new(); - if is_zsh { - script_content.push_str( - "#!/bin/zsh\nsetopt aliases\n[ -f ~/.zshrc ] && source ~/.zshrc >/dev/null 2>&1\n", - ); - } else if is_bash { - script_content.push_str( - "#!/bin/bash\nshopt -s expand_aliases\n[ -f ~/.bash_aliases ] && source ~/.bash_aliases >/dev/null 2>&1\n[ -f ~/.bashrc ] && source ~/.bashrc >/dev/null 2>&1\n", - ); - } else { - script_content.push_str("#!/bin/sh\n"); - } - script_content.push_str(command); - script_content.push('\n'); - - std::fs::write(&temp_file, &script_content)?; + start_command(gui, command, show_output, false) +} - // Execute the temporary script with null stdin - let result = CmdBuilder::new(&shell) - .arg(&temp_file.to_string_lossy().to_string()) - .stdin(String::new()) - .cwd_path(gui.git.repo_path()) - .run(); +fn start_command( + gui: &mut Gui, + command: &str, + show_output: bool, + shell_prompt: bool, +) -> Result<()> { + anyhow::ensure!( + gui.shell_command_job.is_none(), + "A shell command is already running" + ); + let shell = std::env::var_os("SHELL") + .filter(|shell| !shell.is_empty()) + .unwrap_or_else(|| "sh".into()); + let job = ShellJob::spawn(gui.git.repo_path(), command, &shell, shell_prompt)?; + append_log(gui, format!("$ {command}")); + gui.shell_command_job = Some(RunningCommand { job, show_output }); + gui.popup = PopupState::Loading { + title: "Shell command".into(), + message: "Esc cancel · 5m timeout".into(), + }; + Ok(()) +} - // Clean up temporary file - let _ = std::fs::remove_file(&temp_file); +fn append_log(gui: &Gui, message: String) { + if let Ok(mut log) = gui.command_log.lock() { + log.push(message); + let excess = log.len().saturating_sub(100); + log.drain(..excess); + } +} - let result = result?; +pub fn cancel_running_command(gui: &mut Gui) { + if let Some(command) = &gui.shell_command_job { + command.job.cancel(); + if let PopupState::Loading { message, .. } = &mut gui.popup { + *message = "Cancelling command…".into(); + } + } +} - if let Ok(mut log) = gui.command_log.lock() { - log.push(format!("$ {}", command)); +pub fn receive_command_result(gui: &mut Gui) { + let Some(outcome) = gui + .shell_command_job + .as_mut() + .and_then(|command| command.job.try_result()) + else { + return; + }; + let command = gui.shell_command_job.take().unwrap(); + gui.popup = PopupState::None; + // Even failures/cancellation can leave repository changes behind. + gui.needs_refresh = true; + gui.needs_diff_refresh = true; + if gui.diff_mode.active { + refresh_comparison(gui); + } + match outcome { + Ok(result) => { + let (title, message, kind) = result_message(&result); + // The shared log is also rendered every frame; retain a bounded + // preview rather than up to two MiB per entry. Full output remains + // available in the scrollable popup. + let mut end = message.len().min(16 * 1024); + while !message.is_char_boundary(end) { + end -= 1; + } + let suffix = if end < message.len() { + "\n[log output truncated; see command popup]" + } else { + "" + }; + append_log(gui, format!("{title}: {}{suffix}", &message[..end])); + if command.show_output || !result.success { + gui.popup = PopupState::CommandOutput { + title, + message, + kind, + scroll: 0, + }; + } + } + Err(error) => { + let message = format!("{error:#}"); + append_log(gui, format!("Command failed: {message}")); + gui.show_error("Command failed", error); + } } +} - if show_output && !result.stdout.is_empty() { - gui.popup = PopupState::Message { - title: "Command output".to_string(), - message: result.stdout_trimmed().to_string(), - kind: MessageKind::Info, - }; - } else if !result.success { - gui.popup = PopupState::Message { - title: "Command failed".to_string(), - message: result.stderr.trim().to_string(), - kind: MessageKind::Error, +/// Refresh refs after mutations without throwing away comparison navigation. +fn refresh_comparison(gui: &mut Gui) { + let focus = gui.diff_mode.focus; + let sidebar_focus = gui.diff_mode.sidebar_focus; + let source = gui.diff_mode.diff_source; + let files_commit = gui.diff_mode.files_commit.clone(); + let selected_commit = gui.diff_mode.selected_commit().map(|c| c.hash.clone()); + let selected_file = if gui.diff_mode.show_tree { + gui.diff_mode + .tree_nodes + .get(gui.diff_mode.diff_files_selected) + .map(|n| n.path.clone()) + } else { + gui.diff_mode + .diff_files + .get(gui.diff_mode.diff_files_selected) + .map(|f| f.name.clone()) + }; + let collapsed = gui.diff_mode.collapsed_dirs.clone(); + let search_query = gui.diff_mode.file_search_query.clone(); + let search_textarea = gui.diff_mode.file_search_textarea.clone(); + let _ = super::diff_mode::reload_diff_files(gui); + if let Some(hash) = files_commit { + if let Ok(files) = gui.git.commit_files(&hash) { + gui.diff_mode.diff_files = files; + gui.diff_mode.files_commit = Some(hash); + } + } + gui.diff_mode.collapsed_dirs = collapsed; + super::diff_mode::update_diff_mode_tree(gui); + if let Some(hash) = selected_commit { + if let Some(index) = gui.diff_mode.commits.iter().position(|c| c.hash == hash) { + gui.diff_mode.commits_selected = index; + } + } + if let Some(path) = selected_file { + let index = if gui.diff_mode.show_tree { + gui.diff_mode.tree_nodes.iter().position(|n| n.path == path) + } else { + gui.diff_mode.diff_files.iter().position(|f| f.name == path) }; + gui.diff_mode.diff_files_selected = index.unwrap_or(0); } + gui.diff_mode.set_focus(focus); + gui.diff_mode.sidebar_focus = sidebar_focus; + gui.diff_mode.diff_source = source; + gui.diff_mode.file_search_query = search_query; + gui.diff_mode.file_search_textarea = search_textarea; + gui.diff_mode.update_file_search_matches(); +} - gui.needs_refresh = true; +fn result_message(result: &ShellResult) -> (String, String, MessageKind) { + let title = if result.cancelled { + "Command cancelled".into() + } else if result.timed_out { + "Command timed out (5 minutes)".into() + } else if result.success { + "Command output (exit 0)".into() + } else if let Some(code) = result.exit_code { + format!("Command failed (exit {code})") + } else { + "Command failed (terminated by signal)".into() + }; + // Raw terminal controls must never reach the popup or copy/log view. + let clean = |text: &str| { + String::from_utf8_lossy(&strip_ansi_escapes::strip(text.replace('\t', " "))) + .chars() + .filter(|c| !c.is_control() || matches!(c, '\n' | '\t')) + .collect::() + }; + let stdout = clean(&result.stdout); + let stderr = clean(&result.stderr); + let message = match (stdout.is_empty(), stderr.is_empty()) { + (true, true) => "No output.".into(), + (false, true) => stdout, + (true, false) => stderr, + (false, false) => format!("{stdout}\n--- stderr ---\n{stderr}"), + }; + let kind = if result.success { + MessageKind::Info + } else { + MessageKind::Error + }; + (title, message, kind) +} + +/// Explicit universal binding takes precedence over panel shortcuts, not text entry. +pub fn try_handle_prompt_key(gui: &mut Gui, key: KeyEvent) -> Result { + if super::super::matches_key( + key, + &gui.config + .user_config + .keybinding + .universal + .custom_command_prompt, + ) { + open_custom_command_prompt(gui)?; + return Ok(true); + } + Ok(false) +} + +pub fn confirm_shell_prompt(gui: &mut Gui, input: &str) -> Result<()> { + if !input.trim().is_empty() { + start_command(gui, input, true, true)?; + } Ok(()) } @@ -209,17 +349,68 @@ fn context_id_to_name(ctx: ContextId) -> &'static str { } pub fn open_custom_command_prompt(gui: &mut Gui) -> Result<()> { - gui.popup = PopupState::Input { - title: "Execute Shell Command".to_string(), - textarea: crate::gui::popup::make_textarea(""), - on_confirm: Box::new(move |gui, input| { - if !input.trim().is_empty() { - run_command(gui, &input, true)?; - } - Ok(()) - }), - is_commit: false, - confirm_focused: false, + gui.popup = PopupState::ShellCommand { + textarea: crate::gui::popup::make_textarea( + "Command runs in repository; Enter executes, Esc cancels", + ), }; Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + fn outcome(success: bool, stdout: &str, stderr: &str) -> ShellResult { + ShellResult { + stdout: stdout.into(), + stderr: stderr.into(), + success, + exit_code: Some(if success { 0 } else { 7 }), + cancelled: false, + timed_out: false, + } + } + + #[test] + fn command_failure_takes_precedence_over_nonempty_stdout() { + let (title, message, kind) = + result_message(&outcome(false, "partial result", "failure reason")); + assert_eq!(title, "Command failed (exit 7)"); + assert_eq!(kind, MessageKind::Error); + assert!(message.contains("partial result")); + assert!(message.contains("failure reason")); + } + + #[test] + fn stderr_only_success_and_no_output_are_reported() { + let (title, message, kind) = result_message(&outcome(true, "", "warning")); + assert_eq!(title, "Command output (exit 0)"); + assert_eq!(kind, MessageKind::Info); + assert_eq!(message, "warning"); + assert_eq!(result_message(&outcome(true, "", "")).1, "No output."); + assert_eq!( + result_message(&outcome(false, "", "")).2, + MessageKind::Error + ); + } + + #[test] + fn cancellation_timeout_and_signal_have_explicit_status() { + let mut result = outcome(false, "", ""); + result.cancelled = true; + assert_eq!(result_message(&result).0, "Command cancelled"); + result.cancelled = false; + result.timed_out = true; + assert!(result_message(&result).0.contains("timed out")); + result.timed_out = false; + result.exit_code = None; + assert!(result_message(&result).0.contains("signal")); + } + + #[test] + fn shell_output_strips_terminal_controls_but_keeps_lines() { + let message = result_message(&outcome(true, "\x1b[31mred\x1b[0m\nline\tvalue\x07\r", "")).1; + assert_eq!(message, "red\nline value"); + } +} diff --git a/src/gui/controller/diff_mode.rs b/src/gui/controller/diff_mode.rs index c943d9b..a08e512 100644 --- a/src/gui/controller/diff_mode.rs +++ b/src/gui/controller/diff_mode.rs @@ -120,6 +120,15 @@ pub fn handle_key(gui: &mut Gui, key: KeyEvent) -> Result<()> { return handle_file_search_key(gui, key); } + // Keep all text entry local before dispatching configurable shortcuts. + if gui.diff_mode.focus == DiffModeFocus::DiffExploration && gui.diff_view.search_active { + return handle_diff_search_key(gui, key); + } + + if super::custom_commands::try_handle_prompt_key(gui, key)? { + return Ok(()); + } + // Ctrl-F: same-context grep dialog over all hunk contents. if super::diff_grep::is_diff_grep_key(key) { return super::diff_grep::open_diff_grep_picker(gui); @@ -137,10 +146,6 @@ pub fn handle_key(gui: &mut Gui, key: KeyEvent) -> Result<()> { return Ok(()); } - // Keep text-entry shortcuts local, including diff-content search. - if gui.diff_mode.focus == DiffModeFocus::DiffExploration && gui.diff_view.search_active { - return handle_diff_search_key(gui, key); - } if gui.try_handle_layout_key(key) { return Ok(()); } @@ -843,7 +848,7 @@ pub fn reload_diff_files(gui: &mut Gui) -> Result<()> { Ok(()) } -fn update_diff_mode_tree(gui: &mut Gui) { +pub(super) fn update_diff_mode_tree(gui: &mut Gui) { if gui.diff_mode.show_tree { gui.diff_mode.tree_nodes = crate::model::file_tree::build_commit_file_tree( &gui.diff_mode.diff_files, @@ -1032,7 +1037,7 @@ pub fn maybe_request_diff(gui: &mut Gui, generation: u64, diff_key: String) { } fn show_diff_mode_command_palette(gui: &mut Gui) { - let diff_mode_section = CommandSection { + let mut diff_mode_section = CommandSection { title: "Compare / Diff Mode".into(), entries: vec![ CommandEntry::keybinding("q".into(), "Exit diff mode".into()), @@ -1130,12 +1135,26 @@ fn show_diff_mode_command_palette(gui: &mut Gui) { "/".into(), "Search files, loaded commits, or diff content".into(), ), - CommandEntry::keybinding("n/N".into(), "Next / previous search match".into()), - CommandEntry::keybinding("".into(), "Grep diff contents".into()), - CommandEntry::keybinding("y".into(), "Copy to clipboard".into()), - CommandEntry::keybinding("?".into(), "Show command palette".into()), ], }; + let prompt_key = &gui + .config + .user_config + .keybinding + .universal + .custom_command_prompt; + if !prompt_key.trim().is_empty() { + diff_mode_section.entries.push(CommandEntry::keybinding( + prompt_key.clone(), + "Execute shell command".into(), + )); + } + diff_mode_section.entries.extend([ + CommandEntry::keybinding("n/N".into(), "Next / previous search match".into()), + CommandEntry::keybinding("".into(), "Grep diff contents".into()), + CommandEntry::keybinding("y".into(), "Copy to clipboard".into()), + CommandEntry::keybinding("?".into(), "Show command palette".into()), + ]); let combobox_section = CommandSection { title: "Combobox (while editing A or B)".into(), @@ -1197,6 +1216,160 @@ mod tests { } } + fn prompt_test_gui() -> (TempRepo, Gui) { + let path = std::env::temp_dir().join(format!( + "lazygitrs-compare-prompt-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&path).unwrap(); + let repo = TempRepo(path); + let output = Command::new("git") + .current_dir(&repo.0) + .args(["init", "-b", "main"]) + .output() + .unwrap(); + assert!(output.status.success(), "{:?}", output); + let config = AppConfig { + debug: false, + version: String::new(), + user_config: UserConfig::default(), + app_state: AppState::default(), + config_dir: repo.0.clone(), + state_dir: repo.0.clone(), + state_path: repo.0.join("state.yml"), + }; + let gui = Gui::new(config, GitCommands::new(&repo.0).unwrap(), None, false).unwrap(); + (repo, gui) + } + + #[test] + fn compare_shell_prompt_takes_priority_over_shortcuts_unless_disabled() { + let (_repo, mut gui) = prompt_test_gui(); + for binding in [":", "q", "?", "/", "", "+", "", "1"] { + Arc::get_mut(&mut gui.config) + .unwrap() + .user_config + .keybinding + .universal + .custom_command_prompt = binding.into(); + for focus in [ + DiffModeFocus::SelectorA, + DiffModeFocus::SelectorB, + DiffModeFocus::CommitFiles, + DiffModeFocus::Commits, + DiffModeFocus::DiffExploration, + ] { + gui.popup = PopupState::None; + gui.diff_mode.enter(false); + gui.diff_mode.set_focus(focus); + handle_key(&mut gui, parse_key(binding).unwrap()).unwrap(); + assert!( + matches!(gui.popup, PopupState::ShellCommand { .. }), + "{binding} {focus:?}" + ); + assert!(gui.diff_mode.active); + assert_eq!(gui.diff_mode.focus, focus); + assert_eq!(gui.screen_mode, crate::gui::ScreenMode::Normal); + } + } + Arc::get_mut(&mut gui.config) + .unwrap() + .user_config + .keybinding + .universal + .custom_command_prompt + .clear(); + gui.popup = PopupState::None; + handle_key(&mut gui, parse_key(":").unwrap()).unwrap(); + assert!(gui.popup == PopupState::None); + handle_key(&mut gui, parse_key("q").unwrap()).unwrap(); + assert!(!gui.diff_mode.active); + } + + #[test] + fn compare_text_entry_takes_priority_over_shell_prompt_and_shortcuts() { + let (_repo, mut gui) = prompt_test_gui(); + for binding in [":", "q", "?", ""] { + Arc::get_mut(&mut gui.config) + .unwrap() + .user_config + .keybinding + .universal + .custom_command_prompt = binding.into(); + for input_mode in ["ref A", "ref B", "file search", "content search"] { + gui.diff_mode.enter(false); + gui.diff_view = DiffViewState::default(); + match input_mode { + "ref A" => gui.diff_mode.start_editing(DiffModeSelector::A), + "ref B" => gui.diff_mode.start_editing(DiffModeSelector::B), + "file search" => { + gui.diff_mode.set_focus(DiffModeFocus::CommitFiles); + gui.diff_mode.file_search_active = true; + gui.diff_mode.file_search_textarea = + Some(tui_textarea::TextArea::default()); + } + "content search" => { + gui.diff_mode.set_focus(DiffModeFocus::DiffExploration); + gui.diff_view.start_search(); + } + _ => unreachable!(), + } + handle_key(&mut gui, parse_key(binding).unwrap()).unwrap(); + assert!(gui.popup == PopupState::None, "{binding} {input_mode}"); + assert!(gui.diff_mode.active); + if binding != "" { + let text = match input_mode { + "ref A" | "ref B" => gui.diff_mode.query_text(), + "file search" => gui.diff_mode.file_search_query.clone(), + "content search" => gui.diff_view.search_query.clone(), + _ => unreachable!(), + }; + assert_eq!(text, binding, "{input_mode}"); + } + } + } + } + + #[test] + fn compare_palette_lists_only_enabled_shell_prompt_after_text_search() { + use crate::gui::popup::CommandAction; + let (_repo, mut gui) = prompt_test_gui(); + for binding in [":", "", "", " "] { + Arc::get_mut(&mut gui.config) + .unwrap() + .user_config + .keybinding + .universal + .custom_command_prompt = binding.into(); + show_diff_mode_command_palette(&mut gui); + let PopupState::CommandPalette { sections, .. } = &gui.popup else { + panic!("expected compare command palette"); + }; + let entries = §ions[0].entries; + let prompt = entries + .iter() + .position(|entry| entry.description == "Execute shell command"); + if binding.trim().is_empty() { + assert!(prompt.is_none()); + } else { + let prompt = prompt.expect("enabled prompt entry"); + assert_eq!(entries[prompt].key, binding); + assert_eq!( + entries[prompt].action, + CommandAction::Dispatch(parse_key(binding).unwrap()) + ); + assert_eq!( + entries[prompt - 1].description, + "Search files, loaded commits, or diff content" + ); + } + } + } + #[test] fn compare_controller_previews_commits_opens_files_and_routes_mouse() { let path = std::env::temp_dir().join(format!( diff --git a/src/gui/mod.rs b/src/gui/mod.rs index b80db26..12dcc46 100644 --- a/src/gui/mod.rs +++ b/src/gui/mod.rs @@ -21,6 +21,7 @@ use crossterm::terminal::{self, EnterAlternateScreen, LeaveAlternateScreen}; use crossterm::{Command, cursor, execute}; use ratatui::Terminal; use ratatui::backend::CrosstermBackend; +use ratatui::layout::Rect; use crate::config::keybindings::parse_key; use crate::config::{AppConfig, AppState}; @@ -586,6 +587,7 @@ pub struct Gui { pub context_mgr: ContextManager, pub layout: LayoutState, pub popup: PopupState, + shell_command_job: Option, pub diff_view: DiffViewState, /// Cached graph layouts used to render only the visible commit rows. commit_list_cache: presentation::commits::CommitListCache, @@ -1072,6 +1074,7 @@ impl Gui { context_mgr, layout: LayoutState::default(), popup: PopupState::None, + shell_command_job: None, diff_view: { let mut dv = DiffViewState::new(); dv.wrap = diff_line_wrap; @@ -1193,6 +1196,7 @@ impl Gui { let mut keyboard_enhanced = keyboard_enhanced; let result = self.main_loop(&mut terminal, &input, &mut keyboard_enhanced); + self.shell_command_job.take(); // Drop cancels and reaps before leaving the TUI. self.diff_preview_cache.clear(); self.diff_view.reset_keep_prefs(); self.displayed_diff_key.clear(); @@ -1440,6 +1444,7 @@ impl Gui { self.maybe_start_auto_fetch(); // Check for completed background menu item operations + controller::custom_commands::receive_command_result(self); self.receive_menu_async_results(); // Advance spinner animation @@ -1713,7 +1718,8 @@ impl Gui { } fn input_wait_timeout(&self) -> Duration { - if self.ai_commit_generation_active() + if self.shell_command_job.is_some() + || self.ai_commit_generation_active() || self.remote_op_label.is_some() || self.needs_refresh || self.diff_loading @@ -3575,6 +3581,14 @@ impl Gui { fn handle_key(&mut self, key: KeyEvent) -> Result<()> { self.pending_startup_head = false; + if self.shell_command_job.is_some() { + if key.code == KeyCode::Esc + || (key.code == KeyCode::Char('c') && key.modifiers.contains(KeyModifiers::CONTROL)) + { + controller::custom_commands::cancel_running_command(self); + } + return Ok(()); + } if self.handle_ai_commit_cancel_key(key) { return Ok(()); } @@ -3600,6 +3614,14 @@ impl Gui { return Ok(()); } + // Text entry owns its characters, even when the prompt binding is remapped. + if !self.diff_mode.active + && !(self.diff_focused && self.diff_view.search_active) + && controller::custom_commands::try_handle_prompt_key(self, key)? + { + return Ok(()); + } + // Rebase mode takes priority over everything if self.rebase_mode.active { return controller::rebase_mode::handle_key(self, key); @@ -4398,6 +4420,13 @@ impl Gui { } fn handle_paste(&mut self, data: String) { + if self.shell_command_job.is_some() { + return; + } + if let PopupState::ShellCommand { textarea } = &mut self.popup { + textarea.insert_str(&data.replace("\r\n", "\n").replace('\r', "\n")); + return; + } if data.is_empty() { return; } @@ -4563,6 +4592,47 @@ impl Gui { self.popup = PopupState::None; } } + PopupState::ShellCommand { textarea } => { + if key.code == KeyCode::Esc { + self.popup = PopupState::None; + } else if key.code == KeyCode::Enter { + let command = textarea.lines().join("\n"); + self.popup = PopupState::None; + controller::custom_commands::confirm_shell_prompt(self, &command)?; + } else if let PopupState::ShellCommand { textarea } = &mut self.popup { + textarea_input(textarea, key); + } + } + PopupState::CommandOutput { .. } => { + if matches!(key.code, KeyCode::Esc | KeyCode::Enter | KeyCode::Char('q')) { + self.popup = PopupState::None; + } else if let PopupState::CommandOutput { + message, scroll, .. + } = &mut self.popup + { + let max = views::command_output_max_scroll( + message, + Rect::new(0, 0, self.layout.width, self.layout.height), + ); + let page = (usize::from(self.layout.height) * 4 / 5) + .saturating_sub(3) + .max(1); + match key.code { + KeyCode::Char('j') | KeyCode::Down => { + *scroll = scroll.saturating_add(1).min(max) + } + KeyCode::Char('k') | KeyCode::Up => *scroll = scroll.saturating_sub(1), + KeyCode::PageDown => *scroll = scroll.saturating_add(page).min(max), + KeyCode::PageUp => *scroll = scroll.saturating_sub(page), + KeyCode::Home | KeyCode::Char('g') => *scroll = 0, + KeyCode::End | KeyCode::Char('G') => *scroll = max, + KeyCode::Char('y') => { + let _ = Platform::copy_to_clipboard(message); + } + _ => {} + } + } + } PopupState::Message { .. } => { // Any key dismisses the message self.popup = PopupState::None; @@ -5822,7 +5892,7 @@ impl Gui { let active = self.context_mgr.active(); // Universal keybindings - let universal = CommandSection { + let mut universal = CommandSection { title: "Universal".into(), entries: vec![ CommandEntry::keybinding(kb.universal.quit.clone(), "Quit".into()), @@ -5900,6 +5970,13 @@ impl Gui { ], }; + if !kb.universal.custom_command_prompt.is_empty() { + universal.entries.push(CommandEntry::keybinding( + kb.universal.custom_command_prompt.clone(), + "Execute shell command".into(), + )); + } + // Context-specific keybindings let context_section = match active { ContextId::Files => CommandSection { @@ -6973,6 +7050,9 @@ impl Gui { } fn handle_mouse(&mut self, mouse: MouseEvent) { + if self.shell_command_job.is_some() { + return; + } use crossterm::event::{KeyModifiers, MouseButton, MouseEventKind}; if !self.config.user_config.gui.mouse_events { @@ -9418,6 +9498,215 @@ mod terminal_mouse_tests { assert_eq!(keys.len(), 8); } + fn wait_for_shell_command(gui: &mut Gui) { + let deadline = Instant::now() + Duration::from_secs(5); + while gui.shell_command_job.is_some() { + controller::custom_commands::receive_command_result(gui); + assert!(Instant::now() < deadline, "shell job did not finish"); + std::thread::sleep(Duration::from_millis(5)); + } + } + + fn start_test_custom_command(gui: &mut Gui, command: &str, show_output: bool) { + let config = Arc::get_mut(&mut gui.config).unwrap(); + config.user_config.custom_commands = vec![crate::config::user_config::CustomCommand { + key: "".into(), + command: command.into(), + show_output, + ..Default::default() + }]; + gui.handle_key(parse_key("").unwrap()).unwrap(); + } + + #[test] + fn shell_prompt_binding_works_in_lists_and_diff_focus_and_respects_remapping() { + let repo = TempRepo::new("shell-prompt-bindings"); + let mut gui = tree_test_gui(&repo); + for context in [ + ContextId::Files, + ContextId::Commits, + ContextId::CommitFiles, + ContextId::StashFiles, + ContextId::Branches, + ] { + gui.context_mgr.set_active(context); + for focused in [false, true] { + gui.diff_focused = focused; + for binding in [":", "q", "?", "", ""] { + Arc::get_mut(&mut gui.config) + .unwrap() + .user_config + .keybinding + .universal + .custom_command_prompt = binding.into(); + gui.handle_key(parse_key(binding).unwrap()).unwrap(); + assert!( + matches!(gui.popup, PopupState::ShellCommand { .. }), + "{context:?}, {focused}, {binding}" + ); + assert!(!gui.should_quit); + gui.handle_key(parse_key("").unwrap()).unwrap(); + } + } + } + Arc::get_mut(&mut gui.config) + .unwrap() + .user_config + .keybinding + .universal + .custom_command_prompt + .clear(); + gui.diff_focused = false; + gui.context_mgr.set_active(ContextId::Files); + gui.handle_key(parse_key(":").unwrap()).unwrap(); + assert!(gui.popup == PopupState::None); + gui.show_command_palette(); + let PopupState::CommandPalette { sections, .. } = &gui.popup else { + panic!("palette"); + }; + assert!( + !sections + .iter() + .flat_map(|s| &s.entries) + .any(|e| e.description == "Execute shell command") + ); + } + + #[test] + fn shell_prompt_does_not_steal_search_or_popup_input() { + let repo = TempRepo::new("shell-text-priority"); + let mut gui = tree_test_gui(&repo); + gui.search_active = true; + gui.search_textarea = Some(popup::make_textarea("")); + gui.handle_key(parse_key(":").unwrap()).unwrap(); + assert!(gui.popup == PopupState::None); + assert_eq!(gui.search_query, ":"); + gui.search_active = false; + gui.diff_focused = true; + gui.diff_view.search_active = true; + gui.handle_key(parse_key(":").unwrap()).unwrap(); + assert!(gui.popup == PopupState::None); + gui.diff_view.search_active = false; + gui.popup = PopupState::Input { + title: "Other input".into(), + textarea: popup::make_textarea(""), + on_confirm: Box::new(|_, _| Ok(())), + is_commit: false, + confirm_focused: false, + }; + gui.handle_key(parse_key(":").unwrap()).unwrap(); + assert!( + matches!(&gui.popup, PopupState::Input { textarea, .. } if textarea.lines().join("") == ":") + ); + } + + #[test] + fn shell_prompt_keeps_literal_paste_through_resize_and_blank_submit_is_noop() { + let repo = TempRepo::new("shell-literal-input"); + let mut gui = tree_test_gui(&repo); + gui.handle_key(parse_key(":").unwrap()).unwrap(); + let command = "printf '%s' 'multiple spaces; $quoted'\n# pasted newline"; + gui.handle_paste(command.into()); + gui.handle_resize(25, 8); + assert!( + matches!(&gui.popup, PopupState::ShellCommand { textarea } if textarea.lines().join("\n") == command) + ); + gui.handle_key(parse_key("").unwrap()).unwrap(); + assert!(gui.shell_command_job.is_none()); + gui.handle_key(parse_key(":").unwrap()).unwrap(); + gui.handle_paste(" ".into()); + gui.handle_key(parse_key("").unwrap()).unwrap(); + assert!(gui.popup == PopupState::None); + assert!(gui.shell_command_job.is_none()); + } + + #[test] + fn custom_command_is_nonblocking_cancelable_and_refreshes_after_cancellation() { + let repo = TempRepo::new("shell-cancellation"); + let mut gui = tree_test_gui(&repo); + let started = Instant::now(); + start_test_custom_command(&mut gui, "sleep 30", true); + assert!(started.elapsed() < Duration::from_secs(1)); + assert!(gui.shell_command_job.is_some()); + assert_eq!(gui.input_wait_timeout(), Duration::from_millis(16)); + gui.handle_key(parse_key("q").unwrap()).unwrap(); + assert!(!gui.should_quit); + gui.handle_key(parse_key("").unwrap()).unwrap(); + wait_for_shell_command(&mut gui); + assert!(gui.needs_refresh); + assert!(gui.needs_diff_refresh); + assert!( + matches!(&gui.popup, PopupState::CommandOutput { title, kind: MessageKind::Error, .. } if title == "Command cancelled") + ); + } + + #[test] + fn custom_command_logs_and_reports_failure_stdout_and_stderr_without_losing_filters() { + let repo = TempRepo::new("shell-output"); + let mut gui = tree_test_gui(&repo); + gui.commit_path_filter = Some("src".into()); + gui.commit_author_filter = vec!["Example".into()]; + gui.screen_mode = ScreenMode::Half; + start_test_custom_command( + &mut gui, + "printf partial; printf warning >&2; exit 7", + false, + ); + wait_for_shell_command(&mut gui); + assert!( + matches!(&gui.popup, PopupState::CommandOutput { title, message, kind: MessageKind::Error, .. } + if title.contains("exit 7") && message.contains("partial") && message.contains("warning")) + ); + let log = gui.command_log.lock().unwrap(); + assert!(log.iter().any(|line| line.contains("$ printf partial"))); + assert!( + log.iter() + .any(|line| line.contains("Command failed (exit 7)")) + ); + drop(log); + assert_eq!(gui.commit_path_filter.as_deref(), Some("src")); + assert_eq!(gui.commit_author_filter, vec!["Example"]); + assert_eq!(gui.screen_mode, ScreenMode::Half); + gui.handle_key(parse_key("").unwrap()).unwrap(); + start_test_custom_command(&mut gui, "printf changed > created-by-command", false); + wait_for_shell_command(&mut gui); + assert!(gui.popup == PopupState::None); + assert_eq!( + std::fs::read_to_string(repo.path().join("created-by-command")).unwrap(), + "changed" + ); + assert!(gui.needs_refresh); + } + + #[test] + fn command_output_can_scroll_to_last_line_and_back() { + let repo = TempRepo::new("shell-scroll"); + let mut gui = tree_test_gui(&repo); + gui.layout.update_size(80, 24); + let message = (0..100).map(|i| format!("line {i}\n")).collect::(); + gui.popup = PopupState::CommandOutput { + title: "output".into(), + message: message.clone(), + kind: MessageKind::Info, + scroll: 0, + }; + gui.handle_key(parse_key("G").unwrap()).unwrap(); + let max = views::command_output_max_scroll(&message, Rect::new(0, 0, 80, 24)); + assert!(matches!(gui.popup, PopupState::CommandOutput { scroll, .. } if scroll == max)); + gui.handle_key(parse_key("g").unwrap()).unwrap(); + assert!(matches!( + gui.popup, + PopupState::CommandOutput { scroll: 0, .. } + )); + gui.handle_key(parse_key("j").unwrap()).unwrap(); + assert!(matches!( + gui.popup, + PopupState::CommandOutput { scroll: 1, .. } + )); + gui.handle_key(parse_key("").unwrap()).unwrap(); + assert!(gui.popup == PopupState::None); + } + struct TempRepo { path: PathBuf, } diff --git a/src/gui/popup.rs b/src/gui/popup.rs index 6c58f0f..6f4c735 100644 --- a/src/gui/popup.rs +++ b/src/gui/popup.rs @@ -544,6 +544,16 @@ pub enum PopupState { message: String, kind: MessageKind, }, + /// Literal shell input: never soft-wrap or join away user newlines. + ShellCommand { + textarea: TextArea<'static>, + }, + CommandOutput { + title: String, + message: String, + kind: MessageKind, + scroll: usize, + }, /// Shown while a background operation (like AI commit generation) is running. #[allow(dead_code)] Loading { diff --git a/src/gui/presentation/diff_mode.rs b/src/gui/presentation/diff_mode.rs index e782f7a..0fbc82f 100644 --- a/src/gui/presentation/diff_mode.rs +++ b/src/gui/presentation/diff_mode.rs @@ -177,6 +177,57 @@ mod tests { } } + #[test] + fn compare_footer_shows_configured_shell_prompt_in_list_and_diff_contexts() { + for focus in [ + DiffModeFocus::SelectorA, + DiffModeFocus::SelectorB, + DiffModeFocus::CommitFiles, + DiffModeFocus::Commits, + DiffModeFocus::DiffExploration, + ] { + for tree in [false, true] { + for (binding, hint) in [(":", ":"), ("", "ctrl+x"), ("", ""), (" ", "")] { + let mut kb = crate::config::KeybindingConfig::default(); + kb.universal.custom_command_prompt = binding.into(); + let mut state = DiffModeState::new(); + state.show_tree = tree; + state.set_focus(focus); + let mut terminal = Terminal::new(TestBackend::new(240, 1)).unwrap(); + terminal + .draw(|frame| { + render_status_bar( + frame, + frame.area(), + &state, + &DiffViewState::default(), + &Theme::default(), + &kb, + ) + }) + .unwrap(); + let text = buffer_text(&terminal); + assert_eq!( + text.contains(" shell"), + !hint.is_empty(), + "{focus:?} {tree} {binding}: {text}" + ); + if !hint.is_empty() { + assert!(text.contains(&format!("{hint} shell")), "{text}"); + } + assert_eq!(text.contains(": shell"), binding == ":", "{text}"); + assert_eq!( + text.contains("- fold/unfold"), + tree && focus == DiffModeFocus::CommitFiles, + "{text}" + ); + assert!(!text.contains("details"), "{text}"); + assert!(text.contains("? help"), "{text}"); + } + } + } + } + #[test] fn compare_empty_commit_states_are_distinct() { for (counts, expected) in [ @@ -750,6 +801,8 @@ fn render_status_bar( } let fold_key = crate::gui::views::format_key_hint(&keybindings.universal.fold_directory); + let shell_key = + crate::gui::views::format_key_hint(&keybindings.universal.custom_command_prompt); let hints = if state.editing.is_some() { vec![("Enter", "select"), ("Esc", "cancel"), ("↑↓", "navigate")] } else { @@ -775,6 +828,9 @@ fn render_status_bar( if state.show_tree && state.focus == DiffModeFocus::CommitFiles && !fold_key.is_empty() { hints.push((fold_key.as_str(), "fold/unfold")); } + if !shell_key.is_empty() { + hints.push((shell_key.as_str(), "shell")); + } hints.push(("?", "help")); hints }; diff --git a/src/gui/views.rs b/src/gui/views.rs index d8144da..b7f1b7e 100644 --- a/src/gui/views.rs +++ b/src/gui/views.rs @@ -1508,6 +1508,73 @@ mod tests { } } + #[test] + fn footer_shows_configured_shell_prompt_in_list_and_diff_contexts() { + use super::{ContextId, ContextManager, DiffViewState, KeybindingConfig, Model}; + let mut diff_view = DiffViewState::default(); + diff_view.load("test", "old\n", "new\n"); + for ctx in [ + ContextId::Files, + ContextId::Commits, + ContextId::CommitFiles, + ContextId::BranchCommits, + ContextId::BranchCommitFiles, + ContextId::Reflog, + ContextId::StashFiles, + ] { + for focused in [false, true] { + for (binding, hint) in [(":", ":"), ("", "ctrl+x"), ("", ""), (" ", "")] { + let mut kb = KeybindingConfig::default(); + kb.universal.custom_command_prompt = binding.into(); + let mut ctx_mgr = ContextManager::new(); + ctx_mgr.set_active(ctx); + let mut terminal = Terminal::new(TestBackend::new(240, 1)).unwrap(); + terminal + .draw(|frame| { + super::render_status_bar( + frame, + frame.area(), + &ctx_mgr, + &diff_view, + &Theme::default(), + &Model::default(), + focused, + false, + &kb, + false, + false, + ) + }) + .unwrap(); + let text: String = terminal + .backend() + .buffer() + .content + .iter() + .map(|cell| cell.symbol()) + .collect(); + assert_eq!( + text.contains(" shell"), + !hint.is_empty(), + "{ctx:?} {focused} {binding}: {text}" + ); + if !hint.is_empty() { + assert!(text.contains(&format!("{hint} shell")), "{text}"); + } + assert_eq!(text.contains(": shell"), binding == ":", "{text}"); + if ctx != ContextId::StashFiles { + let toggle = if ctx == ContextId::Files { + "head" + } else { + "files" + }; + assert!(text.contains(&format!("ctrl+g {toggle}")), "{text}"); + } + } + } + } + } + #[test] fn commit_list_footers_omit_details_toggle() { for ctx in [ @@ -1896,6 +1963,68 @@ mod tests { .expect("long popup message should render without panicking"); } + #[test] + fn command_output_popup_scrolls_to_final_line_and_handles_tiny_terminals() { + let message = (0..100) + .map(|i| format!("output-line-{i:03}\n")) + .collect::(); + for (width, height) in [(80, 24), (20, 8), (4, 4), (3, 3), (1, 1)] { + let area = Rect::new(0, 0, width, height); + for last in [false, true] { + let popup = PopupState::CommandOutput { + title: "exit 7".into(), + message: message.clone(), + kind: MessageKind::Error, + scroll: if last { + super::command_output_max_scroll(&message, area) + } else { + 0 + }, + }; + let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap(); + terminal + .draw(|frame| { + render_popup(frame, &popup, area, 0, &Theme::default(), false, false) + }) + .unwrap(); + if width >= 20 { + let buffer = terminal.backend().buffer(); + let text: String = buffer.content().iter().map(|cell| cell.symbol()).collect(); + assert!(text.contains("exit 7"), "{text}"); + let line = if last { + "output-line-099" + } else { + "output-line-000" + }; + assert!(text.contains(line), "{width}x{height}: {text}"); + } + } + } + } + + #[test] + fn literal_shell_prompt_renders_in_small_terminals() { + let mut textarea = crate::gui::popup::make_textarea(""); + textarea.insert_str("printf '%s' 'multiple spaces'\n# next line"); + let popup = PopupState::ShellCommand { textarea }; + for (width, height) in [(80, 24), (20, 8), (4, 4), (1, 1)] { + let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap(); + terminal + .draw(|frame| { + render_popup( + frame, + &popup, + Rect::new(0, 0, width, height), + 0, + &Theme::default(), + false, + false, + ) + }) + .unwrap(); + } + } + fn sample_menu() -> PopupState { PopupState::Menu { title: "Copy".to_string(), @@ -2965,6 +3094,7 @@ fn render_status_bar( let open_log_menu_key = format_key_hint(&keybindings.commits.open_log_menu); let toggle_head_key = format_key_hint(&keybindings.universal.toggle_working_tree_and_head); let fold_key = format_key_hint(&keybindings.universal.fold_directory); + let shell_key = format_key_hint(&keybindings.universal.custom_command_prompt); if active_file_tree && !diff_focused && !fold_key.is_empty() { hints.push((fold_key.as_str(), "fold/unfold")); @@ -3142,6 +3272,9 @@ fn render_status_bar( } // Global hints (always last) + if !shell_key.is_empty() { + hints.push((shell_key.as_str(), "shell")); + } hints.extend([("q", "quit"), ("tab/1-5", "panels"), ("j/k", "nav")]); let key_style = Style::default() @@ -3550,6 +3683,28 @@ pub fn render_loading_overlay( frame.render_widget(widget, popup_rect); } +fn command_output_geometry(area: Rect) -> (Rect, usize) { + let width = (area.width.saturating_mul(9) / 10) + .min(120) + .max(4) + .min(area.width); + let height = (area.height.saturating_mul(4) / 5).max(4).min(area.height); + let rect = Rect::new( + area.x + area.width.saturating_sub(width) / 2, + area.y + area.height.saturating_sub(height) / 2, + width, + height, + ); + (rect, usize::from(height.saturating_sub(3))) +} + +pub(super) fn command_output_max_scroll(message: &str, area: Rect) -> usize { + let (rect, rows) = command_output_geometry(area); + wrap_popup_lines(message, usize::from(rect.width.saturating_sub(2))) + .len() + .saturating_sub(rows) +} + pub fn render_popup( frame: &mut Frame, popup: &PopupState, @@ -3568,6 +3723,72 @@ pub fn render_popup( let x = (area.width.saturating_sub(popup_width)) / 2; match popup { + PopupState::ShellCommand { textarea } => { + // Reuse input styling without introducing soft-wrap mutations. + let input = PopupState::Input { + title: "Execute Shell Command".into(), + textarea: textarea.clone(), + on_confirm: Box::new(|_, _| Ok(())), + is_commit: false, + confirm_focused: false, + }; + render_popup(frame, &input, area, spinner_frame, theme, false, false); + } + PopupState::CommandOutput { + title, + message, + kind, + scroll, + } => { + let (rect, rows) = command_output_geometry(area); + let wrapped = wrap_popup_lines(message, usize::from(rect.width.saturating_sub(2))); + let max = wrapped.len().saturating_sub(rows); + let scroll = (*scroll).min(max); + frame.render_widget(Clear, rect); + let color = if *kind == crate::gui::popup::MessageKind::Error { + Color::Red + } else { + theme.accent_secondary + }; + let block = Block::default() + .borders(Borders::ALL) + .title(format!( + " {title} [{}/{}] ", + scroll + 1, + wrapped.len().max(1) + )) + .border_style(Style::default().fg(color)); + frame.render_widget(block, rect); + let inner = rect.inner(ratatui::layout::Margin { + horizontal: 1, + vertical: 1, + }); + let lines: Vec = wrapped + .iter() + .skip(scroll) + .take(rows) + .map(|line| Line::from(line.as_str())) + .collect(); + frame.render_widget( + Paragraph::new(lines), + Rect::new( + inner.x, + inner.y, + inner.width, + inner.height.saturating_sub(1), + ), + ); + frame.render_widget( + Paragraph::new("j/k scroll · PgUp/PgDn · g/G · y copy · esc/enter close") + .style(Style::default().fg(theme.text_dimmed)), + Rect::new( + inner.x, + inner.y + inner.height.saturating_sub(1), + inner.width, + inner.height.min(1), + ), + ); + } PopupState::Confirm { title, message, .. } => { let inner_width = popup_width.saturating_sub(4) as usize; // borders + padding let wrapped = wrap_popup_lines(message, inner_width); diff --git a/src/os/mod.rs b/src/os/mod.rs index cf1e2ae..6134354 100644 --- a/src/os/mod.rs +++ b/src/os/mod.rs @@ -1,3 +1,4 @@ pub mod cmd; pub mod platform; +pub mod shell; pub mod tty; diff --git a/src/os/shell.rs b/src/os/shell.rs new file mode 100644 index 0000000..8068e61 --- /dev/null +++ b/src/os/shell.rs @@ -0,0 +1,958 @@ +//! Non-interactive, bounded shell jobs. A job owns its child until it is reaped; +//! dropping the job cancels it and waits for the worker to finish cleanup. + +use std::ffi::OsStr; +use std::path::Path; +use std::process::Command; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, mpsc}; +use std::thread::{self, JoinHandle}; +use std::time::Duration; + +use anyhow::{Context, Result, anyhow}; + +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(5 * 60); +const OUTPUT_LIMIT: usize = 1024 * 1024; +const TRUNCATION_NOTICE: &str = "\n[output truncated]\n"; + +#[derive(Debug)] +pub struct ShellResult { + pub stdout: String, + pub stderr: String, + pub success: bool, + pub exit_code: Option, + pub cancelled: bool, + pub timed_out: bool, +} + +pub struct ShellJob { + cancel: Arc, + result: mpsc::Receiver>, + worker: Option>, + result_taken: bool, +} + +impl ShellJob { + /// Start a shell job with a five-minute deadline. Launch/I/O errors are + /// delivered through `try_result`; setup and thread creation errors are + /// returned here. An empty shell is not replaced with a default here. + /// + /// With config loading disabled, use `sh -c`, regardless of `shell`, to + /// preserve custom-command semantics. Otherwise bash/zsh load their rc + /// files; other shells receive native `-c` arguments, not a POSIX wrapper. + /// Remaining processes in the job's group are terminated on shell exit. + /// Currently requires Unix process-group support. + pub fn spawn( + repo: &Path, + command: &str, + shell: &OsStr, + load_shell_config: bool, + ) -> Result { + Self::spawn_with_timeout(repo, command, shell, load_shell_config, DEFAULT_TIMEOUT) + } + + fn spawn_with_timeout( + repo: &Path, + command: &str, + shell: &OsStr, + load_shell_config: bool, + timeout: Duration, + ) -> Result { + Self::spawn_command( + shell_command(repo, command, shell, load_shell_config)?, + timeout, + ) + } + + fn spawn_command(command: Command, timeout: Duration) -> Result { + // Without process-group termination and nonblocking pipes we cannot + // uphold the cancellation/Drop contract. Fail explicitly rather than + // pretending killing only a shell is safe on unsupported platforms. + #[cfg(not(unix))] + { + let _ = (command, timeout); + anyhow::bail!("Shell jobs require Unix process-group support"); + } + + #[cfg(unix)] + { + let cancel = Arc::new(AtomicBool::new(false)); + let worker_cancel = Arc::clone(&cancel); + let (sender, result) = mpsc::channel(); + let worker = thread::Builder::new() + .name("shell-job".into()) + .spawn(move || { + let outcome = unix::run(command, &worker_cancel, timeout); + let _ = sender.send(outcome); + }) + .context("Failed to start shell worker")?; + Ok(Self { + cancel, + result, + worker: Some(worker), + result_taken: false, + }) + } + } + + /// Poll without blocking. The result (including errors) is returned once. + pub fn try_result(&mut self) -> Option> { + if self.result_taken { + return None; + } + let result = match self.result.try_recv() { + Ok(result) => result, + Err(mpsc::TryRecvError::Empty) => return None, + Err(mpsc::TryRecvError::Disconnected) => { + Err(anyhow!("Shell worker stopped unexpectedly")) + } + }; + self.result_taken = true; + Some(result) + } + + /// Request cancellation. Idempotent; the worker terminates the whole + /// process group and reaps the shell before publishing its result. + pub fn cancel(&self) { + self.cancel.store(true, Ordering::Release); + } +} + +impl Drop for ShellJob { + fn drop(&mut self) { + self.cancel(); + if let Some(worker) = self.worker.take() { + let _ = worker.join(); + } + } +} + +// The user command and the absolute repo path are positional arguments, never +// interpolated into these scripts. Save them before sourcing: rc files may +// change positional parameters or cwd. `eval` parses the command AFTER aliases +// and functions have been defined. Stay non-interactive to avoid job-control +// process groups and attempts to acquire the TUI's terminal. +const BASH_WRAPPER: &str = r#" +builtin readonly _lazygitrs_repo="$1" _lazygitrs_command="$2" +builtin shopt -s expand_aliases +if [ -r "$HOME/.bash_aliases" ]; then + builtin source "$HOME/.bash_aliases" +fi +if [ -r "$HOME/.bashrc" ]; then + builtin source "$HOME/.bashrc" +fi +builtin shopt -s expand_aliases +builtin cd -- "$_lazygitrs_repo" || exit +builtin eval -- "$_lazygitrs_command" +"#; + +const ZSH_WRAPPER: &str = r#" +builtin readonly _lazygitrs_repo="$1" _lazygitrs_command="$2" +if [ -r "${ZDOTDIR:-$HOME}/.zshrc" ]; then + builtin source "${ZDOTDIR:-$HOME}/.zshrc" +fi +builtin setopt aliases +builtin cd -- "$_lazygitrs_repo" || exit +builtin eval -- "$_lazygitrs_command" +"#; + +fn shell_command( + repo: &Path, + command: &str, + shell: &OsStr, + load_shell_config: bool, +) -> Result { + let repo = repo + .canonicalize() + .context("Failed to resolve shell job directory")?; + let shell = if load_shell_config { + shell + } else { + OsStr::new("sh") + }; + let mut cmd = Command::new(shell); + match Path::new(shell).file_name().and_then(OsStr::to_str) { + Some("bash") if load_shell_config => { + // Load only the explicit rc file, not an inherited BASH_ENV script + // that runs before we can preserve the positional arguments. + cmd.env_remove("BASH_ENV"); + cmd.args(["--noprofile", "--norc", "-c", BASH_WRAPPER, "lazygitrs"]) + .arg(&repo) + .arg(command); + } + Some("zsh") if load_shell_config => { + // Native non-interactive startup loads .zshenv (including ZDOTDIR). + cmd.args(["-c", ZSH_WRAPPER, "lazygitrs"]) + .arg(&repo) + .arg(command); + } + _ => { + cmd.arg("-c").arg(command); + } + } + cmd.current_dir(repo); + Ok(cmd) +} + +#[cfg(unix)] +mod unix { + use std::io::{self, Read}; + use std::os::fd::AsRawFd; + use std::os::unix::process::CommandExt; + use std::process::{Child, ExitStatus, Stdio}; + use std::time::Instant; + + use super::*; + + const POLL_INTERVAL: Duration = Duration::from_millis(20); + // Even a descendant that deliberately leaves the session cannot keep us + // waiting on its inherited output pipes indefinitely. + const DRAIN_GRACE: Duration = Duration::from_millis(100); + const READS_PER_TURN: usize = 16; + + pub(super) fn run( + mut command: Command, + cancel: &AtomicBool, + timeout: Duration, + ) -> Result { + if cancel.load(Ordering::Acquire) { + return Ok(ShellResult { + stdout: String::new(), + stderr: String::new(), + success: false, + exit_code: None, + cancelled: true, + timed_out: false, + }); + } + + command + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + // SAFETY: setsid is async-signal-safe; this closure allocates nothing + // and touches no locks between fork and exec. The new session also + // gives the shell a process group whose id is its pid, and no tty. + unsafe { + command.pre_exec(|| { + if libc::setsid() == -1 { + return Err(io::Error::last_os_error()); + } + Ok(()) + }); + } + + let started = Instant::now(); + let child = command.spawn().context("Failed to spawn shell")?; + let mut process = Process { + child, + reaped: false, + }; + let mut stdout = Capture::new(process.child.stdout.take().expect("piped stdout"))?; + let mut stderr = Capture::new(process.child.stderr.take().expect("piped stderr"))?; + let mut completion: Option<(ExitStatus, Instant)> = None; + let mut cancelled = false; + let mut timed_out = false; + + loop { + // A bounded slice for EACH pipe keeps a prolific writer from + // starving the other stream, cancellation, or deadline checks. + stdout.drain().context("Failed to read shell stdout")?; + stderr.drain().context("Failed to read shell stderr")?; + + if completion.is_none() { + let exited = process.exited().context("Failed to inspect shell status")?; + if !exited { + cancelled = cancel.load(Ordering::Acquire); + timed_out = !cancelled && started.elapsed() >= timeout; + } + if exited || cancelled || timed_out { + // Clean up background descendants on normal completion + // too. Kill BEFORE reaping so the group id cannot be + // recycled and accidentally signal an unrelated job. + let status = process.finish().context("Failed to terminate/reap shell")?; + completion = Some((status, Instant::now())); + } + } + + let wait = if let Some((status, ended)) = completion { + if stdout.pipe.is_none() && stderr.pipe.is_none() || ended.elapsed() >= DRAIN_GRACE + { + return Ok(ShellResult { + stdout: stdout.finish(), + stderr: stderr.finish(), + success: status.success() && !cancelled && !timed_out, + exit_code: status.code(), + cancelled, + timed_out, + }); + } + POLL_INTERVAL.min(DRAIN_GRACE.saturating_sub(ended.elapsed())) + } else { + POLL_INTERVAL.min(timeout.saturating_sub(started.elapsed())) + }; + poll(&stdout, &stderr, wait).context("Failed to poll shell output")?; + } + } + + /// Owns cleanup on every path, including pipe setup/read failures and + /// unwinding. Nothing after `finish` may signal the now-reusable child pid. + struct Process { + child: Child, + reaped: bool, + } + + impl Process { + fn exited(&self) -> io::Result { + // WNOWAIT observes exit without releasing the leader's pid. Using + // Child::try_wait here would reap it before group cleanup. + let mut info: libc::siginfo_t = unsafe { std::mem::zeroed() }; + let result = unsafe { + libc::waitid( + libc::P_PID, + self.child.id() as libc::id_t, + &mut info, + libc::WEXITED | libc::WNOHANG | libc::WNOWAIT, + ) + }; + if result == -1 { + let error = io::Error::last_os_error(); + if error.kind() == io::ErrorKind::Interrupted { + return Ok(false); + } + return Err(error); + } + Ok(info.si_signo != 0) + } + + fn kill_group(&self) -> io::Result<()> { + // setsid established pgid == pid. Never use a shared/TUI group. + if unsafe { libc::kill(-(self.child.id() as libc::pid_t), libc::SIGKILL) } == -1 { + let error = io::Error::last_os_error(); + if error.raw_os_error() != Some(libc::ESRCH) { + return Err(error); + } + } + Ok(()) + } + + fn finish(&mut self) -> io::Result { + let killed = self.kill_group().or_else(|error| { + // Darwin reports EPERM, not ESRCH, for a group containing + // only its zombie leader. Once the leader has exited, this + // cleanup is best effort; a live leader still reports errors. + #[cfg(target_os = "macos")] + if error.raw_os_error() == Some(libc::EPERM) && self.exited()? { + return Ok(()); + } + Err(error) + }); + // Fallback also ensures the leader is killed if group signalling + // failed. Always reap, even when returning that signalling error. + let _ = self.child.kill(); + let status = self.child.wait()?; + self.reaped = true; + killed?; + Ok(status) + } + } + + impl Drop for Process { + fn drop(&mut self) { + if !self.reaped { + let _ = self.finish(); + } + } + } + + struct Capture

{ + pipe: Option

, + bytes: Vec, + truncated: bool, + } + + impl Capture

{ + fn new(pipe: P) -> io::Result { + let fd = pipe.as_raw_fd(); + let flags = unsafe { libc::fcntl(fd, libc::F_GETFL) }; + if flags == -1 + || unsafe { libc::fcntl(fd, libc::F_SETFL, flags | libc::O_NONBLOCK) } == -1 + { + return Err(io::Error::last_os_error()); + } + Ok(Self { + pipe: Some(pipe), + bytes: Vec::new(), + truncated: false, + }) + } + + fn drain(&mut self) -> io::Result<()> { + let Some(pipe) = &mut self.pipe else { + return Ok(()); + }; + let mut buffer = [0; 8192]; + for _ in 0..READS_PER_TURN { + match pipe.read(&mut buffer) { + Ok(0) => { + self.pipe = None; + break; + } + Ok(count) => { + let retained = count.min(OUTPUT_LIMIT - self.bytes.len()); + self.bytes.extend_from_slice(&buffer[..retained]); + self.truncated |= retained != count; + } + Err(error) if error.kind() == io::ErrorKind::WouldBlock => break, + Err(error) if error.kind() == io::ErrorKind::Interrupted => continue, + Err(error) => return Err(error), + } + } + Ok(()) + } + + fn finish(self) -> String { + // Invalid UTF-8 can expand to replacement characters; cap the + // returned String as well as the underlying raw-byte buffer. + let mut output = String::from_utf8_lossy(&self.bytes).into_owned(); + let mut end = output.len().min(OUTPUT_LIMIT); + while !output.is_char_boundary(end) { + end -= 1; + } + let truncated = self.truncated || self.pipe.is_some() || output.len() > end; + output.truncate(end); + if truncated { + output.push_str(TRUNCATION_NOTICE); + } + output + } + + fn fd(&self) -> libc::c_int { + self.pipe.as_ref().map_or(-1, AsRawFd::as_raw_fd) + } + } + + #[cfg(test)] + #[test] + fn invalid_utf8_is_lossy_and_still_bounded() { + let capture = Capture:: { + pipe: None, + bytes: vec![0xff; OUTPUT_LIMIT], + truncated: false, + }; + let output = capture.finish(); + assert!(output.len() <= OUTPUT_LIMIT + TRUNCATION_NOTICE.len()); + assert!(output.starts_with('\u{fffd}')); + assert!(output.ends_with(TRUNCATION_NOTICE)); + } + + fn poll( + stdout: &Capture, + stderr: &Capture, + timeout: Duration, + ) -> io::Result<()> { + let mut pipes = [stdout.fd(), stderr.fd()].map(|fd| libc::pollfd { + fd, + events: libc::POLLIN, + revents: 0, + }); + let result = unsafe { + libc::poll( + pipes.as_mut_ptr(), + pipes.len() as libc::nfds_t, + timeout.as_millis() as libc::c_int, + ) + }; + if result == -1 { + let error = io::Error::last_os_error(); + if error.kind() != io::ErrorKind::Interrupted { + return Err(error); + } + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::ffi::OsString; + + use super::*; + + #[cfg(not(unix))] + #[test] + fn unsupported_platform_returns_an_error() { + let repo = std::env::current_dir().unwrap(); + assert!(ShellJob::spawn(&repo, ":", OsStr::new("sh"), false).is_err()); + } + + #[test] + fn non_posix_shells_receive_only_native_arguments() { + let repo = std::env::current_dir().unwrap(); + let input = "printf '%s' 'quotes; $variables; $(substitution)'"; + for shell in ["/usr/bin/fish", "/usr/bin/nu", "/bin/dash"] { + let cmd = shell_command(&repo, input, OsStr::new(shell), true).unwrap(); + assert_eq!(cmd.get_program(), shell); + assert_eq!(cmd.get_args().collect::>(), ["-c", input]); + } + } + + #[test] + fn custom_commands_always_use_sh_without_a_wrapper() { + let repo = std::env::current_dir().unwrap(); + let cmd = shell_command(&repo, "echo custom", OsStr::new("fish"), false).unwrap(); + assert_eq!(cmd.get_program(), "sh"); + assert_eq!(cmd.get_args().collect::>(), ["-c", "echo custom"]); + } + + #[test] + fn wrappers_keep_paths_and_commands_in_separate_arguments() { + let repo = std::env::current_dir().unwrap().canonicalize().unwrap(); + let input = "printf '%s' '\"; touch should-not-exist; #'"; + for shell in ["/bin/bash", "/bin/zsh"] { + let cmd = shell_command(&repo, input, OsStr::new(shell), true).unwrap(); + let args = cmd.get_args().map(OsStr::to_os_string).collect::>(); + assert_eq!(args[args.len() - 2], repo.as_os_str()); + assert_eq!(args[args.len() - 1], OsString::from(input)); + assert!(!args[args.len() - 4].to_string_lossy().contains(input)); + } + } + + #[cfg(unix)] + mod execution { + use std::fs; + use std::path::PathBuf; + use std::sync::atomic::AtomicUsize; + use std::time::Instant; + + use super::*; + + struct TestDir(PathBuf); + + impl TestDir { + fn new() -> Self { + static NEXT: AtomicUsize = AtomicUsize::new(0); + let path = std::env::temp_dir().join(format!( + "lazygitrs-shell-{}-{} space's $dir", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + fs::create_dir(&path).unwrap(); + Self(path.canonicalize().unwrap()) + } + + fn spawn(&self, script: &str) -> ShellJob { + ShellJob::spawn(&self.0, script, OsStr::new("/bin/sh"), false).unwrap() + } + } + + impl Drop for TestDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + fn result(job: &mut ShellJob) -> Result { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + if let Some(result) = job.try_result() { + assert!(job.try_result().is_none(), "result must be one-shot"); + return result; + } + assert!( + Instant::now() < deadline, + "shell job did not complete promptly" + ); + thread::sleep(Duration::from_millis(5)); + } + } + + fn ready_pids(dir: &TestDir) -> Vec { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + if let Ok(text) = fs::read_to_string(dir.0.join("ready")) { + let pids = text + .split_whitespace() + .filter_map(|s| s.parse().ok()) + .collect::>(); + if pids.len() == 2 { + return pids; + } + } + assert!(Instant::now() < deadline, "shell did not signal readiness"); + thread::sleep(Duration::from_millis(5)); + } + } + + fn assert_reaped(pid: libc::pid_t) { + let mut status = 0; + assert_eq!( + unsafe { libc::waitpid(pid, &mut status, libc::WNOHANG) }, + -1 + ); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ECHILD) + ); + } + + fn assert_descendant_stopped(pid: libc::pid_t) { + let deadline = Instant::now() + Duration::from_secs(2); + loop { + if unsafe { libc::kill(pid, 0) } == -1 { + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ESRCH) + ); + return; + } + // A reparented zombie is dead, but a container's pid 1 may not + // reap it promptly. We can only reap our direct child. + let state = Command::new("ps") + .args(["-o", "stat=", "-p", &pid.to_string()]) + .output() + .unwrap(); + if String::from_utf8_lossy(&state.stdout) + .trim() + .starts_with('Z') + { + return; + } + assert!( + Instant::now() < deadline, + "descendant {pid} is still running" + ); + thread::sleep(Duration::from_millis(5)); + } + } + + const WAIT_WITH_DESCENDANT: &str = + "sleep 30 & descendant=$!; printf '%s %s\\n' \"$$\" \"$descendant\" > ready; wait"; + + #[test] + fn cwd_and_both_output_streams() { + let dir = TestDir::new(); + let outcome = result(&mut dir.spawn("pwd -P; printf out; printf err >&2")).unwrap(); + assert_eq!(outcome.stdout, format!("{}\nout", dir.0.display())); + assert_eq!(outcome.stderr, "err"); + assert!(outcome.success); + assert_eq!(outcome.exit_code, Some(0)); + assert!(!outcome.cancelled && !outcome.timed_out); + } + + #[test] + fn nonzero_exit_and_no_output() { + let dir = TestDir::new(); + let outcome = result(&mut dir.spawn("printf out; printf err >&2; exit 7")).unwrap(); + assert!(!outcome.success); + assert_eq!(outcome.exit_code, Some(7)); + assert_eq!(outcome.stdout, "out"); + assert_eq!(outcome.stderr, "err"); + let empty = result(&mut dir.spawn(":")).unwrap(); + assert!(empty.success); + assert!(empty.stdout.is_empty() && empty.stderr.is_empty()); + } + + #[test] + fn stdin_is_eof_and_there_is_no_controlling_tty() { + let dir = TestDir::new(); + let script = "if read value; then exit 1; fi; if ( : /dev/null; then exit 2; fi; printf eof"; + let outcome = result(&mut dir.spawn(script)).unwrap(); + assert!(outcome.success, "{outcome:?}"); + assert_eq!(outcome.stdout, "eof"); + assert!(outcome.stderr.is_empty()); + } + + #[test] + fn missing_shell_reports_an_error() { + let dir = TestDir::new(); + let mut job = + ShellJob::spawn(&dir.0, ":", dir.0.join("missing-shell").as_os_str(), true) + .unwrap(); + assert!( + result(&mut job) + .unwrap_err() + .to_string() + .contains("Failed to spawn shell") + ); + let mut empty = ShellJob::spawn(&dir.0, ":", OsStr::new(""), true).unwrap(); + assert!( + result(&mut empty).is_err(), + "the caller must choose the fallback shell" + ); + } + + #[test] + fn cancellation_before_launch_does_not_run_the_command() { + let dir = TestDir::new(); + let cmd = + shell_command(&dir.0, "touch should-not-exist", OsStr::new("sh"), false).unwrap(); + let outcome = unix::run(cmd, &AtomicBool::new(true), DEFAULT_TIMEOUT).unwrap(); + assert!(outcome.cancelled && !outcome.timed_out && !outcome.success); + assert_eq!(outcome.exit_code, None); + assert!(!dir.0.join("should-not-exist").exists()); + } + + #[test] + fn cancellation_kills_group_and_reaps_shell() { + let dir = TestDir::new(); + let mut job = dir.spawn(WAIT_WITH_DESCENDANT); + assert!(job.try_result().is_none()); + let pids = ready_pids(&dir); + job.cancel(); + job.cancel(); + let outcome = result(&mut job).unwrap(); + assert!(outcome.cancelled && !outcome.timed_out && !outcome.success); + assert_eq!(outcome.exit_code, None); + assert_reaped(pids[0]); + assert_descendant_stopped(pids[1]); + } + + #[test] + fn escaped_descendant_cannot_hold_the_output_pipes_open() { + use std::os::fd::AsRawFd; + use std::os::unix::process::CommandExt; + + struct EscapedProcess(libc::pid_t); + impl Drop for EscapedProcess { + fn drop(&mut self) { + unsafe { libc::kill(self.0, libc::SIGKILL) }; + } + } + + let dir = TestDir::new(); + let ready = fs::File::create(dir.0.join("escaped")).unwrap(); + let fd = ready.as_raw_fd(); + let fd_limit = unsafe { libc::sysconf(libc::_SC_OPEN_MAX) }; + assert!(fd_limit > 0 && fd_limit <= libc::c_int::MAX as libc::c_long); + let mut cmd = shell_command(&dir.0, "printf done", OsStr::new("sh"), false).unwrap(); + // Make a descendant that inherits the redirected output fds, but + // leaves the job's session. Between fork and _exit use ONLY + // async-signal-safe libc functions, never the Rust runtime. + unsafe { + cmd.pre_exec(move || { + let pid = libc::fork(); + if pid == -1 { + return Err(std::io::Error::last_os_error()); + } + if pid == 0 { + if libc::setsid() == -1 { + libc::_exit(1); + } + // Unlike an exec, a bare fork retains even CLOEXEC + // fds: close the spawn handshake and unrelated jobs' + // pipe ends before holding ONLY stdout/stderr open. + for fd in 3..fd_limit as libc::c_int { + libc::close(fd); + } + libc::sleep(30); + libc::_exit(0); + } + if libc::write( + fd, + (&pid as *const libc::pid_t).cast(), + std::mem::size_of_val(&pid), + ) == -1 + { + libc::kill(pid, libc::SIGKILL); + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } + let mut job = ShellJob::spawn_command(cmd, DEFAULT_TIMEOUT).unwrap(); + let deadline = Instant::now() + Duration::from_secs(5); + let escaped = loop { + let bytes = fs::read(dir.0.join("escaped")).unwrap(); + if let Ok(bytes) = bytes.try_into() { + break EscapedProcess(libc::pid_t::from_ne_bytes(bytes)); + } + assert!( + Instant::now() < deadline, + "escaped descendant did not start" + ); + thread::sleep(Duration::from_millis(5)); + }; + let outcome = result(&mut job).unwrap(); + assert!(outcome.success, "{outcome:?}"); + assert_eq!(outcome.stdout, format!("done{TRUNCATION_NOTICE}")); + assert_eq!(outcome.stderr, TRUNCATION_NOTICE); + assert_eq!( + unsafe { libc::kill(escaped.0, 0) }, + 0, + "descendant really escaped the group" + ); + drop(escaped); + } + + #[test] + fn cancellation_is_not_starved_by_continuously_readable_streams() { + let dir = TestDir::new(); + let mut job = dir.spawn("printf '%s %s\\n' \"$$\" \"$$\" > ready; while :; do printf abcdefghijklmnopqrstuvwxyz; printf abcdefghijklmnopqrstuvwxyz >&2; done"); + let pids = ready_pids(&dir); + job.cancel(); + let outcome = result(&mut job).unwrap(); + assert!(outcome.cancelled && !outcome.success); + assert_reaped(pids[0]); + } + + #[test] + fn timeout_kills_group_and_reaps_shell() { + let dir = TestDir::new(); + let mut job = ShellJob::spawn_with_timeout( + &dir.0, + WAIT_WITH_DESCENDANT, + OsStr::new("/bin/sh"), + false, + Duration::from_millis(250), + ) + .unwrap(); + let pids = ready_pids(&dir); + let outcome = result(&mut job).unwrap(); + assert!(outcome.timed_out && !outcome.cancelled && !outcome.success); + assert_eq!(outcome.exit_code, None); + assert_reaped(pids[0]); + assert_descendant_stopped(pids[1]); + } + + #[test] + fn drop_cancels_joins_and_reaps() { + let dir = TestDir::new(); + let job = dir.spawn(WAIT_WITH_DESCENDANT); + let pids = ready_pids(&dir); + let started = Instant::now(); + drop(job); + assert!(started.elapsed() < Duration::from_secs(2)); + assert_reaped(pids[0]); + assert_descendant_stopped(pids[1]); + } + + #[test] + fn large_output_is_drained_but_bounded_per_stream() { + let dir = TestDir::new(); + let script = "awk 'BEGIN { for (i=0; i<32768; i++) { print \"0123456789012345678901234567890123456789012345678901234567890123\"; print \"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijkl\" > \"/dev/stderr\" } }'"; + let outcome = result(&mut dir.spawn(script)).unwrap(); + assert!(outcome.success, "{outcome:?}"); + for output in [&outcome.stdout, &outcome.stderr] { + assert_eq!(output.len(), OUTPUT_LIMIT + TRUNCATION_NOTICE.len()); + assert!(output.ends_with(TRUNCATION_NOTICE)); + } + } + + #[test] + fn closed_pipes_do_not_disable_timeout() { + let dir = TestDir::new(); + let mut job = ShellJob::spawn_with_timeout( + &dir.0, + "exec >/dev/null 2>&1; sleep 30", + OsStr::new("/bin/sh"), + false, + Duration::from_millis(100), + ) + .unwrap(); + let outcome = result(&mut job).unwrap(); + assert!(outcome.timed_out && !outcome.success); + } + + #[test] + fn shell_exit_does_not_wait_for_background_output_pipes() { + let dir = TestDir::new(); + let script = "sleep 30 & descendant=$!; printf '%s %s\\n' \"$$\" \"$descendant\" > ready; printf done"; + let mut job = dir.spawn(script); + let outcome = result(&mut job).unwrap(); + assert!(outcome.success); + assert_eq!(outcome.stdout, "done"); + let pids = ready_pids(&dir); + assert_reaped(pids[0]); + assert_descendant_stopped(pids[1]); + } + + fn installed(shell: &str) -> bool { + Command::new(shell) + .arg("--version") + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|status| status.success()) + } + + fn startup_test(shell: &str, rc: &str, zdotdir: bool) { + if !installed(shell) { + return; + } + let dir = TestDir::new(); + let home = TestDir::new(); + fs::write(home.0.join(rc), "alias shell_alias='printf alias'; shell_function() { printf function; }; cd /; set -- changed\n").unwrap(); + let mut cmd = shell_command(&dir.0, "shell_alias; shell_function; printf '\\n'; pwd -P; printf '%s' '\"; literal $value; #'", OsStr::new(shell), true).unwrap(); + cmd.env("HOME", &home.0).env_remove("ZDOTDIR"); + if zdotdir { + cmd.env("HOME", &dir.0).env("ZDOTDIR", &home.0); + } + let outcome = + result(&mut ShellJob::spawn_command(cmd, DEFAULT_TIMEOUT).unwrap()).unwrap(); + assert!(outcome.success, "{outcome:?}"); + assert_eq!( + outcome.stdout, + format!("aliasfunction\n{}\n\"; literal $value; #", dir.0.display()) + ); + assert!(outcome.stderr.is_empty(), "{outcome:?}"); + } + + #[test] + fn bash_loads_aliases_functions_and_restores_cwd() { + startup_test("/bin/bash", ".bashrc", false); + startup_test("/bin/bash", ".bash_aliases", false); + } + + #[test] + fn zsh_loads_aliases_functions_and_restores_cwd() { + startup_test("/bin/zsh", ".zshrc", false); + startup_test("/bin/zsh", ".zshrc", true); + } + + #[test] + fn custom_commands_do_not_load_aliases_or_functions() { + let dir = TestDir::new(); + let home = TestDir::new(); + fs::write( + home.0.join(".bashrc"), + "alias shell_alias='printf alias'; shell_function() { printf function; }\n", + ) + .unwrap(); + let mut cmd = shell_command( + &dir.0, + "command -v shell_alias; command -v shell_function", + OsStr::new("/bin/bash"), + false, + ) + .unwrap(); + cmd.env("HOME", &home.0); + let outcome = + result(&mut ShellJob::spawn_command(cmd, DEFAULT_TIMEOUT).unwrap()).unwrap(); + assert!(!outcome.success); + assert!(outcome.stdout.is_empty()); + } + + #[test] + fn fish_runs_native_syntax_when_installed() { + if !installed("fish") { + return; + } + let dir = TestDir::new(); + let home = TestDir::new(); + let mut cmd = shell_command( + &dir.0, + "set value fish; printf '%s' $value", + OsStr::new("fish"), + true, + ) + .unwrap(); + cmd.env("HOME", &home.0).env("XDG_CONFIG_HOME", &home.0); + let outcome = + result(&mut ShellJob::spawn_command(cmd, DEFAULT_TIMEOUT).unwrap()).unwrap(); + assert!(outcome.success, "{outcome:?}"); + assert_eq!(outcome.stdout, "fish"); + } + } +} From fe49a4943235365bd2133289b765bc528c74b7f9 Mon Sep 17 00:00:00 2001 From: Blankeos Date: Wed, 7 Oct 2026 03:25:17 +0800 Subject: [PATCH 3/4] Preserve asynchronous commit drafts and make shell popups mouse-modal --- README.md | 4 +- src/gui/controller/diff_mode.rs | 2 +- src/gui/mod.rs | 271 ++++++++++++++++++++++++++++++++ src/gui/views.rs | 10 +- 4 files changed, 276 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index aabdd88..0060d15 100644 --- a/README.md +++ b/README.md @@ -97,12 +97,12 @@ minutes. Output includes the exit status, stdout, and stderr, with a 1 MiB captu cap per stream (excess output is truncated). Background children are terminated when the job ends, including on completion, cancellation, or timeout; this is not a way to launch persistent background services. Processes that explicitly -create their own session can escape process-group cleanup; this prompt is not a +create their own process group or session can escape cleanup; this prompt is not a sandbox. The process runner currently requires Unix (macOS/Linux). Input preserves pasted newlines and quoted spacing; `Enter` executes the whole command and `Esc` dismisses without running it. Scroll command results with -`j`/`k`, arrow keys, `PgUp`/`PgDn`, or `g`/`G`; `y` copies the result and +`j`/`k`, arrow keys, the mouse wheel, `PgUp`/`PgDn`, or `g`/`G`; `y` copies the result and `Esc`/`Enter` closes it. The command log retains a bounded output preview. Configured `customCommands` still use `sh -c` rather than the prompt's `$SHELL` diff --git a/src/gui/controller/diff_mode.rs b/src/gui/controller/diff_mode.rs index a08e512..3e2d025 100644 --- a/src/gui/controller/diff_mode.rs +++ b/src/gui/controller/diff_mode.rs @@ -848,7 +848,7 @@ pub fn reload_diff_files(gui: &mut Gui) -> Result<()> { Ok(()) } -pub(super) fn update_diff_mode_tree(gui: &mut Gui) { +pub(in crate::gui) fn update_diff_mode_tree(gui: &mut Gui) { if gui.diff_mode.show_tree { gui.diff_mode.tree_nodes = crate::model::file_tree::build_commit_file_tree( &gui.diff_mode.diff_files, diff --git a/src/gui/mod.rs b/src/gui/mod.rs index 402ecf7..d79bac0 100644 --- a/src/gui/mod.rs +++ b/src/gui/mod.rs @@ -1717,6 +1717,14 @@ impl Gui { Ok(()) } + fn shell_command_ui_active(&self) -> bool { + self.shell_command_job.is_some() + || matches!( + self.popup, + PopupState::ShellCommand { .. } | PopupState::CommandOutput { .. } + ) + } + fn input_wait_timeout(&self) -> Duration { if self.shell_command_job.is_some() || self.ai_commit_generation_active() @@ -2239,6 +2247,12 @@ impl Gui { /// Check for completed AI commit message generation results. fn receive_ai_commit_results(&mut self) { + // Shell input/results own the modal until dismissed. Leave AI results + // queued instead of replacing a typed command or restoring an editor + // that shell completion would immediately discard. + if self.shell_command_ui_active() { + return; + } while let Ok(result) = self.ai_commit_rx.try_recv() { let active_generation = self.ai_commit_job.as_ref().map(|job| job.generation); if active_generation != Some(result.generation) { @@ -2566,6 +2580,11 @@ impl Gui { /// Check for completed background remote operations (push, pull, fetch). fn receive_remote_op_results(&mut self) { + // In particular, a remote failure must not replace the shell's modal + // and then be lost when the shell completes. + if self.shell_command_ui_active() { + return; + } if let Ok(result) = self.remote_op_rx.try_recv() { self.remote_op_label = None; match result { @@ -2596,6 +2615,13 @@ impl Gui { } } Err(e) => { + // AI generation may have restored the editor just before + // a queued remote error arrives. Keep that draft when the + // error needs to take over the popup. + if matches!(self.popup, PopupState::CommitInput { .. }) { + self.saved_commit_popup = + Some(std::mem::replace(&mut self.popup, PopupState::None)); + } let err = format!("{}", e); if let Some(name) = self .pending_checkout_by_name @@ -7036,6 +7062,26 @@ impl Gui { return; } + match &mut self.popup { + PopupState::ShellCommand { .. } => return, + PopupState::CommandOutput { + message, scroll, .. + } => { + let max = views::command_output_max_scroll( + message, + Rect::new(0, 0, self.layout.width, self.layout.height), + ); + match mouse.kind { + MouseEventKind::ScrollDown => *scroll = scroll.saturating_add(3).min(max), + MouseEventKind::ScrollUp => *scroll = scroll.saturating_sub(3), + _ => {} + } + // All events are modal, including clicks outside the popup. + return; + } + _ => {} + } + // Sidebar divider drag (Normal mode only). Must run before text-select / // focus paths so the hit strip wins the gesture. if self.sidebar_resizing { @@ -9581,6 +9627,190 @@ mod terminal_mouse_tests { assert!(gui.shell_command_job.is_none()); } + #[test] + fn shell_modal_defers_ctrl_g_result_without_losing_command_or_commit_draft() { + let repo = TempRepo::new("shell-ai-popup-ownership"); + let mut gui = tree_test_gui(&repo); + repo.git(&["add", "."]); + gui.model.lock().unwrap().files = gui.git.load_files().unwrap(); + gui.handle_key(parse_key("c").unwrap()).unwrap(); + assert!(matches!(gui.popup, PopupState::CommitInput { .. })); + gui.pending_commit_popup = Some(std::mem::replace(&mut gui.popup, PopupState::None)); + // Deterministic completion of a Ctrl-G job, without an external agent. + gui.ai_commit_job = Some(AiCommitJob { + generation: 1, + cancel: Arc::new(AtomicBool::new(false)), + cancel_armed_at: None, + }); + gui.ai_commit_tx + .send(AiCommitResult { + generation: 1, + result: Ok(Some("feat: generated summary\n\nGenerated body".into())), + }) + .unwrap(); + gui.remote_op_tx + .send(Err(anyhow::anyhow!("concurrent fetch failed"))) + .unwrap(); + gui.handle_key(parse_key(":").unwrap()).unwrap(); + gui.handle_paste("printf shell-result".into()); + gui.receive_ai_commit_results(); + gui.receive_remote_op_results(); + gui.receive_remote_op_results(); + assert!(matches!(&gui.popup, PopupState::ShellCommand { textarea } + if textarea.lines().join("\n") == "printf shell-result")); + assert!(gui.pending_commit_popup.is_some()); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_ai_commit_results(); + gui.receive_remote_op_results(); + assert!(matches!(gui.popup, PopupState::Loading { .. })); + wait_for_shell_command(&mut gui); + gui.receive_ai_commit_results(); + assert!( + matches!(&gui.popup, PopupState::CommandOutput { message, .. } + if message.contains("shell-result")) + ); + assert!(gui.pending_commit_popup.is_some()); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_ai_commit_results(); + assert!(gui.pending_commit_popup.is_none()); + assert!(gui.ai_commit_job.is_none()); + assert!(matches!(&gui.popup, PopupState::CommitInput { + summary_textarea, body_state, .. + } if summary_textarea.lines().join("") == "feat: generated summary" + && body_state.raw() == "Generated body")); + // This is the main-loop receiver order after shell output closes. + gui.receive_remote_op_results(); + assert!(matches!(&gui.popup, PopupState::Message { message, .. } + if message.contains("concurrent fetch failed"))); + assert!( + matches!(&gui.saved_commit_popup, Some(PopupState::CommitInput { + summary_textarea, body_state, .. + }) if summary_textarea.lines().join("") == "feat: generated summary" + && body_state.raw() == "Generated body") + ); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.handle_key(parse_key("c").unwrap()).unwrap(); + assert!( + matches!(&gui.popup, PopupState::CommitInput { summary_textarea, .. } + if summary_textarea.lines().join("") == "feat: generated summary") + ); + } + + #[test] + fn shell_modal_defers_remote_error_until_output_is_dismissed() { + let repo = TempRepo::new("shell-remote-popup-ownership"); + let mut gui = tree_test_gui(&repo); + gui.remote_op_label = Some("Fetch".into()); + gui.remote_op_tx + .send(Err(anyhow::anyhow!("fetch failed"))) + .unwrap(); + start_test_custom_command(&mut gui, "printf shell-result", true); + gui.receive_remote_op_results(); + assert!(matches!(gui.popup, PopupState::Loading { .. })); + wait_for_shell_command(&mut gui); + gui.receive_remote_op_results(); + assert!( + matches!(&gui.popup, PopupState::CommandOutput { message, .. } + if message.contains("shell-result")) + ); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_remote_op_results(); + assert!(matches!(&gui.popup, PopupState::Message { message, .. } + if message.contains("fetch failed"))); + assert!(gui.remote_op_label.is_none()); + } + + #[test] + fn shell_popups_capture_mouse_without_changing_underlying_navigation() { + use crate::gui::modes::diff_mode::DiffModeFocus; + use crossterm::event::{MouseButton, MouseEventKind}; + + let repo = TempRepo::new("shell-modal-mouse"); + let mut gui = tree_test_gui(&repo); + Arc::get_mut(&mut gui.config) + .unwrap() + .user_config + .gui + .mouse_events = true; + gui.layout.update_size(100, 30); + for compare in [false, true] { + if compare { + gui.diff_mode.enter(true); + gui.diff_mode.set_focus(DiffModeFocus::Commits); + } + let focus = gui.diff_mode.focus; + let context = gui.context_mgr.active(); + let selection = gui.context_mgr.selected_active(); + let diff_scroll = gui.diff_view.scroll_offset; + for output in [false, true] { + let message = (0..100).map(|i| format!("line {i}\n")).collect::(); + gui.popup = if output { + PopupState::CommandOutput { + title: "output".into(), + message: message.clone(), + kind: MessageKind::Info, + scroll: 0, + } + } else { + let mut textarea = popup::make_textarea(""); + textarea.insert_str("typed command"); + PopupState::ShellCommand { textarea } + }; + for (column, row) in [(1, 1), (50, 12), (99, 29)] { + for kind in [ + MouseEventKind::Down(MouseButton::Left), + MouseEventKind::Drag(MouseButton::Left), + MouseEventKind::Up(MouseButton::Left), + MouseEventKind::ScrollDown, + ] { + gui.handle_mouse(MouseEvent { + kind, + column, + row, + modifiers: KeyModifiers::NONE, + }); + } + } + if output { + assert!(matches!( + gui.popup, + PopupState::CommandOutput { scroll: 9, .. } + )); + for _ in 0..100 { + gui.handle_mouse(MouseEvent { + kind: MouseEventKind::ScrollDown, + column: 50, + row: 12, + modifiers: KeyModifiers::NONE, + }); + } + let max = views::command_output_max_scroll(&message, Rect::new(0, 0, 100, 30)); + assert!( + matches!(gui.popup, PopupState::CommandOutput { scroll, .. } if scroll == max) + ); + gui.handle_mouse(MouseEvent { + kind: MouseEventKind::ScrollUp, + column: 50, + row: 12, + modifiers: KeyModifiers::NONE, + }); + assert!( + matches!(gui.popup, PopupState::CommandOutput { scroll, .. } if scroll == max.saturating_sub(3)) + ); + } else { + assert!(matches!(&gui.popup, PopupState::ShellCommand { textarea } + if textarea.lines().join("") == "typed command")); + } + assert_eq!(gui.context_mgr.active(), context); + assert_eq!(gui.context_mgr.selected_active(), selection); + assert_eq!(gui.diff_view.scroll_offset, diff_scroll); + assert_eq!(gui.diff_mode.focus, focus); + assert!(gui.diff_mode.editing.is_none()); + assert!(!gui.sidebar_resizing); + } + } + } + #[test] fn custom_command_is_nonblocking_cancelable_and_refreshes_after_cancellation() { let repo = TempRepo::new("shell-cancellation"); @@ -9668,6 +9898,47 @@ mod terminal_mouse_tests { assert!(gui.popup == PopupState::None); } + #[test] + fn shell_completion_preserves_compare_commit_files_tree_search_and_focus() { + use crate::gui::modes::diff_mode::DiffModeFocus; + + let repo = TempRepo::new("shell-compare-refresh"); + let mut gui = tree_test_gui(&repo); + // Both configured commands and shell prompts use this result path. + start_test_custom_command(&mut gui, "printf refreshed", true); + gui.diff_mode.enter(true); + gui.diff_mode.ref_a = "HEAD~2".into(); + gui.diff_mode.ref_b = "HEAD".into(); + controller::diff_mode::reload_diff_files(&mut gui).unwrap(); + gui.diff_mode.commits_selected = 1; + controller::diff_mode::open_selected_commit_files(&mut gui).unwrap(); + let commit = gui.diff_mode.files_commit.clone(); + gui.diff_mode.collapsed_dirs.insert("src".into()); + controller::diff_mode::update_diff_mode_tree(&mut gui); + gui.diff_mode.file_search_query = "src".into(); + gui.diff_mode.set_focus(DiffModeFocus::DiffExploration); + let source = gui.diff_mode.diff_source; + let files = gui.diff_mode.diff_files.clone(); + wait_for_shell_command(&mut gui); + assert_eq!(gui.diff_mode.focus, DiffModeFocus::DiffExploration); + assert_eq!(gui.diff_mode.sidebar_focus, DiffModeFocus::CommitFiles); + assert_eq!(gui.diff_mode.diff_source, source); + assert_eq!(gui.diff_mode.files_commit, commit); + assert_eq!(gui.diff_mode.diff_files.len(), files.len()); + for (actual, expected) in gui.diff_mode.diff_files.iter().zip(files) { + assert_eq!(actual.name, expected.name); + } + assert!(gui.diff_mode.collapsed_dirs.contains("src")); + assert_eq!(gui.diff_mode.file_search_query, "src"); + assert!(matches!( + gui.popup, + PopupState::CommandOutput { + kind: MessageKind::Info, + .. + } + )); + } + struct TempRepo { path: PathBuf, } diff --git a/src/gui/views.rs b/src/gui/views.rs index c76ab7d..9b9f48c 100644 --- a/src/gui/views.rs +++ b/src/gui/views.rs @@ -1562,14 +1562,8 @@ mod tests { assert!(text.contains(&format!("{hint} shell")), "{text}"); } assert_eq!(text.contains(": shell"), binding == ":", "{text}"); - if ctx != ContextId::StashFiles { - let toggle = if ctx == ContextId::Files { - "head" - } else { - "files" - }; - assert!(text.contains(&format!("ctrl+g {toggle}")), "{text}"); - } + assert!(!text.contains("ctrl+g head"), "{text}"); + assert!(!text.contains("ctrl+g files"), "{text}"); } } } From dc042355d37d83aed38eeab844c050724f8365a0 Mon Sep 17 00:00:00 2001 From: Blankeos Date: Thu, 8 Oct 2026 03:27:26 +0800 Subject: [PATCH 4/4] Fix shell prompt review regressions and style command footer --- README.md | 14 +- src/gui/controller/custom_commands.rs | 231 +++++++++++++++++++- src/gui/controller/diff_mode.rs | 17 +- src/gui/mod.rs | 297 ++++++++++++++++++++++++-- src/gui/views.rs | 81 ++++++- src/os/shell.rs | 160 ++++++++++++-- 6 files changed, 747 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 0060d15..ba3a5e9 100644 --- a/README.md +++ b/README.md @@ -83,12 +83,18 @@ keybinding: ``` The prompt uses `$SHELL`, falling back to `sh` when it is unset or empty. Bash -loads `~/.bash_aliases` and `~/.bashrc` and enables alias expansion; zsh loads `${ZDOTDIR:-$HOME}/.zshrc`. +loads `~/.bash_aliases` and `~/.bashrc` and enables alias expansion; zsh keeps +native `.zshenv` startup (including changes to `ZDOTDIR`) and then loads +`${ZDOTDIR:-$HOME}/.zshrc`. Fish keeps its native startup configuration. Aliases and functions defined there can be used, but a noninteractive guard in your rc file may skip their definitions. Definitions that exist only in your -current interactive shell are not inherited. Fish and other shells receive the -command using their native `-c` invocation, so use that shell's syntax rather -than POSIX syntax. +current interactive shell are not inherited. Bash, zsh, and Fish restore the +repository root after startup and clear positional arguments (`$argv` in Fish) +before evaluating your command, even if startup changed them. Paths and commands +are passed as literal arguments or environment values, not interpolated into +wrapper code. Fish uses a Fish-native wrapper; other shells receive a direct +native `-c` invocation (their startup may change the working directory). Use +your shell's syntax rather than POSIX syntax. Commands run asynchronously and **noninteractively** with no input/TTY: editors, password prompts, and other interactive programs are not supported here. Press diff --git a/src/gui/controller/custom_commands.rs b/src/gui/controller/custom_commands.rs index a5b487c..c4c4780 100644 --- a/src/gui/controller/custom_commands.rs +++ b/src/gui/controller/custom_commands.rs @@ -13,6 +13,16 @@ pub(in crate::gui) struct RunningCommand { show_output: bool, } +fn comparison_contains_commit(gui: &Gui, hash: &str) -> bool { + // Symmetric-difference membership, without materializing the history. + // A failed reload (or tree-only comparison) has no valid commit range. + // is_ancestor returns false on errors, so XOR alone would misclassify a + // deleted ref and resurrect stale drilled-down files. + gui.diff_mode.ahead_behind.is_some() + && (gui.git.is_ancestor(hash, &gui.diff_mode.ref_a) + != gui.git.is_ancestor(hash, &gui.diff_mode.ref_b)) +} + /// Try to handle a key as a custom command. Returns Ok(true) if handled. pub fn try_handle_key(gui: &mut Gui, key: KeyEvent) -> Result { let active = gui.context_mgr.active(); @@ -124,14 +134,16 @@ pub fn receive_command_result(gui: &mut Gui) { else { return; }; - let command = gui.shell_command_job.take().unwrap(); - gui.popup = PopupState::None; + // Retain job/modal ownership through refresh: a refresh failure should be + // queued, not overwritten by the command result shown below. // Even failures/cancellation can leave repository changes behind. gui.needs_refresh = true; gui.needs_diff_refresh = true; if gui.diff_mode.active { refresh_comparison(gui); } + let command = gui.shell_command_job.take().unwrap(); + gui.popup = PopupState::None; match outcome { Ok(result) => { let (title, message, kind) = result_message(&result); @@ -167,11 +179,19 @@ pub fn receive_command_result(gui: &mut Gui) { /// Refresh refs after mutations without throwing away comparison navigation. fn refresh_comparison(gui: &mut Gui) { + use crate::gui::modes::diff_mode::CompareDiffSource; + let focus = gui.diff_mode.focus; let sidebar_focus = gui.diff_mode.sidebar_focus; let source = gui.diff_mode.diff_source; let files_commit = gui.diff_mode.files_commit.clone(); let selected_commit = gui.diff_mode.selected_commit().map(|c| c.hash.clone()); + let selected_index = gui.diff_mode.commits_selected; + let loaded_count = gui + .diff_mode + .commits + .len() + .max(crate::git::DEFAULT_COMMIT_LIMIT); let selected_file = if gui.diff_mode.show_tree { gui.diff_mode .tree_nodes @@ -186,20 +206,40 @@ fn refresh_comparison(gui: &mut Gui) { let collapsed = gui.diff_mode.collapsed_dirs.clone(); let search_query = gui.diff_mode.file_search_query.clone(); let search_textarea = gui.diff_mode.file_search_textarea.clone(); - let _ = super::diff_mode::reload_diff_files(gui); + let _ = super::diff_mode::reload_diff_files_with_commit_limit(gui, loaded_count); + // New commits can push the selection past the retained pages. Only walk + // further when the hash actually remains in A...B, and stop as soon as it + // is found. Rewritten/removed commits must not cause an unbounded reload. + if let Some(hash) = selected_commit.as_deref() { + if !gui.diff_mode.commits.iter().any(|c| c.hash == hash) + && comparison_contains_commit(gui, hash) + { + while gui.diff_mode.has_more_commits() { + let before = gui.diff_mode.commits.len(); + if super::diff_mode::load_more_commits(gui).is_err() + || gui.diff_mode.commits.len() == before + || gui.diff_mode.commits.iter().any(|c| c.hash == hash) + { + break; + } + } + } + } if let Some(hash) = files_commit { - if let Ok(files) = gui.git.commit_files(&hash) { + if comparison_contains_commit(gui, &hash) + && let Ok(files) = gui.git.commit_files(&hash) + { gui.diff_mode.diff_files = files; gui.diff_mode.files_commit = Some(hash); } } gui.diff_mode.collapsed_dirs = collapsed; super::diff_mode::update_diff_mode_tree(gui); - if let Some(hash) = selected_commit { - if let Some(index) = gui.diff_mode.commits.iter().position(|c| c.hash == hash) { - gui.diff_mode.commits_selected = index; - } - } + // If the selected hash genuinely left the comparison, keep the nearest + // available row rather than silently jumping to the newest commit. + gui.diff_mode.commits_selected = selected_commit + .and_then(|hash| gui.diff_mode.commits.iter().position(|c| c.hash == hash)) + .unwrap_or_else(|| selected_index.min(gui.diff_mode.commits.len().saturating_sub(1))); if let Some(path) = selected_file { let index = if gui.diff_mode.show_tree { gui.diff_mode.tree_nodes.iter().position(|n| n.path == path) @@ -210,7 +250,15 @@ fn refresh_comparison(gui: &mut Gui) { } gui.diff_mode.set_focus(focus); gui.diff_mode.sidebar_focus = sidebar_focus; - gui.diff_mode.diff_source = source; + gui.diff_mode.diff_source = match source { + CompareDiffSource::CommitFiles if gui.diff_mode.files_commit.is_none() => { + CompareDiffSource::Comparison + } + CompareDiffSource::Commit if gui.diff_mode.selected_commit().is_none() => { + CompareDiffSource::Comparison + } + _ => source, + }; gui.diff_mode.file_search_query = search_query; gui.diff_mode.file_search_textarea = search_textarea; gui.diff_mode.update_file_search_matches(); @@ -360,6 +408,169 @@ pub fn open_custom_command_prompt(gui: &mut Gui) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use crate::config::{AppConfig, AppState, UserConfig}; + use crate::git::GitCommands; + use crate::gui::modes::diff_mode::{CompareDiffSource, DiffModeFocus}; + use std::process::Command; + + struct HistoryRepo(std::path::PathBuf); + + impl Drop for HistoryRepo { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + fn git_script(repo: &HistoryRepo, script: &str) { + let output = Command::new("sh") + .args(["-eu", "-c", script]) + .current_dir(&repo.0) + .env("GIT_AUTHOR_NAME", "Test") + .env("GIT_AUTHOR_EMAIL", "test@example.com") + .env("GIT_COMMITTER_NAME", "Test") + .env("GIT_COMMITTER_EMAIL", "test@example.com") + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } + + fn comparison_history() -> (HistoryRepo, Gui) { + let path = std::env::temp_dir().join(format!( + "lazygitrs-comparison-refresh-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&path).unwrap(); + let repo = HistoryRepo(path); + // Reuse two trees: commit-tree avoids 950 working-tree/index updates. + git_script( + &repo, + r#" + git init -q -b main + printf 'base\n' > file.txt + git add file.txt + a=$(git write-tree) + printf 'changed\n' > file.txt + git add file.txt + b=$(git write-tree) + root=$(git -c commit.gpgsign=false commit-tree "$a" -m root) + git update-ref refs/heads/main "$root" + parent=$root + i=1 + while [ "$i" -le 950 ]; do + if [ $((i % 2)) -eq 0 ]; then tree=$a; else tree=$b; fi + parent=$(git -c commit.gpgsign=false commit-tree "$tree" -p "$parent" -m "commit $i") + i=$((i + 1)) + done + git update-ref refs/heads/feature "$parent" + git reset -q --hard main + "#, + ); + let config = AppConfig { + debug: false, + version: String::new(), + user_config: UserConfig::default(), + app_state: AppState::default(), + config_dir: repo.0.clone(), + state_dir: repo.0.clone(), + state_path: repo.0.join("state.yml"), + }; + let mut gui = Gui::new(config, GitCommands::new(&repo.0).unwrap(), None, false).unwrap(); + gui.diff_mode.enter(false); + gui.diff_mode.ref_a = "main".into(); + gui.diff_mode.ref_b = "feature".into(); + super::super::diff_mode::reload_diff_files(&mut gui).unwrap(); + super::super::diff_mode::load_more_commits(&mut gui).unwrap(); + assert_eq!(gui.diff_mode.commits.len(), 600); + gui.diff_mode.commits_selected = 550; + gui.diff_mode.set_focus(DiffModeFocus::Commits); + (repo, gui) + } + + #[test] + fn comparison_completion_preserves_paged_commit_and_drilled_files() { + let (repo, mut gui) = comparison_history(); + let hash = gui.diff_mode.selected_commit().unwrap().hash.clone(); + refresh_comparison(&mut gui); + assert_eq!(gui.diff_mode.commits.len(), 600); + assert_eq!(gui.diff_mode.selected_commit().unwrap().hash, hash); + assert_eq!(gui.diff_mode.diff_source, CompareDiffSource::Commit); + super::super::diff_mode::open_selected_commit_files(&mut gui).unwrap(); + gui.diff_mode.set_focus(DiffModeFocus::DiffExploration); + let file = gui.diff_mode.diff_files[0].name.clone(); + git_script( + &repo, + r#" + parent=$(git rev-parse feature) + tree=$(git rev-parse 'feature^{tree}') + i=1 + while [ "$i" -le 60 ]; do + parent=$(git -c commit.gpgsign=false commit-tree "$tree" -p "$parent" -m "new $i") + i=$((i + 1)) + done + git update-ref refs/heads/feature "$parent" + "#, + ); + refresh_comparison(&mut gui); + assert_eq!(gui.diff_mode.selected_commit().unwrap().hash, hash); + assert_eq!(gui.diff_mode.commits_selected, 610); + assert_eq!(gui.diff_mode.commits.len(), 900); + assert!(gui.diff_mode.has_more_commits()); + assert_eq!(gui.diff_mode.files_commit.as_deref(), Some(hash.as_str())); + assert_eq!(gui.diff_mode.diff_source, CompareDiffSource::CommitFiles); + assert_eq!(gui.diff_mode.focus, DiffModeFocus::DiffExploration); + assert_eq!( + gui.diff_mode.diff_files[gui.diff_mode.diff_files_selected].name, + file + ); + } + + #[test] + fn comparison_completion_removed_hash_or_ref_drops_stale_drilled_files() { + let (repo, mut gui) = comparison_history(); + super::super::diff_mode::open_selected_commit_files(&mut gui).unwrap(); + let hash = gui.diff_mode.files_commit.clone().unwrap(); + // The object still exists, but main now contains it: it has genuinely + // left A...B and must not retain an unrelated drilled file context. + git_script(&repo, &format!("git update-ref refs/heads/main {hash}")); + refresh_comparison(&mut gui); + assert_eq!(gui.diff_mode.commits.len(), 550); + assert_eq!(gui.diff_mode.commits_selected, 549); + assert_ne!(gui.diff_mode.selected_commit().unwrap().hash, hash); + assert!(gui.diff_mode.files_commit.is_none()); + assert_eq!(gui.diff_mode.diff_source, CompareDiffSource::Comparison); + assert_eq!( + gui.diff_mode + .diff_files + .iter() + .map(|f| &f.name) + .collect::>(), + gui.git + .diff_refs_files("main", "feature") + .unwrap() + .iter() + .map(|f| &f.name) + .collect::>() + ); + super::super::diff_mode::open_selected_commit_files(&mut gui).unwrap(); + assert!(gui.diff_mode.files_commit.is_some()); + // The remaining ref still contains the drilled commit. An invalid + // ref must not be mistaken for "not an ancestor" by the XOR check. + git_script(&repo, "git update-ref -d refs/heads/main"); + refresh_comparison(&mut gui); + assert!(gui.diff_mode.ahead_behind.is_none()); + assert!(gui.diff_mode.files_commit.is_none()); + assert!(gui.diff_mode.diff_files.is_empty()); + assert!(gui.diff_mode.commits.is_empty()); + assert_eq!(gui.diff_mode.diff_source, CompareDiffSource::Comparison); + } fn outcome(success: bool, stdout: &str, stderr: &str) -> ShellResult { ShellResult { diff --git a/src/gui/controller/diff_mode.rs b/src/gui/controller/diff_mode.rs index 3e2d025..c3a806d 100644 --- a/src/gui/controller/diff_mode.rs +++ b/src/gui/controller/diff_mode.rs @@ -811,6 +811,15 @@ fn handle_diff_exploration_key(gui: &mut Gui, key: KeyEvent) -> Result<()> { /// Reload the file list for the current A..B diff. pub fn reload_diff_files(gui: &mut Gui) -> Result<()> { + reload_diff_files_with_commit_limit(gui, crate::git::DEFAULT_COMMIT_LIMIT) +} + +/// Completion refreshes retain the pages the user has already loaded. Ref +/// changes still use the normal first-page limit via `reload_diff_files`. +pub(in crate::gui) fn reload_diff_files_with_commit_limit( + gui: &mut Gui, + commit_limit: usize, +) -> Result<()> { let ref_a = gui.diff_mode.ref_a.clone(); let ref_b = gui.diff_mode.ref_b.clone(); gui.diff_mode.ahead_behind = None; @@ -828,7 +837,7 @@ pub fn reload_diff_files(gui: &mut Gui) -> Result<()> { if gui.diff_mode.ahead_behind.is_some() { gui.diff_mode.commits = gui .git - .load_compare_commits(&ref_a, &ref_b, crate::git::DEFAULT_COMMIT_LIMIT, 0) + .load_compare_commits(&ref_a, &ref_b, commit_limit.max(1), 0) .unwrap_or_default(); gui.diff_mode.commits_revision = gui.diff_mode.commits_revision.wrapping_add(1); } @@ -837,11 +846,7 @@ pub fn reload_diff_files(gui: &mut Gui) -> Result<()> { Err(e) => { gui.diff_mode.diff_files.clear(); gui.diff_mode.tree_nodes.clear(); - gui.popup = PopupState::Message { - title: "Diff error".to_string(), - message: format!("{}", e), - kind: crate::gui::popup::MessageKind::Error, - }; + gui.show_error("Diff error", e); } } gui.diff_mode.set_focus(gui.diff_mode.focus); diff --git a/src/gui/mod.rs b/src/gui/mod.rs index d79bac0..cf297b9 100644 --- a/src/gui/mod.rs +++ b/src/gui/mod.rs @@ -588,6 +588,9 @@ pub struct Gui { pub layout: LayoutState, pub popup: PopupState, shell_command_job: Option, + /// Refresh notifications delayed by a shell modal, in arrival order. + deferred_notifications: VecDeque, + deferred_notification_visible: bool, pub diff_view: DiffViewState, /// Cached graph layouts used to render only the visible commit rows. commit_list_cache: presentation::commits::CommitListCache, @@ -955,11 +958,45 @@ fn image_diff_payload( impl Gui { fn show_error(&mut self, title: &str, err: anyhow::Error) { - self.popup = PopupState::Message { + self.show_notification(PopupState::Message { title: title.to_string(), message: format!("{:#}", err), kind: MessageKind::Error, - }; + }); + } + + fn show_notification(&mut self, notification: PopupState) { + if self.shell_command_ui_active() || self.deferred_notification_ui_active() { + // Auto-refresh can report the same failure repeatedly during a + // five-minute command. Keep one adjacent copy, not an error flood. + let duplicate = matches!( + (self.deferred_notifications.back(), ¬ification), + (Some(PopupState::Message { title: a, message: b, kind: c }), + PopupState::Message { title: x, message: y, kind: z }) + if a == x && b == y && c == z + ); + if !duplicate { + self.deferred_notifications.push_back(notification); + } + } else { + self.popup = notification; + } + } + + fn deferred_notification_ui_active(&self) -> bool { + self.deferred_notification_visible || !self.deferred_notifications.is_empty() + } + + fn receive_deferred_notifications(&mut self) { + // Never replace another editor, result, or notification. Dispatch one + // per dismissal and keep AI/remote results queued until it is seen. + if self.popup == PopupState::None && !self.shell_command_ui_active() { + self.deferred_notification_visible = false; + if let Some(notification) = self.deferred_notifications.pop_front() { + self.popup = notification; + self.deferred_notification_visible = true; + } + } } pub fn new( @@ -1075,6 +1112,8 @@ impl Gui { layout: LayoutState::default(), popup: PopupState::None, shell_command_job: None, + deferred_notifications: VecDeque::new(), + deferred_notification_visible: false, diff_view: { let mut dv = DiffViewState::new(); dv.wrap = diff_line_wrap; @@ -1429,6 +1468,7 @@ impl Gui { self.maybe_request_commit_details(); // Check for AI commit message generation results + self.receive_deferred_notifications(); self.receive_ai_commit_results(); // Check for completed incremental commit page loads @@ -2250,7 +2290,7 @@ impl Gui { // Shell input/results own the modal until dismissed. Leave AI results // queued instead of replacing a typed command or restoring an editor // that shell completion would immediately discard. - if self.shell_command_ui_active() { + if self.shell_command_ui_active() || self.deferred_notification_ui_active() { return; } while let Ok(result) = self.ai_commit_rx.try_recv() { @@ -2360,6 +2400,10 @@ impl Gui { } fn receive_filter_paths(&mut self) { + // A completed picker must not replace a shell input/output popup. + if self.shell_command_ui_active() || self.deferred_notification_ui_active() { + return; + } let result = { let Some(rx) = self.filter_paths_rx.as_ref() else { return; @@ -2435,12 +2479,15 @@ impl Gui { } Err(e) => { self.commit_history_complete = true; - if self.popup == PopupState::None { - self.popup = PopupState::Message { + if self.popup == PopupState::None + || self.shell_command_ui_active() + || self.deferred_notification_ui_active() + { + self.show_notification(PopupState::Message { title: "Commits".to_string(), message: format!("Could not load commits: {}", e), kind: MessageKind::Error, - }; + }); } } } @@ -2582,7 +2629,7 @@ impl Gui { fn receive_remote_op_results(&mut self) { // In particular, a remote failure must not replace the shell's modal // and then be lost when the shell completes. - if self.shell_command_ui_active() { + if self.shell_command_ui_active() || self.deferred_notification_ui_active() { return; } if let Ok(result) = self.remote_op_rx.try_recv() { @@ -3615,11 +3662,19 @@ impl Gui { } return Ok(()); } - if self.handle_ai_commit_cancel_key(key) { + if has_command_modifier(key.modifiers) && matches!(key.code, KeyCode::Char(_)) { return Ok(()); } - - if has_command_modifier(key.modifiers) && matches!(key.code, KeyCode::Char(_)) { + // Shell input/output own Escape as well as text. An armed AI cancel + // gesture must not intercept dismissal of an unrelated modal. + if matches!( + self.popup, + PopupState::ShellCommand { .. } | PopupState::CommandOutput { .. } + ) || self.deferred_notification_visible + { + return self.handle_popup_key(key); + } + if self.handle_ai_commit_cancel_key(key) { return Ok(()); } @@ -4652,6 +4707,7 @@ impl Gui { PopupState::Message { .. } => { // Any key dismisses the message self.popup = PopupState::None; + self.deferred_notification_visible = false; } PopupState::Menu { items, @@ -8784,16 +8840,16 @@ impl Gui { let branch = self.rebase_mode.branch_name.clone(); let count = self.rebase_mode.total_count; self.rebase_mode.exit(); - self.popup = crate::gui::popup::PopupState::Message { - title: "Rebase complete".to_string(), + self.show_notification(PopupState::Message { + title: "Rebase ended".to_string(), message: format!( - "Successfully rebased '{}' ({} commit{}).", + "Rebase of '{}' is no longer in progress ({} commit{}). It may have completed or been aborted.", branch, count, if count == 1 { "" } else { "s" }, ), - kind: crate::gui::popup::MessageKind::Info, - }; + kind: MessageKind::Info, + }); } } // Clear the dismissal flag once no rebase is in progress, so the next @@ -9627,6 +9683,217 @@ mod terminal_mouse_tests { assert!(gui.shell_command_job.is_none()); } + #[test] + fn shell_modal_defers_refresh_errors_and_rebase_notifications_in_order() { + for phase in 0..3 { + let repo = TempRepo::new("shell-refresh-notification-ownership"); + let mut gui = tree_test_gui(&repo); + match phase { + 0 => { + gui.handle_key(parse_key(":").unwrap()).unwrap(); + gui.handle_paste("printf typed-command".into()); + } + 1 => start_test_custom_command(&mut gui, "printf shell-result", true), + _ => { + gui.popup = PopupState::CommandOutput { + title: "Command output".into(), + message: "shell-result".into(), + kind: MessageKind::Info, + scroll: 0, + }; + } + } + let (tx, rx) = mpsc::channel(); + gui.files_refresh_rx = Some(rx); + gui.files_refresh_in_progress = true; + tx.send(Err(anyhow::anyhow!("refresh fixture failure"))) + .unwrap(); + gui.receive_files_refresh(); + assert!(!gui.files_refresh_in_progress); + assert!(gui.files_refresh_rx.is_none()); + // Repeated auto-refresh failures are coalesced while the shell is open. + gui.show_error("Refresh failed", anyhow::anyhow!("refresh fixture failure")); + assert_eq!(gui.deferred_notifications.len(), 1); + gui.rebase_mode.active = true; + gui.rebase_mode.phase = RebasePhase::InProgress; + gui.rebase_mode.branch_name = "main".into(); + gui.rebase_mode.total_count = 3; + gui.model.lock().unwrap().is_rebasing = false; + gui.after_model_refresh().unwrap(); + assert!( + !gui.rebase_mode.active, + "model/view refresh must still apply" + ); + assert_eq!(gui.deferred_notifications.len(), 2); + gui.remote_op_label = Some("Fetch".into()); + gui.remote_op_tx + .send(Err(anyhow::anyhow!("queued fetch failure"))) + .unwrap(); + gui.receive_deferred_notifications(); + gui.receive_remote_op_results(); + match phase { + 0 => assert!(matches!(&gui.popup, PopupState::ShellCommand { textarea } + if textarea.lines().join("\n") == "printf typed-command")), + 1 => { + assert!(matches!(gui.popup, PopupState::Loading { .. })); + wait_for_shell_command(&mut gui); + gui.receive_deferred_notifications(); + assert!( + matches!(&gui.popup, PopupState::CommandOutput { message, .. } + if message.contains("shell-result")) + ); + } + _ => assert!( + matches!(&gui.popup, PopupState::CommandOutput { message, .. } + if message == "shell-result") + ), + } + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_deferred_notifications(); + assert!( + matches!(&gui.popup, PopupState::Message { title, message, .. } + if title == "Refresh failed" && message == "refresh fixture failure") + ); + gui.receive_remote_op_results(); + assert!(gui.remote_op_label.is_some()); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_deferred_notifications(); + assert!( + matches!(&gui.popup, PopupState::Message { title, message, .. } + if title == "Rebase ended" && message.contains("completed or been aborted")) + ); + gui.receive_remote_op_results(); + assert!(gui.remote_op_label.is_some()); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_deferred_notifications(); + gui.receive_remote_op_results(); + assert!(matches!(&gui.popup, PopupState::Message { message, .. } + if message == "queued fetch failure")); + assert!(gui.deferred_notifications.is_empty()); + } + } + + #[test] + fn shell_modal_defers_filter_picker_and_notifications_defer_ai_results() { + let repo = TempRepo::new("shell-filter-and-notification-ai"); + let mut gui = tree_test_gui(&repo); + gui.handle_key(parse_key(":").unwrap()).unwrap(); + let (tx, rx) = mpsc::channel(); + gui.filter_paths_rx = Some(rx); + tx.send(Ok(vec!["src".into()])).unwrap(); + gui.receive_filter_paths(); + assert!(matches!(gui.popup, PopupState::ShellCommand { .. })); + assert!(gui.filter_paths_rx.is_some()); + gui.show_error("Refresh failed", anyhow::anyhow!("fixture failure")); + gui.ai_commit_job = Some(AiCommitJob { + generation: 1, + cancel: Arc::new(AtomicBool::new(false)), + cancel_armed_at: Some(Instant::now()), + }); + gui.ai_commit_tx + .send(AiCommitResult { + generation: 1, + result: Err(anyhow::anyhow!("AI fixture failure")), + }) + .unwrap(); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_deferred_notifications(); + gui.receive_ai_commit_results(); + gui.receive_filter_paths(); + assert!(matches!(&gui.popup, PopupState::Message { title, .. } + if title == "Refresh failed")); + assert!(gui.ai_commit_job.is_some()); + assert!(gui.filter_paths_rx.is_some()); + gui.commit_page_loading = true; + gui.commit_page_tx + .send(CommitPageResult { + generation: gui.commit_page_generation, + replace: false, + result: Err(anyhow::anyhow!("page fixture failure")), + }) + .unwrap(); + gui.receive_commit_page_results(); + assert!(!gui.commit_page_loading); + assert_eq!(gui.deferred_notifications.len(), 1); + // Notification Escape is modal too, not the second half of AI cancel. + gui.handle_key(parse_key("").unwrap()).unwrap(); + assert!(gui.ai_commit_job.is_some()); + gui.receive_deferred_notifications(); + gui.receive_ai_commit_results(); + assert!(matches!(&gui.popup, PopupState::Message { title, .. } + if title == "Commits")); + assert!(gui.ai_commit_job.is_some()); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_deferred_notifications(); + gui.receive_ai_commit_results(); + assert!(matches!(&gui.popup, PopupState::Message { title, .. } + if title == "AI generation failed")); + assert!(gui.ai_commit_job.is_none()); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_filter_paths(); + assert!(matches!(&gui.popup, PopupState::ListPicker { core, .. } + if core.items.iter().any(|item| item.value == "src"))); + } + + #[test] + fn shell_completion_keeps_output_and_defers_invalid_comparison_error() { + let repo = TempRepo::new("shell-comparison-refresh-error"); + let mut gui = tree_test_gui(&repo); + start_test_custom_command(&mut gui, "printf shell-result", true); + gui.diff_mode.enter(true); + gui.diff_mode.ref_a = "nonexistent-ref".into(); + gui.diff_mode.ref_b = "HEAD".into(); + wait_for_shell_command(&mut gui); + assert!( + matches!(&gui.popup, PopupState::CommandOutput { message, .. } + if message == "shell-result") + ); + assert_eq!(gui.deferred_notifications.len(), 1); + gui.handle_key(parse_key("").unwrap()).unwrap(); + gui.receive_deferred_notifications(); + assert!(matches!(&gui.popup, PopupState::Message { title, kind, .. } + if title == "Diff error" && *kind == MessageKind::Error)); + } + + #[test] + fn shell_popup_escape_does_not_arm_or_cancel_ai_generation() { + let repo = TempRepo::new("shell-ai-escape-priority"); + let mut gui = tree_test_gui(&repo); + for output in [false, true] { + for armed in [false, true] { + let cancel = Arc::new(AtomicBool::new(false)); + gui.ai_commit_job = Some(AiCommitJob { + generation: 1, + cancel: Arc::clone(&cancel), + cancel_armed_at: armed.then(Instant::now), + }); + let original_armed = gui.ai_commit_job.as_ref().unwrap().cancel_armed_at; + gui.pending_commit_popup = Some(PopupState::None); + if output { + gui.popup = PopupState::CommandOutput { + title: "Command output".into(), + message: "done".into(), + kind: MessageKind::Info, + scroll: 0, + }; + } else { + gui.handle_key(parse_key(":").unwrap()).unwrap(); + } + gui.handle_key(parse_key("").unwrap()).unwrap(); + assert!(gui.popup == PopupState::None); + assert!(!cancel.load(Ordering::Relaxed)); + assert_eq!( + gui.ai_commit_job.as_ref().unwrap().cancel_armed_at, + original_armed + ); + assert!(gui.pending_commit_popup.is_some()); + assert!(gui.saved_commit_popup.is_none()); + gui.pending_commit_popup = None; + gui.ai_commit_job = None; + } + } + } + #[test] fn shell_modal_defers_ctrl_g_result_without_losing_command_or_commit_draft() { let repo = TempRepo::new("shell-ai-popup-ownership"); diff --git a/src/gui/views.rs b/src/gui/views.rs index 9b9f48c..8812692 100644 --- a/src/gui/views.rs +++ b/src/gui/views.rs @@ -1845,6 +1845,66 @@ mod tests { } } + #[test] + fn shell_footers_use_bold_keys_and_muted_action_labels() { + let theme = Theme::default(); + for (popup, hint, action) in [ + ( + PopupState::ShellCommand { + textarea: crate::gui::popup::make_textarea(""), + }, + " enter confirm esc cancel", + "confirm", + ), + ( + PopupState::CommandOutput { + title: "Command output".into(), + message: "result".into(), + kind: MessageKind::Info, + scroll: 0, + }, + " enter close esc close j/k scroll y copy pgup/pgdn page g/G top/bottom", + "close", + ), + ] { + let mut terminal = Terminal::new(TestBackend::new(120, 24)).unwrap(); + terminal + .draw(|frame| { + render_popup( + frame, + &popup, + Rect::new(0, 0, 120, 24), + 0, + &theme, + false, + false, + ); + }) + .unwrap(); + let buffer = terminal.backend().buffer(); + let (row, text) = (0..24) + .find_map(|y| { + let text: String = (0..120).map(|x| buffer[(x, y)].symbol()).collect(); + text.contains(hint).then_some((y, text)) + }) + .expect("styled footer should be visible in the rendered popup"); + for key in ["enter", "esc"] { + let x = text[..text.find(key).unwrap()].chars().count() as u16; + for col in x..x + key.len() as u16 { + let cell = &buffer[(col, row)]; + assert_eq!(cell.fg, theme.text); + assert!(cell.modifier.contains(ratatui::style::Modifier::BOLD)); + } + } + let x = text[..text.find(action).unwrap()].chars().count() as u16; + for col in x..x + action.len() as u16 { + let cell = &buffer[(col, row)]; + assert_eq!(cell.fg, theme.text_dimmed); + assert!(!cell.modifier.contains(ratatui::style::Modifier::BOLD)); + } + } + } + #[test] fn literal_shell_prompt_renders_in_small_terminals() { let mut textarea = crate::gui::popup::make_textarea(""); @@ -3592,9 +3652,26 @@ pub fn render_popup( inner.height.saturating_sub(1), ), ); + let key_style = Style::default() + .fg(theme.text) + .add_modifier(ratatui::style::Modifier::BOLD); + let desc_style = Style::default().fg(theme.text_dimmed); + let hint_line = Line::from(vec![ + Span::styled(" enter ", key_style), + Span::styled("close ", desc_style), + Span::styled("esc ", key_style), + Span::styled("close ", desc_style), + Span::styled("j/k ", key_style), + Span::styled("scroll ", desc_style), + Span::styled("y ", key_style), + Span::styled("copy ", desc_style), + Span::styled("pgup/pgdn ", key_style), + Span::styled("page ", desc_style), + Span::styled("g/G ", key_style), + Span::styled("top/bottom", desc_style), + ]); frame.render_widget( - Paragraph::new("j/k scroll · PgUp/PgDn · g/G · y copy · esc/enter close") - .style(Style::default().fg(theme.text_dimmed)), + Paragraph::new(hint_line), Rect::new( inner.x, inner.y + inner.height.saturating_sub(1), diff --git a/src/os/shell.rs b/src/os/shell.rs index 8068e61..8eb2ce8 100644 --- a/src/os/shell.rs +++ b/src/os/shell.rs @@ -39,7 +39,7 @@ impl ShellJob { /// /// With config loading disabled, use `sh -c`, regardless of `shell`, to /// preserve custom-command semantics. Otherwise bash/zsh load their rc - /// files; other shells receive native `-c` arguments, not a POSIX wrapper. + /// files; fish uses a native wrapper, and other shells receive native `-c`. /// Remaining processes in the job's group are terminated on shell exit. /// Currently requires Unix process-group support. pub fn spawn( @@ -127,8 +127,8 @@ impl Drop for ShellJob { } } -// The user command and the absolute repo path are positional arguments, never -// interpolated into these scripts. Save them before sourcing: rc files may +// The user command and absolute repo path are arguments/environment values, +// never interpolated into these scripts. Save them before sourcing: rc files may // change positional parameters or cwd. `eval` parses the command AFTER aliases // and functions have been defined. Stay non-interactive to avoid job-control // process groups and attempts to acquire the TUI's terminal. @@ -143,19 +143,34 @@ if [ -r "$HOME/.bashrc" ]; then fi builtin shopt -s expand_aliases builtin cd -- "$_lazygitrs_repo" || exit +builtin set -- builtin eval -- "$_lazygitrs_command" "#; const ZSH_WRAPPER: &str = r#" -builtin readonly _lazygitrs_repo="$1" _lazygitrs_command="$2" +builtin readonly _lazygitrs_repo="$_LAZYGITRS_JOB_REPO" _lazygitrs_command="$_LAZYGITRS_JOB_COMMAND" +builtin unset _LAZYGITRS_JOB_REPO _LAZYGITRS_JOB_COMMAND if [ -r "${ZDOTDIR:-$HOME}/.zshrc" ]; then builtin source "${ZDOTDIR:-$HOME}/.zshrc" fi builtin setopt aliases builtin cd -- "$_lazygitrs_repo" || exit +builtin set -- builtin eval -- "$_lazygitrs_command" "#; +// fish reads its config before -c. Environment transport survives startup +// changes to $argv; clear $argv before eval so the command sees no job metadata +// or startup arguments. Quoted expansions preserve spaces, quotes and newlines. +const FISH_WRAPPER: &str = r#" +builtin set --local _lazygitrs_repo "$_LAZYGITRS_JOB_REPO" +builtin set --local _lazygitrs_command "$_LAZYGITRS_JOB_COMMAND" +builtin set --erase --global _LAZYGITRS_JOB_REPO _LAZYGITRS_JOB_COMMAND +builtin cd "$_lazygitrs_repo"; or exit +builtin set --global argv +builtin eval "$_lazygitrs_command" +"#; + fn shell_command( repo: &Path, command: &str, @@ -182,9 +197,16 @@ fn shell_command( } Some("zsh") if load_shell_config => { // Native non-interactive startup loads .zshenv (including ZDOTDIR). - cmd.args(["-c", ZSH_WRAPPER, "lazygitrs"]) - .arg(&repo) - .arg(command); + // Transport via env instead of $1/$2: .zshenv can run `set --` + // before our wrapper starts. Do not disable it with -f. + cmd.env("_LAZYGITRS_JOB_REPO", &repo) + .env("_LAZYGITRS_JOB_COMMAND", command) + .args(["-c", ZSH_WRAPPER, "lazygitrs"]); + } + Some("fish") if load_shell_config => { + cmd.env("_LAZYGITRS_JOB_REPO", &repo) + .env("_LAZYGITRS_JOB_COMMAND", command) + .args(["-c", FISH_WRAPPER]); } _ => { cmd.arg("-c").arg(command); @@ -490,7 +512,7 @@ mod tests { fn non_posix_shells_receive_only_native_arguments() { let repo = std::env::current_dir().unwrap(); let input = "printf '%s' 'quotes; $variables; $(substitution)'"; - for shell in ["/usr/bin/fish", "/usr/bin/nu", "/bin/dash"] { + for shell in ["/usr/bin/nu", "/bin/dash"] { let cmd = shell_command(&repo, input, OsStr::new(shell), true).unwrap(); assert_eq!(cmd.get_program(), shell); assert_eq!(cmd.get_args().collect::>(), ["-c", input]); @@ -509,7 +531,7 @@ mod tests { fn wrappers_keep_paths_and_commands_in_separate_arguments() { let repo = std::env::current_dir().unwrap().canonicalize().unwrap(); let input = "printf '%s' '\"; touch should-not-exist; #'"; - for shell in ["/bin/bash", "/bin/zsh"] { + for shell in ["/bin/bash"] { let cmd = shell_command(&repo, input, OsStr::new(shell), true).unwrap(); let args = cmd.get_args().map(OsStr::to_os_string).collect::>(); assert_eq!(args[args.len() - 2], repo.as_os_str()); @@ -518,6 +540,33 @@ mod tests { } } + #[test] + fn startup_safe_wrappers_transport_literal_values_in_environment() { + let repo = std::env::current_dir().unwrap().canonicalize().unwrap(); + let input = "printf '%s' '\"; literal $value; #'"; + for (shell, wrapper) in [("/bin/zsh", ZSH_WRAPPER), ("/usr/bin/fish", FISH_WRAPPER)] { + let cmd = shell_command(&repo, input, OsStr::new(shell), true).unwrap(); + let args = cmd.get_args().collect::>(); + assert_eq!(args[0], "-c"); + assert_eq!(args[1], wrapper); + assert!(!wrapper.contains(input)); + let env = cmd.get_envs().collect::>(); + assert!(env.contains(&(OsStr::new("_LAZYGITRS_JOB_REPO"), Some(repo.as_os_str())))); + assert!(env.contains(&( + OsStr::new("_LAZYGITRS_JOB_COMMAND"), + Some(OsStr::new(input)) + ))); + } + } + + #[test] + fn wrappers_clear_positional_arguments_before_eval() { + for wrapper in [BASH_WRAPPER, ZSH_WRAPPER] { + assert!(wrapper.contains("builtin set --\nbuiltin eval")); + } + assert!(FISH_WRAPPER.contains("builtin set --global argv\nbuiltin eval")); + } + #[cfg(unix)] mod execution { use std::fs; @@ -884,7 +933,7 @@ mod tests { let dir = TestDir::new(); let home = TestDir::new(); fs::write(home.0.join(rc), "alias shell_alias='printf alias'; shell_function() { printf function; }; cd /; set -- changed\n").unwrap(); - let mut cmd = shell_command(&dir.0, "shell_alias; shell_function; printf '\\n'; pwd -P; printf '%s' '\"; literal $value; #'", OsStr::new(shell), true).unwrap(); + let mut cmd = shell_command(&dir.0, "shell_alias; shell_function; printf '\\n'; pwd -P; printf '%s' '\"; literal $value; #'; printf '\\nargs:%s' \"$#\"; printf '<%s>' \"$@\"", OsStr::new(shell), true).unwrap(); cmd.env("HOME", &home.0).env_remove("ZDOTDIR"); if zdotdir { cmd.env("HOME", &dir.0).env("ZDOTDIR", &home.0); @@ -894,7 +943,10 @@ mod tests { assert!(outcome.success, "{outcome:?}"); assert_eq!( outcome.stdout, - format!("aliasfunction\n{}\n\"; literal $value; #", dir.0.display()) + format!( + "aliasfunction\n{}\n\"; literal $value; #\nargs:0<>", + dir.0.display() + ) ); assert!(outcome.stderr.is_empty(), "{outcome:?}"); } @@ -911,6 +963,67 @@ mod tests { startup_test("/bin/zsh", ".zshrc", true); } + #[test] + fn bash_and_zsh_commands_have_no_wrapper_arguments_without_rc() { + for shell in ["/bin/bash", "/bin/zsh"] { + if !installed(shell) { + continue; + } + let dir = TestDir::new(); + let home = TestDir::new(); + let mut cmd = shell_command( + &dir.0, + "printf 'args:%s' \"$#\"; printf '<%s>' \"$@\"", + OsStr::new(shell), + true, + ) + .unwrap(); + cmd.env("HOME", &home.0).env_remove("ZDOTDIR"); + let outcome = + result(&mut ShellJob::spawn_command(cmd, DEFAULT_TIMEOUT).unwrap()).unwrap(); + assert!(outcome.success, "{outcome:?}"); + assert_eq!(outcome.stdout, "args:0<>"); + assert!(outcome.stderr.is_empty(), "{outcome:?}"); + } + } + + #[test] + fn zshenv_can_replace_arguments_and_redirect_zdotdir() { + if !installed("/bin/zsh") { + return; + } + let dir = TestDir::new(); + let home = TestDir::new(); + let config = home.0.join("redirected config"); + fs::create_dir(&config).unwrap(); + fs::write( + home.0.join(".zshenv"), + "set -- startup changed; cd /; export ZDOTDIR=\"$HOME/redirected config\"\n", + ) + .unwrap(); + fs::write( + config.join(".zshrc"), + "alias shell_alias='printf alias'; set -- rc changed; cd /\n", + ) + .unwrap(); + let mut cmd = shell_command( + &dir.0, + "shell_alias; printf '\\n'; pwd -P; printf 'args:%s' \"$#\"", + OsStr::new("/bin/zsh"), + true, + ) + .unwrap(); + cmd.env("HOME", &home.0).env_remove("ZDOTDIR"); + let outcome = + result(&mut ShellJob::spawn_command(cmd, DEFAULT_TIMEOUT).unwrap()).unwrap(); + assert!(outcome.success, "{outcome:?}"); + assert_eq!( + outcome.stdout, + format!("alias\n{}\nargs:0", dir.0.display()) + ); + assert!(outcome.stderr.is_empty(), "{outcome:?}"); + } + #[test] fn custom_commands_do_not_load_aliases_or_functions() { let dir = TestDir::new(); @@ -936,15 +1049,23 @@ mod tests { #[test] fn fish_runs_native_syntax_when_installed() { - if !installed("fish") { + let Some(shell) = ["fish", "/opt/homebrew/bin/fish", "/usr/local/bin/fish"] + .into_iter() + .find(|shell| installed(shell)) + else { + eprintln!( + "Skipping fish execution test: fish is not installed (including Homebrew paths)" + ); return; - } + }; let dir = TestDir::new(); let home = TestDir::new(); + fs::create_dir(home.0.join("fish")).unwrap(); + fs::write(home.0.join("fish/config.fish"), "function shell_function; printf function; end\ncd /\nset --global argv startup changed\n").unwrap(); let mut cmd = shell_command( &dir.0, - "set value fish; printf '%s' $value", - OsStr::new("fish"), + "set value fish; printf '%s' $value; shell_function; printf '\\n'; pwd -P; printf 'args:%s\\n' (count $argv); printf '%s' '\"; literal $value; #'; printf '%s' \"single'quote\"", + OsStr::new(shell), true, ) .unwrap(); @@ -952,7 +1073,14 @@ mod tests { let outcome = result(&mut ShellJob::spawn_command(cmd, DEFAULT_TIMEOUT).unwrap()).unwrap(); assert!(outcome.success, "{outcome:?}"); - assert_eq!(outcome.stdout, "fish"); + assert_eq!( + outcome.stdout, + format!( + "fishfunction\n{}\nargs:0\n\"; literal $value; #single'quote", + dir.0.display() + ) + ); + assert!(outcome.stderr.is_empty(), "{outcome:?}"); } } }