From f6c3c7d7831aa4e8a8ec507b7e46ecb80169dfd3 Mon Sep 17 00:00:00 2001 From: HuaGu-Dragon <1801943622@qq.com> Date: Thu, 14 May 2026 14:39:33 +0800 Subject: [PATCH] Vim/Helix: Prevent undo grouping when any LSP completion (#53980) 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 ## Summary Prevent undo grouping when an LSP completion includes extra edits so that the completion and its extra edits are applied and reverted atomically. ## Problem When applying an LSP completion that also applies extra edits, the editor may merge that completion into the surrounding undo group or split the transaction while waiting for edits, causing undo to leave the buffer in an inconsistent state. ## Solution * Always block undo merging for completions Closes #ISSUE Release Notes: - Prevent undo grouping when any LSP completion --- crates/editor/src/completions.rs | 5 + crates/vim/src/test.rs | 198 +++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+) diff --git a/crates/editor/src/completions.rs b/crates/editor/src/completions.rs index 2be7f28c5b..72bb0af3e7 100644 --- a/crates/editor/src/completions.rs +++ b/crates/editor/src/completions.rs @@ -975,6 +975,11 @@ impl Editor { self.show_signature_help(&ShowSignatureHelp, window, cx); } + // After the code completion is finished, we should finalize the last transaction. + // This ensure vim/helix not group the edits together. + self.buffer + .update(cx, |buffer, cx| buffer.finalize_last_transaction(cx)); + Some(cx.spawn_in(window, async move |editor, cx| { let additional_edits_tx = apply_edits.await?; diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index 8b5570125c..99eda3d6c4 100644 --- a/crates/vim/src/test.rs +++ b/crates/vim/src/test.rs @@ -1402,6 +1402,204 @@ async fn test_undo(cx: &mut gpui::TestAppContext) { 3"}); } +#[perf] +#[gpui::test] +async fn test_lsp_completions_undo(cx: &mut gpui::TestAppContext) { + use editor::test::editor_lsp_test_context::EditorLspTestContext; + VimTestContext::init(cx); + let mut cx = VimTestContext::new_with_lsp( + EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string()]), + resolve_provider: Some(true), + ..Default::default() + }), + signature_help_provider: Some(lsp::SignatureHelpOptions::default()), + ..Default::default() + }, + cx, + ) + .await, + true, + ); + + cx.set_state("fn main() { let a = ˇ }", Mode::Normal); + cx.simulate_keystroke("i"); + cx.set_state("fn main() { let a = ˇ }", Mode::Insert); + + cx.simulate_keystroke(";"); + cx.assert_state("fn main() { let a = ;ˇ }", Mode::Insert); + + cx.simulate_keystroke("escape"); + cx.assert_state("fn main() { let a = ˇ; }", Mode::Normal); + + cx.simulate_keystroke("i"); + cx.simulate_keystroke("2"); + cx.assert_state("fn main() { let a = 2ˇ; }", Mode::Insert); + cx.simulate_keystroke("."); + + let completion_item = lsp::CompletionItem { + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 22, + }, + end: lsp::Position { + line: 0, + character: 22, + }, + }, + new_text: "completion".to_string(), + })), + additional_text_edits: None, + ..Default::default() + }; + + let closure_completion_item = completion_item.clone(); + let mut request = cx.set_request_handler::(move |_, _, _| { + let task_completion_item = closure_completion_item.clone(); + async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + task_completion_item, + ]))) + } + }); + + request.next().await; + + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + + let _ = cx.update_editor(|editor, window, cx| { + editor + .confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx) + .unwrap() + }); + + cx.assert_editor_state("fn main() { let a = 2.completionˇ; }"); + + cx.simulate_keystrokes("escape u"); + + cx.assert_editor_state("fn main() { let a = 2.ˇ; }"); +} + +#[perf] +#[gpui::test] +async fn test_lsp_completions_with_additional_edits_undo(cx: &mut gpui::TestAppContext) { + use editor::test::editor_lsp_test_context::EditorLspTestContext; + VimTestContext::init(cx); + let mut cx = VimTestContext::new_with_lsp( + EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string()]), + resolve_provider: Some(true), + ..Default::default() + }), + ..Default::default() + }, + cx, + ) + .await, + true, + ); + + cx.set_state("fn main() { let a = ˇ }", Mode::Normal); + cx.simulate_keystroke("i"); + cx.set_state("fn main() { let a = ˇ }", Mode::Insert); + + cx.simulate_keystroke("2"); + cx.simulate_keystroke(";"); + cx.assert_state("fn main() { let a = 2;ˇ }", Mode::Insert); + + cx.simulate_keystroke("escape"); + cx.assert_state("fn main() { let a = 2ˇ; }", Mode::Normal); + + cx.simulate_keystroke("i"); + cx.assert_state("fn main() { let a = 2ˇ; }", Mode::Insert); + + cx.simulate_keystroke("."); + let completion_item = lsp::CompletionItem { + label: "some".into(), + kind: Some(lsp::CompletionItemKind::SNIPPET), + detail: Some("Wrap the expression in an `Option::Some`".to_string()), + documentation: Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { + kind: lsp::MarkupKind::Markdown, + value: "```rust\nSome(2)\n```".to_string(), + })), + deprecated: Some(false), + sort_text: Some("fffffff2".to_string()), + filter_text: Some("some".to_string()), + insert_text_format: Some(lsp::InsertTextFormat::SNIPPET), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 22, + }, + end: lsp::Position { + line: 0, + character: 22, + }, + }, + new_text: "Some(2)".to_string(), + })), + additional_text_edits: Some(vec![lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 20, + }, + end: lsp::Position { + line: 0, + character: 22, + }, + }, + new_text: "".to_string(), + }]), + ..Default::default() + }; + + let closure_completion_item = completion_item.clone(); + let mut request = cx.set_request_handler::(move |_, _, _| { + let task_completion_item = closure_completion_item.clone(); + async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + task_completion_item, + ]))) + } + }); + + request.next().await; + + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + + let apply_additional_edits = cx.update_editor(|editor, window, cx| { + editor + .confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx) + .unwrap() + }); + cx.assert_editor_state("fn main() { let a = 2.Some(2)ˇ; }"); + + cx.set_request_handler::(move |_, _, _| { + let task_completion_item = completion_item.clone(); + async move { Ok(task_completion_item) } + }) + .next() + .await + .unwrap(); + + apply_additional_edits.await.unwrap(); + cx.assert_editor_state("fn main() { let a = Some(2)ˇ; }"); + + cx.simulate_keystrokes("escape u"); + + cx.assert_editor_state("fn main() { let a = 2.ˇ; }"); +} + #[perf] #[gpui::test] async fn test_mouse_selection(cx: &mut TestAppContext) {