From 6e43e77c3f3eed3b7509152f54b3273a0997c331 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 31 Mar 2023 17:08:41 +0200 Subject: [PATCH 1/4] 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(()) From b588ba1435d54286a1568d9f7a1b87c91537a066 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 31 Mar 2023 17:17:35 +0200 Subject: [PATCH 2/4] Avoid auto-indenting when accepting copilot suggestion Co-Authored-By: Nathan Sobo --- crates/editor/src/editor.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 623df221c4..62efad288d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2145,6 +2145,21 @@ impl Editor { } pub fn insert(&mut self, text: &str, cx: &mut ViewContext) { + self.insert_with_autoindent_mode( + text, + Some(AutoindentMode::Block { + original_indent_columns: Vec::new(), + }), + cx, + ); + } + + fn insert_with_autoindent_mode( + &mut self, + text: &str, + autoindent_mode: Option, + cx: &mut ViewContext, + ) { let text: Arc = text.into(); self.transact(cx, |this, cx| { let old_selections = this.selections.all_adjusted(cx); @@ -2163,9 +2178,7 @@ impl Editor { old_selections .iter() .map(|s| (s.start..s.end, text.clone())), - Some(AutoindentMode::Block { - original_indent_columns: Vec::new(), - }), + autoindent_mode, cx, ); anchors @@ -2975,7 +2988,7 @@ impl Editor { .copilot_state .text_for_active_completion(cursor, &snapshot) { - self.insert(&text.to_string(), cx); + self.insert_with_autoindent_mode(&text.to_string(), None, cx); self.clear_copilot_suggestions(cx); true } else { From 5f579a4287625a109cfa855f97bdd6fccbe1a7da Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 31 Mar 2023 18:03:38 +0200 Subject: [PATCH 3/4] Fix prefix/suffix calculation when determining copilot suggestion Co-Authored-By: Nathan Sobo Co-Authored-By: Mikayla Maki --- crates/editor/src/editor.rs | 38 +++++++++++++++---------------- crates/editor/src/multi_buffer.rs | 4 ++++ crates/text/src/text.rs | 8 +++++++ 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 62efad288d..0b6d19e34a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1041,29 +1041,26 @@ impl CopilotState { cursor: Anchor, buffer: &MultiBufferSnapshot, ) -> Option<&str> { + use language::ToOffset as _; + let completion = self.completions.get(self.active_completion_index)?; 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 completion_buffer = buffer.buffer_for_excerpt(excerpt_id)?; - 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) + let mut completion_range = completion.range.to_offset(&completion_buffer); + let prefix_len = common_prefix( + completion_buffer.chars_for_range(completion_range.clone()), + completion.text.chars(), + ); + completion_range.start += prefix_len; + let suffix_len = common_prefix( + completion_buffer.reversed_chars_for_range(completion_range.clone()), + completion.text[prefix_len..].chars().rev(), + ); + completion_range.end = completion_range.end.saturating_sub(suffix_len); + + if completion_range.is_empty() + && completion_range.start == cursor.text_anchor.to_offset(&completion_buffer) { Some(&completion.text[prefix_len..completion.text.len() - suffix_len]) } else { @@ -6492,6 +6489,7 @@ impl Editor { multi_buffer::Event::Edited => { self.refresh_active_diagnostics(cx); self.refresh_code_actions(cx); + self.refresh_copilot_suggestions(cx); cx.emit(Event::BufferEdited); } multi_buffer::Event::ExcerptsAdded { diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 0b85f94f08..ed4d297c51 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -2933,6 +2933,10 @@ impl MultiBufferSnapshot { Some(self.excerpt(excerpt_id)?.buffer_id) } + pub fn buffer_for_excerpt(&self, excerpt_id: ExcerptId) -> Option<&BufferSnapshot> { + Some(&self.excerpt(excerpt_id)?.buffer) + } + fn excerpt<'a>(&'a self, excerpt_id: ExcerptId) -> Option<&'a Excerpt> { let mut cursor = self.excerpts.cursor::>(); let locator = self.excerpt_locator_for_id(excerpt_id); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index c7d36e29de..11d451eab0 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -1579,6 +1579,14 @@ impl BufferSnapshot { self.text_for_range(range).flat_map(str::chars) } + pub fn reversed_chars_for_range( + &self, + range: Range, + ) -> impl Iterator + '_ { + self.reversed_chunks_in_range(range) + .flat_map(|chunk| chunk.chars().rev()) + } + pub fn contains_str_at(&self, position: T, needle: &str) -> bool where T: ToOffset, From b208d1a489228c661eb38cd09d15b45fb532b07f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 31 Mar 2023 18:10:10 +0200 Subject: [PATCH 4/4] :art: --- crates/editor/src/editor.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 0b6d19e34a..a5722e183e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1028,13 +1028,6 @@ 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, @@ -1048,12 +1041,12 @@ impl CopilotState { let completion_buffer = buffer.buffer_for_excerpt(excerpt_id)?; let mut completion_range = completion.range.to_offset(&completion_buffer); - let prefix_len = common_prefix( + let prefix_len = Self::common_prefix( completion_buffer.chars_for_range(completion_range.clone()), completion.text.chars(), ); completion_range.start += prefix_len; - let suffix_len = common_prefix( + let suffix_len = Self::common_prefix( completion_buffer.reversed_chars_for_range(completion_range.clone()), completion.text[prefix_len..].chars().rev(), ); @@ -1076,6 +1069,13 @@ impl CopilotState { } self.completions.push(new_completion); } + + 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() + } } #[derive(Debug)]