Skip to content

Commit 04ff8de

Browse files
kubkonJosephTLyons
authored andcommitted
terminal: Fix escaping arguments when using CMD as the shell (#39701)
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/A
1 parent 37e7f30 commit 04ff8de

File tree

8 files changed

+48
-19
lines changed

8 files changed

+48
-19
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/languages/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ tree-sitter-typescript = { workspace = true, optional = true }
9191
tree-sitter-yaml = { workspace = true, optional = true }
9292
util.workspace = true
9393
workspace-hack.workspace = true
94-
shlex.workspace = true
9594

9695
[dev-dependencies]
9796
pretty_assertions.workspace = true

crates/languages/src/python.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,15 +1180,7 @@ impl ToolchainLister for PythonToolchainProvider {
11801180
}
11811181
Some(PythonEnvironmentKind::Venv | PythonEnvironmentKind::VirtualEnv) => {
11821182
if let Some(prefix) = &toolchain.prefix {
1183-
let activate_keyword = match shell {
1184-
ShellKind::Cmd => ".",
1185-
ShellKind::Nushell => "overlay use",
1186-
ShellKind::PowerShell => ".",
1187-
ShellKind::Fish => "source",
1188-
ShellKind::Csh => "source",
1189-
ShellKind::Tcsh => "source",
1190-
ShellKind::Posix | ShellKind::Rc => "source",
1191-
};
1183+
let activate_keyword = shell.activate_keyword();
11921184
let activate_script_name = match shell {
11931185
ShellKind::Posix | ShellKind::Rc => "activate",
11941186
ShellKind::Csh => "activate.csh",
@@ -1200,8 +1192,7 @@ impl ToolchainLister for PythonToolchainProvider {
12001192
};
12011193
let path = prefix.join(BINARY_DIR).join(activate_script_name);
12021194

1203-
if let Ok(quoted) =
1204-
shlex::try_quote(&path.to_string_lossy()).map(Cow::into_owned)
1195+
if let Some(quoted) = shell.try_quote(&path.to_string_lossy())
12051196
&& fs.is_file(&path).await
12061197
{
12071198
activation_script.push(format!("{activate_keyword} {quoted}"));

crates/project/src/terminals.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,15 +201,24 @@ impl Project {
201201
},
202202
None => match activation_script.clone() {
203203
activation_script if !activation_script.is_empty() => {
204-
let activation_script = activation_script.join("; ");
204+
let separator = shell_kind.sequential_commands_separator();
205+
let activation_script =
206+
activation_script.join(&format!("{separator} "));
205207
let to_run = format_to_run();
206208

207-
let arg = format!("{activation_script}; {to_run}");
209+
let mut arg = format!("{activation_script}{separator} {to_run}");
210+
if shell_kind == ShellKind::Cmd {
211+
// We need to put the entire command in quotes since otherwise CMD tries to execute them
212+
// as separate commands rather than chaining one after another.
213+
arg = format!("\"{arg}\"");
214+
}
215+
216+
let args = shell_kind.args_for_shell(false, arg);
208217

209218
(
210219
Shell::WithArguments {
211220
program: shell,
212-
args: vec!["-c".to_owned(), arg],
221+
args,
213222
title_override: None,
214223
},
215224
env,

crates/task/src/shell_builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ impl ShellBuilder {
4949
format!("{} -C '{}'", self.program, command_to_use_in_label)
5050
}
5151
ShellKind::Cmd => {
52-
format!("{} /C '{}'", self.program, command_to_use_in_label)
52+
format!("{} /C \"{}\"", self.program, command_to_use_in_label)
5353
}
5454
ShellKind::Posix
5555
| ShellKind::Nushell

crates/task/src/task.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,13 +345,22 @@ impl Shell {
345345
Shell::System => get_system_shell(),
346346
}
347347
}
348+
348349
pub fn program_and_args(&self) -> (String, &[String]) {
349350
match self {
350351
Shell::Program(program) => (program.clone(), &[]),
351352
Shell::WithArguments { program, args, .. } => (program.clone(), args),
352353
Shell::System => (get_system_shell(), &[]),
353354
}
354355
}
356+
357+
pub fn shell_kind(&self) -> ShellKind {
358+
match self {
359+
Shell::Program(program) => ShellKind::new(program),
360+
Shell::WithArguments { program, .. } => ShellKind::new(program),
361+
Shell::System => ShellKind::system(),
362+
}
363+
}
355364
}
356365

357366
type VsCodeEnvVariable = String;

crates/terminal/src/terminal.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,7 @@ impl TerminalBuilder {
409409
events_rx,
410410
})
411411
}
412+
412413
pub fn new(
413414
working_directory: Option<PathBuf>,
414415
task: Option<TaskState>,
@@ -507,8 +508,10 @@ impl TerminalBuilder {
507508
working_directory: working_directory.clone(),
508509
drain_on_exit: true,
509510
env: env.clone().into_iter().collect(),
511+
// We do not want to escape arguments if we are using CMD as our shell.
512+
// If we do we end up with too many quotes/escaped quotes for CMD to handle.
510513
#[cfg(windows)]
511-
escape_args: true,
514+
escape_args: shell.shell_kind() != util::shell::ShellKind::Cmd,
512515
}
513516
};
514517

crates/util/src/shell.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,14 +353,21 @@ impl ShellKind {
353353
}
354354
}
355355

356-
pub fn command_prefix(&self) -> Option<char> {
356+
pub const fn command_prefix(&self) -> Option<char> {
357357
match self {
358358
ShellKind::PowerShell => Some('&'),
359359
ShellKind::Nushell => Some('^'),
360360
_ => None,
361361
}
362362
}
363363

364+
pub const fn sequential_commands_separator(&self) -> char {
365+
match self {
366+
ShellKind::Cmd => '&',
367+
_ => ';',
368+
}
369+
}
370+
364371
pub fn try_quote<'a>(&self, arg: &'a str) -> Option<Cow<'a, str>> {
365372
shlex::try_quote(arg).ok().map(|arg| match self {
366373
// If we are running in PowerShell, we want to take extra care when escaping strings.
@@ -370,4 +377,16 @@ impl ShellKind {
370377
_ => arg,
371378
})
372379
}
380+
381+
pub const fn activate_keyword(&self) -> &'static str {
382+
match self {
383+
ShellKind::Cmd => "",
384+
ShellKind::Nushell => "overlay use",
385+
ShellKind::PowerShell => ".",
386+
ShellKind::Fish => "source",
387+
ShellKind::Csh => "source",
388+
ShellKind::Tcsh => "source",
389+
ShellKind::Posix | ShellKind::Rc => "source",
390+
}
391+
}
373392
}

0 commit comments

Comments
 (0)