From c2ee9483a96aba35cda96d08251a33e5a5215b85 Mon Sep 17 00:00:00 2001 From: ARNO Date: Mon, 3 Aug 2026 15:11:55 +0800 Subject: [PATCH] feat(shell): add detected and custom shells to the new terminal menu (#311) * feat(windows): add Nushell support * feat(shell): add custom shell to the terminal menu * feat(shell): add cross-platform Nushell support * fix(shell): preserve custom arguments across shell inventories * fix(shell): preserve configured command identity --------- Co-authored-by: thomas --- crates/tty7-core/src/core/shells.rs | 317 ++++++++++++++++++++++++++- crates/tty7-core/src/daemon/spawn.rs | 35 +-- crates/tty7-core/src/host/remote.rs | 5 +- src/ui/app.rs | 16 +- src/ui/remote_workspace.rs | 15 +- src/ui/settings.rs | 5 +- src/ui/tab_strip.rs | 33 ++- 7 files changed, 382 insertions(+), 44 deletions(-) diff --git a/crates/tty7-core/src/core/shells.rs b/crates/tty7-core/src/core/shells.rs index dca7c4df..95235f99 100644 --- a/crates/tty7-core/src/core/shells.rs +++ b/crates/tty7-core/src/core/shells.rs @@ -1,3 +1,5 @@ +#[cfg(windows)] +use std::ffi::OsStr; use std::path::Path; #[cfg(windows)] use std::path::PathBuf; @@ -9,6 +11,11 @@ pub struct DetectedShell { pub label: String, pub program: String, pub args: Vec, + /// Marks arguments supplied by tty7's shell detection rather than by the user. + /// Older peers omit this field, and their inventory arguments were all treated + /// as tty7 defaults, so `true` preserves the previous protocol behavior. + #[serde(default = "default_true")] + pub args_are_tty7_defaults: bool, } impl DetectedShell { @@ -17,10 +24,15 @@ impl DetectedShell { label: label.into(), program: program.into(), args: Vec::new(), + args_are_tty7_defaults: true, } } } +fn default_true() -> bool { + true +} + #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct ShellInventory { pub shells: Vec, @@ -29,10 +41,7 @@ pub struct ShellInventory { pub fn inventory() -> ShellInventory { let configured = crate::core::config::shell_command(); - ShellInventory { - shells: detect_shells(), - default_name: default_shell_name(configured.as_ref().map(|(p, _)| p.as_str())), - } + inventory_from(detect_shells(), configured, &login_shell()) } pub fn detect_shells() -> Vec { @@ -54,6 +63,107 @@ pub fn default_shell_name(configured: Option<&str>) -> String { basename(&program) } +/// Combines the detected shells with the explicit shell from Settings. +/// +/// A bare configured command such as `nu` matches the detected executable with +/// the same basename because both resolve through PATH. Explicit paths only +/// match the same path, so choosing a second installation of the same shell +/// still creates a useful, distinct menu entry. The detected label wins for a +/// match, preserving friendly names such as "Nushell" and "PowerShell 7". +fn inventory_from( + mut shells: Vec, + configured: Option<(String, Vec)>, + fallback_program: &str, +) -> ShellInventory { + let configured = configured.filter(|(program, _)| !program.trim().is_empty()); + let default_program = configured + .as_ref() + .map_or(fallback_program, |(program, _)| program.as_str()); + + if let Some(default_index) = shells + .iter() + .position(|shell| same_shell_program(&shell.program, default_program)) + { + // A configured command may resolve to an already detected executable. + // Keep the detected friendly label, but retain the user's command, + // launch arguments, and their origin so local and remote menus behave + // identically. + if let Some((program, args)) = configured.as_ref() { + // Keep a bare command bare: it must continue to resolve through PATH. + // The detected entry can be the login shell (which intentionally wins + // inventory deduplication), and that absolute path is not necessarily + // the executable the configured bare command would resolve to. + shells[default_index].program.clone_from(program); + shells[default_index].args.clone_from(args); + shells[default_index].args_are_tty7_defaults = false; + } + let default_name = shells[default_index].label.clone(); + return ShellInventory { + shells, + default_name, + }; + } + + let mut default_name = basename(default_program); + if let Some((program, args)) = configured { + if shells.iter().any(|shell| shell.label == default_name) { + default_name.push_str(" (Configured)"); + } + // The configured shell is the platform default, so keep it at the top + // just like the detected login shell while retaining its custom args. + shells.insert( + 0, + DetectedShell { + label: default_name.clone(), + program, + args, + args_are_tty7_defaults: false, + }, + ); + } + + ShellInventory { + shells, + default_name, + } +} + +/// Compares executable identities without collapsing two explicit installs. +fn same_shell_program(detected: &str, configured: &str) -> bool { + let detected = detected.trim(); + let configured = configured.trim(); + if detected.is_empty() || configured.is_empty() { + return false; + } + + let detected_path = Path::new(detected); + let configured_path = Path::new(configured); + if is_bare_program(configured_path) { + return basename(detected) == basename(configured); + } + if is_bare_program(detected_path) { + return false; + } + + comparable_program_path(detected_path) == comparable_program_path(configured_path) +} + +fn is_bare_program(program: &Path) -> bool { + program.components().count() <= 1 +} + +/// Canonicalization folds harmless `.` and `..` differences when the target +/// exists. The textual fallback still gives stable behavior for configured +/// paths that have not been installed yet. +fn comparable_program_path(program: &Path) -> String { + let resolved = std::fs::canonicalize(program).unwrap_or_else(|_| program.to_path_buf()); + let mut value = resolved.to_string_lossy().into_owned(); + if cfg!(windows) { + value = value.replace('/', "\\").to_ascii_lowercase(); + } + value +} + /// The user's login shell, straight from the passwd database. /// /// `$SHELL` is a snapshot taken when the session logged in, so `chsh` does not @@ -132,6 +242,16 @@ fn basename(program: &str) -> String { } } +/// Keeps conventional executable names for POSIX shells while giving Nushell +/// the same product name in the menu on every supported desktop platform. +fn shell_label(name: &str) -> String { + if name.eq_ignore_ascii_case("nu") { + "Nushell".to_string() + } else { + name.to_string() + } +} + #[cfg_attr(windows, allow(dead_code))] fn parse_etc_shells(content: &str) -> Vec { content @@ -155,7 +275,7 @@ fn unix_shells_from( } let name = basename(&path); if seen.insert(name.clone()) { - out.push(DetectedShell::bare(name, path)); + out.push(DetectedShell::bare(shell_label(&name), path)); } } out @@ -239,7 +359,18 @@ fn pick_first_existing(candidates: impl IntoIterator) -> Option< #[cfg(windows)] fn find_in_path(exe: &str) -> Option { let path = std::env::var_os("PATH")?; - std::env::split_paths(&path) + find_in_path_var(exe, &path) +} + +/// Finds an exact executable name using Windows' ordered `PATH` directories. +/// +/// The caller supplies the `.exe` suffix explicitly so a directory with the +/// same name is never mistaken for a launchable shell. Keeping the path value +/// separate also makes the lookup testable without mutating process-global +/// environment variables while other tests are running. +#[cfg(windows)] +fn find_in_path_var(exe: &str, path: &OsStr) -> Option { + std::env::split_paths(path) .map(|dir| dir.join(exe)) .find(|p| p.is_file()) } @@ -257,6 +388,16 @@ fn detect_windows() -> Vec { )); } + // Nushell has no stable Windows installation directory across package + // managers, so mirror command resolution and offer the first `nu.exe` + // reachable through the current system PATH. + if let Some(nu) = find_in_path("nu.exe") { + out.push(DetectedShell::bare( + "Nushell", + nu.to_string_lossy().into_owned(), + )); + } + let ps5 = system_root .join("System32") .join("WindowsPowerShell") @@ -285,6 +426,7 @@ fn detect_windows() -> Vec { label: "Git Bash".into(), program: bash.to_string_lossy().into_owned(), args: vec!["-i".into(), "-l".into()], + args_are_tty7_defaults: true, }); } @@ -293,6 +435,7 @@ fn detect_windows() -> Vec { label: format!("WSL · {distro}"), program: "wsl.exe".into(), args: vec!["--distribution".into(), distro, "--cd".into(), "~".into()], + args_are_tty7_defaults: true, }); } @@ -421,6 +564,29 @@ mod tests { assert_eq!(cands.len(), PATH_PROBED_SHELLS.len() * 2); } + #[test] + fn nushell_is_probed_from_every_unix_path_directory() { + let candidates = path_shell_candidates("/opt/homebrew/bin:/home/user/.local/bin"); + let nushell: Vec<_> = candidates + .iter() + .filter(|candidate| candidate.ends_with("/nu")) + .map(String::as_str) + .collect(); + + assert_eq!( + nushell, + ["/opt/homebrew/bin/nu", "/home/user/.local/bin/nu"] + ); + + let detected = unix_shells_from(candidates, |candidate| { + candidate == "/home/user/.local/bin/nu" + }); + assert_eq!( + detected, + [DetectedShell::bare("Nushell", "/home/user/.local/bin/nu")] + ); + } + #[test] fn etc_shells_still_contributes_what_path_does_not_reach() { let etc = ["/bin/zsh".to_string(), "/opt/weird/ksh".to_string()]; @@ -549,6 +715,145 @@ mod tests { } } + #[cfg(windows)] + #[test] + fn windows_path_probe_finds_the_first_nushell_file() { + let first = tempfile::tempdir().unwrap(); + let second = tempfile::tempdir().unwrap(); + let directory_named_like_nu = first.path().join("nu.exe"); + std::fs::create_dir(&directory_named_like_nu).unwrap(); + let expected = second.path().join("nu.exe"); + std::fs::write(&expected, b"test executable placeholder").unwrap(); + + let path = std::env::join_paths([first.path(), second.path()]).unwrap(); + assert_eq!(find_in_path_var("nu.exe", &path), Some(expected)); + } + + #[test] + fn a_unique_configured_shell_is_added_first_with_its_args() { + let detected = vec![DetectedShell::bare("System Shell", "system-shell")]; + let inventory = inventory_from( + detected, + Some(( + "custom-shell".into(), + vec!["--login".into(), "--verbose".into()], + )), + "system-shell", + ); + + assert_eq!(inventory.default_name, "custom-shell"); + assert_eq!(inventory.shells.len(), 2); + assert_eq!(inventory.shells[0].label, "custom-shell"); + assert_eq!(inventory.shells[0].program, "custom-shell"); + assert_eq!(inventory.shells[0].args, ["--login", "--verbose"]); + assert!(!inventory.shells[0].args_are_tty7_defaults); + } + + #[test] + fn a_bare_configured_name_reuses_the_detected_friendly_entry() { + let detected_program = if cfg!(windows) { + r"C:\Tools\Nushell\nu.exe" + } else { + "/opt/nushell/bin/nu" + }; + let inventory = inventory_from( + vec![DetectedShell::bare("Nushell", detected_program)], + Some(("nu".into(), vec!["--login".into()])), + "fallback-shell", + ); + + assert_eq!(inventory.shells.len(), 1, "the same shell was duplicated"); + assert_eq!(inventory.default_name, "Nushell"); + assert_eq!(inventory.shells[0].program, "nu"); + assert_eq!(inventory.shells[0].args, ["--login"]); + assert!(!inventory.shells[0].args_are_tty7_defaults); + } + + #[test] + fn a_bare_configured_command_keeps_path_resolution_after_deduplication() { + let inventory = inventory_from( + vec![DetectedShell::bare("bash", "/bin/bash")], + Some(("bash".into(), Vec::new())), + "fallback-shell", + ); + + assert_eq!(inventory.shells.len(), 1); + assert_eq!(inventory.shells[0].program, "bash"); + assert_eq!(inventory.default_name, "bash"); + } + + #[test] + fn explicit_same_named_shells_at_different_paths_stay_distinct() { + let first = if cfg!(windows) { + r"C:\Shells\first\custom.exe" + } else { + "/opt/shells/first/custom" + }; + let second = if cfg!(windows) { + r"D:\Shells\second\custom.exe" + } else { + "/opt/shells/second/custom" + }; + let inventory = inventory_from( + vec![DetectedShell::bare("custom", first)], + Some((second.into(), Vec::new())), + "fallback-shell", + ); + + assert_eq!(inventory.shells.len(), 2); + assert_eq!(inventory.shells[0].program, second); + assert_eq!(inventory.shells[0].label, "custom (Configured)"); + assert_eq!(inventory.shells[1].label, "custom"); + assert_eq!(inventory.default_name, "custom (Configured)"); + } + + #[test] + fn an_explicit_configured_path_does_not_collapse_a_bare_detected_name() { + let configured = if cfg!(windows) { + r"C:\Shells\custom.exe" + } else { + "/opt/shells/custom" + }; + let detected = if cfg!(windows) { + "custom.exe" + } else { + "custom" + }; + let inventory = inventory_from( + vec![DetectedShell::bare("custom", detected)], + Some((configured.into(), Vec::new())), + "fallback-shell", + ); + + assert_eq!(inventory.shells.len(), 2); + assert_eq!(inventory.shells[0].program, configured); + } + + #[test] + fn detected_label_names_the_unconfigured_platform_default() { + let program = if cfg!(windows) { + r"C:\Program Files\PowerShell\7\pwsh.exe" + } else { + "/opt/homebrew/bin/zsh" + }; + let label = if cfg!(windows) { "PowerShell 7" } else { "zsh" }; + let inventory = inventory_from(vec![DetectedShell::bare(label, program)], None, program); + + assert_eq!(inventory.default_name, label); + assert_eq!(inventory.shells.len(), 1); + assert!(inventory.shells[0].args_are_tty7_defaults); + } + + #[test] + fn inventories_from_older_peers_treat_arguments_as_tty7_defaults() { + let inventory: ShellInventory = serde_json::from_str( + r#"{"shells":[{"label":"Git Bash","program":"bash","args":["-l"]}],"default_name":"Git Bash"}"#, + ) + .expect("an inventory without argument-origin metadata should remain compatible"); + + assert!(inventory.shells[0].args_are_tty7_defaults); + } + #[test] fn default_shell_name_prefers_the_configured_program() { assert_eq!(default_shell_name(Some("/usr/bin/fish")), "fish"); diff --git a/crates/tty7-core/src/daemon/spawn.rs b/crates/tty7-core/src/daemon/spawn.rs index 978e1ab1..c3102d08 100644 --- a/crates/tty7-core/src/daemon/spawn.rs +++ b/crates/tty7-core/src/daemon/spawn.rs @@ -347,7 +347,15 @@ fn is_supported_shell(path: &Path) -> bool { }; matches!( name.to_ascii_lowercase().as_str(), - "zsh" | "bash" | "fish" | "pwsh" | "powershell" | "powershell.exe" | "pwsh.exe" + "zsh" + | "bash" + | "fish" + | "nu" + | "nu.exe" + | "pwsh" + | "powershell" + | "powershell.exe" + | "pwsh.exe" ) } @@ -635,6 +643,19 @@ mod exe_name_tests { assert_eq!(strip_exe_suffix(".exe"), ""); assert_eq!(strip_exe_suffix("exe"), "exe"); } + + #[test] + fn supported_shell_detection_matches_shell_basenames_only() { + assert!(is_supported_shell(Path::new("/opt/homebrew/bin/fish"))); + assert!(is_supported_shell(Path::new("/bin/zsh"))); + assert!(is_supported_shell(Path::new("/usr/bin/bash"))); + assert!(is_supported_shell(Path::new("/opt/homebrew/bin/nu"))); + assert!(is_supported_shell(Path::new("/portable/Nu.EXE"))); + assert!(!is_supported_shell(Path::new( + "/Applications/kitty.app/kitty" + ))); + assert!(!is_supported_shell(Path::new("/usr/bin/omp"))); + } } #[cfg(all(test, unix))] @@ -642,18 +663,6 @@ mod tests { use super::*; use std::io::ErrorKind; use std::os::unix::net::UnixStream; - use std::path::Path; - - #[test] - fn supported_shell_detection_matches_shell_basenames_only() { - assert!(is_supported_shell(Path::new("/opt/homebrew/bin/fish"))); - assert!(is_supported_shell(Path::new("/bin/zsh"))); - assert!(is_supported_shell(Path::new("/usr/bin/bash"))); - assert!(!is_supported_shell(Path::new( - "/Applications/kitty.app/kitty" - ))); - assert!(!is_supported_shell(Path::new("/usr/bin/omp"))); - } #[cfg(any(target_os = "macos", target_os = "linux"))] #[test] diff --git a/crates/tty7-core/src/host/remote.rs b/crates/tty7-core/src/host/remote.rs index 1c3398de..3f2090e9 100644 --- a/crates/tty7-core/src/host/remote.rs +++ b/crates/tty7-core/src/host/remote.rs @@ -1016,7 +1016,8 @@ mod tests { shells: vec![crate::core::shells::DetectedShell { label: "zsh".into(), program: "/usr/bin/zsh".into(), - args: vec![], + args: vec!["--no-rcs".into()], + args_are_tty7_defaults: false, }], default_name: "zsh".into(), })), @@ -1029,6 +1030,8 @@ mod tests { assert_eq!(seen.recv().unwrap(), ControlRequest::Shells); assert_eq!(inv.default_name, "zsh"); assert_eq!(inv.shells[0].program, "/usr/bin/zsh"); + assert_eq!(inv.shells[0].args, ["--no-rcs"]); + assert!(!inv.shells[0].args_are_tty7_defaults); } #[test] diff --git a/src/ui/app.rs b/src/ui/app.rs index b4c3737f..f4f6a6cd 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -4041,13 +4041,17 @@ impl Tty7App { } else { Some(ShellConfig { program, args }) }; - let cfg = cx.global_mut::(); - if cfg.shell == shell { - return; + { + let cfg = cx.global_mut::(); + if cfg.shell == shell { + return; + } + cfg.shell = shell; + cfg.save(); } - cfg.shell = shell; - cfg.save(); - cx.notify(); + // Shell discovery runs off the UI thread and now includes the saved + // configured shell, so refresh the menu without blocking Settings. + self.refresh_shells(cx); } pub(crate) fn set_working_directory_strategy( diff --git a/src/ui/remote_workspace.rs b/src/ui/remote_workspace.rs index 0a448d78..fa34bcaf 100644 --- a/src/ui/remote_workspace.rs +++ b/src/ui/remote_workspace.rs @@ -216,17 +216,10 @@ impl Tty7App { } } - pub(crate) fn default_shell_label(&self, cx: &gpui::App) -> String { - if self.shells_host.is_local() { - crate::core::shells::default_shell_name( - cx.global::() - .shell - .as_ref() - .map(|s| s.program.as_str()), - ) - } else { - self.shells.default_name.clone() - } + pub(crate) fn default_shell_label(&self, _cx: &gpui::App) -> String { + // ShellInventory maps configured programs to their displayed labels, + // including friendly Windows names that differ from the executable. + self.shells.default_name.clone() } pub(crate) fn refresh_shells(&mut self, cx: &mut Context) { diff --git a/src/ui/settings.rs b/src/ui/settings.rs index 9b7e08cb..c1954de4 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -165,7 +165,7 @@ fn settings_search_entries() -> &'static [SearchEntry] { SearchEntry { section: Terminal, title: "Program", - keywords: "shell binary zsh bash fish pwsh powershell executable launch", + keywords: "shell binary zsh bash fish nu nushell pwsh powershell executable launch", }, SearchEntry { section: Terminal, @@ -3257,7 +3257,7 @@ impl Tty7App { )) .child(self.settings_row( "Program", - "Executable name on PATH or an absolute path. e.g. zsh, fish, pwsh.", + "Executable name on PATH or an absolute path. e.g. zsh, fish, nu, pwsh.", program_control, cx, )) @@ -4790,6 +4790,7 @@ mod tests { ("grouping", WindowTabs), ("threshold", WindowTabs), ("report mouse", Terminal), + ("nushell", Terminal), ("open files with", Terminal), ("bell", Terminal), ("known_hosts", Ssh), diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index e473c129..11f18f93 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -16,6 +16,7 @@ use crate::core::actions::{ TogglePalette, }; use crate::core::config::RightPanelTab; +use crate::core::shells::DetectedShell; use crate::daemon::protocol::ShellSpec; use crate::ui::app::{TILE_GLYPH, TILE_GLYPH_LINE, TILE_SIZE, Tab, Tty7App, tile_trailing_inset}; use crate::ui::hints::tab_badge_label; @@ -28,6 +29,17 @@ pub(crate) const GRAB_HANDLE_W: f32 = 80.; const KEEP_SEGMENTS: usize = 3; +/// Builds a launch specification without recomputing argument ownership locally. +/// The inventory may originate from a remote host, so only its transported +/// metadata can distinguish tty7 launch defaults from user-authored arguments. +fn shell_spec(shell: &DetectedShell) -> ShellSpec { + ShellSpec { + program: shell.program.clone(), + args: shell.args.clone(), + args_are_tty7_defaults: shell.args_are_tty7_defaults, + } +} + pub(crate) fn abbreviate_home(path: &str) -> std::borrow::Cow<'_, str> { use std::borrow::Cow; if path.starts_with('~') { @@ -539,11 +551,7 @@ impl Tty7App { button.dropdown_menu(move |menu, _window, _cx| { let mut menu = menu.min_w(px(220.)); for shell in &shells { - let spec = ShellSpec { - program: shell.program.clone(), - args: shell.args.clone(), - args_are_tty7_defaults: true, - }; + let spec = shell_spec(shell); let open = app.clone(); let item = if shell.label == default_name { let label: SharedString = shell.label.clone().into(); @@ -1115,4 +1123,19 @@ mod tests { assert_eq!(out.chars().count(), 41); assert!(out.ends_with('…')); } + + #[test] + fn configured_shell_arguments_remain_user_authored_in_the_menu() { + let shell = DetectedShell { + label: "custom".into(), + program: "custom-shell".into(), + args: vec!["--login".into()], + args_are_tty7_defaults: false, + }; + let spec = shell_spec(&shell); + + assert_eq!(spec.program, "custom-shell"); + assert_eq!(spec.args, ["--login"]); + assert!(!spec.args_are_tty7_defaults); + } }