Merge pull request #2373 from zed-industries/fix-copilot-panic

Antonio Scandurra created 2 years ago

Avoid interpolating Copilot suggestion if cursor excerpt differs

Change summary

crates/editor/src/editor.rs       |   3 
crates/editor/src/editor_tests.rs | 104 +++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 1 deletion(-)

Detailed changes

crates/editor/src/editor.rs

@@ -1040,7 +1040,8 @@ impl CopilotState {
         let completion = self.completions.get(self.active_completion_index)?;
         let excerpt_id = self.excerpt_id?;
         let completion_buffer = buffer.buffer_for_excerpt(excerpt_id)?;
-        if !completion.range.start.is_valid(completion_buffer)
+        if excerpt_id != cursor.excerpt_id
+            || !completion.range.start.is_valid(completion_buffer)
             || !completion.range.end.is_valid(completion_buffer)
         {
             return None;

crates/editor/src/editor_tests.rs

@@ -6163,6 +6163,110 @@ async fn test_copilot_completion_invalidation(
     });
 }
 
+#[gpui::test]
+async fn test_copilot_multibuffer(
+    deterministic: Arc<Deterministic>,
+    cx: &mut gpui::TestAppContext,
+) {
+    let (copilot, copilot_lsp) = Copilot::fake(cx);
+    cx.update(|cx| {
+        cx.set_global(Settings::test(cx));
+        cx.set_global(copilot)
+    });
+
+    let buffer_1 = cx.add_model(|cx| Buffer::new(0, "a = 1\nb = 2\n", cx));
+    let buffer_2 = cx.add_model(|cx| Buffer::new(0, "c = 3\nd = 4\n", cx));
+    let multibuffer = cx.add_model(|cx| {
+        let mut multibuffer = MultiBuffer::new(0);
+        multibuffer.push_excerpts(
+            buffer_1.clone(),
+            [ExcerptRange {
+                context: Point::new(0, 0)..Point::new(2, 0),
+                primary: None,
+            }],
+            cx,
+        );
+        multibuffer.push_excerpts(
+            buffer_2.clone(),
+            [ExcerptRange {
+                context: Point::new(0, 0)..Point::new(2, 0),
+                primary: None,
+            }],
+            cx,
+        );
+        multibuffer
+    });
+    let (_, editor) = cx.add_window(|cx| build_editor(multibuffer, cx));
+
+    handle_copilot_completion_request(
+        &copilot_lsp,
+        vec![copilot::request::Completion {
+            text: "b = 2 + a".into(),
+            range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 5)),
+            ..Default::default()
+        }],
+        vec![],
+    );
+    editor.update(cx, |editor, cx| {
+        // Ensure copilot suggestions are shown for the first excerpt.
+        editor.change_selections(None, cx, |s| {
+            s.select_ranges([Point::new(1, 5)..Point::new(1, 5)])
+        });
+        editor.next_copilot_suggestion(&Default::default(), cx);
+    });
+    deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT);
+    editor.update(cx, |editor, cx| {
+        assert!(editor.has_active_copilot_suggestion(cx));
+        assert_eq!(
+            editor.display_text(cx),
+            "\n\na = 1\nb = 2 + a\n\n\n\nc = 3\nd = 4\n"
+        );
+        assert_eq!(editor.text(cx), "a = 1\nb = 2\n\nc = 3\nd = 4\n");
+    });
+
+    handle_copilot_completion_request(
+        &copilot_lsp,
+        vec![copilot::request::Completion {
+            text: "d = 4 + c".into(),
+            range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 6)),
+            ..Default::default()
+        }],
+        vec![],
+    );
+    editor.update(cx, |editor, cx| {
+        // Move to another excerpt, ensuring the suggestion gets cleared.
+        editor.change_selections(None, cx, |s| {
+            s.select_ranges([Point::new(4, 5)..Point::new(4, 5)])
+        });
+        assert!(!editor.has_active_copilot_suggestion(cx));
+        assert_eq!(
+            editor.display_text(cx),
+            "\n\na = 1\nb = 2\n\n\n\nc = 3\nd = 4\n"
+        );
+        assert_eq!(editor.text(cx), "a = 1\nb = 2\n\nc = 3\nd = 4\n");
+
+        // Type a character, ensuring we don't even try to interpolate the previous suggestion.
+        editor.handle_input(" ", cx);
+        assert!(!editor.has_active_copilot_suggestion(cx));
+        assert_eq!(
+            editor.display_text(cx),
+            "\n\na = 1\nb = 2\n\n\n\nc = 3\nd = 4 \n"
+        );
+        assert_eq!(editor.text(cx), "a = 1\nb = 2\n\nc = 3\nd = 4 \n");
+    });
+
+    // Ensure the new suggestion is displayed when the debounce timeout expires.
+    deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT);
+    editor.update(cx, |editor, cx| {
+        assert!(editor.has_active_copilot_suggestion(cx));
+        assert_eq!(
+            editor.display_text(cx),
+            "\n\na = 1\nb = 2\n\n\n\nc = 3\nd = 4 + c\n"
+        );
+        assert_eq!(editor.text(cx), "a = 1\nb = 2\n\nc = 3\nd = 4 \n");
+    });
+}
+
 fn empty_range(row: usize, column: usize) -> Range<DisplayPoint> {
     let point = DisplayPoint::new(row as u32, column as u32);
     point..point