From 22342206183005b8dd58ecb6d6cac8efcac54a42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20L=C3=BCthy?= Date: Thu, 7 Aug 2025 17:27:29 +0200 Subject: [PATCH] completions: Add subtle/eager behavior to Supermaven and Copilot (#35548) This pull request introduces changes to improve the behavior and consistency of multiple completion providers (`CopilotCompletionProvider`, `SupermavenCompletionProvider`) and their integration with UI elements like menus and inline completion buttons. It now allows to see the prediction with the completion menu open whilst pressing `opt` and also enables the subtle/eager setting that was introduced with zeta. Edit: I managed to get the preview working with correct icons! image CleanShot 2025-08-04 at 01 36 31@2x Correct icons are also displayed: image Edit2: I added some comments, would be very happy to receive feedback (still learning rust) Release Notes: - Added Subtle and Eager edit prediction modes to Copilot and Supermaven --- .../src/copilot_completion_provider.rs | 23 +-- crates/edit_prediction/src/edit_prediction.rs | 9 ++ .../src/edit_prediction_button.rs | 7 +- crates/editor/src/edit_prediction_tests.rs | 152 ++++++++++++++++++ crates/editor/src/editor.rs | 141 +++++++++++----- crates/supermaven/src/supermaven.rs | 92 +++++++++-- .../src/supermaven_completion_provider.rs | 11 +- 7 files changed, 372 insertions(+), 63 deletions(-) diff --git a/crates/copilot/src/copilot_completion_provider.rs b/crates/copilot/src/copilot_completion_provider.rs index 2a7225c4e3..2fd6df27b9 100644 --- a/crates/copilot/src/copilot_completion_provider.rs +++ b/crates/copilot/src/copilot_completion_provider.rs @@ -58,11 +58,19 @@ impl EditPredictionProvider for CopilotCompletionProvider { } fn show_completions_in_menu() -> bool { + true + } + + fn show_tab_accept_marker() -> bool { + true + } + + fn supports_jump_to_edit() -> bool { false } fn is_refreshing(&self) -> bool { - self.pending_refresh.is_some() + self.pending_refresh.is_some() && self.completions.is_empty() } fn is_enabled( @@ -343,8 +351,8 @@ mod tests { executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); cx.update_editor(|editor, window, cx| { assert!(editor.context_menu_visible()); - assert!(!editor.has_active_edit_prediction()); - // Since we have both, the copilot suggestion is not shown inline + assert!(editor.has_active_edit_prediction()); + // Since we have both, the copilot suggestion is existing but does not show up as ghost text assert_eq!(editor.text(cx), "one.\ntwo\nthree\n"); assert_eq!(editor.display_text(cx), "one.\ntwo\nthree\n"); @@ -934,8 +942,9 @@ mod tests { executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); cx.update_editor(|editor, _, cx| { assert!(editor.context_menu_visible()); - assert!(!editor.has_active_edit_prediction(),); + assert!(editor.has_active_edit_prediction()); assert_eq!(editor.text(cx), "one\ntwo.\nthree\n"); + assert_eq!(editor.display_text(cx), "one\ntwo.\nthree\n"); }); } @@ -1077,8 +1086,6 @@ mod tests { vec![complete_from_marker.clone(), replace_range_marker.clone()], ); - let complete_from_position = - cx.to_lsp(marked_ranges.remove(&complete_from_marker).unwrap()[0].start); let replace_range = cx.to_lsp_range(marked_ranges.remove(&replace_range_marker).unwrap()[0].clone()); @@ -1087,10 +1094,6 @@ mod tests { let completions = completions.clone(); async move { assert_eq!(params.text_document_position.text_document.uri, url.clone()); - assert_eq!( - params.text_document_position.position, - complete_from_position - ); Ok(Some(lsp::CompletionResponse::Array( completions .iter() diff --git a/crates/edit_prediction/src/edit_prediction.rs b/crates/edit_prediction/src/edit_prediction.rs index fd4e9bb21d..c8502f75de 100644 --- a/crates/edit_prediction/src/edit_prediction.rs +++ b/crates/edit_prediction/src/edit_prediction.rs @@ -61,6 +61,10 @@ pub trait EditPredictionProvider: 'static + Sized { fn show_tab_accept_marker() -> bool { false } + fn supports_jump_to_edit() -> bool { + true + } + fn data_collection_state(&self, _cx: &App) -> DataCollectionState { DataCollectionState::Unsupported } @@ -116,6 +120,7 @@ pub trait EditPredictionProviderHandle { ) -> bool; fn show_completions_in_menu(&self) -> bool; fn show_tab_accept_marker(&self) -> bool; + fn supports_jump_to_edit(&self) -> bool; fn data_collection_state(&self, cx: &App) -> DataCollectionState; fn usage(&self, cx: &App) -> Option; fn toggle_data_collection(&self, cx: &mut App); @@ -166,6 +171,10 @@ where T::show_tab_accept_marker() } + fn supports_jump_to_edit(&self) -> bool { + T::supports_jump_to_edit() + } + fn data_collection_state(&self, cx: &App) -> DataCollectionState { self.read(cx).data_collection_state(cx) } diff --git a/crates/edit_prediction_button/src/edit_prediction_button.rs b/crates/edit_prediction_button/src/edit_prediction_button.rs index 9ab94a4095..3d3b43d71b 100644 --- a/crates/edit_prediction_button/src/edit_prediction_button.rs +++ b/crates/edit_prediction_button/src/edit_prediction_button.rs @@ -491,7 +491,12 @@ impl EditPredictionButton { let subtle_mode = matches!(current_mode, EditPredictionsMode::Subtle); let eager_mode = matches!(current_mode, EditPredictionsMode::Eager); - if matches!(provider, EditPredictionProvider::Zed) { + if matches!( + provider, + EditPredictionProvider::Zed + | EditPredictionProvider::Copilot + | EditPredictionProvider::Supermaven + ) { menu = menu .separator() .header("Display Modes") diff --git a/crates/editor/src/edit_prediction_tests.rs b/crates/editor/src/edit_prediction_tests.rs index 527dfb8832..7bf51e45d7 100644 --- a/crates/editor/src/edit_prediction_tests.rs +++ b/crates/editor/src/edit_prediction_tests.rs @@ -228,6 +228,49 @@ async fn test_edit_prediction_invalidation_range(cx: &mut gpui::TestAppContext) }); } +#[gpui::test] +async fn test_edit_prediction_jump_disabled_for_non_zed_providers(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeNonZedEditPredictionProvider::default()); + assign_editor_completion_provider_non_zed(provider.clone(), &mut cx); + + // Cursor is 2+ lines above the proposed edit + cx.set_state(indoc! {" + line 0 + line ˇ1 + line 2 + line 3 + line + "}); + + propose_edits_non_zed( + &provider, + vec![(Point::new(4, 3)..Point::new(4, 3), " 4")], + &mut cx, + ); + + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + // For non-Zed providers, there should be no move completion (jump functionality disabled) + cx.editor(|editor, _, _| { + if let Some(completion_state) = &editor.active_edit_prediction { + // Should be an Edit prediction, not a Move prediction + match &completion_state.completion { + EditPrediction::Edit { .. } => { + // This is expected for non-Zed providers + } + EditPrediction::Move { .. } => { + panic!( + "Non-Zed providers should not show Move predictions (jump functionality)" + ); + } + } + } + }); +} + fn assert_editor_active_edit_completion( cx: &mut EditorTestContext, assert: impl FnOnce(MultiBufferSnapshot, &Vec<(Range, String)>), @@ -301,6 +344,37 @@ fn assign_editor_completion_provider( }) } +fn propose_edits_non_zed( + provider: &Entity, + edits: Vec<(Range, &str)>, + cx: &mut EditorTestContext, +) { + let snapshot = cx.buffer_snapshot(); + let edits = edits.into_iter().map(|(range, text)| { + let range = snapshot.anchor_after(range.start)..snapshot.anchor_before(range.end); + (range, text.into()) + }); + + cx.update(|_, cx| { + provider.update(cx, |provider, _| { + provider.set_edit_prediction(Some(edit_prediction::EditPrediction { + id: None, + edits: edits.collect(), + edit_preview: None, + })) + }) + }); +} + +fn assign_editor_completion_provider_non_zed( + provider: Entity, + cx: &mut EditorTestContext, +) { + cx.update_editor(|editor, window, cx| { + editor.set_edit_prediction_provider(Some(provider), window, cx); + }) +} + #[derive(Default, Clone)] pub struct FakeEditPredictionProvider { pub completion: Option, @@ -325,6 +399,84 @@ impl EditPredictionProvider for FakeEditPredictionProvider { false } + fn supports_jump_to_edit() -> bool { + true + } + + fn is_enabled( + &self, + _buffer: &gpui::Entity, + _cursor_position: language::Anchor, + _cx: &gpui::App, + ) -> bool { + true + } + + fn is_refreshing(&self) -> bool { + false + } + + fn refresh( + &mut self, + _project: Option>, + _buffer: gpui::Entity, + _cursor_position: language::Anchor, + _debounce: bool, + _cx: &mut gpui::Context, + ) { + } + + fn cycle( + &mut self, + _buffer: gpui::Entity, + _cursor_position: language::Anchor, + _direction: edit_prediction::Direction, + _cx: &mut gpui::Context, + ) { + } + + fn accept(&mut self, _cx: &mut gpui::Context) {} + + fn discard(&mut self, _cx: &mut gpui::Context) {} + + fn suggest<'a>( + &mut self, + _buffer: &gpui::Entity, + _cursor_position: language::Anchor, + _cx: &mut gpui::Context, + ) -> Option { + self.completion.clone() + } +} + +#[derive(Default, Clone)] +pub struct FakeNonZedEditPredictionProvider { + pub completion: Option, +} + +impl FakeNonZedEditPredictionProvider { + pub fn set_edit_prediction(&mut self, completion: Option) { + self.completion = completion; + } +} + +impl EditPredictionProvider for FakeNonZedEditPredictionProvider { + fn name() -> &'static str { + "fake-non-zed-provider" + } + + fn display_name() -> &'static str { + "Fake Non-Zed Provider" + } + + fn show_completions_in_menu() -> bool { + false + } + + fn supports_jump_to_edit() -> bool { + false + } + fn is_enabled( &self, _buffer: &gpui::Entity, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 73a81bea19..677acd9fd8 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7760,8 +7760,14 @@ impl Editor { } else { None }; - let is_move = - move_invalidation_row_range.is_some() || self.edit_predictions_hidden_for_vim_mode; + let supports_jump = self + .edit_prediction_provider + .as_ref() + .map(|provider| provider.provider.supports_jump_to_edit()) + .unwrap_or(true); + + let is_move = supports_jump + && (move_invalidation_row_range.is_some() || self.edit_predictions_hidden_for_vim_mode); let completion = if is_move { invalidation_row_range = move_invalidation_row_range.unwrap_or(edit_start_row..edit_end_row); @@ -8799,8 +8805,12 @@ impl Editor { return None; } - let highlighted_edits = - crate::edit_prediction_edit_text(&snapshot, edits, edit_preview.as_ref()?, false, cx); + let highlighted_edits = if let Some(edit_preview) = edit_preview.as_ref() { + crate::edit_prediction_edit_text(&snapshot, edits, edit_preview, false, cx) + } else { + // Fallback for providers without edit_preview + crate::edit_prediction_fallback_text(edits, cx) + }; let styled_text = highlighted_edits.to_styled_text(&style.text); let line_count = highlighted_edits.text.lines().count(); @@ -9068,6 +9078,18 @@ impl Editor { let editor_bg_color = cx.theme().colors().editor_background; editor_bg_color.blend(accent_color.opacity(0.6)) } + fn get_prediction_provider_icon_name( + provider: &Option, + ) -> IconName { + match provider { + Some(provider) => match provider.provider.name() { + "copilot" => IconName::Copilot, + "supermaven" => IconName::Supermaven, + _ => IconName::ZedPredict, + }, + None => IconName::ZedPredict, + } + } fn render_edit_prediction_cursor_popover( &self, @@ -9080,6 +9102,7 @@ impl Editor { cx: &mut Context, ) -> Option { let provider = self.edit_prediction_provider.as_ref()?; + let provider_icon = Self::get_prediction_provider_icon_name(&self.edit_prediction_provider); if provider.provider.needs_terms_acceptance(cx) { return Some( @@ -9106,7 +9129,7 @@ impl Editor { h_flex() .flex_1() .gap_2() - .child(Icon::new(IconName::ZedPredict)) + .child(Icon::new(provider_icon)) .child(Label::new("Accept Terms of Service")) .child(div().w_full()) .child( @@ -9122,12 +9145,8 @@ impl Editor { let is_refreshing = provider.provider.is_refreshing(cx); - fn pending_completion_container() -> Div { - h_flex() - .h_full() - .flex_1() - .gap_2() - .child(Icon::new(IconName::ZedPredict)) + fn pending_completion_container(icon: IconName) -> Div { + h_flex().h_full().flex_1().gap_2().child(Icon::new(icon)) } let completion = match &self.active_edit_prediction { @@ -9157,7 +9176,7 @@ impl Editor { Icon::new(IconName::ZedPredictUp) } } - EditPrediction::Edit { .. } => Icon::new(IconName::ZedPredict), + EditPrediction::Edit { .. } => Icon::new(provider_icon), })) .child( h_flex() @@ -9224,15 +9243,15 @@ impl Editor { cx, )?, - None => { - pending_completion_container().child(Label::new("...").size(LabelSize::Small)) - } + None => pending_completion_container(provider_icon) + .child(Label::new("...").size(LabelSize::Small)), }, - None => pending_completion_container().child(Label::new("No Prediction")), + None => pending_completion_container(provider_icon) + .child(Label::new("...").size(LabelSize::Small)), }; - let completion = if is_refreshing { + let completion = if is_refreshing || self.active_edit_prediction.is_none() { completion .with_animation( "loading-completion", @@ -9332,23 +9351,35 @@ impl Editor { .child(Icon::new(arrow).color(Color::Muted).size(IconSize::Small)) } + let supports_jump = self + .edit_prediction_provider + .as_ref() + .map(|provider| provider.provider.supports_jump_to_edit()) + .unwrap_or(true); + match &completion.completion { EditPrediction::Move { target, snapshot, .. - } => Some( - h_flex() - .px_2() - .gap_2() - .flex_1() - .child( - if target.text_anchor.to_point(&snapshot).row > cursor_point.row { - Icon::new(IconName::ZedPredictDown) - } else { - Icon::new(IconName::ZedPredictUp) - }, - ) - .child(Label::new("Jump to Edit")), - ), + } => { + if !supports_jump { + return None; + } + + Some( + h_flex() + .px_2() + .gap_2() + .flex_1() + .child( + if target.text_anchor.to_point(&snapshot).row > cursor_point.row { + Icon::new(IconName::ZedPredictDown) + } else { + Icon::new(IconName::ZedPredictUp) + }, + ) + .child(Label::new("Jump to Edit")), + ) + } EditPrediction::Edit { edits, @@ -9358,14 +9389,13 @@ impl Editor { } => { let first_edit_row = edits.first()?.0.start.text_anchor.to_point(&snapshot).row; - let (highlighted_edits, has_more_lines) = crate::edit_prediction_edit_text( - &snapshot, - &edits, - edit_preview.as_ref()?, - true, - cx, - ) - .first_line_preview(); + let (highlighted_edits, has_more_lines) = + if let Some(edit_preview) = edit_preview.as_ref() { + crate::edit_prediction_edit_text(&snapshot, &edits, edit_preview, true, cx) + .first_line_preview() + } else { + crate::edit_prediction_fallback_text(&edits, cx).first_line_preview() + }; let styled_text = gpui::StyledText::new(highlighted_edits.text) .with_default_highlights(&style.text, highlighted_edits.highlights); @@ -9376,11 +9406,13 @@ impl Editor { .child(styled_text) .when(has_more_lines, |parent| parent.child("…")); - let left = if first_edit_row != cursor_point.row { + let left = if supports_jump && first_edit_row != cursor_point.row { render_relative_row_jump("", cursor_point.row, first_edit_row) .into_any_element() } else { - Icon::new(IconName::ZedPredict).into_any_element() + let icon_name = + Editor::get_prediction_provider_icon_name(&self.edit_prediction_provider); + Icon::new(icon_name).into_any_element() }; Some( @@ -23270,6 +23302,33 @@ fn edit_prediction_edit_text( edit_preview.highlight_edits(current_snapshot, &edits, include_deletions, cx) } +fn edit_prediction_fallback_text(edits: &[(Range, String)], cx: &App) -> HighlightedText { + // Fallback for providers that don't provide edit_preview (like Copilot/Supermaven) + // Just show the raw edit text with basic styling + let mut text = String::new(); + let mut highlights = Vec::new(); + + let insertion_highlight_style = HighlightStyle { + color: Some(cx.theme().colors().text), + ..Default::default() + }; + + for (_, edit_text) in edits { + let start_offset = text.len(); + text.push_str(edit_text); + let end_offset = text.len(); + + if start_offset < end_offset { + highlights.push((start_offset..end_offset, insertion_highlight_style)); + } + } + + HighlightedText { + text: text.into(), + highlights, + } +} + pub fn diagnostic_style(severity: lsp::DiagnosticSeverity, colors: &StatusColors) -> Hsla { match severity { lsp::DiagnosticSeverity::ERROR => colors.error, diff --git a/crates/supermaven/src/supermaven.rs b/crates/supermaven/src/supermaven.rs index ab500fb79d..a31b96d882 100644 --- a/crates/supermaven/src/supermaven.rs +++ b/crates/supermaven/src/supermaven.rs @@ -234,16 +234,14 @@ fn find_relevant_completion<'a>( } let original_cursor_offset = buffer.clip_offset(state.prefix_offset, text::Bias::Left); - let text_inserted_since_completion_request = - buffer.text_for_range(original_cursor_offset..current_cursor_offset); - let mut trimmed_completion = state_completion; - for chunk in text_inserted_since_completion_request { - if let Some(suffix) = trimmed_completion.strip_prefix(chunk) { - trimmed_completion = suffix; - } else { - continue 'completions; - } - } + let text_inserted_since_completion_request: String = buffer + .text_for_range(original_cursor_offset..current_cursor_offset) + .collect(); + let trimmed_completion = + match state_completion.strip_prefix(&text_inserted_since_completion_request) { + Some(suffix) => suffix, + None => continue 'completions, + }; if best_completion.map_or(false, |best| best.len() > trimmed_completion.len()) { continue; @@ -439,3 +437,77 @@ pub struct SupermavenCompletion { pub id: SupermavenCompletionStateId, pub updates: watch::Receiver<()>, } + +#[cfg(test)] +mod tests { + use super::*; + use collections::BTreeMap; + use gpui::TestAppContext; + use language::Buffer; + + #[gpui::test] + async fn test_find_relevant_completion_no_first_letter_skip(cx: &mut TestAppContext) { + let buffer = cx.new(|cx| Buffer::local("hello world", cx)); + let buffer_snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot()); + + let mut states = BTreeMap::new(); + let state_id = SupermavenCompletionStateId(1); + let (updates_tx, _) = watch::channel(); + + states.insert( + state_id, + SupermavenCompletionState { + buffer_id: buffer.entity_id(), + prefix_anchor: buffer_snapshot.anchor_before(0), // Start of buffer + prefix_offset: 0, + text: "hello".to_string(), + dedent: String::new(), + updates_tx, + }, + ); + + let cursor_position = buffer_snapshot.anchor_after(1); + + let result = find_relevant_completion( + &states, + buffer.entity_id(), + &buffer_snapshot, + cursor_position, + ); + + assert_eq!(result, Some("ello")); + } + + #[gpui::test] + async fn test_find_relevant_completion_with_multiple_chars(cx: &mut TestAppContext) { + let buffer = cx.new(|cx| Buffer::local("hello world", cx)); + let buffer_snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot()); + + let mut states = BTreeMap::new(); + let state_id = SupermavenCompletionStateId(1); + let (updates_tx, _) = watch::channel(); + + states.insert( + state_id, + SupermavenCompletionState { + buffer_id: buffer.entity_id(), + prefix_anchor: buffer_snapshot.anchor_before(0), // Start of buffer + prefix_offset: 0, + text: "hello".to_string(), + dedent: String::new(), + updates_tx, + }, + ); + + let cursor_position = buffer_snapshot.anchor_after(3); + + let result = find_relevant_completion( + &states, + buffer.entity_id(), + &buffer_snapshot, + cursor_position, + ); + + assert_eq!(result, Some("lo")); + } +} diff --git a/crates/supermaven/src/supermaven_completion_provider.rs b/crates/supermaven/src/supermaven_completion_provider.rs index 2660a03e6f..1b1fc54a7a 100644 --- a/crates/supermaven/src/supermaven_completion_provider.rs +++ b/crates/supermaven/src/supermaven_completion_provider.rs @@ -108,6 +108,14 @@ impl EditPredictionProvider for SupermavenCompletionProvider { } fn show_completions_in_menu() -> bool { + true + } + + fn show_tab_accept_marker() -> bool { + true + } + + fn supports_jump_to_edit() -> bool { false } @@ -116,7 +124,7 @@ impl EditPredictionProvider for SupermavenCompletionProvider { } fn is_refreshing(&self) -> bool { - self.pending_refresh.is_some() + self.pending_refresh.is_some() && self.completion_id.is_none() } fn refresh( @@ -197,6 +205,7 @@ impl EditPredictionProvider for SupermavenCompletionProvider { let mut point = cursor_position.to_point(&snapshot); point.column = snapshot.line_len(point.row); let range = cursor_position..snapshot.anchor_after(point); + Some(completion_from_diff( snapshot, completion_text,