Cargo.lock 🔗
@@ -8778,7 +8778,6 @@ dependencies = [
"serde_json",
"serde_json_lenient",
"settings",
- "shlex",
"smol",
"task",
"text",
terminal: Fix escaping arguments when using CMD as the shell (#39701)
Jakub Konka created
A couple of caveats:
- We should not auto-escape arguments with Alacritty's `escape_args`
option if using CMD otherwise, the generated command will have way too
many escaped characters for CMD to parse correctly.
- When composing a full command for CMD, we need to put it in double
quotes manually: `cmd /C "activate.bat& pwsh.exe -C do_something"` so
that CMD executes the entire string as a sequence of commands.
- CMD requires `&` as a chaining operator for commands (`;` for other
shells).
Release Notes:
- N/AChange summary
Cargo.lock | 1 -
crates/languages/Cargo.toml | 1 -
crates/languages/src/python.rs | 13 ++-----------
crates/project/src/terminals.rs | 15 ++++++++++++---
crates/task/src/shell_builder.rs | 2 +-
crates/task/src/task.rs | 9 +++++++++
crates/terminal/src/terminal.rs | 5 ++++-
crates/util/src/shell.rs | 21 ++++++++++++++++++++-
8 files changed, 48 insertions(+), 19 deletions(-)
Detailed changes
crates/languages/Cargo.toml 🔗
@@ -91,7 +91,6 @@ tree-sitter-typescript = { workspace = true, optional = true }
tree-sitter-yaml = { workspace = true, optional = true }
util.workspace = true
workspace-hack.workspace = true
-shlex.workspace = true
[dev-dependencies]
pretty_assertions.workspace = true
crates/languages/src/python.rs 🔗
@@ -1180,15 +1180,7 @@ impl ToolchainLister for PythonToolchainProvider {
}
Some(PythonEnvironmentKind::Venv | PythonEnvironmentKind::VirtualEnv) => {
if let Some(prefix) = &toolchain.prefix {
- let activate_keyword = match shell {
- ShellKind::Cmd => ".",
- ShellKind::Nushell => "overlay use",
- ShellKind::PowerShell => ".",
- ShellKind::Fish => "source",
- ShellKind::Csh => "source",
- ShellKind::Tcsh => "source",
- ShellKind::Posix | ShellKind::Rc => "source",
- };
+ let activate_keyword = shell.activate_keyword();
let activate_script_name = match shell {
ShellKind::Posix | ShellKind::Rc => "activate",
ShellKind::Csh => "activate.csh",
@@ -1200,8 +1192,7 @@ impl ToolchainLister for PythonToolchainProvider {
};
let path = prefix.join(BINARY_DIR).join(activate_script_name);
- if let Ok(quoted) =
- shlex::try_quote(&path.to_string_lossy()).map(Cow::into_owned)
+ if let Some(quoted) = shell.try_quote(&path.to_string_lossy())
&& fs.is_file(&path).await
{
activation_script.push(format!("{activate_keyword} {quoted}"));
crates/project/src/terminals.rs 🔗
@@ -201,15 +201,24 @@ impl Project {
},
None => match activation_script.clone() {
activation_script if !activation_script.is_empty() => {
- let activation_script = activation_script.join("; ");
+ let separator = shell_kind.sequential_commands_separator();
+ let activation_script =
+ activation_script.join(&format!("{separator} "));
let to_run = format_to_run();
- let arg = format!("{activation_script}; {to_run}");
+ let mut arg = format!("{activation_script}{separator} {to_run}");
+ if shell_kind == ShellKind::Cmd {
+ // We need to put the entire command in quotes since otherwise CMD tries to execute them
+ // as separate commands rather than chaining one after another.
+ arg = format!("\"{arg}\"");
+ }
+
+ let args = shell_kind.args_for_shell(false, arg);
(
Shell::WithArguments {
program: shell,
- args: vec!["-c".to_owned(), arg],
+ args,
title_override: None,
},
env,
crates/task/src/shell_builder.rs 🔗
@@ -49,7 +49,7 @@ impl ShellBuilder {
format!("{} -C '{}'", self.program, command_to_use_in_label)
}
ShellKind::Cmd => {
- format!("{} /C '{}'", self.program, command_to_use_in_label)
+ format!("{} /C \"{}\"", self.program, command_to_use_in_label)
}
ShellKind::Posix
| ShellKind::Nushell
crates/task/src/task.rs 🔗
@@ -345,6 +345,7 @@ impl Shell {
Shell::System => get_system_shell(),
}
}
+
pub fn program_and_args(&self) -> (String, &[String]) {
match self {
Shell::Program(program) => (program.clone(), &[]),
@@ -352,6 +353,14 @@ impl Shell {
Shell::System => (get_system_shell(), &[]),
}
}
+
+ pub fn shell_kind(&self) -> ShellKind {
+ match self {
+ Shell::Program(program) => ShellKind::new(program),
+ Shell::WithArguments { program, .. } => ShellKind::new(program),
+ Shell::System => ShellKind::system(),
+ }
+ }
}
type VsCodeEnvVariable = String;
crates/terminal/src/terminal.rs 🔗
@@ -409,6 +409,7 @@ impl TerminalBuilder {
events_rx,
})
}
+
pub fn new(
working_directory: Option<PathBuf>,
task: Option<TaskState>,
@@ -507,8 +508,10 @@ impl TerminalBuilder {
working_directory: working_directory.clone(),
drain_on_exit: true,
env: env.clone().into_iter().collect(),
+ // We do not want to escape arguments if we are using CMD as our shell.
+ // If we do we end up with too many quotes/escaped quotes for CMD to handle.
#[cfg(windows)]
- escape_args: true,
+ escape_args: shell.shell_kind() != util::shell::ShellKind::Cmd,
}
};
crates/util/src/shell.rs 🔗
@@ -353,7 +353,7 @@ impl ShellKind {
}
}
- pub fn command_prefix(&self) -> Option<char> {
+ pub const fn command_prefix(&self) -> Option<char> {
match self {
ShellKind::PowerShell => Some('&'),
ShellKind::Nushell => Some('^'),
@@ -361,6 +361,13 @@ impl ShellKind {
}
}
+ pub const fn sequential_commands_separator(&self) -> char {
+ match self {
+ ShellKind::Cmd => '&',
+ _ => ';',
+ }
+ }
+
pub fn try_quote<'a>(&self, arg: &'a str) -> Option<Cow<'a, str>> {
shlex::try_quote(arg).ok().map(|arg| match self {
// If we are running in PowerShell, we want to take extra care when escaping strings.
@@ -370,4 +377,16 @@ impl ShellKind {
_ => arg,
})
}
+
+ pub const fn activate_keyword(&self) -> &'static str {
+ match self {
+ ShellKind::Cmd => "",
+ ShellKind::Nushell => "overlay use",
+ ShellKind::PowerShell => ".",
+ ShellKind::Fish => "source",
+ ShellKind::Csh => "source",
+ ShellKind::Tcsh => "source",
+ ShellKind::Posix | ShellKind::Rc => "source",
+ }
+ }
}