Unassigned myself as apparently do not understand Vim specific related to all this.

I have assumed we need a generic fix like #52699 or similar and got quite confused on the current impl.

Unassigned myself as apparently do not understand Vim specific related to all this.

I have assumed we need a generic fix like #52699 or similar and got quite confused on the current impl.

Yeah....
Perhaps I should change the logic for group transactions rather than using a hack here.

Maybe I need some help here. After digging deeper, the core issue is very specific to Vim/Helix mode:

Vim tracks an entire Insert mode session as a single undo group via self.current_tx. When confirming an LSP completion, the editor inserts the main text (moving the cursor) and then await additional text edits. During this await, vim.rs sees the cursor move (current_anchor != newest), assumes the user manually moved it (e.g., via arrow keys), and prematurely drops current_tx, breaking the undo group.

While it might be tempting to add metadata to the generic Transaction struct to tell Vim "this is a completion, don't break the group", doing so for a Vim-specific behavior would require a huge refactor and break the crate layering?

Maybe explicitly calls buffer.finalize_last_transaction() is better, since project/src/lsp_store.rs also do that before applying the additional text.

we can actually solve this elegantly without adding any Vim-specific boolean flags (completion_additional_edits_pending) to the Editor struct.

This definitely looks like a proper direction!

2 small questions I have left:

• pending LSP completion edits are the additional text edits, if I understand the issue right?

If so, then can we move the fix under if let Some checks in

https://github.com/zed-industries/zed/blob/d4f06ee3740b0da1bd55476a5ff19d7c2ac1e57f/crates/editor/src/editor.rs#L6981-L6991

Or do you think this fix should apply to all, "synchronous" also, edits?

• The only fix-related comment I've found is

zed/crates/language/src/buffer.rs

Lines 2479 to 2481 in d4f06ee

	/// Prevent the last transaction from being grouped with any subsequent transactions,
	/// even if they occur with the buffer's undo grouping duration.
	pub fn finalize_last_transaction(&mut self) -> Option<&Transaction> {

and that does not explain much what happens here. Can we add a small comment on why this is needed and how the rest of the code interacts with it?

If it helps, a good example of such comment is a few lines above: https://github.com/zed-industries/zed/blob/d4f06ee3740b0da1bd55476a5ff19d7c2ac1e57f/crates/editor/src/editor.rs#L6953-L6954

Yes — the pending LSP completion edits are the additional text edits.

Regarding your second question: I intentionally placed the fix (finalize_last_transaction) outside the if let block so that it applies to all completions, including synchronous ones. Here is my reasoning:

• After we enter completion, the cursor moves, then vim.rs detects this in local_selections_changed And Vim immediately issues a group_until_transaction. Since apply_edits.await is asynchronous, If we wait to finalize until inside the if let Some block, the merge_transaction will be incorrect.

• You raised a great point about whether this should apply to all synchronous edits as well. I tested the native behavior in Neovim and Helix, When you type let a = and accept the completion Some, pressing u should only revert Some, not the let a = typed beforehand. (This is also why the existing test case expects the state to revert to 2. instead of 2). I also add a test to cover this.

Here is the video about their behaviour. Sorry for that I do not have neovim locally, so I asked my friend to have a test in his machine, but the video have a bit blurry.

Does this solve your concerns?

Sorry for the delay—I've been under academic pressure
I updated the PR title and description and rebased the branch to resolve conflicts.