Cargo.lock 🔗
@@ -3558,7 +3558,6 @@ dependencies = [
"slotmap",
"smol",
"tempfile",
- "terminal",
"tiny_http",
"url",
"util",
terminal: Use system shell for non-terminal uses (like spinning up external agents) (#51741)
Oliver Azevedo Barnes created
Closes #46551
Zed was using the `terminal.shell` setting for some actions unrelated to
the terminal GUI, like environment capture, ACP agent startup, context
server startup, and vim `:!`, and so if a user set it to `tmux` or
`nushell`, those paths would fail.
This PR ensures paths unrelated to Zed's terminal use the system path
instead.
Manual testing:
Set `terminal.shell` to `tmux` or any another non-bash executable.
Starting an external agent thread shouldn't break.
Release Notes:
- Fixed ACP agent and other breakage when `terminal.shell` was set to a
non-shell program like `tmux`Change summary
Cargo.lock | 1
crates/agent_servers/src/acp.rs | 8 +-
crates/context_server/Cargo.toml | 1
crates/context_server/src/transport/stdio_transport.rs | 6 -
crates/project/src/environment.rs | 33 +----------
crates/project/src/terminals.rs | 14 +++-
crates/vim/src/command.rs | 4
7 files changed, 20 insertions(+), 47 deletions(-)
Detailed changes
crates/agent_servers/src/acp.rs 🔗
@@ -16,12 +16,11 @@ use project::agent_server_store::{AgentServerCommand, AgentServerStore};
use project::{AgentId, Project};
use remote::remote_client::Interactive;
use serde::Deserialize;
-use settings::Settings as _;
use std::path::PathBuf;
use std::process::Stdio;
use std::rc::Rc;
use std::{any::Any, cell::RefCell};
-use task::{ShellBuilder, SpawnInTerminal};
+use task::{Shell, ShellBuilder, SpawnInTerminal};
use thiserror::Error;
use util::ResultExt as _;
use util::path_list::PathList;
@@ -34,7 +33,7 @@ use gpui::{App, AppContext as _, AsyncApp, Entity, SharedString, Task, WeakEntit
use acp_thread::{AcpThread, AuthRequired, LoadError, TerminalProviderEvent};
use terminal::TerminalBuilder;
-use terminal::terminal_settings::{AlternateScroll, CursorShape, TerminalSettings};
+use terminal::terminal_settings::{AlternateScroll, CursorShape};
use crate::GEMINI_ID;
@@ -251,8 +250,7 @@ impl AcpConnection {
)
});
- let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone());
- let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive();
+ let builder = ShellBuilder::new(&Shell::System, cfg!(windows)).non_interactive();
let mut child = builder.build_std_command(Some(path.clone()), &args);
child.envs(env.clone());
if let Some(cwd) = project.read_with(cx, |project, _cx| {
crates/context_server/Cargo.toml 🔗
@@ -38,7 +38,6 @@ tempfile.workspace = true
tiny_http.workspace = true
url = { workspace = true, features = ["serde"] }
util.workspace = true
-terminal.workspace = true
[dev-dependencies]
gpui = { workspace = true, features = ["test-support"] }
crates/context_server/src/transport/stdio_transport.rs 🔗
@@ -8,11 +8,10 @@ use futures::{
AsyncBufReadExt as _, AsyncRead, AsyncWrite, AsyncWriteExt as _, Stream, StreamExt as _,
};
use gpui::AsyncApp;
-use settings::Settings as _;
use smol::channel;
use smol::process::Child;
-use terminal::terminal_settings::TerminalSettings;
use util::TryFutureExt as _;
+use util::shell::Shell;
use util::shell_builder::ShellBuilder;
use crate::client::ModelContextServerBinary;
@@ -31,8 +30,7 @@ impl StdioTransport {
working_directory: &Option<PathBuf>,
cx: &AsyncApp,
) -> Result<Self> {
- let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone());
- let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive();
+ let builder = ShellBuilder::new(&Shell::System, cfg!(windows)).non_interactive();
let mut command =
builder.build_smol_command(Some(binary.executable.display().to_string()), &binary.args);
crates/project/src/environment.rs 🔗
@@ -5,8 +5,7 @@ use remote::RemoteClient;
use rpc::proto::{self, REMOTE_SERVER_PROJECT_ID};
use std::{collections::VecDeque, path::Path, sync::Arc};
use task::{Shell, shell_to_proto};
-use terminal::terminal_settings::TerminalSettings;
-use util::{ResultExt, command::new_command, rel_path::RelPath};
+use util::{ResultExt, command::new_command};
use worktree::Worktree;
use collections::HashMap;
@@ -134,19 +133,7 @@ impl ProjectEnvironment {
None if self.is_remote_project => {
Some(self.local_directory_environment(&Shell::System, abs_path, cx))
}
- None => Some({
- let shell = TerminalSettings::get(
- Some(settings::SettingsLocation {
- worktree_id: worktree.id(),
- path: RelPath::empty(),
- }),
- cx,
- )
- .shell
- .clone();
-
- self.local_directory_environment(&shell, abs_path, cx)
- }),
+ None => Some(self.local_directory_environment(&Shell::System, abs_path, cx)),
}
.unwrap_or_else(|| Task::ready(None).shared())
}
@@ -175,21 +162,7 @@ impl ProjectEnvironment {
worktree_store.find_worktree(&abs_path, cx)
})
.ok()
- .map(|worktree| {
- let shell = terminal::terminal_settings::TerminalSettings::get(
- worktree
- .as_ref()
- .map(|(worktree, path)| settings::SettingsLocation {
- worktree_id: worktree.read(cx).id(),
- path: &path,
- }),
- cx,
- )
- .shell
- .clone();
-
- self.local_directory_environment(&shell, abs_path, cx)
- }),
+ .map(|_| self.local_directory_environment(&Shell::System, abs_path, cx)),
}
.unwrap_or_else(|| Task::ready(None).shared())
}
crates/project/src/terminals.rs 🔗
@@ -17,7 +17,9 @@ use terminal::{
TaskState, TaskStatus, Terminal, TerminalBuilder, insert_zed_terminal_env,
terminal_settings::TerminalSettings,
};
-use util::{command::new_std_command, get_default_system_shell, maybe, rel_path::RelPath};
+use util::{
+ command::new_std_command, get_default_system_shell, get_system_shell, maybe, rel_path::RelPath,
+};
use crate::{Project, ProjectPath};
@@ -103,7 +105,7 @@ impl Project {
.read(cx)
.shell()
.unwrap_or_else(get_default_system_shell),
- None => settings.shell.program(),
+ None => get_system_shell(),
};
let path_style = self.path_style(cx);
let shell_kind = ShellKind::new(&shell, path_style.is_windows());
@@ -363,12 +365,16 @@ impl Project {
.unwrap_or_else(get_default_system_shell),
None => settings.shell.program(),
};
+ let env_shell = match &remote_client {
+ Some(_) => shell.clone(),
+ None => get_system_shell(),
+ };
let path_style = self.path_style(cx);
// Prepare a task for resolving the environment
let env_task =
- self.resolve_directory_environment(&shell, path.clone(), remote_client.clone(), cx);
+ self.resolve_directory_environment(&env_shell, path.clone(), remote_client.clone(), cx);
let lang_registry = self.languages.clone();
cx.spawn(async move |project, cx| {
@@ -526,7 +532,7 @@ impl Project {
.as_ref()
.and_then(|remote_client| remote_client.read(cx).shell())
.map(Shell::Program)
- .unwrap_or_else(|| settings.shell.clone());
+ .unwrap_or(Shell::System);
let is_windows = self.path_style(cx).is_windows();
let builder = ShellBuilder::new(&shell, is_windows).non_interactive();
let (command, args) = builder.build(Some(command), &Vec::new());
crates/vim/src/command.rs 🔗
@@ -28,7 +28,7 @@ use std::{
sync::OnceLock,
time::Instant,
};
-use task::{HideStrategy, RevealStrategy, SaveStrategy, SpawnInTerminal, TaskId};
+use task::{HideStrategy, RevealStrategy, SaveStrategy, Shell, SpawnInTerminal, TaskId};
use ui::ActiveTheme;
use util::{
ResultExt,
@@ -2476,7 +2476,7 @@ impl ShellExec {
workspace.update(cx, |workspace, cx| {
let project = workspace.project().read(cx);
let cwd = project.first_project_directory(cx);
- let shell = project.terminal_settings(&cwd, cx).shell.clone();
+ let shell = Shell::System;
let spawn_in_terminal = SpawnInTerminal {
id: TaskId("vim".to_string()),