project: Fix task arguments being quoted incorrectly for nushell and powershell (#38056)

Lukas Wirth and Piotr Osiewicz created

Release Notes:

- Fixed task arguments being quoted incorrectly for nushell and
powershell

Co-authored-by: Piotr Osiewicz <piotr@zed.dev>

Change summary

crates/languages/src/python.rs   |  16 ++--
crates/project/src/terminals.rs  | 116 +++++++++++++++++++--------------
crates/task/src/shell_builder.rs |   8 ++
3 files changed, 84 insertions(+), 56 deletions(-)

Detailed changes

crates/languages/src/python.rs 🔗

@@ -708,7 +708,7 @@ impl ContextProvider for PythonContextProvider {
             // Execute a selection
             TaskTemplate {
                 label: "execute selection".to_owned(),
-                command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value_with_whitespace(),
+                command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value(),
                 args: vec![
                     "-c".to_owned(),
                     VariableName::SelectedText.template_value_with_whitespace(),
@@ -719,7 +719,7 @@ impl ContextProvider for PythonContextProvider {
             // Execute an entire file
             TaskTemplate {
                 label: format!("run '{}'", VariableName::File.template_value()),
-                command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value_with_whitespace(),
+                command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value(),
                 args: vec![VariableName::File.template_value_with_whitespace()],
                 cwd: Some(VariableName::WorktreeRoot.template_value()),
                 ..TaskTemplate::default()
@@ -727,10 +727,10 @@ impl ContextProvider for PythonContextProvider {
             // Execute a file as module
             TaskTemplate {
                 label: format!("run module '{}'", VariableName::File.template_value()),
-                command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value_with_whitespace(),
+                command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value(),
                 args: vec![
                     "-m".to_owned(),
-                    PYTHON_MODULE_NAME_TASK_VARIABLE.template_value_with_whitespace(),
+                    PYTHON_MODULE_NAME_TASK_VARIABLE.template_value(),
                 ],
                 cwd: Some(VariableName::WorktreeRoot.template_value()),
                 tags: vec!["python-module-main-method".to_owned()],
@@ -744,7 +744,7 @@ impl ContextProvider for PythonContextProvider {
                     // Run tests for an entire file
                     TaskTemplate {
                         label: format!("unittest '{}'", VariableName::File.template_value()),
-                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value_with_whitespace(),
+                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value(),
                         args: vec![
                             "-m".to_owned(),
                             "unittest".to_owned(),
@@ -756,7 +756,7 @@ impl ContextProvider for PythonContextProvider {
                     // Run test(s) for a specific target within a file
                     TaskTemplate {
                         label: "unittest $ZED_CUSTOM_PYTHON_TEST_TARGET".to_owned(),
-                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value_with_whitespace(),
+                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value(),
                         args: vec![
                             "-m".to_owned(),
                             "unittest".to_owned(),
@@ -776,7 +776,7 @@ impl ContextProvider for PythonContextProvider {
                     // Run tests for an entire file
                     TaskTemplate {
                         label: format!("pytest '{}'", VariableName::File.template_value()),
-                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value_with_whitespace(),
+                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value(),
                         args: vec![
                             "-m".to_owned(),
                             "pytest".to_owned(),
@@ -788,7 +788,7 @@ impl ContextProvider for PythonContextProvider {
                     // Run test(s) for a specific target within a file
                     TaskTemplate {
                         label: "pytest $ZED_CUSTOM_PYTHON_TEST_TARGET".to_owned(),
-                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value_with_whitespace(),
+                        command: PYTHON_ACTIVE_TOOLCHAIN_PATH.template_value(),
                         args: vec![
                             "-m".to_owned(),
                             "pytest".to_owned(),

crates/project/src/terminals.rs 🔗

@@ -135,6 +135,7 @@ impl Project {
         let lang_registry = self.languages.clone();
         let fs = self.fs.clone();
         cx.spawn(async move |project, cx| {
+            let shell_kind = ShellKind::new(&shell);
             let activation_script = maybe!(async {
                 for toolchain in toolchains {
                     let Some(toolchain) = toolchain.await else {
@@ -147,7 +148,7 @@ impl Project {
                     let lister = language?.toolchain_lister();
                     return Some(
                         lister?
-                            .activation_script(&toolchain, ShellKind::new(&shell), fs.as_ref())
+                            .activation_script(&toolchain, shell_kind, fs.as_ref())
                             .await,
                     );
                 }
@@ -157,26 +158,36 @@ impl Project {
             .unwrap_or_default();
 
             project.update(cx, move |this, cx| {
+                let format_to_run = || {
+                    if let Some(command) = &spawn_task.command {
+                        let mut command: Option<Cow<str>> = shlex::try_quote(command).ok();
+                        if let Some(command) = &mut command
+                            && command.starts_with('"')
+                            && let Some(prefix) = shell_kind.command_prefix()
+                        {
+                            *command = Cow::Owned(format!("{prefix}{command}"));
+                        }
+
+                        let args = spawn_task
+                            .args
+                            .iter()
+                            .filter_map(|arg| shlex::try_quote(arg).ok());
+                        command.into_iter().chain(args).join(" ")
+                    } else {
+                        // todo: this breaks for remotes to windows
+                        format!("exec {shell} -l")
+                    }
+                };
+
                 let shell = {
                     env.extend(spawn_task.env);
                     match remote_client {
                         Some(remote_client) => match activation_script.clone() {
                             activation_script if !activation_script.is_empty() => {
                                 let activation_script = activation_script.join("; ");
-                                let to_run = if let Some(command) = spawn_task.command {
-                                    let command: Option<Cow<str>> = shlex::try_quote(&command).ok();
-                                    let args = spawn_task
-                                        .args
-                                        .iter()
-                                        .filter_map(|arg| shlex::try_quote(arg).ok());
-                                    command.into_iter().chain(args).join(" ")
-                                } else {
-                                    format!("exec {shell} -l")
-                                };
-                                let args = vec![
-                                    "-c".to_owned(),
-                                    format!("{activation_script}; {to_run}",),
-                                ];
+                                let to_run = format_to_run();
+                                let args =
+                                    vec!["-c".to_owned(), format!("{activation_script}; {to_run}")];
                                 create_remote_shell(
                                     Some((&shell, &args)),
                                     &mut env,
@@ -197,43 +208,21 @@ impl Project {
                             )?,
                         },
                         None => match activation_script.clone() {
-                            #[cfg(not(windows))]
                             activation_script if !activation_script.is_empty() => {
                                 let activation_script = activation_script.join("; ");
-                                let to_run = if let Some(command) = spawn_task.command {
-                                    let command: Option<Cow<str>> = shlex::try_quote(&command).ok();
-                                    let args = spawn_task
-                                        .args
-                                        .iter()
-                                        .filter_map(|arg| shlex::try_quote(arg).ok());
-                                    command.into_iter().chain(args).join(" ")
-                                } else {
-                                    format!("exec {shell} -l")
-                                };
+                                let to_run = format_to_run();
+
+                                // todo(lw): Alacritty uses `CreateProcessW` on windows with the entire command and arg sequence merged into a single string,
+                                // without quoting the arguments
+                                #[cfg(windows)]
+                                let arg =
+                                    quote_arg(&format!("{activation_script}; {to_run}"), true);
+                                #[cfg(not(windows))]
+                                let arg = format!("{activation_script}; {to_run}");
+
                                 Shell::WithArguments {
                                     program: shell,
-                                    args: vec![
-                                        "-c".to_owned(),
-                                        // alacritty formats all args into a single string literally without extra quoting before handing it off to powershell
-                                        // so we work around this here
-                                        if cfg!(windows) {
-                                            println!(
-                                                "{}",
-                                                shlex::try_quote(&format!(
-                                                    "{activation_script}; {to_run}",
-                                                ))
-                                                .unwrap()
-                                                .into_owned()
-                                            );
-                                            shlex::try_quote(&format!(
-                                                "{activation_script}; {to_run}",
-                                            ))
-                                            .unwrap()
-                                            .into_owned()
-                                        } else {
-                                            format!("{activation_script}; {to_run}",)
-                                        },
-                                    ],
+                                    args: vec!["-c".to_owned(), arg],
                                     title_override: None,
                                 }
                             }
@@ -523,6 +512,37 @@ impl Project {
     }
 }
 
+/// We're not using shlex for windows as it is overly eager with escaping some of the special characters (^) we need for nu. Hence, we took
+/// that quote impl straight from Rust stdlib (Command API).
+#[cfg(windows)]
+fn quote_arg(argument: &str, quote: bool) -> String {
+    let mut arg = String::new();
+    if quote {
+        arg.push('"');
+    }
+
+    let mut backslashes: usize = 0;
+    for x in argument.chars() {
+        if x == '\\' {
+            backslashes += 1;
+        } else {
+            if x == '"' {
+                // Add n+1 backslashes to total 2n+1 before internal '"'.
+                arg.extend((0..=backslashes).map(|_| '\\'));
+            }
+            backslashes = 0;
+        }
+        arg.push(x);
+    }
+
+    if quote {
+        // Add n backslashes to total 2n before ending '"'.
+        arg.extend((0..backslashes).map(|_| '\\'));
+        arg.push('"');
+    }
+    arg
+}
+
 fn create_remote_shell(
     spawn_command: Option<(&String, &Vec<String>)>,
     env: &mut HashMap<String, String>,

crates/task/src/shell_builder.rs 🔗

@@ -193,6 +193,14 @@ impl ShellKind {
                 .collect(),
         }
     }
+
+    pub fn command_prefix(&self) -> Option<char> {
+        match self {
+            ShellKind::Powershell => Some('&'),
+            ShellKind::Nushell => Some('^'),
+            _ => None,
+        }
+    }
 }
 
 /// ShellBuilder is used to turn a user-requested task into a