ssh lsp completions (#17665)

Conrad Irwin created

Release Notes:

* ssh-remoting: Fixed shell environment loading for remote shells.

Change summary

crates/language/src/language_registry.rs      | 15 +--
crates/markdown/examples/markdown.rs          |  5 
crates/markdown/examples/markdown_as_child.rs |  5 -
crates/project/src/environment.rs             | 80 ++++++++++++--------
crates/project/src/lsp_store.rs               | 11 +-
crates/remote_server/src/headless_project.rs  |  6 -
crates/repl/src/repl_editor.rs                |  7 -
crates/zed/src/main.rs                        | 23 ++---
8 files changed, 74 insertions(+), 78 deletions(-)

Detailed changes

crates/language/src/language_registry.rs 🔗

@@ -12,7 +12,7 @@ use collections::{hash_map, HashMap, HashSet};
 use futures::{
     channel::{mpsc, oneshot},
     future::Shared,
-    Future, FutureExt as _,
+    Future,
 };
 use globset::GlobSet;
 use gpui::{AppContext, BackgroundExecutor, Task};
@@ -79,7 +79,6 @@ impl<'a> From<&'a str> for LanguageName {
 pub struct LanguageRegistry {
     state: RwLock<LanguageRegistryState>,
     language_server_download_dir: Option<Arc<Path>>,
-    login_shell_env_loaded: Shared<Task<()>>,
     executor: BackgroundExecutor,
     lsp_binary_status_tx: LspBinaryStatusSender,
 }
@@ -204,7 +203,7 @@ struct LspBinaryStatusSender {
 }
 
 impl LanguageRegistry {
-    pub fn new(login_shell_env_loaded: Task<()>, executor: BackgroundExecutor) -> Self {
+    pub fn new(executor: BackgroundExecutor) -> Self {
         let this = Self {
             state: RwLock::new(LanguageRegistryState {
                 next_language_server_id: 0,
@@ -224,7 +223,6 @@ impl LanguageRegistry {
                 fake_server_txs: Default::default(),
             }),
             language_server_download_dir: None,
-            login_shell_env_loaded: login_shell_env_loaded.shared(),
             lsp_binary_status_tx: Default::default(),
             executor,
         };
@@ -234,7 +232,7 @@ impl LanguageRegistry {
 
     #[cfg(any(test, feature = "test-support"))]
     pub fn test(executor: BackgroundExecutor) -> Self {
-        let mut this = Self::new(Task::ready(()), executor);
+        let mut this = Self::new(executor);
         this.language_server_download_dir = Some(Path::new("/the-download-dir").into());
         this
     }
@@ -828,7 +826,7 @@ impl LanguageRegistry {
         adapter: Arc<CachedLspAdapter>,
         root_path: Arc<Path>,
         delegate: Arc<dyn LspAdapterDelegate>,
-        cli_environment: Option<HashMap<String, String>>,
+        cli_environment: Shared<Task<Option<HashMap<String, String>>>>,
         cx: &mut AppContext,
     ) -> Option<PendingLanguageServer> {
         let server_id = self.state.write().next_language_server_id();
@@ -844,15 +842,12 @@ impl LanguageRegistry {
             .log_err()?;
         let container_dir: Arc<Path> = Arc::from(download_dir.join(adapter.name.0.as_ref()));
         let root_path = root_path.clone();
-        let login_shell_env_loaded = self.login_shell_env_loaded.clone();
         let this = Arc::downgrade(self);
 
         let task = cx.spawn({
             let container_dir = container_dir.clone();
             move |mut cx| async move {
-                // If we want to install a binary globally, we need to wait for
-                // the login shell to be set on our process.
-                login_shell_env_loaded.await;
+                let cli_environment = cli_environment.await;
 
                 let binary_result = adapter
                     .clone()

crates/markdown/examples/markdown.rs 🔗

@@ -1,5 +1,5 @@
 use assets::Assets;
-use gpui::{prelude::*, rgb, App, KeyBinding, StyleRefinement, Task, View, WindowOptions};
+use gpui::{prelude::*, rgb, App, KeyBinding, StyleRefinement, View, WindowOptions};
 use language::{language_settings::AllLanguageSettings, LanguageRegistry};
 use markdown::{Markdown, MarkdownStyle};
 use node_runtime::FakeNodeRuntime;
@@ -105,8 +105,7 @@ pub fn main() {
         let node_runtime = FakeNodeRuntime::new();
         theme::init(LoadThemes::JustBase, cx);
 
-        let language_registry =
-            LanguageRegistry::new(Task::ready(()), cx.background_executor().clone());
+        let language_registry = LanguageRegistry::new(cx.background_executor().clone());
         language_registry.set_theme(cx.theme().clone());
         let language_registry = Arc::new(language_registry);
         languages::init(language_registry.clone(), node_runtime, cx);

crates/markdown/examples/markdown_as_child.rs 🔗

@@ -29,10 +29,7 @@ pub fn main() {
         cx.bind_keys([KeyBinding::new("cmd-c", markdown::Copy, None)]);
 
         let node_runtime = FakeNodeRuntime::new();
-        let language_registry = Arc::new(LanguageRegistry::new(
-            Task::ready(()),
-            cx.background_executor().clone(),
-        ));
+        let language_registry = Arc::new(LanguageRegistry::new(cx.background_executor().clone()));
         languages::init(language_registry.clone(), node_runtime, cx);
         theme::init(LoadThemes::JustBase, cx);
         Assets.load_fonts(cx).unwrap();

crates/project/src/environment.rs 🔗

@@ -1,10 +1,7 @@
-use anyhow::{anyhow, Context as _, Result};
+use anyhow::Result;
 use futures::{future::Shared, FutureExt};
-use std::{
-    path::{Path, PathBuf},
-    sync::Arc,
-};
-use util::{parse_env_output, ResultExt};
+use std::{path::Path, sync::Arc};
+use util::ResultExt;
 
 use collections::HashMap;
 use gpui::{AppContext, Context, Model, ModelContext, Task};
@@ -168,10 +165,53 @@ impl From<EnvironmentOrigin> for String {
     }
 }
 
+#[cfg(any(test, feature = "test-support"))]
+async fn load_shell_environment(
+    _dir: &Path,
+    _load_direnv: &DirenvSettings,
+) -> Result<HashMap<String, String>> {
+    Ok([("ZED_FAKE_TEST_ENV".into(), "true".into())]
+        .into_iter()
+        .collect())
+}
+
+#[cfg(not(any(test, feature = "test-support")))]
 async fn load_shell_environment(
     dir: &Path,
     load_direnv: &DirenvSettings,
 ) -> Result<HashMap<String, String>> {
+    use anyhow::{anyhow, Context};
+    use std::path::PathBuf;
+    use util::parse_env_output;
+
+    async fn load_direnv_environment(dir: &Path) -> Result<Option<HashMap<String, String>>> {
+        let Ok(direnv_path) = which::which("direnv") else {
+            return Ok(None);
+        };
+
+        let direnv_output = smol::process::Command::new(direnv_path)
+            .args(["export", "json"])
+            .current_dir(dir)
+            .output()
+            .await
+            .context("failed to spawn direnv to get local environment variables")?;
+
+        anyhow::ensure!(
+            direnv_output.status.success(),
+            "direnv exited with error {:?}",
+            direnv_output.status
+        );
+
+        let output = String::from_utf8_lossy(&direnv_output.stdout);
+        if output.is_empty() {
+            return Ok(None);
+        }
+
+        Ok(Some(
+            serde_json::from_str(&output).context("failed to parse direnv output")?,
+        ))
+    }
+
     let direnv_environment = match load_direnv {
         DirenvSettings::ShellHook => None,
         DirenvSettings::Direct => load_direnv_environment(dir).await?,
@@ -248,31 +288,3 @@ async fn load_shell_environment(
 
     Ok(parsed_env)
 }
-
-async fn load_direnv_environment(dir: &Path) -> Result<Option<HashMap<String, String>>> {
-    let Ok(direnv_path) = which::which("direnv") else {
-        return Ok(None);
-    };
-
-    let direnv_output = smol::process::Command::new(direnv_path)
-        .args(["export", "json"])
-        .current_dir(dir)
-        .output()
-        .await
-        .context("failed to spawn direnv to get local environment variables")?;
-
-    anyhow::ensure!(
-        direnv_output.status.success(),
-        "direnv exited with error {:?}",
-        direnv_output.status
-    );
-
-    let output = String::from_utf8_lossy(&direnv_output.stdout);
-    if output.is_empty() {
-        return Ok(None);
-    }
-
-    Ok(Some(
-        serde_json::from_str(&output).context("failed to parse direnv output")?,
-    ))
-}

crates/project/src/lsp_store.rs 🔗

@@ -4629,14 +4629,13 @@ impl LspStore {
             return;
         }
 
+        let local = self.as_local().unwrap();
+
         let stderr_capture = Arc::new(Mutex::new(Some(String::new())));
         let lsp_adapter_delegate = ProjectLspAdapterDelegate::for_local(self, worktree_handle, cx);
-        let cli_environment = self
-            .as_local()
-            .unwrap()
-            .environment
-            .read(cx)
-            .get_cli_environment();
+        let cli_environment = local.environment.update(cx, |environment, cx| {
+            environment.get_environment(Some(worktree_id), Some(worktree_path.clone()), cx)
+        });
 
         let pending_server = match self.languages.create_pending_language_server(
             stderr_capture.clone(),

crates/remote_server/src/headless_project.rs 🔗

@@ -1,6 +1,6 @@
 use anyhow::{anyhow, Result};
 use fs::Fs;
-use gpui::{AppContext, AsyncAppContext, Context, Model, ModelContext, Task};
+use gpui::{AppContext, AsyncAppContext, Context, Model, ModelContext};
 use language::LanguageRegistry;
 use project::{
     buffer_store::BufferStore, project_settings::SettingsObserver, search::SearchQuery,
@@ -36,9 +36,7 @@ impl HeadlessProject {
     }
 
     pub fn new(session: Arc<SshSession>, fs: Arc<dyn Fs>, cx: &mut ModelContext<Self>) -> Self {
-        // TODO: we should load the env correctly (as we do in login_shell_env_loaded when stdout is not a pty). Can we re-use the ProjectEnvironment for that?
-        let mut languages =
-            LanguageRegistry::new(Task::ready(()), cx.background_executor().clone());
+        let mut languages = LanguageRegistry::new(cx.background_executor().clone());
         languages
             .set_language_server_download_dir(PathBuf::from("/Users/conrad/what-could-go-wrong"));
 

crates/repl/src/repl_editor.rs 🔗

@@ -321,7 +321,7 @@ fn get_language(editor: WeakView<Editor>, cx: &mut AppContext) -> Option<Arc<Lan
 #[cfg(test)]
 mod tests {
     use super::*;
-    use gpui::{Context, Task};
+    use gpui::Context;
     use indoc::indoc;
     use language::{Buffer, Language, LanguageConfig, LanguageRegistry};
 
@@ -475,10 +475,7 @@ mod tests {
         let typescript =
             languages::language("typescript", tree_sitter_typescript::language_typescript());
         let python = languages::language("python", tree_sitter_python::language());
-        let language_registry = Arc::new(LanguageRegistry::new(
-            Task::ready(()),
-            cx.background_executor().clone(),
-        ));
+        let language_registry = Arc::new(LanguageRegistry::new(cx.background_executor().clone()));
         language_registry.add(markdown.clone());
         language_registry.add(typescript.clone());
         language_registry.add(python.clone());

crates/zed/src/main.rs 🔗

@@ -400,16 +400,16 @@ fn main() {
         paths::keymap_file().clone(),
     );
 
-    let login_shell_env_loaded = if stdout_is_a_pty() {
-        Task::ready(())
-    } else {
-        app.background_executor().spawn(async {
-            #[cfg(unix)]
-            {
-                load_shell_from_passwd().await.log_err();
-            }
-            load_login_shell_environment().await.log_err();
-        })
+    if !stdout_is_a_pty() {
+        app.background_executor()
+            .spawn(async {
+                #[cfg(unix)]
+                {
+                    load_shell_from_passwd().await.log_err();
+                }
+                load_login_shell_environment().await.log_err();
+            })
+            .detach()
     };
 
     app.on_open_urls({
@@ -451,8 +451,7 @@ fn main() {
         client::init_settings(cx);
         let client = Client::production(cx);
         cx.set_http_client(client.http_client().clone());
-        let mut languages =
-            LanguageRegistry::new(login_shell_env_loaded, cx.background_executor().clone());
+        let mut languages = LanguageRegistry::new(cx.background_executor().clone());
         languages.set_language_server_download_dir(paths::languages_dir().clone());
         let languages = Arc::new(languages);
         let node_runtime = RealNodeRuntime::new(client.http_client());