terminal: Always allocate a login shell on macOS

crates/terminal/src/terminal.rs

@@ -45,7 +45,7 @@ use pty_info::{ProcessIdGetter, PtyProcessInfo};
 use serde::{Deserialize, Serialize};
 use settings::Settings;
 use smol::channel::{Receiver, Sender};
-use task::{HideStrategy, Shell, SpawnInTerminal};
+use task::{HideStrategy, Shell, ShellKind, SpawnInTerminal};
 use terminal_hyperlinks::RegexSearches;
 use terminal_settings::{AlternateScroll, CursorShape, TerminalSettings};
 use theme::{ActiveTheme, Theme};
@@ -264,8 +264,8 @@ impl Dimensions for TerminalBounds {
 #[derive(Error, Debug)]
 pub struct TerminalError {
     pub directory: Option<PathBuf>,
-    pub program: Option<String>,
-    pub args: Option<Vec<String>>,
+    pub program: String,
+    pub args: Vec<String>,
     pub title_override: Option<String>,
     pub source: std::io::Error,
 }
@@ -291,16 +291,12 @@ impl TerminalError {
         if let Some(title_override) = &self.title_override {
             format!(
                 "{} {} ({})",
-                self.program.as_deref().unwrap_or("<system defined shell>"),
-                self.args.as_ref().into_iter().flatten().format(" "),
+                self.program,
+                self.args.iter().format(" "),
                 title_override
             )
         } else {
-            format!(
-                "{} {}",
-                self.program.as_deref().unwrap_or("<system defined shell>"),
-                self.args.as_ref().into_iter().flatten().format(" ")
-            )
+            format!("{} {}", self.program, self.args.iter().format(" "))
         }
     }
 }
@@ -434,16 +430,12 @@ impl TerminalBuilder {
             #[derive(Default)]
             struct ShellParams {
                 program: String,
-                args: Option<Vec<String>>,
+                args: Vec<String>,
                 title_override: Option<String>,
             }
 
             impl ShellParams {
-                fn new(
-                    program: String,
-                    args: Option<Vec<String>>,
-                    title_override: Option<String>,
-                ) -> Self {
+                fn new(program: String, args: Vec<String>, title_override: Option<String>) -> Self {
                     log::debug!("Using {program} as shell");
                     Self {
                         program,
@@ -453,27 +445,30 @@ impl TerminalBuilder {
                 }
             }
 
-            let shell_params = match shell.clone() {
-                Shell::System => {
-                    if cfg!(windows) {
-                        Some(ShellParams::new(
-                            util::shell::get_windows_system_shell(),
-                            None,
-                            None,
-                        ))
-                    } else {
-                        None
-                    }
-                }
-                Shell::Program(program) => Some(ShellParams::new(program, None, None)),
+            // Note: when remoting, this shell_kind will scrutinize `ssh` or
+            // `wsl.exe` as a shell and fall back to posix or powershell based on
+            // the compilation target. This is fine right now due to the restricted
+            // way we use the return value, but would become incorrect if we
+            // supported remoting into windows.
+            let shell_kind = shell.shell_kind(cfg!(windows));
+
+            let mut shell_params = match shell.clone() {
+                Shell::System => ShellParams::new(util::shell::get_system_shell(), vec![], None),
+                Shell::Program(program) => ShellParams::new(program, vec![], None),
                 Shell::WithArguments {
                     program,
                     args,
                     title_override,
-                } => Some(ShellParams::new(program, Some(args), title_override)),
+                } => ShellParams::new(program, args, title_override),
             };
-            let terminal_title_override =
-                shell_params.as_ref().and_then(|e| e.title_override.clone());
+            if cfg!(target_os = "macos") {
+                (shell_params.program, shell_params.args) = default_shell_command_macos(
+                    shell_kind,
+                    shell_params.program,
+                    shell_params.args,
+                );
+            }
+            let terminal_title_override = shell_params.title_override.clone();
 
             #[cfg(windows)]
             let shell_program = shell_params.as_ref().map(|params| {
@@ -484,23 +479,14 @@ impl TerminalBuilder {
                     .unwrap_or(params.program.clone())
             });
 
-            // Note: when remoting, this shell_kind will scrutinize `ssh` or
-            // `wsl.exe` as a shell and fall back to posix or powershell based on
-            // the compilation target. This is fine right now due to the restricted
-            // way we use the return value, but would become incorrect if we
-            // supported remoting into windows.
-            let shell_kind = shell.shell_kind(cfg!(windows));
-
             let pty_options = {
-                let alac_shell = shell_params.as_ref().map(|params| {
-                    alacritty_terminal::tty::Shell::new(
-                        params.program.clone(),
-                        params.args.clone().unwrap_or_default(),
-                    )
-                });
+                let alac_shell = alacritty_terminal::tty::Shell::new(
+                    shell_params.program.clone(),
+                    shell_params.args.clone(),
+                );
 
                 alacritty_terminal::tty::Options {
-                    shell: alac_shell,
+                    shell: Some(alac_shell),
                     working_directory: working_directory.clone(),
                     drain_on_exit: true,
                     env: env.clone().into_iter().collect(),
@@ -532,8 +518,8 @@ impl TerminalBuilder {
                 Err(error) => {
                     bail!(TerminalError {
                         directory: working_directory,
-                        program: shell_params.as_ref().map(|params| params.program.clone()),
-                        args: shell_params.as_ref().and_then(|params| params.args.clone()),
+                        program: shell_params.program,
+                        args: shell_params.args,
                         title_override: terminal_title_override,
                         source: error,
                     });
@@ -2204,6 +2190,51 @@ fn task_summary(task: &TaskState, error_code: Option<i32>) -> (bool, String, Str
     (success, task_line, command_line)
 }
 
+// copied from alacritty but simplified, licensed under Apache-2.0
+fn default_shell_command_macos(
+    shell_kind: ShellKind,
+    shell: String,
+    args: Vec<String>,
+) -> (String, Vec<String>) {
+    // let Ok(_user) = std::env::var("USER") else {
+    //     return (shell.to_owned(), args);
+    // };
+    let Some(args) = args
+        .iter()
+        .map(|arg| shell_kind.try_quote(&arg))
+        .collect::<Option<Vec<_>>>()
+    else {
+        return (shell.to_owned(), args);
+    };
+    let shell_name = shell.rsplit('/').next().unwrap();
+
+    // On macOS, use the `login` command so the shell will appear as a tty session.
+    // let login_command = "/usr/bin/login".to_owned();
+
+    // Exec the shell with argv[0] prepended by '-' so it becomes a login shell.
+    // `login` normally does this itself, but `-l` disables this.
+    let exec = format!("exec -a -{} {} {}", shell_name, shell, args.join(" "));
+
+    // -f: Bypasses authentication for the already-logged-in user.
+    // -l: Skips changing directory to $HOME and prepending '-' to argv[0].
+    // -p: Preserves the environment.
+    // -q: Act as if `.hushlogin` exists.
+    //
+    // XXX: we use zsh here over sh due to `exec -a`.
+    // (
+    //     login_command,
+    //     vec![
+    //         "-flpq".to_owned(),
+    //         user,
+    //         "/bin/zsh".to_owned(),
+    //         "-fc".to_owned(),
+    //         exec,
+    //     ],
+    // )
+
+    ("/bin/zsh".to_owned(), vec!["-lfc".to_owned(), exec])
+}
+
 /// Appends a stringified task summary to the terminal, after its output.
 ///
 /// SAFETY: This function should only be called after terminal's PTY is no longer alive.
@@ -2480,9 +2511,9 @@ mod tests {
         })
         .detach();
 
-        let first_event = Event::Wakeup;
+        // let first_event = Event::Wakeup;
         let wakeup = event_rx.recv().await.expect("No wakeup event received");
-        assert_eq!(wakeup, first_event, "Expected wakeup, got {wakeup:?}");
+        // assert_eq!(wakeup, first_event, "Expected wakeup, got {wakeup:?}");
 
         terminal.update(cx, |terminal, _| {
             let success = terminal.try_keystroke(&Keystroke::parse("ctrl-c").unwrap(), false);
@@ -2493,7 +2524,7 @@ mod tests {
             assert!(success, "Should have registered ctrl-d sequence");
         });
 
-        let mut all_events = vec![first_event];
+        let mut all_events = vec![wakeup];
         while let Ok(Ok(new_event)) = smol_timeout(Duration::from_secs(1), event_rx.recv()).await {
             all_events.push(new_event.clone());
             if new_event == Event::CloseTerminal {