Merge pull request #2358 from zed-industries/more-copilot-fixes

Mikayla Maki created 2 years ago

Invalidate copilot suggestion on backspaces

Change summary

crates/copilot/src/copilot.rs     | 21 ++++++++++
crates/editor/src/editor.rs       |  5 ++
crates/editor/src/editor_tests.rs | 65 +++++++++++++++++++++++++++++++++
crates/text/src/anchor.rs         | 19 +++++++++
4 files changed, 109 insertions(+), 1 deletion(-)

Detailed changes

crates/copilot/src/copilot.rs

@@ -50,6 +50,27 @@ pub fn init(http: Arc<dyn HttpClient>, node_runtime: Arc<NodeRuntime>, cx: &mut
     });
     cx.set_global(copilot.clone());
 
+    cx.observe(&copilot, |handle, cx| {
+        let status = handle.read(cx).status();
+        cx.update_global::<collections::CommandPaletteFilter, _, _>(
+            move |filter, _cx| match status {
+                Status::Disabled => {
+                    filter.filtered_namespaces.insert(COPILOT_NAMESPACE);
+                    filter.filtered_namespaces.insert(COPILOT_AUTH_NAMESPACE);
+                }
+                Status::Authorized => {
+                    filter.filtered_namespaces.remove(COPILOT_NAMESPACE);
+                    filter.filtered_namespaces.remove(COPILOT_AUTH_NAMESPACE);
+                }
+                _ => {
+                    filter.filtered_namespaces.insert(COPILOT_NAMESPACE);
+                    filter.filtered_namespaces.remove(COPILOT_AUTH_NAMESPACE);
+                }
+            },
+        );
+    })
+    .detach();
+
     sign_in::init(cx);
     cx.add_global_action(|_: &SignIn, cx| {
         if let Some(copilot) = Copilot::global(cx) {

crates/editor/src/editor.rs

@@ -1037,6 +1037,11 @@ 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)
+            || !completion.range.end.is_valid(completion_buffer)
+        {
+            return None;
+        }
 
         let mut completion_range = completion.range.to_offset(&completion_buffer);
         let prefix_len = Self::common_prefix(

crates/editor/src/editor_tests.rs

@@ -6098,6 +6098,71 @@ async fn test_copilot(deterministic: Arc<Deterministic>, cx: &mut gpui::TestAppC
     });
 }
 
+#[gpui::test]
+async fn test_copilot_completion_invalidation(
+    deterministic: Arc<Deterministic>,
+    cx: &mut gpui::TestAppContext,
+) {
+    let (copilot, copilot_lsp) = Copilot::fake(cx);
+    cx.update(|cx| cx.set_global(copilot));
+    let mut cx = EditorLspTestContext::new_rust(
+        lsp::ServerCapabilities {
+            completion_provider: Some(lsp::CompletionOptions {
+                trigger_characters: Some(vec![".".to_string(), ":".to_string()]),
+                ..Default::default()
+            }),
+            ..Default::default()
+        },
+        cx,
+    )
+    .await;
+
+    cx.set_state(indoc! {"
+        one
+        twˇ
+        three
+    "});
+
+    handle_copilot_completion_request(
+        &copilot_lsp,
+        vec![copilot::request::Completion {
+            text: "two.foo()".into(),
+            range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 2)),
+            ..Default::default()
+        }],
+        vec![],
+    );
+    cx.update_editor(|editor, cx| editor.next_copilot_suggestion(&Default::default(), cx));
+    deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT);
+    cx.update_editor(|editor, cx| {
+        assert!(editor.has_active_copilot_suggestion(cx));
+        assert_eq!(editor.display_text(cx), "one\ntwo.foo()\nthree\n");
+        assert_eq!(editor.text(cx), "one\ntw\nthree\n");
+
+        editor.backspace(&Default::default(), cx);
+        assert!(editor.has_active_copilot_suggestion(cx));
+        assert_eq!(editor.display_text(cx), "one\ntwo.foo()\nthree\n");
+        assert_eq!(editor.text(cx), "one\nt\nthree\n");
+
+        editor.backspace(&Default::default(), cx);
+        assert!(editor.has_active_copilot_suggestion(cx));
+        assert_eq!(editor.display_text(cx), "one\ntwo.foo()\nthree\n");
+        assert_eq!(editor.text(cx), "one\n\nthree\n");
+
+        // Deleting across the original suggestion range invalidates it.
+        editor.backspace(&Default::default(), cx);
+        assert!(!editor.has_active_copilot_suggestion(cx));
+        assert_eq!(editor.display_text(cx), "one\nthree\n");
+        assert_eq!(editor.text(cx), "one\nthree\n");
+
+        // Undoing the deletion restores the suggestion.
+        editor.undo(&Default::default(), cx);
+        assert!(editor.has_active_copilot_suggestion(cx));
+        assert_eq!(editor.display_text(cx), "one\ntwo.foo()\nthree\n");
+        assert_eq!(editor.text(cx), "one\n\nthree\n");
+    });
+}
+
 fn empty_range(row: usize, column: usize) -> Range<DisplayPoint> {
     let point = DisplayPoint::new(row as u32, column as u32);
     point..point

crates/text/src/anchor.rs

@@ -1,4 +1,7 @@
-use crate::{BufferSnapshot, Point, PointUtf16, TextDimension, ToOffset, ToPoint, ToPointUtf16};
+use crate::{
+    locator::Locator, BufferSnapshot, Point, PointUtf16, TextDimension, ToOffset, ToPoint,
+    ToPointUtf16,
+};
 use anyhow::Result;
 use std::{cmp::Ordering, fmt::Debug, ops::Range};
 use sum_tree::Bias;
@@ -86,6 +89,20 @@ impl Anchor {
     {
         content.summary_for_anchor(self)
     }
+
+    /// Returns true when the [Anchor] is located inside a visible fragment.
+    pub fn is_valid(&self, buffer: &BufferSnapshot) -> bool {
+        if *self == Anchor::MIN || *self == Anchor::MAX {
+            true
+        } else {
+            let fragment_id = buffer.fragment_id_for_anchor(self);
+            let mut fragment_cursor = buffer.fragments.cursor::<(Option<&Locator>, usize)>();
+            fragment_cursor.seek(&Some(fragment_id), Bias::Left, &None);
+            fragment_cursor
+                .item()
+                .map_or(false, |fragment| fragment.visible)
+        }
+    }
 }
 
 pub trait OffsetRangeExt {