Improve LSP tasks ergonomics (#31551)

Kirill Bulatov created

* stopped fetching LSP tasks for too long (but still use the hardcoded
value for the time being — the LSP tasks settings part is a simple bool
key and it's not very simple to fit in another value there)

* introduced `prefer_lsp` language task settings value, to control
whether in the gutter/modal/both/none LSP tasks are shown exclusively,
if possible

Release Notes:

- Added a way to prefer LSP tasks over Zed tasks

Change summary

assets/settings/default.json             | 12 +++
crates/editor/Cargo.toml                 |  1 
crates/editor/src/editor.rs              | 30 +++++++++
crates/editor/src/editor_tests.rs        |  3 
crates/editor/src/hover_popover.rs       |  4 
crates/editor/src/lsp_ext.rs             | 80 +++++++++++++++----------
crates/language/src/language_settings.rs |  9 ++
crates/markdown/src/markdown.rs          |  2 
crates/tasks_ui/src/modal.rs             | 31 ++++++++-
9 files changed, 128 insertions(+), 44 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -1314,7 +1314,17 @@
   // Settings related to running tasks.
   "tasks": {
     "variables": {},
-    "enabled": true
+    "enabled": true,
+    // Use LSP tasks over Zed language extension ones.
+    // If no LSP tasks are returned due to error/timeout or regular execution,
+    // Zed language extension tasks will be used instead.
+    //
+    // Other Zed tasks will still be shown:
+    // * Zed task from either of the task config file
+    // * Zed task from history (e.g. one-off task was spawned before)
+    //
+    // Default: true
+    "prefer_lsp": true
   },
   // An object whose keys are language names, and whose values
   // are arrays of filenames or extensions of files that should

crates/editor/Cargo.toml 🔗

@@ -98,6 +98,7 @@ gpui = { workspace = true, features = ["test-support"] }
 language = { workspace = true, features = ["test-support"] }
 languages = {workspace = true, features = ["test-support"] }
 lsp = { workspace = true, features = ["test-support"] }
+markdown = { workspace = true, features = ["test-support"] }
 multi_buffer = { workspace = true, features = ["test-support"] }
 project = { workspace = true, features = ["test-support"] }
 release_channel.workspace = true

crates/editor/src/editor.rs 🔗

@@ -1670,6 +1670,13 @@ impl Editor {
                             editor
                                 .refresh_inlay_hints(InlayHintRefreshReason::RefreshRequested, cx);
                         }
+                        project::Event::LanguageServerAdded(..)
+                        | project::Event::LanguageServerRemoved(..) => {
+                            if editor.tasks_update_task.is_none() {
+                                editor.tasks_update_task =
+                                    Some(editor.refresh_runnables(window, cx));
+                            }
+                        }
                         project::Event::SnippetEdit(id, snippet_edits) => {
                             if let Some(buffer) = editor.buffer.read(cx).buffer(*id) {
                                 let focus_handle = editor.focus_handle(cx);
@@ -13543,6 +13550,7 @@ impl Editor {
         }
         let project = self.project.as_ref().map(Entity::downgrade);
         let task_sources = self.lsp_task_sources(cx);
+        let multi_buffer = self.buffer.downgrade();
         cx.spawn_in(window, async move |editor, cx| {
             cx.background_executor().timer(UPDATE_DEBOUNCE).await;
             let Some(project) = project.and_then(|p| p.upgrade()) else {
@@ -13626,7 +13634,19 @@ impl Editor {
                 return;
             };
 
-            let rows = Self::runnable_rows(project, display_snapshot, new_rows, cx.clone());
+            let Ok(prefer_lsp) = multi_buffer.update(cx, |buffer, cx| {
+                buffer.language_settings(cx).tasks.prefer_lsp
+            }) else {
+                return;
+            };
+
+            let rows = Self::runnable_rows(
+                project,
+                display_snapshot,
+                prefer_lsp && !lsp_tasks_by_rows.is_empty(),
+                new_rows,
+                cx.clone(),
+            );
             editor
                 .update(cx, |editor, _| {
                     editor.clear_tasks();
@@ -13654,15 +13674,21 @@ impl Editor {
     fn runnable_rows(
         project: Entity<Project>,
         snapshot: DisplaySnapshot,
+        prefer_lsp: bool,
         runnable_ranges: Vec<RunnableRange>,
         mut cx: AsyncWindowContext,
     ) -> Vec<((BufferId, BufferRow), RunnableTasks)> {
         runnable_ranges
             .into_iter()
             .filter_map(|mut runnable| {
-                let tasks = cx
+                let mut tasks = cx
                     .update(|_, cx| Self::templates_with_tags(&project, &mut runnable.runnable, cx))
                     .ok()?;
+                if prefer_lsp {
+                    tasks.retain(|(task_kind, _)| {
+                        !matches!(task_kind, TaskSourceKind::Language { .. })
+                    });
+                }
                 if tasks.is_empty() {
                     return None;
                 }

crates/editor/src/editor_tests.rs 🔗

@@ -9111,11 +9111,10 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) {
                 lsp::Url::from_file_path(path!("/file.rs")).unwrap()
             );
             assert_eq!(params.options.tab_size, 8);
-            Ok(Some(vec![]))
+            Ok(Some(Vec::new()))
         })
         .next()
         .await;
-    cx.executor().start_waiting();
     save.await;
 }
 

crates/editor/src/hover_popover.rs 🔗

@@ -1050,7 +1050,9 @@ mod tests {
 
                 for (range, event) in slice.iter() {
                     match event {
-                        MarkdownEvent::SubstitutedText(parsed) => rendered_text.push_str(parsed),
+                        MarkdownEvent::SubstitutedText(parsed) => {
+                            rendered_text.push_str(parsed.as_str())
+                        }
                         MarkdownEvent::Text | MarkdownEvent::Code => {
                             rendered_text.push_str(&text[range.clone()])
                         }

crates/editor/src/lsp_ext.rs 🔗

@@ -1,4 +1,5 @@
 use std::sync::Arc;
+use std::time::Duration;
 
 use crate::Editor;
 use collections::HashMap;
@@ -16,6 +17,7 @@ use project::LocationLink;
 use project::Project;
 use project::TaskSourceKind;
 use project::lsp_store::lsp_ext_command::GetLspRunnables;
+use smol::future::FutureExt as _;
 use smol::stream::StreamExt;
 use task::ResolvedTask;
 use task::TaskContext;
@@ -130,44 +132,58 @@ pub fn lsp_tasks(
         .collect::<FuturesUnordered<_>>();
 
     cx.spawn(async move |cx| {
-        let mut lsp_tasks = Vec::new();
-        while let Some(server_to_query) = lsp_task_sources.next().await {
-            if let Some((server_id, buffers)) = server_to_query {
-                let source_kind = TaskSourceKind::Lsp(server_id);
-                let id_base = source_kind.to_id_base();
-                let mut new_lsp_tasks = Vec::new();
-                for buffer in buffers {
-                    let lsp_buffer_context = lsp_task_context(&project, &buffer, cx)
-                        .await
-                        .unwrap_or_default();
+        cx.spawn(async move |cx| {
+            let mut lsp_tasks = Vec::new();
+            while let Some(server_to_query) = lsp_task_sources.next().await {
+                if let Some((server_id, buffers)) = server_to_query {
+                    let source_kind = TaskSourceKind::Lsp(server_id);
+                    let id_base = source_kind.to_id_base();
+                    let mut new_lsp_tasks = Vec::new();
+                    for buffer in buffers {
+                        let lsp_buffer_context = lsp_task_context(&project, &buffer, cx)
+                            .await
+                            .unwrap_or_default();
 
-                    if let Ok(runnables_task) = project.update(cx, |project, cx| {
-                        let buffer_id = buffer.read(cx).remote_id();
-                        project.request_lsp(
-                            buffer,
-                            LanguageServerToQuery::Other(server_id),
-                            GetLspRunnables {
-                                buffer_id,
-                                position: for_position,
-                            },
-                            cx,
-                        )
-                    }) {
-                        if let Some(new_runnables) = runnables_task.await.log_err() {
-                            new_lsp_tasks.extend(new_runnables.runnables.into_iter().filter_map(
-                                |(location, runnable)| {
-                                    let resolved_task =
-                                        runnable.resolve_task(&id_base, &lsp_buffer_context)?;
-                                    Some((location, resolved_task))
+                        if let Ok(runnables_task) = project.update(cx, |project, cx| {
+                            let buffer_id = buffer.read(cx).remote_id();
+                            project.request_lsp(
+                                buffer,
+                                LanguageServerToQuery::Other(server_id),
+                                GetLspRunnables {
+                                    buffer_id,
+                                    position: for_position,
                                 },
-                            ));
+                                cx,
+                            )
+                        }) {
+                            if let Some(new_runnables) = runnables_task.await.log_err() {
+                                new_lsp_tasks.extend(
+                                    new_runnables.runnables.into_iter().filter_map(
+                                        |(location, runnable)| {
+                                            let resolved_task = runnable
+                                                .resolve_task(&id_base, &lsp_buffer_context)?;
+                                            Some((location, resolved_task))
+                                        },
+                                    ),
+                                );
+                            }
                         }
                     }
+                    lsp_tasks.push((source_kind, new_lsp_tasks));
                 }
-                lsp_tasks.push((source_kind, new_lsp_tasks));
             }
-        }
-        lsp_tasks
+            lsp_tasks
+        })
+        .race({
+            // `lsp::LSP_REQUEST_TIMEOUT` is larger than we want for the modal to open fast
+            let timer = cx.background_executor().timer(Duration::from_millis(200));
+            async move {
+                timer.await;
+                log::info!("Timed out waiting for LSP tasks");
+                Vec::new()
+            }
+        })
+        .await
     })
 }
 

crates/language/src/language_settings.rs 🔗

@@ -1045,6 +1045,15 @@ pub struct LanguageTaskConfig {
     pub variables: HashMap<String, String>,
     #[serde(default = "default_true")]
     pub enabled: bool,
+    /// Use LSP tasks over Zed language extension ones.
+    /// If no LSP tasks are returned due to error/timeout or regular execution,
+    /// Zed language extension tasks will be used instead.
+    ///
+    /// Other Zed tasks will still be shown:
+    /// * Zed task from either of the task config file
+    /// * Zed task from history (e.g. one-off task was spawned before)
+    #[serde(default = "default_true")]
+    pub prefer_lsp: bool,
 }
 
 impl InlayHintSettings {

crates/markdown/src/markdown.rs 🔗

@@ -225,7 +225,7 @@ impl Markdown {
         self.parse(cx);
     }
 
-    #[cfg(feature = "test-support")]
+    #[cfg(any(test, feature = "test-support"))]
     pub fn parsed_markdown(&self) -> &ParsedMarkdown {
         &self.parsed_markdown
     }

crates/tasks_ui/src/modal.rs 🔗

@@ -1,6 +1,7 @@
 use std::sync::Arc;
 
 use crate::TaskContexts;
+use editor::Editor;
 use fuzzy::{StringMatch, StringMatchCandidate};
 use gpui::{
     Action, AnyElement, App, AppContext as _, Context, DismissEvent, Entity, EventEmitter,
@@ -246,15 +247,28 @@ impl PickerDelegate for TasksModalDelegate {
                     let workspace = self.workspace.clone();
                     let lsp_task_sources = self.task_contexts.lsp_task_sources.clone();
                     let task_position = self.task_contexts.latest_selection;
-
                     cx.spawn(async move |picker, cx| {
-                        let Ok(lsp_tasks) = workspace.update(cx, |workspace, cx| {
-                            editor::lsp_tasks(
+                        let Ok((lsp_tasks, prefer_lsp)) = workspace.update(cx, |workspace, cx| {
+                            let lsp_tasks = editor::lsp_tasks(
                                 workspace.project().clone(),
                                 &lsp_task_sources,
                                 task_position,
                                 cx,
-                            )
+                            );
+                            let prefer_lsp = workspace
+                                .active_item(cx)
+                                .and_then(|item| item.downcast::<Editor>())
+                                .map(|editor| {
+                                    editor
+                                        .read(cx)
+                                        .buffer()
+                                        .read(cx)
+                                        .language_settings(cx)
+                                        .tasks
+                                        .prefer_lsp
+                                })
+                                .unwrap_or(false);
+                            (lsp_tasks, prefer_lsp)
                         }) else {
                             return Vec::new();
                         };
@@ -269,6 +283,8 @@ impl PickerDelegate for TasksModalDelegate {
                                 };
 
                                 let mut new_candidates = used;
+                                let add_current_language_tasks =
+                                    !prefer_lsp || lsp_tasks.is_empty();
                                 new_candidates.extend(lsp_tasks.into_iter().flat_map(
                                     |(kind, tasks_with_locations)| {
                                         tasks_with_locations
@@ -279,7 +295,12 @@ impl PickerDelegate for TasksModalDelegate {
                                             .map(move |(_, task)| (kind.clone(), task))
                                     },
                                 ));
-                                new_candidates.extend(current);
+                                new_candidates.extend(current.into_iter().filter(
+                                    |(task_kind, _)| {
+                                        add_current_language_tasks
+                                            || !matches!(task_kind, TaskSourceKind::Language { .. })
+                                    },
+                                ));
                                 let match_candidates = string_match_candidates(&new_candidates);
                                 let _ = picker.delegate.candidates.insert(new_candidates);
                                 match_candidates