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
This commit is contained in:
HuaGu-Dragon authored and GitHub committed
1 parent
a937e4e8be
commit
f6c3c7d783
2 files changed
+203
No files matched your search
@@ -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?;
|
||||
|
||||
|
||||
@@ -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::<lsp::request::Completion, _, _>(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::<lsp::request::Completion, _, _>(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::<lsp::request::ResolveCompletionItem, _, _>(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) {
|
||||
|
||||
Reference in new issue
Block a user