From c847d986eca9081df10619430bea7ad0490c670f Mon Sep 17 00:00:00 2001 From: "zed-zippy[bot]" <234243425+zed-zippy[bot]@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:53:24 +0000 Subject: [PATCH] Fix regression preventing new predictions from being previewed in subtle mode (#51887) (cherry-pick to preview) (#52224) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of #51887 to preview ---- ## Context Fixes some issues with https://github.com/zed-industries/zed/pull/51842 Namely that the tests were scattered and not well organized (this PR also makes them more thorough), and a regression where holding the modifiers for the accept prediction keybind would not cause an incoming prediction to be immediately previewed. ## How to Review ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - (Preview v0.229.x only) Fixed a regression where holding the modifiers for the accept edit prediction keybind would not immediately preview predictions as they arrived Co-authored-by: Ben Kunkle --- assets/keymaps/default-linux.json | 7 +- assets/keymaps/default-macos.json | 7 +- assets/keymaps/default-windows.json | 8 +- assets/keymaps/vim.json | 2 +- crates/editor/src/edit_prediction_tests.rs | 888 ++++++++++++--------- crates/editor/src/editor.rs | 57 +- 6 files changed, 587 insertions(+), 382 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 24299f6a4c0dba33f8c1417ea2e688b33c9b9152..ecd603a9afd64a97e2f4e54cd687c94c06e7fc41 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -770,11 +770,16 @@ "bindings": { "alt-tab": "editor::AcceptEditPrediction", "alt-l": "editor::AcceptEditPrediction", - "tab": "editor::AcceptEditPrediction", "alt-k": "editor::AcceptNextWordEditPrediction", "alt-j": "editor::AcceptNextLineEditPrediction", }, }, + { + "context": "Editor && edit_prediction && edit_prediction_mode == eager", + "bindings": { + "tab": "editor::AcceptEditPrediction", + }, + }, { "context": "Editor && showing_code_actions", "bindings": { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index b58b35d2b50ce9fa12695ac8fa20968072eb2c01..3310d060baf4a063fc7f9a9cd23609b3b07c1f7f 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -834,11 +834,16 @@ "context": "Editor && edit_prediction", "bindings": { "alt-tab": "editor::AcceptEditPrediction", - "tab": "editor::AcceptEditPrediction", "ctrl-cmd-right": "editor::AcceptNextWordEditPrediction", "ctrl-cmd-down": "editor::AcceptNextLineEditPrediction", }, }, + { + "context": "Editor && edit_prediction && edit_prediction_mode == eager", + "bindings": { + "tab": "editor::AcceptEditPrediction", + }, + }, { "context": "Editor && showing_code_actions", "use_key_equivalents": true, diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index 426076a497055dbc350231d2889ae53006076582..08a85a92d994e1d77ff9de79f2b28fd625cc457c 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -766,11 +766,17 @@ "bindings": { "alt-tab": "editor::AcceptEditPrediction", "alt-l": "editor::AcceptEditPrediction", - "tab": "editor::AcceptEditPrediction", "alt-k": "editor::AcceptNextWordEditPrediction", "alt-j": "editor::AcceptNextLineEditPrediction", }, }, + { + "context": "Editor && edit_prediction && edit_prediction_mode == eager", + "use_key_equivalents": true, + "bindings": { + "tab": "editor::AcceptEditPrediction", + }, + }, { "context": "Editor && showing_code_actions", "use_key_equivalents": true, diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 11b18040bb3f1e25eb8d5c148d023d181a1dbf6b..ce5a363dbebe465e732e365f3527f26880f9f770 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -1060,7 +1060,7 @@ }, }, { - "context": "Editor && edit_prediction", + "context": "Editor && edit_prediction && edit_prediction_mode == eager", "bindings": { // This is identical to the binding in the base keymap, but the vim bindings above to // "vim::Tab" shadow it, so it needs to be bound again. diff --git a/crates/editor/src/edit_prediction_tests.rs b/crates/editor/src/edit_prediction_tests.rs index 40d915b841c1cf4a53025f7dce654dd129cb8de5..684213e481762d7fb09a0bd6d8b7a0b9fc6d4a36 100644 --- a/crates/editor/src/edit_prediction_tests.rs +++ b/crates/editor/src/edit_prediction_tests.rs @@ -1,13 +1,17 @@ use edit_prediction_types::{ EditPredictionDelegate, EditPredictionIconSet, PredictedCursorPosition, }; -use gpui::{Entity, KeyBinding, Modifiers, prelude::*}; +use gpui::{ + Entity, KeyBinding, KeybindingKeystroke, Keystroke, Modifiers, NoAction, Task, prelude::*, +}; use indoc::indoc; -use language::Buffer; use language::EditPredictionsMode; -use multi_buffer::{Anchor, MultiBufferSnapshot, ToPoint}; +use language::{Buffer, CodeLabel}; +use multi_buffer::{Anchor, ExcerptId, MultiBufferSnapshot, ToPoint}; +use project::{Completion, CompletionResponse, CompletionSource}; use std::{ ops::Range, + rc::Rc, sync::{ Arc, atomic::{self, AtomicUsize}, @@ -17,8 +21,9 @@ use text::{Point, ToOffset}; use ui::prelude::*; use crate::{ - AcceptEditPrediction, EditPrediction, EditPredictionKeybindAction, - EditPredictionKeybindSurface, MenuEditPredictionsPolicy, + AcceptEditPrediction, CompletionContext, CompletionProvider, EditPrediction, + EditPredictionKeybindAction, EditPredictionKeybindSurface, MenuEditPredictionsPolicy, + ShowCompletions, editor_tests::{init_test, update_test_language_settings}, test::editor_test_context::EditorTestContext, }; @@ -482,57 +487,10 @@ 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) { +async fn test_edit_prediction_preview_activates_when_prediction_arrives_with_modifier_held( + cx: &mut gpui::TestAppContext, +) { init_test(cx, |_| {}); load_default_keymap(cx); update_test_language_settings(cx, &|settings| { @@ -544,227 +502,324 @@ async fn test_subtle_in_code_indicator_prefers_preview_binding(cx: &mut gpui::Te 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" - ); + cx.editor(|editor, _, _| { + assert!(!editor.has_active_edit_prediction()); + assert!(!editor.edit_prediction_preview_is_active()); }); -} -#[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"); + let preview_modifiers = cx.update_editor(|editor, window, cx| { + *editor + .preview_edit_prediction_keystroke(window, cx) + .unwrap() + .modifiers() }); - cx.simulate_keystroke("tab"); + cx.simulate_modifiers_change(preview_modifiers); 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 = ˇ;"); + cx.editor(|editor, _, _| { + assert!(!editor.has_active_edit_prediction()); + assert!(editor.edit_prediction_preview_is_active()); + }); 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()); + editor.set_menu_edit_predictions_policy(MenuEditPredictionsPolicy::ByProvider); + editor.update_visible_edit_prediction(window, cx) + }); - let keybind_display = editor.edit_prediction_keybind_display( - EditPredictionKeybindSurface::CursorPopoverExpanded, - window, - cx, + cx.editor(|editor, _, _| { + assert!(editor.has_active_edit_prediction()); + assert!( + editor.edit_prediction_preview_is_active(), + "prediction preview should activate immediately when the prediction arrives while the preview modifier is still held", ); + }); +} - 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" +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"), ); - 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()); +async fn test_inline_edit_prediction_keybind_selection_cases(cx: &mut gpui::TestAppContext) { + enum InlineKeybindState { + Normal, + ShowingCompletions, + InLeadingWhitespace, + ShowingCompletionsAndLeadingWhitespace, + } - 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"); + enum ExpectedKeystroke { + DefaultAccept, + DefaultPreview, + Literal(&'static str), + } - assert_eq!( - keybind_display.action, - EditPredictionKeybindAction::Preview, - "multi-line prediction should show the preview action" - ); - assert!(preview_keystroke.modifiers().modified()); - }); -} + struct InlineKeybindCase { + name: &'static str, + use_default_keymap: bool, + mode: EditPredictionsMode, + extra_bindings: Vec, + state: InlineKeybindState, + expected_accept_keystroke: ExpectedKeystroke, + expected_preview_keystroke: ExpectedKeystroke, + expected_displayed_keystroke: ExpectedKeystroke, + } -#[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 default_cx = EditorTestContext::new(cx).await; + let provider = default_cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut default_cx); + default_cx.set_state("let x = ˇ;"); + propose_edits(&provider, vec![(8..8, "42")], &mut default_cx); + default_cx + .update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + let (default_accept_keystroke, default_preview_keystroke) = + default_cx.update_editor(|editor, window, cx| { + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::Inline, + window, + cx, + ); + let accept_keystroke = keybind_display + .accept_keystroke + .as_ref() + .expect("default inline edit prediction should have an accept binding") + .clone(); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .expect("default inline edit prediction should have a preview binding") + .clone(); + (accept_keystroke, preview_keystroke) + }); + + let cases = [ + InlineKeybindCase { + name: "default setup prefers tab over alt-tab for accept", + use_default_keymap: true, + mode: EditPredictionsMode::Eager, + extra_bindings: Vec::new(), + state: InlineKeybindState::Normal, + expected_accept_keystroke: ExpectedKeystroke::DefaultAccept, + expected_preview_keystroke: ExpectedKeystroke::DefaultPreview, + expected_displayed_keystroke: ExpectedKeystroke::DefaultAccept, + }, + InlineKeybindCase { + name: "subtle mode displays preview binding inline", + use_default_keymap: true, + mode: EditPredictionsMode::Subtle, + extra_bindings: Vec::new(), + state: InlineKeybindState::Normal, + expected_accept_keystroke: ExpectedKeystroke::DefaultPreview, + expected_preview_keystroke: ExpectedKeystroke::DefaultPreview, + expected_displayed_keystroke: ExpectedKeystroke::DefaultPreview, + }, + InlineKeybindCase { + name: "removing default tab binding still displays tab", + use_default_keymap: true, + mode: EditPredictionsMode::Eager, + extra_bindings: vec![KeyBinding::new( + "tab", + NoAction, + Some("Editor && edit_prediction && edit_prediction_mode == eager"), + )], + state: InlineKeybindState::Normal, + expected_accept_keystroke: ExpectedKeystroke::DefaultPreview, + expected_preview_keystroke: ExpectedKeystroke::DefaultPreview, + expected_displayed_keystroke: ExpectedKeystroke::DefaultPreview, + }, + InlineKeybindCase { + name: "custom-only rebound accept key uses replacement key", + use_default_keymap: true, + mode: EditPredictionsMode::Eager, + extra_bindings: vec![KeyBinding::new( + "ctrl-enter", + AcceptEditPrediction, + Some("Editor && edit_prediction"), + )], + state: InlineKeybindState::Normal, + expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + }, + InlineKeybindCase { + name: "showing completions restores conflict-context binding", + use_default_keymap: true, + mode: EditPredictionsMode::Eager, + extra_bindings: vec![KeyBinding::new( + "ctrl-enter", + AcceptEditPrediction, + Some("Editor && edit_prediction && showing_completions"), + )], + state: InlineKeybindState::ShowingCompletions, + expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + }, + InlineKeybindCase { + name: "leading whitespace restores conflict-context binding", + use_default_keymap: false, + mode: EditPredictionsMode::Eager, + extra_bindings: vec![KeyBinding::new( + "ctrl-enter", + AcceptEditPrediction, + Some("Editor && edit_prediction && in_leading_whitespace"), + )], + state: InlineKeybindState::InLeadingWhitespace, + expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + }, + InlineKeybindCase { + name: "showing completions and leading whitespace restore combined conflict binding", + use_default_keymap: false, + mode: EditPredictionsMode::Eager, + extra_bindings: vec![KeyBinding::new( + "ctrl-enter", + AcceptEditPrediction, + Some("Editor && edit_prediction && showing_completions && in_leading_whitespace"), + )], + state: InlineKeybindState::ShowingCompletionsAndLeadingWhitespace, + expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"), + }, + ]; + + for case in cases { + init_test(cx, |_| {}); + if case.use_default_keymap { + load_default_keymap(cx); + } + update_test_language_settings(cx, &|settings| { + settings.edit_predictions.get_or_insert_default().mode = Some(case.mode); + }); - 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 = ˇ;"); + if !case.extra_bindings.is_empty() { + cx.update(|cx| cx.bind_keys(case.extra_bindings.clone())); + } - 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)); + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); - cx.update_editor(|editor, window, cx| { - assert!(editor.has_active_edit_prediction()); + match case.state { + InlineKeybindState::Normal | InlineKeybindState::ShowingCompletions => { + cx.set_state("let x = ˇ;"); + } + InlineKeybindState::InLeadingWhitespace + | InlineKeybindState::ShowingCompletionsAndLeadingWhitespace => { + cx.set_state(indoc! {" + fn main() { + ˇ + } + "}); + } + } - let keybind_display = editor.edit_prediction_keybind_display( - EditPredictionKeybindSurface::CursorPopoverExpanded, - window, - cx, - ); + propose_edits(&provider, vec![(8..8, "42")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + if matches!( + case.state, + InlineKeybindState::ShowingCompletions + | InlineKeybindState::ShowingCompletionsAndLeadingWhitespace + ) { + assign_editor_completion_menu_provider(&mut cx); + cx.update_editor(|editor, window, cx| { + editor.show_completions(&ShowCompletions, window, cx); + }); + cx.run_until_parked(); + } - 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"); + cx.update_editor(|editor, window, cx| { + assert!( + editor.has_active_edit_prediction(), + "case '{}' should have an active edit prediction", + case.name + ); - 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()); - }); + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::Inline, + window, + cx, + ); + let accept_keystroke = keybind_display + .accept_keystroke + .as_ref() + .unwrap_or_else(|| panic!("case '{}' should have an accept binding", case.name)); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .unwrap_or_else(|| panic!("case '{}' should have a preview binding", case.name)); + let displayed_keystroke = keybind_display + .displayed_keystroke + .as_ref() + .unwrap_or_else(|| panic!("case '{}' should have a displayed binding", case.name)); + + let expected_accept_keystroke = match case.expected_accept_keystroke { + ExpectedKeystroke::DefaultAccept => default_accept_keystroke.clone(), + ExpectedKeystroke::DefaultPreview => default_preview_keystroke.clone(), + ExpectedKeystroke::Literal(keystroke) => KeybindingKeystroke::from_keystroke( + Keystroke::parse(keystroke).expect("expected test keystroke to parse"), + ), + }; + let expected_preview_keystroke = match case.expected_preview_keystroke { + ExpectedKeystroke::DefaultAccept => default_accept_keystroke.clone(), + ExpectedKeystroke::DefaultPreview => default_preview_keystroke.clone(), + ExpectedKeystroke::Literal(keystroke) => KeybindingKeystroke::from_keystroke( + Keystroke::parse(keystroke).expect("expected test keystroke to parse"), + ), + }; + let expected_displayed_keystroke = match case.expected_displayed_keystroke { + ExpectedKeystroke::DefaultAccept => default_accept_keystroke.clone(), + ExpectedKeystroke::DefaultPreview => default_preview_keystroke.clone(), + ExpectedKeystroke::Literal(keystroke) => KeybindingKeystroke::from_keystroke( + Keystroke::parse(keystroke).expect("expected test keystroke to parse"), + ), + }; + + assert_eq!( + accept_keystroke, &expected_accept_keystroke, + "case '{}' selected the wrong accept binding", + case.name + ); + assert_eq!( + preview_keystroke, &expected_preview_keystroke, + "case '{}' selected the wrong preview binding", + case.name + ); + assert_eq!( + displayed_keystroke, &expected_displayed_keystroke, + "case '{}' selected the wrong displayed binding", + case.name + ); + + if matches!(case.mode, EditPredictionsMode::Subtle) { + assert!( + editor.edit_prediction_requires_modifier(), + "case '{}' should require a modifier", + case.name + ); + } + }); + } } #[gpui::test] -async fn test_multi_line_prediction_with_preview_uses_preview_cursor_popover_action( - cx: &mut gpui::TestAppContext, -) { +async fn test_tab_accepts_edit_prediction_over_completion(cx: &mut gpui::TestAppContext) { init_test(cx, |_| {}); load_default_keymap(cx); @@ -773,131 +828,194 @@ async fn test_multi_line_prediction_with_preview_uses_preview_cursor_popover_act 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; + 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 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()); + assert_editor_active_edit_completion(&mut cx, |_, edits| { + assert_eq!(edits.len(), 1); + assert_eq!(edits[0].1.as_ref(), "42"); }); -} - -#[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"); + cx.simulate_keystroke("tab"); + cx.run_until_parked(); - 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()); - }); + cx.assert_editor_state("let x = 42ˇ;"); } #[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()); - }); +async fn test_cursor_popover_edit_prediction_keybind_cases(cx: &mut gpui::TestAppContext) { + enum CursorPopoverPredictionKind { + SingleLine, + MultiLine, + SingleLineWithPreview, + MultiLineWithPreview, + DeleteSingleNewline, + StaleSingleLineAfterMultiLine, + } - propose_edits(&provider, vec![(8..8, "42")], &mut cx); - cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + struct CursorPopoverCase { + name: &'static str, + prediction_kind: CursorPopoverPredictionKind, + expected_action: EditPredictionKeybindAction, + } - cx.update_editor(|editor, window, cx| { - assert!(editor.has_active_edit_prediction()); + let cases = [ + CursorPopoverCase { + name: "single line prediction uses accept action", + prediction_kind: CursorPopoverPredictionKind::SingleLine, + expected_action: EditPredictionKeybindAction::Accept, + }, + CursorPopoverCase { + name: "multi line prediction uses preview action", + prediction_kind: CursorPopoverPredictionKind::MultiLine, + expected_action: EditPredictionKeybindAction::Preview, + }, + CursorPopoverCase { + name: "single line prediction with preview still uses accept action", + prediction_kind: CursorPopoverPredictionKind::SingleLineWithPreview, + expected_action: EditPredictionKeybindAction::Accept, + }, + CursorPopoverCase { + name: "multi line prediction with preview uses preview action", + prediction_kind: CursorPopoverPredictionKind::MultiLineWithPreview, + expected_action: EditPredictionKeybindAction::Preview, + }, + CursorPopoverCase { + name: "single line newline deletion uses accept action", + prediction_kind: CursorPopoverPredictionKind::DeleteSingleNewline, + expected_action: EditPredictionKeybindAction::Accept, + }, + CursorPopoverCase { + name: "stale multi line prediction does not force preview action", + prediction_kind: CursorPopoverPredictionKind::StaleSingleLineAfterMultiLine, + expected_action: EditPredictionKeybindAction::Accept, + }, + ]; + + for case in cases { + 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); + + match case.prediction_kind { + CursorPopoverPredictionKind::SingleLine => { + 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) + }); + } + CursorPopoverPredictionKind::MultiLine => { + 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) + }); + } + CursorPopoverPredictionKind::SingleLineWithPreview => { + 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) + }); + } + CursorPopoverPredictionKind::MultiLineWithPreview => { + 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) + }); + } + CursorPopoverPredictionKind::DeleteSingleNewline => { + 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) + }); + } + CursorPopoverPredictionKind::StaleSingleLineAfterMultiLine => { + 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) + }); + } + } - 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"); + cx.update_editor(|editor, window, cx| { + assert!( + editor.has_active_edit_prediction(), + "case '{}' should have an active edit prediction", + case.name + ); - 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"); - }); + let keybind_display = editor.edit_prediction_keybind_display( + EditPredictionKeybindSurface::CursorPopoverExpanded, + window, + cx, + ); + let accept_keystroke = keybind_display + .accept_keystroke + .as_ref() + .unwrap_or_else(|| panic!("case '{}' should have an accept binding", case.name)); + let preview_keystroke = keybind_display + .preview_keystroke + .as_ref() + .unwrap_or_else(|| panic!("case '{}' should have a preview binding", case.name)); + + assert_eq!( + keybind_display.action, case.expected_action, + "case '{}' selected the wrong cursor popover action", + case.name + ); + assert_eq!( + accept_keystroke.key(), + "tab", + "case '{}' selected the wrong accept binding", + case.name + ); + assert!( + preview_keystroke.modifiers().modified(), + "case '{}' should use a modified preview binding", + case.name + ); + + if matches!( + case.prediction_kind, + CursorPopoverPredictionKind::StaleSingleLineAfterMultiLine + ) { + assert!( + editor.stale_edit_prediction_in_menu.is_none(), + "case '{}' should clear stale menu state", + case.name + ); + } + }); + } } fn assert_editor_active_edit_completion( @@ -1054,6 +1172,12 @@ fn assign_editor_completion_provider( }) } +fn assign_editor_completion_menu_provider(cx: &mut EditorTestContext) { + cx.update_editor(|editor, _, _| { + editor.set_completion_provider(Some(Rc::new(FakeCompletionMenuProvider))); + }); +} + fn propose_edits_non_zed( provider: &Entity, edits: Vec<(Range, &str)>, @@ -1086,6 +1210,54 @@ fn assign_editor_completion_provider_non_zed( }) } +struct FakeCompletionMenuProvider; + +impl CompletionProvider for FakeCompletionMenuProvider { + fn completions( + &self, + _excerpt_id: ExcerptId, + _buffer: &Entity, + _buffer_position: text::Anchor, + _trigger: CompletionContext, + _window: &mut Window, + _cx: &mut Context, + ) -> Task>> { + let completion = Completion { + replace_range: text::Anchor::MIN..text::Anchor::MAX, + new_text: "fake_completion".to_string(), + label: CodeLabel::plain("fake_completion".to_string(), None), + documentation: None, + source: CompletionSource::Custom, + icon_path: None, + match_start: None, + snippet_deduplication_key: None, + insert_text_mode: None, + confirm: None, + }; + + Task::ready(Ok(vec![CompletionResponse { + completions: vec![completion], + display_options: Default::default(), + is_incomplete: false, + }])) + } + + fn is_completion_trigger( + &self, + _buffer: &Entity, + _position: language::Anchor, + _text: &str, + _trigger_in_words: bool, + _cx: &mut Context, + ) -> bool { + false + } + + fn filter_completions(&self) -> bool { + false + } +} + #[derive(Default, Clone)] pub struct FakeEditPredictionDelegate { pub completion: Option, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 69081c07fd97ff49204893cdca6d4fbc081f2750..eae3e4100b277d58d030fc71b9f0e582bec7088a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2884,6 +2884,11 @@ impl Editor { if self.in_leading_whitespace { key_context.add("in_leading_whitespace"); } + if self.edit_prediction_requires_modifier() { + key_context.set("edit_prediction_mode", "subtle") + } else { + key_context.set("edit_prediction_mode", "eager"); + } if self.selection_mark_mode { key_context.add("selection_mode"); @@ -2942,7 +2947,7 @@ impl Editor { window: &mut Window, cx: &mut App, ) -> Option { - let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx); + let key_context = self.key_context_internal(true, window, cx); let bindings = match granularity { @@ -2969,7 +2974,7 @@ impl Editor { window: &mut Window, cx: &mut App, ) -> Option { - let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx); + let key_context = self.key_context_internal(true, window, cx); let bindings = window.bindings_for_action_in_context(&AcceptEditPrediction, key_context); bindings .into_iter() @@ -2980,6 +2985,32 @@ impl Editor { }) } + fn edit_prediction_preview_modifiers_held( + &self, + modifiers: &Modifiers, + window: &mut Window, + cx: &mut App, + ) -> bool { + let key_context = self.key_context_internal(true, window, cx); + let actions: [&dyn Action; 3] = [ + &AcceptEditPrediction, + &AcceptNextWordEditPrediction, + &AcceptNextLineEditPrediction, + ]; + + actions.into_iter().any(|action| { + window + .bindings_for_action_in_context(action, key_context.clone()) + .into_iter() + .rev() + .any(|binding| { + binding.keystrokes().first().is_some_and(|keystroke| { + keystroke.modifiers().modified() && keystroke.modifiers() == modifiers + }) + }) + }) + } + fn edit_prediction_cursor_popover_prefers_preview( &self, completion: &EditPredictionState, @@ -8486,9 +8517,12 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { + self.update_edit_prediction_settings(cx); + // Ensure that the edit prediction preview is updated, even when not // enabled, if there's an active edit prediction preview. if self.show_edit_predictions_in_menu() + || self.edit_prediction_requires_modifier() || matches!( self.edit_prediction_preview, EditPredictionPreview::Active { .. } @@ -8581,24 +8615,7 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { - let mut modifiers_held = false; - - let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx); - let actions: [&dyn Action; 3] = [ - &AcceptEditPrediction, - &AcceptNextWordEditPrediction, - &AcceptNextLineEditPrediction, - ]; - - 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()); - } - } - } + let modifiers_held = self.edit_prediction_preview_modifiers_held(modifiers, window, cx); if modifiers_held { if matches!(