Clear buffer colors on empty LSP response (#38742)

Kirill Bulatov created

Follow-up of https://github.com/zed-industries/zed/pull/32816
Closes https://github.com/zed-industries/zed/issues/38602


https://github.com/user-attachments/assets/26058c91-4ffd-4c6f-a41d-17da0c3d7220

Release Notes:

- Fixed buffer colors not cleared on empty LSP responses

Change summary

crates/editor/src/editor_tests.rs |  46 +++++++++++++
crates/editor/src/lsp_colors.rs   | 112 +++++++++++++++++---------------
2 files changed, 106 insertions(+), 52 deletions(-)

Detailed changes

crates/editor/src/editor_tests.rs 🔗

@@ -25701,7 +25701,7 @@ async fn test_document_colors(cx: &mut TestAppContext) {
     workspace
         .update(cx, |workspace, window, cx| {
             workspace.active_pane().update(cx, |pane, cx| {
-                pane.navigate_backward(&Default::default(), window, cx);
+                pane.navigate_backward(&workspace::GoBack, window, cx);
             })
         })
         .unwrap();
@@ -25729,6 +25729,50 @@ async fn test_document_colors(cx: &mut TestAppContext) {
             "Should have an initial inlay"
         );
     });
+
+    drop(color_request_handle);
+    let closure_requests_made = Arc::clone(&requests_made);
+    let mut empty_color_request_handle = fake_language_server
+        .set_request_handler::<lsp::request::DocumentColor, _, _>(move |params, _| {
+            let requests_made = Arc::clone(&closure_requests_made);
+            async move {
+                assert_eq!(
+                    params.text_document.uri,
+                    lsp::Uri::from_file_path(path!("/a/first.rs")).unwrap()
+                );
+                requests_made.fetch_add(1, atomic::Ordering::Release);
+                Ok(Vec::new())
+            }
+        });
+    let save = editor.update_in(cx, |editor, window, cx| {
+        editor.move_to_end(&MoveToEnd, window, cx);
+        editor.handle_input("dirty_again", window, cx);
+        editor.save(
+            SaveOptions {
+                format: false,
+                autosave: true,
+            },
+            project.clone(),
+            window,
+            cx,
+        )
+    });
+    save.await.unwrap();
+
+    empty_color_request_handle.next().await.unwrap();
+    cx.run_until_parked();
+    assert_eq!(
+        4,
+        requests_made.load(atomic::Ordering::Acquire),
+        "Should query for colors once per save only, as formatting was not requested"
+    );
+    editor.update(cx, |editor, cx| {
+        assert_eq!(
+            Vec::<Rgba>::new(),
+            extract_color_inlays(editor, cx),
+            "Should clear all colors when the server returns an empty response"
+        );
+    });
 }
 
 #[gpui::test]

crates/editor/src/lsp_colors.rs 🔗

@@ -228,60 +228,70 @@ impl Editor {
                 };
                 match colors {
                     Ok(colors) => {
-                        for color in colors.colors {
-                            let color_start = point_from_lsp(color.lsp_range.start);
-                            let color_end = point_from_lsp(color.lsp_range.end);
+                        if colors.colors.is_empty() {
+                            let new_entry =
+                                new_editor_colors.entry(buffer_id).or_insert_with(|| {
+                                    (Vec::<(Range<Anchor>, DocumentColor)>::new(), None)
+                                });
+                            new_entry.0.clear();
+                            new_entry.1 = colors.cache_version;
+                        } else {
+                            for color in colors.colors {
+                                let color_start = point_from_lsp(color.lsp_range.start);
+                                let color_end = point_from_lsp(color.lsp_range.end);
 
-                            for (excerpt_id, buffer_snapshot, excerpt_range) in excerpts {
-                                if !excerpt_range.contains(&color_start.0)
-                                    || !excerpt_range.contains(&color_end.0)
-                                {
-                                    continue;
-                                }
-                                let Some(color_start_anchor) = multi_buffer_snapshot
-                                    .anchor_in_excerpt(
-                                        *excerpt_id,
-                                        buffer_snapshot.anchor_before(
-                                            buffer_snapshot
-                                                .clip_point_utf16(color_start, Bias::Left),
-                                        ),
-                                    )
-                                else {
-                                    continue;
-                                };
-                                let Some(color_end_anchor) = multi_buffer_snapshot
-                                    .anchor_in_excerpt(
-                                        *excerpt_id,
-                                        buffer_snapshot.anchor_after(
-                                            buffer_snapshot
-                                                .clip_point_utf16(color_end, Bias::Right),
-                                        ),
-                                    )
-                                else {
-                                    continue;
-                                };
+                                for (excerpt_id, buffer_snapshot, excerpt_range) in excerpts {
+                                    if !excerpt_range.contains(&color_start.0)
+                                        || !excerpt_range.contains(&color_end.0)
+                                    {
+                                        continue;
+                                    }
+                                    let Some(color_start_anchor) = multi_buffer_snapshot
+                                        .anchor_in_excerpt(
+                                            *excerpt_id,
+                                            buffer_snapshot.anchor_before(
+                                                buffer_snapshot
+                                                    .clip_point_utf16(color_start, Bias::Left),
+                                            ),
+                                        )
+                                    else {
+                                        continue;
+                                    };
+                                    let Some(color_end_anchor) = multi_buffer_snapshot
+                                        .anchor_in_excerpt(
+                                            *excerpt_id,
+                                            buffer_snapshot.anchor_after(
+                                                buffer_snapshot
+                                                    .clip_point_utf16(color_end, Bias::Right),
+                                            ),
+                                        )
+                                    else {
+                                        continue;
+                                    };
 
-                                let new_entry =
-                                    new_editor_colors.entry(buffer_id).or_insert_with(|| {
-                                        (Vec::<(Range<Anchor>, DocumentColor)>::new(), None)
-                                    });
-                                new_entry.1 = colors.cache_version;
-                                let new_buffer_colors = &mut new_entry.0;
+                                    let new_entry =
+                                        new_editor_colors.entry(buffer_id).or_insert_with(|| {
+                                            (Vec::<(Range<Anchor>, DocumentColor)>::new(), None)
+                                        });
+                                    new_entry.1 = colors.cache_version;
+                                    let new_buffer_colors = &mut new_entry.0;
 
-                                let (Ok(i) | Err(i)) =
-                                    new_buffer_colors.binary_search_by(|(probe, _)| {
-                                        probe
-                                            .start
-                                            .cmp(&color_start_anchor, &multi_buffer_snapshot)
-                                            .then_with(|| {
-                                                probe
-                                                    .end
-                                                    .cmp(&color_end_anchor, &multi_buffer_snapshot)
-                                            })
-                                    });
-                                new_buffer_colors
-                                    .insert(i, (color_start_anchor..color_end_anchor, color));
-                                break;
+                                    let (Ok(i) | Err(i)) =
+                                        new_buffer_colors.binary_search_by(|(probe, _)| {
+                                            probe
+                                                .start
+                                                .cmp(&color_start_anchor, &multi_buffer_snapshot)
+                                                .then_with(|| {
+                                                    probe.end.cmp(
+                                                        &color_end_anchor,
+                                                        &multi_buffer_snapshot,
+                                                    )
+                                                })
+                                        });
+                                    new_buffer_colors
+                                        .insert(i, (color_start_anchor..color_end_anchor, color));
+                                    break;
+                                }
                             }
                         }
                     }