From 6e43e77c3f3eed3b7509152f54b3273a0997c331 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 31 Mar 2023 17:08:41 +0200 Subject: [PATCH] Use copilot's `Completion::{range,text}` to determine suggestion Previously, we were using display text, but this isn't always correct. Now, we just attempt to determine what text Copilot wants to insert by finding a prefix and suffix in the existing text with the suggested text. Co-Authored-By: Nathan Sobo --- crates/copilot/src/copilot.rs | 10 ++-- crates/editor/src/editor.rs | 100 ++++++++++++++++++++-------------- 2 files changed, 66 insertions(+), 44 deletions(-) diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index 5b3b066ea8..af07fc3e42 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -17,6 +17,7 @@ use settings::Settings; use smol::{fs, io::BufReader, stream::StreamExt}; use std::{ ffi::OsString, + ops::Range, path::{Path, PathBuf}, sync::Arc, }; @@ -130,7 +131,7 @@ impl Status { #[derive(Debug, PartialEq, Eq)] pub struct Completion { - pub position: Anchor, + pub range: Range, pub text: String, } @@ -548,10 +549,11 @@ where } fn completion_from_lsp(completion: request::Completion, buffer: &BufferSnapshot) -> Completion { - let position = buffer.clip_point_utf16(point_from_lsp(completion.position), Bias::Left); + let start = buffer.clip_point_utf16(point_from_lsp(completion.range.start), Bias::Left); + let end = buffer.clip_point_utf16(point_from_lsp(completion.range.end), Bias::Left); Completion { - position: buffer.anchor_before(position), - text: completion.display_text, + range: buffer.anchor_before(start)..buffer.anchor_after(end), + text: completion.text, } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index e0ab8d84b4..623df221c4 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1028,42 +1028,56 @@ impl Default for CopilotState { } } +fn common_prefix, T2: Iterator>(a: T1, b: T2) -> usize { + a.zip(b) + .take_while(|(a, b)| a == b) + .map(|(a, _)| a.len_utf8()) + .sum() +} + impl CopilotState { fn text_for_active_completion( &self, cursor: Anchor, buffer: &MultiBufferSnapshot, ) -> Option<&str> { - let cursor_offset = cursor.to_offset(buffer); let completion = self.completions.get(self.active_completion_index)?; - if self.excerpt_id == Some(cursor.excerpt_id) { - let completion_offset: usize = buffer.summary_for_anchor(&Anchor { - excerpt_id: cursor.excerpt_id, - buffer_id: cursor.buffer_id, - text_anchor: completion.position, - }); - let prefix_len = cursor_offset.saturating_sub(completion_offset); - if completion_offset <= cursor_offset && prefix_len <= completion.text.len() { - let (prefix, suffix) = completion.text.split_at(prefix_len); - if buffer.contains_str_at(completion_offset, prefix) && !suffix.is_empty() { - return Some(suffix); - } - } + let excerpt_id = self.excerpt_id?; + let completion_buffer_id = buffer.buffer_id_for_excerpt(excerpt_id); + let completion_start = Anchor { + excerpt_id, + buffer_id: completion_buffer_id, + text_anchor: completion.range.start, + }; + let completion_end = Anchor { + excerpt_id, + buffer_id: completion_buffer_id, + text_anchor: completion.range.end, + }; + let prefix_len = common_prefix(buffer.chars_at(completion_start), completion.text.chars()); + let suffix_len = common_prefix( + buffer.reversed_chars_at(completion_end), + completion.text.chars().rev(), + ); + + let prefix_end_offset = completion_start.to_offset(&buffer) + prefix_len; + let suffix_start_offset = completion_end.to_offset(&buffer) - suffix_len; + if prefix_end_offset == suffix_start_offset + && prefix_end_offset == cursor.to_offset(&buffer) + { + Some(&completion.text[prefix_len..completion.text.len() - suffix_len]) + } else { + None } - None } - fn push_completion( - &mut self, - new_completion: copilot::Completion, - ) -> Option<&copilot::Completion> { + fn push_completion(&mut self, new_completion: copilot::Completion) { for completion in &self.completions { if *completion == new_completion { - return None; + return; } } self.completions.push(new_completion); - self.completions.last() } } @@ -2806,7 +2820,8 @@ impl Editor { self.copilot_state.active_completion_index = 0; cx.notify(); } else { - self.clear_copilot_suggestions(cx); + self.display_map + .update(cx, |map, cx| map.replace_suggestion::(None, cx)); } if !copilot.read(cx).status().is_authorized() { @@ -2828,26 +2843,31 @@ impl Editor { completions.extend(completion.log_err().flatten()); completions.extend(completions_cycling.log_err().into_iter().flatten()); this.upgrade(&cx)?.update(&mut cx, |this, cx| { - this.copilot_state.completions.clear(); - this.copilot_state.active_completion_index = 0; - this.copilot_state.excerpt_id = Some(cursor.excerpt_id); - for completion in completions { - let was_empty = this.copilot_state.completions.is_empty(); - if let Some(completion) = this.copilot_state.push_completion(completion) { - if was_empty { - this.display_map.update(cx, |map, cx| { - map.replace_suggestion( - Some(Suggestion { - position: cursor, - text: completion.text.as_str().into(), - }), - cx, - ) - }); - } + if !completions.is_empty() { + this.copilot_state.completions.clear(); + this.copilot_state.active_completion_index = 0; + this.copilot_state.excerpt_id = Some(cursor.excerpt_id); + for completion in completions { + this.copilot_state.push_completion(completion); } + + let buffer = this.buffer.read(cx).snapshot(cx); + if let Some(text) = this + .copilot_state + .text_for_active_completion(cursor, &buffer) + { + this.display_map.update(cx, |map, cx| { + map.replace_suggestion( + Some(Suggestion { + position: cursor, + text: text.into(), + }), + cx, + ) + }); + } + cx.notify(); } - cx.notify(); }); Some(())