From 433d9299a3fad19eeaa023d6a9d33f551797f56e Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Wed, 18 Mar 2026 11:47:46 -0500 Subject: [PATCH] Remove edit prediction conflict state (#51842) Closes #ISSUE Release Notes: - N/A *or* Added/Fixed/Improved ... --------- Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com> (cherry picked from commit 5f12c92924fd4df007eb72fd4d788ed73fbb2409) --- assets/keymaps/default-linux.json | 9 - assets/keymaps/default-macos.json | 9 - assets/keymaps/default-windows.json | 10 - assets/keymaps/vim.json | 10 +- crates/editor/src/edit_prediction_tests.rs | 462 ++++++++++++++++++++- crates/editor/src/editor.rs | 435 +++++++++++++------ crates/editor/src/element.rs | 36 +- 7 files changed, 768 insertions(+), 203 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 4e566c1c6f5e62b022d1312d89fe3276b3207963..24299f6a4c0dba33f8c1417ea2e688b33c9b9152 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -775,15 +775,6 @@ "alt-j": "editor::AcceptNextLineEditPrediction", }, }, - { - "context": "Editor && edit_prediction_conflict", - "bindings": { - "alt-tab": "editor::AcceptEditPrediction", - "alt-l": "editor::AcceptEditPrediction", - "alt-k": "editor::AcceptNextWordEditPrediction", - "alt-j": "editor::AcceptNextLineEditPrediction", - }, - }, { "context": "Editor && showing_code_actions", "bindings": { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index d3999918f338e1c36501f7388c37028451d54c67..b58b35d2b50ce9fa12695ac8fa20968072eb2c01 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -839,15 +839,6 @@ "ctrl-cmd-down": "editor::AcceptNextLineEditPrediction", }, }, - { - "context": "Editor && edit_prediction_conflict", - "use_key_equivalents": true, - "bindings": { - "alt-tab": "editor::AcceptEditPrediction", - "ctrl-cmd-right": "editor::AcceptNextWordEditPrediction", - "ctrl-cmd-down": "editor::AcceptNextLineEditPrediction", - }, - }, { "context": "Editor && showing_code_actions", "use_key_equivalents": true, diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index b1249b6710ff459762c8cf2b8bb53aff4c876aca..426076a497055dbc350231d2889ae53006076582 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -771,16 +771,6 @@ "alt-j": "editor::AcceptNextLineEditPrediction", }, }, - { - "context": "Editor && edit_prediction_conflict", - "use_key_equivalents": true, - "bindings": { - "alt-tab": "editor::AcceptEditPrediction", - "alt-l": "editor::AcceptEditPrediction", - "alt-k": "editor::AcceptNextWordEditPrediction", - "alt-j": "editor::AcceptNextLineEditPrediction", - }, - }, { "context": "Editor && showing_code_actions", "use_key_equivalents": true, diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index dc69909a60102ba96f819107bde2c7653b0db1c7..11b18040bb3f1e25eb8d5c148d023d181a1dbf6b 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -1073,15 +1073,7 @@ "enter": "agent::Chat", }, }, - { - "context": "os != macos && Editor && edit_prediction_conflict", - "bindings": { - // alt-l is provided as an alternative to tab/alt-tab. and will be displayed in the UI. This - // is because alt-tab may not be available, as it is often used for window switching on Linux - // and Windows. - "alt-l": "editor::AcceptEditPrediction", - }, - }, + { "context": "SettingsWindow > NavigationMenu && !search", "bindings": { diff --git a/crates/editor/src/edit_prediction_tests.rs b/crates/editor/src/edit_prediction_tests.rs index c82915c686e977178398430948f28f8178f216df..40d915b841c1cf4a53025f7dce654dd129cb8de5 100644 --- a/crates/editor/src/edit_prediction_tests.rs +++ b/crates/editor/src/edit_prediction_tests.rs @@ -3,6 +3,8 @@ use edit_prediction_types::{ }; use gpui::{Entity, KeyBinding, Modifiers, prelude::*}; use indoc::indoc; +use language::Buffer; +use language::EditPredictionsMode; use multi_buffer::{Anchor, MultiBufferSnapshot, ToPoint}; use std::{ ops::Range, @@ -15,7 +17,9 @@ use text::{Point, ToOffset}; use ui::prelude::*; use crate::{ - AcceptEditPrediction, EditPrediction, MenuEditPredictionsPolicy, editor_tests::init_test, + AcceptEditPrediction, EditPrediction, EditPredictionKeybindAction, + EditPredictionKeybindSurface, MenuEditPredictionsPolicy, + editor_tests::{init_test, update_test_language_settings}, test::editor_test_context::EditorTestContext, }; use rpc::proto::PeerId; @@ -478,6 +482,424 @@ async fn test_edit_prediction_preview_cleanup_on_toggle_off(cx: &mut gpui::TestA }); } +fn load_default_keymap(cx: &mut gpui::TestAppContext) { + cx.update(|cx| { + cx.bind_keys( + settings::KeymapFile::load_asset_allow_partial_failure( + settings::DEFAULT_KEYMAP_PATH, + cx, + ) + .expect("failed to load default keymap"), + ); + }); +} + +#[gpui::test] +async fn test_tab_is_preferred_accept_binding_over_alt_tab(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits(&provider, vec![(8..8, "42")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::Inline, + window, + cx, + ); + let keystroke = keybind_display + .accept_keystroke + .as_ref() + .expect("should have an accept binding"); + assert!( + !keystroke.modifiers().modified(), + "preferred accept binding should be unmodified (tab), got modifiers: {:?}", + keystroke.modifiers() + ); + assert_eq!( + keystroke.key(), + "tab", + "preferred accept binding should be tab" + ); + }); +} + +#[gpui::test] +async fn test_subtle_in_code_indicator_prefers_preview_binding(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + load_default_keymap(cx); + update_test_language_settings(cx, &|settings| { + settings.edit_predictions.get_or_insert_default().mode = Some(EditPredictionsMode::Subtle); + }); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits(&provider, vec![(8..8, "42")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + assert!( + editor.edit_prediction_requires_modifier(), + "subtle mode should require a modifier" + ); + + let inline_keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::Inline, + window, + cx, + ); + let compact_keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverCompact, + window, + cx, + ); + + let accept_keystroke = inline_keybind_display + .accept_keystroke + .as_ref() + .expect("should have an accept binding"); + let preview_keystroke = inline_keybind_display + .preview_keystroke + .as_ref() + .expect("should have a preview binding"); + let in_code_keystroke = inline_keybind_display + .displayed_keystroke + .as_ref() + .expect("should have an in-code binding"); + let compact_cursor_popover_keystroke = compact_keybind_display + .displayed_keystroke + .as_ref() + .expect("should have a compact cursor popover binding"); + + assert_eq!(accept_keystroke.key(), "tab"); + assert!( + !editor.has_visible_completions_menu(), + "compact cursor-popover branch should be used without a completions menu" + ); + assert!( + preview_keystroke.modifiers().modified(), + "preview binding should use modifiers in subtle mode" + ); + assert_eq!( + compact_cursor_popover_keystroke.key(), + preview_keystroke.key(), + "subtle compact cursor popover should prefer the preview binding" + ); + assert_eq!( + compact_cursor_popover_keystroke.modifiers(), + preview_keystroke.modifiers(), + "subtle compact cursor popover should use the preview binding modifiers" + ); + assert_eq!( + in_code_keystroke.key(), + preview_keystroke.key(), + "subtle in-code indicator should prefer the preview binding" + ); + assert_eq!( + in_code_keystroke.modifiers(), + preview_keystroke.modifiers(), + "subtle in-code indicator should use the preview binding modifiers" + ); + }); +} + +#[gpui::test] +async fn test_tab_accepts_edit_prediction_over_completion(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits(&provider, vec![(8..8, "42")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + assert_editor_active_edit_completion(&mut cx, |_, edits| { + assert_eq!(edits.len(), 1); + assert_eq!(edits[0].1.as_ref(), "42"); + }); + + cx.simulate_keystroke("tab"); + cx.run_until_parked(); + + cx.assert_editor_state("let x = 42ˇ;"); +} + +#[gpui::test] +async fn test_single_line_prediction_uses_accept_cursor_popover_action( + cx: &mut gpui::TestAppContext, +) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits(&provider, vec![(8..8, "42")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); + + let accept_keystroke = keybind_display + .accept_keystroke + .as_ref() + .expect("should have an accept binding"); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .expect("should have a preview binding"); + + assert_eq!( + keybind_display.action, + EditPredictionKeybindAction::Accept, + "single-line prediction should show the accept action" + ); + assert_eq!(accept_keystroke.key(), "tab"); + assert!(preview_keystroke.modifiers().modified()); + }); +} + +#[gpui::test] +async fn test_multi_line_prediction_uses_preview_cursor_popover_action( + cx: &mut gpui::TestAppContext, +) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits(&provider, vec![(8..8, "42\n43")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .expect("should have a preview binding"); + + assert_eq!( + keybind_display.action, + EditPredictionKeybindAction::Preview, + "multi-line prediction should show the preview action" + ); + assert!(preview_keystroke.modifiers().modified()); + }); +} + +#[gpui::test] +async fn test_single_line_prediction_with_preview_uses_accept_cursor_popover_action( + cx: &mut gpui::TestAppContext, +) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits_with_preview(&provider, vec![(8..8, "42")], &mut cx).await; + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); + + let accept_keystroke = keybind_display + .accept_keystroke + .as_ref() + .expect("should have an accept binding"); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .expect("should have a preview binding"); + + assert_eq!( + keybind_display.action, + EditPredictionKeybindAction::Accept, + "single-line prediction should show the accept action even with edit_preview" + ); + assert_eq!(accept_keystroke.key(), "tab"); + assert!(preview_keystroke.modifiers().modified()); + }); +} + +#[gpui::test] +async fn test_multi_line_prediction_with_preview_uses_preview_cursor_popover_action( + cx: &mut gpui::TestAppContext, +) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits_with_preview(&provider, vec![(8..8, "42\n43")], &mut cx).await; + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .expect("should have a preview binding"); + + assert_eq!( + keybind_display.action, + EditPredictionKeybindAction::Preview, + "multi-line prediction should show the preview action with edit_preview" + ); + assert!(preview_keystroke.modifiers().modified()); + }); +} + +#[gpui::test] +async fn test_single_line_deletion_of_newline_uses_accept_cursor_popover_action( + cx: &mut gpui::TestAppContext, +) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state(indoc! {" + fn main() { + let value = 1; + ˇprintln!(\"done\"); + } + "}); + + propose_edits( + &provider, + vec![(Point::new(1, 18)..Point::new(2, 17), "")], + &mut cx, + ); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); + + let accept_keystroke = keybind_display + .accept_keystroke + .as_ref() + .expect("should have an accept binding"); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .expect("should have a preview binding"); + + assert_eq!( + keybind_display.action, + EditPredictionKeybindAction::Accept, + "deleting one newline plus adjacent text should show the accept action" + ); + assert_eq!(accept_keystroke.key(), "tab"); + assert!(preview_keystroke.modifiers().modified()); + }); +} + +#[gpui::test] +async fn test_stale_single_line_prediction_does_not_force_preview_cursor_popover_action( + cx: &mut gpui::TestAppContext, +) { + init_test(cx, |_| {}); + load_default_keymap(cx); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.set_state("let x = ˇ;"); + + propose_edits(&provider, vec![(8..8, "42\n43")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + cx.update_editor(|editor, _window, cx| { + assert!(editor.active_edit_prediction.is_some()); + assert!(editor.stale_edit_prediction_in_menu.is_none()); + editor.take_active_edit_prediction(cx); + assert!(editor.active_edit_prediction.is_none()); + assert!(editor.stale_edit_prediction_in_menu.is_some()); + }); + + propose_edits(&provider, vec![(8..8, "42")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + cx.update_editor(|editor, window, cx| { + assert!(editor.has_active_edit_prediction()); + + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); + let accept_keystroke = keybind_display + .accept_keystroke + .as_ref() + .expect("should have an accept binding"); + + assert_eq!( + keybind_display.action, + EditPredictionKeybindAction::Accept, + "single-line active prediction should show the accept action" + ); + assert!( + editor.stale_edit_prediction_in_menu.is_none(), + "refreshing the visible prediction should clear stale menu state" + ); + assert_eq!(accept_keystroke.key(), "tab"); + }); +} + fn assert_editor_active_edit_completion( cx: &mut EditorTestContext, assert: impl FnOnce(MultiBufferSnapshot, &Vec<(Range, Arc)>), @@ -528,6 +950,44 @@ fn propose_edits( propose_edits_with_cursor_position(provider, edits, None, cx); } +async fn propose_edits_with_preview( + provider: &Entity, + edits: Vec<(Range, &str)>, + cx: &mut EditorTestContext, +) { + let snapshot = cx.buffer_snapshot(); + let edits = edits + .into_iter() + .map(|(range, text)| { + let anchor_range = + snapshot.anchor_after(range.start.clone())..snapshot.anchor_before(range.end); + (anchor_range, Arc::::from(text)) + }) + .collect::>(); + + let preview_edits = edits + .iter() + .map(|(range, text)| (range.clone(), text.clone())) + .collect::>(); + + let edit_preview = cx + .buffer(|buffer: &Buffer, app| buffer.preview_edits(preview_edits, app)) + .await; + + let provider_edits = edits.into_iter().collect(); + + cx.update(|_, cx| { + provider.update(cx, |provider, _| { + provider.set_edit_prediction(Some(edit_prediction_types::EditPrediction::Local { + id: None, + edits: provider_edits, + cursor_position: None, + edit_preview: Some(edit_preview), + })) + }) + }); +} + fn propose_edits_with_cursor_position( provider: &Entity, edits: Vec<(Range, &str)>, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 11b754701ce15586a1f46114e7044fd24158befd..69081c07fd97ff49204893cdca6d4fbc081f2750 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -105,7 +105,7 @@ use edit_prediction_types::{ EditPredictionGranularity, SuggestionDisplayType, }; use editor_settings::{GoToDefinitionFallback, Minimap as MinimapSettings}; -use element::{AcceptEditPredictionBinding, LineWithInvisibles, PositionMap, layout_line}; +use element::{LineWithInvisibles, PositionMap, layout_line}; use futures::{ FutureExt, future::{self, Shared, join}, @@ -256,7 +256,6 @@ pub(crate) const SCROLL_CENTER_TOP_BOTTOM_DEBOUNCE_TIMEOUT: Duration = Duration: pub const LSP_REQUEST_DEBOUNCE_TIMEOUT: Duration = Duration::from_millis(50); pub(crate) const EDIT_PREDICTION_KEY_CONTEXT: &str = "edit_prediction"; -pub(crate) const EDIT_PREDICTION_CONFLICT_KEY_CONTEXT: &str = "edit_prediction_conflict"; pub(crate) const MINIMAP_FONT_SIZE: AbsoluteLength = AbsoluteLength::Pixels(px(2.)); pub type RenderDiffHunkControlsFn = Arc< @@ -701,6 +700,30 @@ pub enum EditPredictionPreview { }, } +#[derive(Copy, Clone, Eq, PartialEq)] +enum EditPredictionKeybindSurface { + Inline, + CursorPopoverCompact, + CursorPopoverExpanded, +} + +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +enum EditPredictionKeybindAction { + Accept, + Preview, +} + +struct EditPredictionKeybindDisplay { + #[cfg(test)] + accept_keystroke: Option, + #[cfg(test)] + preview_keystroke: Option, + displayed_keystroke: Option, + action: EditPredictionKeybindAction, + missing_accept_keystroke: bool, + show_hold_label: bool, +} + impl EditPredictionPreview { pub fn released_too_fast(&self) -> bool { match self { @@ -1225,8 +1248,7 @@ pub struct Editor { show_completions_on_input_override: Option, menu_edit_predictions_policy: MenuEditPredictionsPolicy, edit_prediction_preview: EditPredictionPreview, - edit_prediction_indent_conflict: bool, - edit_prediction_requires_modifier_in_indent_conflict: bool, + in_leading_whitespace: bool, next_inlay_id: usize, next_color_inlay_id: usize, _subscriptions: Vec, @@ -2473,8 +2495,7 @@ impl Editor { show_completions_on_input_override: None, menu_edit_predictions_policy: MenuEditPredictionsPolicy::ByProvider, edit_prediction_settings: EditPredictionSettings::Disabled, - edit_prediction_indent_conflict: false, - edit_prediction_requires_modifier_in_indent_conflict: true, + in_leading_whitespace: false, custom_context_menu: None, show_git_blame_gutter: false, show_git_blame_inline: false, @@ -2856,12 +2877,12 @@ impl Editor { } if has_active_edit_prediction { - if self.edit_prediction_in_conflict() { - key_context.add(EDIT_PREDICTION_CONFLICT_KEY_CONTEXT); - } else { - key_context.add(EDIT_PREDICTION_KEY_CONTEXT); - key_context.add("copilot_suggestion"); - } + key_context.add(EDIT_PREDICTION_KEY_CONTEXT); + key_context.add("copilot_suggestion"); + } + + if self.in_leading_whitespace { + key_context.add("in_leading_whitespace"); } if self.selection_mark_mode { @@ -2915,32 +2936,13 @@ impl Editor { } } - pub fn edit_prediction_in_conflict(&self) -> bool { - if !self.show_edit_predictions_in_menu() { - return false; - } - - let showing_completions = self - .context_menu - .borrow() - .as_ref() - .is_some_and(|context| matches!(context, CodeContextMenu::Completions(_))); - - showing_completions - || self.edit_prediction_requires_modifier() - // Require modifier key when the cursor is on leading whitespace, to allow `tab` - // bindings to insert tab characters. - || (self.edit_prediction_requires_modifier_in_indent_conflict && self.edit_prediction_indent_conflict) - } - - pub fn accept_edit_prediction_keybind( + fn accept_edit_prediction_keystroke( &self, granularity: EditPredictionGranularity, window: &mut Window, cx: &mut App, - ) -> AcceptEditPredictionBinding { - let key_context = self.key_context_internal(true, window, cx); - let in_conflict = self.edit_prediction_in_conflict(); + ) -> Option { + let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx); let bindings = match granularity { @@ -2953,13 +2955,131 @@ impl Editor { } }; - AcceptEditPredictionBinding(bindings.into_iter().rev().find(|binding| { - !in_conflict - || binding - .keystrokes() - .first() - .is_some_and(|keystroke| keystroke.modifiers().modified()) - })) + bindings + .into_iter() + .rev() + .find_map(|binding| match binding.keystrokes() { + [keystroke, ..] => Some(keystroke.clone()), + _ => None, + }) + } + + fn preview_edit_prediction_keystroke( + &self, + window: &mut Window, + cx: &mut App, + ) -> Option { + let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx); + let bindings = window.bindings_for_action_in_context(&AcceptEditPrediction, key_context); + bindings + .into_iter() + .rev() + .find_map(|binding| match binding.keystrokes() { + [keystroke, ..] if keystroke.modifiers().modified() => Some(keystroke.clone()), + _ => None, + }) + } + + fn edit_prediction_cursor_popover_prefers_preview( + &self, + completion: &EditPredictionState, + ) -> bool { + match &completion.completion { + EditPrediction::Edit { + edits, snapshot, .. + } => { + let mut start_row: Option = None; + let mut end_row: Option = None; + + for (range, text) in edits { + let edit_start_row = range.start.text_anchor.to_point(snapshot).row; + let old_end_row = range.end.text_anchor.to_point(snapshot).row; + let inserted_newline_count = text + .as_ref() + .chars() + .filter(|character| *character == '\n') + .count() as u32; + let deleted_newline_count = old_end_row - edit_start_row; + let preview_end_row = edit_start_row + inserted_newline_count; + + start_row = + Some(start_row.map_or(edit_start_row, |row| row.min(edit_start_row))); + end_row = Some(end_row.map_or(preview_end_row, |row| row.max(preview_end_row))); + + if deleted_newline_count > 1 { + end_row = Some(end_row.map_or(old_end_row, |row| row.max(old_end_row))); + } + } + + start_row + .zip(end_row) + .is_some_and(|(start_row, end_row)| end_row > start_row) + } + EditPrediction::MoveWithin { .. } | EditPrediction::MoveOutside { .. } => false, + } + } + + fn edit_prediction_keybind_display( + &self, + surface: EditPredictionKeybindSurface, + window: &mut Window, + cx: &mut App, + ) -> EditPredictionKeybindDisplay { + let accept_keystroke = + self.accept_edit_prediction_keystroke(EditPredictionGranularity::Full, window, cx); + let preview_keystroke = self.preview_edit_prediction_keystroke(window, cx); + + let action = match surface { + EditPredictionKeybindSurface::Inline + | EditPredictionKeybindSurface::CursorPopoverCompact => { + if self.edit_prediction_requires_modifier() { + EditPredictionKeybindAction::Preview + } else { + EditPredictionKeybindAction::Accept + } + } + EditPredictionKeybindSurface::CursorPopoverExpanded => self + .active_edit_prediction + .as_ref() + .filter(|completion| { + self.edit_prediction_cursor_popover_prefers_preview(completion) + }) + .map_or(EditPredictionKeybindAction::Accept, |_| { + EditPredictionKeybindAction::Preview + }), + }; + #[cfg(test)] + let preview_copy = preview_keystroke.clone(); + #[cfg(test)] + let accept_copy = accept_keystroke.clone(); + + let displayed_keystroke = match surface { + EditPredictionKeybindSurface::Inline => match action { + EditPredictionKeybindAction::Accept => accept_keystroke, + EditPredictionKeybindAction::Preview => preview_keystroke, + }, + EditPredictionKeybindSurface::CursorPopoverCompact + | EditPredictionKeybindSurface::CursorPopoverExpanded => match action { + EditPredictionKeybindAction::Accept => accept_keystroke, + EditPredictionKeybindAction::Preview => { + preview_keystroke.or_else(|| accept_keystroke.clone()) + } + }, + }; + + let missing_accept_keystroke = displayed_keystroke.is_none(); + + EditPredictionKeybindDisplay { + #[cfg(test)] + accept_keystroke: accept_copy, + #[cfg(test)] + preview_keystroke: preview_copy, + displayed_keystroke, + action, + missing_accept_keystroke, + show_hold_label: matches!(surface, EditPredictionKeybindSurface::CursorPopoverCompact) + && self.edit_prediction_preview.released_too_fast(), + } } pub fn new_file( @@ -3596,7 +3716,6 @@ impl Editor { self.refresh_matching_bracket_highlights(&display_map, cx); self.refresh_outline_symbols_at_cursor(cx); self.update_visible_edit_prediction(window, cx); - self.edit_prediction_requires_modifier_in_indent_conflict = true; self.inline_blame_popover.take(); if self.git_blame_inline_enabled { self.start_inline_blame_timer(window, cx); @@ -8216,8 +8335,6 @@ impl Editor { } } } - - self.edit_prediction_requires_modifier_in_indent_conflict = false; } pub fn accept_next_word_edit_prediction( @@ -8466,21 +8583,20 @@ impl Editor { ) { let mut modifiers_held = false; - // Check bindings for all granularities. - // If the user holds the key for Word, Line, or Full, we want to show the preview. - let granularities = [ - EditPredictionGranularity::Full, - EditPredictionGranularity::Line, - EditPredictionGranularity::Word, + let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx); + let actions: [&dyn Action; 3] = [ + &AcceptEditPrediction, + &AcceptNextWordEditPrediction, + &AcceptNextLineEditPrediction, ]; - for granularity in granularities { - if let Some(keystroke) = self - .accept_edit_prediction_keybind(granularity, window, cx) - .keystroke() - { - modifiers_held = modifiers_held - || (keystroke.modifiers() == modifiers && keystroke.modifiers().modified()); + for action in actions { + let bindings = window.bindings_for_action_in_context(action, key_context.clone()); + for binding in bindings { + if let Some(keystroke) = binding.keystrokes().first() { + modifiers_held = modifiers_held + || (keystroke.modifiers() == modifiers && keystroke.modifiers().modified()); + } } } @@ -8580,9 +8696,9 @@ impl Editor { self.edit_prediction_settings = self.edit_prediction_settings_at_position(&buffer, cursor_buffer_position, cx); - self.edit_prediction_indent_conflict = multibuffer.is_line_whitespace_upto(cursor); + self.in_leading_whitespace = multibuffer.is_line_whitespace_upto(cursor); - if self.edit_prediction_indent_conflict { + if self.in_leading_whitespace { let cursor_point = cursor.to_point(&multibuffer); let mut suggested_indent = None; multibuffer.suggested_indents_callback( @@ -8597,7 +8713,7 @@ impl Editor { if let Some(indent) = suggested_indent && indent.len == cursor_point.column { - self.edit_prediction_indent_conflict = false; + self.in_leading_whitespace = false; } } @@ -9610,7 +9726,7 @@ impl Editor { const BORDER_WIDTH: Pixels = px(1.); - let keybind = self.render_edit_prediction_accept_keybind(window, cx); + let keybind = self.render_edit_prediction_keybind(window, cx); let has_keybind = keybind.is_some(); let mut element = h_flex() @@ -9766,49 +9882,81 @@ impl Editor { } } - fn render_edit_prediction_accept_keybind( + fn render_edit_prediction_inline_keystroke( &self, - window: &mut Window, - cx: &mut App, - ) -> Option { - let accept_binding = - self.accept_edit_prediction_keybind(EditPredictionGranularity::Full, window, cx); - let accept_keystroke = accept_binding.keystroke()?; - + keystroke: &gpui::KeybindingKeystroke, + modifiers_color: Color, + cx: &App, + ) -> AnyElement { let is_platform_style_mac = PlatformStyle::platform() == PlatformStyle::Mac; - let modifiers_color = if *accept_keystroke.modifiers() == window.modifiers() { - Color::Accent - } else { - Color::Muted - }; - h_flex() .px_0p5() .when(is_platform_style_mac, |parent| parent.gap_0p5()) .font(theme::ThemeSettings::get_global(cx).buffer_font.clone()) .text_size(TextSize::XSmall.rems(cx)) .child(h_flex().children(ui::render_modifiers( - accept_keystroke.modifiers(), + keystroke.modifiers(), PlatformStyle::platform(), Some(modifiers_color), Some(IconSize::XSmall.rems().into()), true, ))) .when(is_platform_style_mac, |parent| { - parent.child(accept_keystroke.key().to_string()) + parent.child(keystroke.key().to_string()) }) .when(!is_platform_style_mac, |parent| { parent.child( - Key::new( - util::capitalize(accept_keystroke.key()), - Some(Color::Default), - ) - .size(Some(IconSize::XSmall.rems().into())), + Key::new(util::capitalize(keystroke.key()), Some(Color::Default)) + .size(Some(IconSize::XSmall.rems().into())), ) }) .into_any() - .into() + } + + fn render_edit_prediction_popover_keystroke( + &self, + keystroke: &gpui::KeybindingKeystroke, + color: Color, + cx: &App, + ) -> AnyElement { + let is_platform_style_mac = PlatformStyle::platform() == PlatformStyle::Mac; + + if keystroke.modifiers().modified() { + h_flex() + .font(theme::ThemeSettings::get_global(cx).buffer_font.clone()) + .when(is_platform_style_mac, |parent| parent.gap_1()) + .child(h_flex().children(ui::render_modifiers( + keystroke.modifiers(), + PlatformStyle::platform(), + Some(color), + None, + false, + ))) + .into_any() + } else { + Key::new(util::capitalize(keystroke.key()), Some(color)) + .size(Some(IconSize::XSmall.rems().into())) + .into_any_element() + } + } + + fn render_edit_prediction_keybind( + &self, + window: &mut Window, + cx: &mut App, + ) -> Option { + let keybind_display = + self.edit_prediction_keybind_display(EditPredictionKeybindSurface::Inline, window, cx); + let keystroke = keybind_display.displayed_keystroke.as_ref()?; + + let modifiers_color = if *keystroke.modifiers() == window.modifiers() { + Color::Accent + } else { + Color::Muted + }; + + Some(self.render_edit_prediction_inline_keystroke(keystroke, modifiers_color, cx)) } fn render_edit_prediction_line_popover( @@ -9820,7 +9968,7 @@ impl Editor { ) -> Stateful
{ let padding_right = if icon.is_some() { px(4.) } else { px(8.) }; - let keybind = self.render_edit_prediction_accept_keybind(window, cx); + let keybind = self.render_edit_prediction_keybind(window, cx); let has_keybind = keybind.is_some(); let icons = Self::get_prediction_provider_icons(&self.edit_prediction_provider, cx); @@ -9879,7 +10027,7 @@ impl Editor { window: &mut Window, cx: &mut App, ) -> Stateful
{ - let keybind = self.render_edit_prediction_accept_keybind(window, cx); + let keybind = self.render_edit_prediction_keybind(window, cx); let has_keybind = keybind.is_some(); let icons = Self::get_prediction_provider_icons(&self.edit_prediction_provider, cx); @@ -9962,8 +10110,7 @@ impl Editor { max_width: Pixels, cursor_point: Point, style: &EditorStyle, - accept_keystroke: Option<&gpui::KeybindingKeystroke>, - _window: &Window, + window: &mut Window, cx: &mut Context, ) -> Option { let provider = self.edit_prediction_provider.as_ref()?; @@ -9980,13 +10127,18 @@ impl Editor { if !self.has_visible_completions_menu() { const RADIUS: Pixels = px(6.); const BORDER_WIDTH: Pixels = px(1.); + let keybind_display = self.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverCompact, + window, + cx, + ); return Some( h_flex() .elevation_2(cx) .border(BORDER_WIDTH) .border_color(cx.theme().colors().border) - .when(accept_keystroke.is_none(), |el| { + .when(keybind_display.missing_accept_keystroke, |el| { el.border_color(cx.theme().status().error) }) .rounded(RADIUS) @@ -10017,18 +10169,19 @@ impl Editor { .border_l_1() .border_color(cx.theme().colors().border) .bg(Self::edit_prediction_line_popover_bg_color(cx)) - .when(self.edit_prediction_preview.released_too_fast(), |el| { + .when(keybind_display.show_hold_label, |el| { el.child( Label::new("Hold") .size(LabelSize::Small) - .when(accept_keystroke.is_none(), |el| { - el.strikethrough() - }) + .when( + keybind_display.missing_accept_keystroke, + |el| el.strikethrough(), + ) .line_height_style(LineHeightStyle::UiLabel), ) }) .id("edit_prediction_cursor_popover_keybind") - .when(accept_keystroke.is_none(), |el| { + .when(keybind_display.missing_accept_keystroke, |el| { let status_colors = cx.theme().status(); el.bg(status_colors.error_background) @@ -10041,15 +10194,13 @@ impl Editor { }) }) .when_some( - accept_keystroke.as_ref(), - |el, accept_keystroke| { - el.child(h_flex().children(ui::render_modifiers( - accept_keystroke.modifiers(), - PlatformStyle::platform(), - Some(Color::Default), - Some(IconSize::XSmall.rems().into()), - false, - ))) + keybind_display.displayed_keystroke.as_ref(), + |el, compact_keystroke| { + el.child(self.render_edit_prediction_popover_keystroke( + compact_keystroke, + Color::Default, + cx, + )) }, ), ) @@ -10096,8 +10247,12 @@ impl Editor { }; let has_completion = self.active_edit_prediction.is_some(); + let keybind_display = self.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); - let is_platform_style_mac = PlatformStyle::platform() == PlatformStyle::Mac; Some( h_flex() .min_w(min_width) @@ -10113,41 +10268,51 @@ impl Editor { .overflow_hidden() .child(completion), ) - .when_some(accept_keystroke, |el, accept_keystroke| { - if !accept_keystroke.modifiers().modified() { - return el; - } + .when_some( + keybind_display.displayed_keystroke.as_ref(), + |el, keystroke| { + let key_color = if !has_completion { + Color::Muted + } else { + Color::Default + }; - el.child( - h_flex() - .h_full() - .border_l_1() - .rounded_r_lg() - .border_color(cx.theme().colors().border) - .bg(Self::edit_prediction_line_popover_bg_color(cx)) - .gap_1() - .py_1() - .px_2() - .child( + if keybind_display.action == EditPredictionKeybindAction::Preview { + el.child( h_flex() - .font(theme::ThemeSettings::get_global(cx).buffer_font.clone()) - .when(is_platform_style_mac, |parent| parent.gap_1()) - .child(h_flex().children(ui::render_modifiers( - accept_keystroke.modifiers(), - PlatformStyle::platform(), - Some(if !has_completion { - Color::Muted - } else { - Color::Default - }), - None, - false, - ))), + .h_full() + .border_l_1() + .rounded_r_lg() + .border_color(cx.theme().colors().border) + .bg(Self::edit_prediction_line_popover_bg_color(cx)) + .gap_1() + .py_1() + .px_2() + .child(self.render_edit_prediction_popover_keystroke( + keystroke, key_color, cx, + )) + .child(Label::new("Preview").into_any_element()) + .opacity(if has_completion { 1.0 } else { 0.4 }), ) - .child(Label::new("Preview").into_any_element()) - .opacity(if has_completion { 1.0 } else { 0.4 }), - ) - }) + } else { + el.child( + h_flex() + .h_full() + .border_l_1() + .rounded_r_lg() + .border_color(cx.theme().colors().border) + .bg(Self::edit_prediction_line_popover_bg_color(cx)) + .gap_1() + .py_1() + .px_2() + .child(self.render_edit_prediction_popover_keystroke( + keystroke, key_color, cx, + )) + .opacity(if has_completion { 1.0 } else { 0.4 }), + ) + } + }, + ) .into_any(), ) } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index acca539ceac3a549641f6abfb8b1a906114f47fe..a3d047bde663664ca5d789a168c4eff5a12ad429 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -43,13 +43,12 @@ use gpui::{ Bounds, ClickEvent, ClipboardItem, ContentMask, Context, Corner, Corners, CursorStyle, DispatchPhase, Edges, Element, ElementInputHandler, Entity, Focusable as _, Font, FontId, FontWeight, GlobalElementId, Hitbox, HitboxBehavior, Hsla, InteractiveElement, IntoElement, - IsZero, KeybindingKeystroke, Length, Modifiers, ModifiersChangedEvent, MouseButton, - MouseClickEvent, MouseDownEvent, MouseMoveEvent, MousePressureEvent, MouseUpEvent, PaintQuad, - ParentElement, Pixels, PressureStage, ScrollDelta, ScrollHandle, ScrollWheelEvent, ShapedLine, - SharedString, Size, StatefulInteractiveElement, Style, Styled, StyledText, TextAlign, TextRun, - TextStyleRefinement, WeakEntity, Window, anchored, deferred, div, fill, linear_color_stop, - linear_gradient, outline, pattern_slash, point, px, quad, relative, size, solid_background, - transparent_black, + IsZero, Length, Modifiers, ModifiersChangedEvent, MouseButton, MouseClickEvent, MouseDownEvent, + MouseMoveEvent, MousePressureEvent, MouseUpEvent, PaintQuad, ParentElement, Pixels, + PressureStage, ScrollDelta, ScrollHandle, ScrollWheelEvent, ShapedLine, SharedString, Size, + StatefulInteractiveElement, Style, Styled, StyledText, TextAlign, TextRun, TextStyleRefinement, + WeakEntity, Window, anchored, deferred, div, fill, linear_color_stop, linear_gradient, outline, + pattern_slash, point, px, quad, relative, size, solid_background, transparent_black, }; use itertools::Itertools; use language::{HighlightedText, IndentGuideSettings, language_settings::ShowWhitespaceSetting}; @@ -59,8 +58,6 @@ use multi_buffer::{ MultiBufferRow, RowInfo, }; -use edit_prediction_types::EditPredictionGranularity; - use project::{ DisableAiSettings, Entry, ProjectPath, debugger::breakpoint_store::{Breakpoint, BreakpointSessionState}, @@ -4838,17 +4835,11 @@ impl EditorElement { let edit_prediction = if edit_prediction_popover_visible { self.editor.update(cx, move |editor, cx| { - let accept_binding = editor.accept_edit_prediction_keybind( - EditPredictionGranularity::Full, - window, - cx, - ); let mut element = editor.render_edit_prediction_cursor_popover( min_width, max_width, cursor_point, style, - accept_binding.keystroke(), window, cx, )?; @@ -8618,21 +8609,6 @@ pub(crate) fn render_buffer_header( }) } -pub struct AcceptEditPredictionBinding(pub(crate) Option); - -impl AcceptEditPredictionBinding { - pub fn keystroke(&self) -> Option<&KeybindingKeystroke> { - if let Some(binding) = self.0.as_ref() { - match &binding.keystrokes() { - [keystroke, ..] => Some(keystroke), - _ => None, - } - } else { - None - } - } -} - fn prepaint_gutter_button( mut button: AnyElement, row: DisplayRow,