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 <thomas@gmail.com>
2026-09-22 08:02:24 +00:00 · 2026-08-03 15:11:55 +08:00
parent f2c0c2dcdf
commit c2ee9483a9
7 changed files with 382 additions and 44 deletions
@@ -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<String>,
+    /// 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<DetectedShell>,
@@ -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<DetectedShell> {
@@ -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<DetectedShell>,
+    configured: Option<(String, Vec<String>)>,
+    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<String> {
    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<Item = PathBuf>) -> Option<
 #[cfg(windows)]
 fn find_in_path(exe: &str) -> Option<PathBuf> {
    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<PathBuf> {
+    std::env::split_paths(path)
        .map(|dir| dir.join(exe))
        .find(|p| p.is_file())
 }
@@ -257,6 +388,16 @@ fn detect_windows() -> Vec<DetectedShell> {
        ));
    }

+    // 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<DetectedShell> {
            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<DetectedShell> {
            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");
@@ -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]
@@ -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]
@@ -4041,13 +4041,17 @@ impl Tty7App {
        } else {
            Some(ShellConfig { program, args })
        };
-        let cfg = cx.global_mut::<Config>();
-        if cfg.shell == shell {
-            return;
+        {
+            let cfg = cx.global_mut::<Config>();
+            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(
@@ -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::<crate::core::config::Config>()
-                    .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<Self>) {
@@ -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),
@@ -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);
+    }
 }