Merge pull request #1195 from zed-industries/lsp-errors

Antonio Scandurra created

Open a buffer for every language server error when clicking on status

Change summary

Cargo.lock                               |  16 ++
crates/language/src/language.rs          |  11 +
crates/lsp_status/Cargo.toml             |  19 ++
crates/lsp_status/src/lsp_status.rs      | 188 +++++++++++++++----------
crates/workspace/src/workspace.rs        |   1 
crates/zed/Cargo.toml                    |   1 
crates/zed/src/languages/installation.rs |   9 
crates/zed/src/languages/json.rs         |  37 ----
crates/zed/src/zed.rs                    |   6 
9 files changed, 174 insertions(+), 114 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -2563,6 +2563,21 @@ dependencies = [
  "url",
 ]
 
+[[package]]
+name = "lsp_status"
+version = "0.1.0"
+dependencies = [
+ "editor",
+ "futures",
+ "gpui",
+ "language",
+ "project",
+ "settings",
+ "smallvec",
+ "util",
+ "workspace",
+]
+
 [[package]]
 name = "malloc_buf"
 version = "0.0.6"
@@ -6048,6 +6063,7 @@ dependencies = [
  "libc",
  "log",
  "lsp",
+ "lsp_status",
  "num_cpus",
  "outline",
  "parking_lot 0.11.2",

crates/language/src/language.rs 🔗

@@ -184,7 +184,7 @@ pub enum LanguageServerBinaryStatus {
     Downloading,
     Downloaded,
     Cached,
-    Failed,
+    Failed { error: String },
 }
 
 pub struct LanguageRegistry {
@@ -382,7 +382,7 @@ async fn get_server_binary_path(
         statuses.clone(),
     )
     .await;
-    if path.is_err() {
+    if let Err(error) = path.as_ref() {
         if let Some(cached_path) = adapter.cached_server_binary(container_dir).await {
             statuses
                 .broadcast((language.clone(), LanguageServerBinaryStatus::Cached))
@@ -390,7 +390,12 @@ async fn get_server_binary_path(
             return Ok(cached_path);
         } else {
             statuses
-                .broadcast((language.clone(), LanguageServerBinaryStatus::Failed))
+                .broadcast((
+                    language.clone(),
+                    LanguageServerBinaryStatus::Failed {
+                        error: format!("{:?}", error),
+                    },
+                ))
                 .await?;
         }
     }

crates/lsp_status/Cargo.toml 🔗

@@ -0,0 +1,19 @@
+[package]
+name = "lsp_status"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+path = "src/lsp_status.rs"
+doctest = false
+
+[dependencies]
+editor = { path = "../editor" }
+language = { path = "../language" }
+gpui = { path = "../gpui" }
+project = { path = "../project" }
+settings = { path = "../settings" }
+util = { path = "../util" }
+workspace = { path = "../workspace" }
+futures = "0.3"
+smallvec = { version = "1.6", features = ["union"] }

crates/workspace/src/lsp_status.rs → crates/lsp_status/src/lsp_status.rs 🔗

@@ -1,84 +1,108 @@
-use crate::{ItemHandle, StatusItemView};
+use editor::Editor;
 use futures::StreamExt;
-use gpui::{actions, AppContext, EventContext};
 use gpui::{
-    elements::*, platform::CursorStyle, Entity, ModelHandle, MutableAppContext, RenderContext,
-    View, ViewContext,
+    actions, elements::*, platform::CursorStyle, AppContext, Entity, EventContext, ModelHandle,
+    MutableAppContext, RenderContext, View, ViewContext, ViewHandle,
 };
 use language::{LanguageRegistry, LanguageServerBinaryStatus};
 use project::{LanguageServerProgress, Project};
 use settings::Settings;
 use smallvec::SmallVec;
-use std::cmp::Reverse;
-use std::fmt::Write;
-use std::sync::Arc;
+use std::{cmp::Reverse, fmt::Write, sync::Arc};
+use util::ResultExt;
+use workspace::{ItemHandle, StatusItemView, Workspace};
 
-actions!(lsp_status, [DismissErrorMessage]);
+actions!(lsp_status, [ShowErrorMessage]);
 
-pub struct LspStatus {
-    checking_for_update: Vec<String>,
-    downloading: Vec<String>,
-    failed: Vec<String>,
+pub enum Event {
+    ShowError { lsp_name: Arc<str>, error: String },
+}
+
+pub struct LspStatusItem {
+    statuses: Vec<LspStatus>,
     project: ModelHandle<Project>,
 }
 
+struct LspStatus {
+    name: Arc<str>,
+    status: LanguageServerBinaryStatus,
+}
+
 pub fn init(cx: &mut MutableAppContext) {
-    cx.add_action(LspStatus::dismiss_error_message);
+    cx.add_action(LspStatusItem::show_error_message);
 }
 
-impl LspStatus {
+impl LspStatusItem {
     pub fn new(
-        project: &ModelHandle<Project>,
+        workspace: &mut Workspace,
         languages: Arc<LanguageRegistry>,
-        cx: &mut ViewContext<Self>,
-    ) -> Self {
-        let mut status_events = languages.language_server_binary_statuses();
-        cx.spawn_weak(|this, mut cx| async move {
-            while let Some((language, event)) = status_events.next().await {
-                if let Some(this) = this.upgrade(&cx) {
-                    this.update(&mut cx, |this, cx| {
-                        for vector in [
-                            &mut this.checking_for_update,
-                            &mut this.downloading,
-                            &mut this.failed,
-                        ] {
-                            vector.retain(|name| name != language.name().as_ref());
-                        }
-
-                        match event {
-                            LanguageServerBinaryStatus::CheckingForUpdate => {
-                                this.checking_for_update.push(language.name().to_string());
-                            }
-                            LanguageServerBinaryStatus::Downloading => {
-                                this.downloading.push(language.name().to_string());
-                            }
-                            LanguageServerBinaryStatus::Failed => {
-                                this.failed.push(language.name().to_string());
-                            }
-                            LanguageServerBinaryStatus::Downloaded
-                            | LanguageServerBinaryStatus::Cached => {}
-                        }
-
-                        cx.notify();
+        cx: &mut ViewContext<Workspace>,
+    ) -> ViewHandle<LspStatusItem> {
+        let project = workspace.project().clone();
+        let this = cx.add_view(|cx: &mut ViewContext<Self>| {
+            let mut status_events = languages.language_server_binary_statuses();
+            cx.spawn_weak(|this, mut cx| async move {
+                while let Some((language, event)) = status_events.next().await {
+                    if let Some(this) = this.upgrade(&cx) {
+                        this.update(&mut cx, |this, cx| {
+                            this.statuses.retain(|s| s.name != language.name());
+                            this.statuses.push(LspStatus {
+                                name: language.name(),
+                                status: event,
+                            });
+                            cx.notify();
+                        });
+                    } else {
+                        break;
+                    }
+                }
+            })
+            .detach();
+            cx.observe(&project, |_, _, cx| cx.notify()).detach();
+
+            Self {
+                statuses: Default::default(),
+                project: project.clone(),
+            }
+        });
+        cx.subscribe(&this, move |workspace, _, event, cx| match event {
+            Event::ShowError { lsp_name, error } => {
+                if let Some(buffer) = project
+                    .update(cx, |project, cx| project.create_buffer(&error, None, cx))
+                    .log_err()
+                {
+                    buffer.update(cx, |buffer, cx| {
+                        buffer.edit(
+                            [(0..0, format!("Language server error: {}\n\n", lsp_name))],
+                            cx,
+                        );
                     });
-                } else {
-                    break;
+                    workspace.add_item(
+                        Box::new(
+                            cx.add_view(|cx| Editor::for_buffer(buffer, Some(project.clone()), cx)),
+                        ),
+                        cx,
+                    );
                 }
             }
         })
         .detach();
-        cx.observe(project, |_, _, cx| cx.notify()).detach();
-
-        Self {
-            checking_for_update: Default::default(),
-            downloading: Default::default(),
-            failed: Default::default(),
-            project: project.clone(),
-        }
+        this
     }
 
-    fn dismiss_error_message(&mut self, _: &DismissErrorMessage, cx: &mut ViewContext<Self>) {
-        self.failed.clear();
+    fn show_error_message(&mut self, _: &ShowErrorMessage, cx: &mut ViewContext<Self>) {
+        self.statuses.retain(|status| {
+            if let LanguageServerBinaryStatus::Failed { error } = &status.status {
+                cx.emit(Event::ShowError {
+                    lsp_name: status.name.clone(),
+                    error: error.clone(),
+                });
+                false
+            } else {
+                true
+            }
+        });
+
         cx.notify();
     }
 
@@ -107,11 +131,11 @@ impl LspStatus {
     }
 }
 
-impl Entity for LspStatus {
-    type Event = ();
+impl Entity for LspStatusItem {
+    type Event = Event;
 }
 
-impl View for LspStatus {
+impl View for LspStatusItem {
     fn ui_name() -> &'static str {
         "LspStatus"
     }
@@ -143,33 +167,51 @@ impl View for LspStatus {
         } else {
             drop(pending_work);
 
-            if !self.downloading.is_empty() {
+            let mut downloading = SmallVec::<[_; 3]>::new();
+            let mut checking_for_update = SmallVec::<[_; 3]>::new();
+            let mut failed = SmallVec::<[_; 3]>::new();
+            for status in &self.statuses {
+                match status.status {
+                    LanguageServerBinaryStatus::CheckingForUpdate => {
+                        checking_for_update.push(status.name.clone());
+                    }
+                    LanguageServerBinaryStatus::Downloading => {
+                        downloading.push(status.name.clone());
+                    }
+                    LanguageServerBinaryStatus::Failed { .. } => {
+                        failed.push(status.name.clone());
+                    }
+                    LanguageServerBinaryStatus::Downloaded | LanguageServerBinaryStatus::Cached => {
+                    }
+                }
+            }
+
+            if !downloading.is_empty() {
                 icon = Some("icons/download-solid-14.svg");
                 message = format!(
                     "Downloading {} language server{}...",
-                    self.downloading.join(", "),
-                    if self.downloading.len() > 1 { "s" } else { "" }
+                    downloading.join(", "),
+                    if downloading.len() > 1 { "s" } else { "" }
                 );
-            } else if !self.checking_for_update.is_empty() {
+            } else if !checking_for_update.is_empty() {
                 icon = Some("icons/download-solid-14.svg");
                 message = format!(
                     "Checking for updates to {} language server{}...",
-                    self.checking_for_update.join(", "),
-                    if self.checking_for_update.len() > 1 {
+                    checking_for_update.join(", "),
+                    if checking_for_update.len() > 1 {
                         "s"
                     } else {
                         ""
                     }
                 );
-            } else if !self.failed.is_empty() {
+            } else if !failed.is_empty() {
                 icon = Some("icons/warning-solid-14.svg");
                 message = format!(
-                    "Failed to download {} language server{}. Click to dismiss.",
-                    self.failed.join(", "),
-                    if self.failed.len() > 1 { "s" } else { "" }
+                    "Failed to download {} language server{}. Click to show error.",
+                    failed.join(", "),
+                    if failed.len() > 1 { "s" } else { "" }
                 );
-                handler =
-                    Some(|_, _, cx: &mut EventContext| cx.dispatch_action(DismissErrorMessage));
+                handler = Some(|_, _, cx: &mut EventContext| cx.dispatch_action(ShowErrorMessage));
             } else {
                 return Empty::new().boxed();
             }
@@ -217,6 +259,6 @@ impl View for LspStatus {
     }
 }
 
-impl StatusItemView for LspStatus {
+impl StatusItemView for LspStatusItem {
     fn set_active_pane_item(&mut self, _: Option<&dyn ItemHandle>, _: &mut ViewContext<Self>) {}
 }

crates/zed/Cargo.toml 🔗

@@ -37,6 +37,7 @@ gpui = { path = "../gpui" }
 journal = { path = "../journal" }
 language = { path = "../language" }
 lsp = { path = "../lsp" }
+lsp_status = { path = "../lsp_status" }
 outline = { path = "../outline" }
 project = { path = "../project" }
 project_panel = { path = "../project_panel" }

crates/zed/src/languages/installation.rs 🔗

@@ -39,10 +39,12 @@ pub async fn npm_package_latest_version(name: &str) -> Result<String> {
     let output = smol::process::Command::new("npm")
         .args(["info", name, "--json"])
         .output()
-        .await?;
+        .await
+        .context("failed to run npm info")?;
     if !output.status.success() {
         Err(anyhow!(
-            "failed to execute npm info: {:?}",
+            "failed to execute npm info:\nstdout: {:?}\nstderr: {:?}",
+            String::from_utf8_lossy(&output.stdout),
             String::from_utf8_lossy(&output.stderr)
         ))?;
     }
@@ -71,7 +73,8 @@ pub async fn npm_install_packages(
         .context("failed to run npm install")?;
     if !output.status.success() {
         Err(anyhow!(
-            "failed to execute npm install: {:?}",
+            "failed to execute npm install:\nstdout: {:?}\nstderr: {:?}",
+            String::from_utf8_lossy(&output.stdout),
             String::from_utf8_lossy(&output.stderr)
         ))?;
     }

crates/zed/src/languages/json.rs 🔗

@@ -1,8 +1,8 @@
+use super::installation::{npm_install_packages, npm_package_latest_version};
 use anyhow::{anyhow, Context, Result};
 use client::http::HttpClient;
 use futures::{future::BoxFuture, FutureExt, StreamExt};
 use language::{LanguageServerName, LspAdapter};
-use serde::Deserialize;
 use serde_json::json;
 use smol::fs;
 use std::{
@@ -33,25 +33,7 @@ impl LspAdapter for JsonLspAdapter {
         _: Arc<dyn HttpClient>,
     ) -> BoxFuture<'static, Result<Box<dyn 'static + Any + Send>>> {
         async move {
-            #[derive(Deserialize)]
-            struct NpmInfo {
-                versions: Vec<String>,
-            }
-
-            let output = smol::process::Command::new("npm")
-                .args(["info", "vscode-json-languageserver", "--json"])
-                .output()
-                .await?;
-            if !output.status.success() {
-                Err(anyhow!("failed to execute npm info"))?;
-            }
-            let mut info: NpmInfo = serde_json::from_slice(&output.stdout)?;
-
-            Ok(Box::new(
-                info.versions
-                    .pop()
-                    .ok_or_else(|| anyhow!("no versions found in npm info"))?,
-            ) as Box<_>)
+            Ok(Box::new(npm_package_latest_version("vscode-json-languageserver").await?) as Box<_>)
         }
         .boxed()
     }
@@ -71,16 +53,11 @@ impl LspAdapter for JsonLspAdapter {
             let binary_path = version_dir.join(Self::BIN_PATH);
 
             if fs::metadata(&binary_path).await.is_err() {
-                let output = smol::process::Command::new("npm")
-                    .current_dir(&version_dir)
-                    .arg("install")
-                    .arg(format!("vscode-json-languageserver@{}", version))
-                    .output()
-                    .await
-                    .context("failed to run npm install")?;
-                if !output.status.success() {
-                    Err(anyhow!("failed to install vscode-json-languageserver"))?;
-                }
+                npm_install_packages(
+                    [("vscode-json-languageserver", version.as_str())],
+                    &version_dir,
+                )
+                .await?;
 
                 if let Some(mut entries) = fs::read_dir(&container_dir).await.log_err() {
                     while let Some(entry) = entries.next().await {

crates/zed/src/zed.rs 🔗

@@ -129,7 +129,7 @@ pub fn init(app_state: &Arc<AppState>, cx: &mut gpui::MutableAppContext) {
         },
     );
 
-    workspace::lsp_status::init(cx);
+    lsp_status::init(cx);
     settings::KeymapFileContent::load_defaults(cx);
 }
 
@@ -209,9 +209,7 @@ pub fn initialize_workspace(
 
     let diagnostic_summary =
         cx.add_view(|cx| diagnostics::items::DiagnosticIndicator::new(workspace.project(), cx));
-    let lsp_status = cx.add_view(|cx| {
-        workspace::lsp_status::LspStatus::new(workspace.project(), app_state.languages.clone(), cx)
-    });
+    let lsp_status = lsp_status::LspStatusItem::new(workspace, app_state.languages.clone(), cx);
     let cursor_position = cx.add_view(|_| editor::items::CursorPosition::new());
     let auto_update = cx.add_view(|cx| auto_update::AutoUpdateIndicator::new(cx));
     let feedback_link = cx.add_view(|_| feedback::FeedbackLink);