Completion replaces existing word incorrectly if they end on the same letter

[aral]

Token following where a completion should be inserted is erroneously replaced with the completion.

I press i and start typing.

So far, so good…

I select the first completion. And it replaces “events” instead of being inserted before it.

helix 22.12 (08c35ab6) on Fedora Silverblue 37 in Black Box terminal under GNOME.

Possibly related to #2580

[archseer]

I think this is just a missing config here:

helix/helix-term/src/ui/completion.rs

Lines 114 to 117 in 91dca3f

	lsp::CompletionTextEdit::InsertAndReplace(item) => {
	// TODO: support using "insert" instead of "replace" via user config
	lsp::TextEdit::new(item.replace, item.new_text.clone())
	}

[archseer]

@pascalkuthe you mentioned this on another issue somewhere ^

[pascalkuthe]

Sorry forgot about this @archseer .

This is indeed intended behavior. LSPs provide two options for handeling autocompletions: replace or insert.
Helix is currently hardcoded to always use the replace edit. Many editors default to insert instead (vscode and nvim.cmp do atleast). I think adding a config option here would be an easy PR. all we need to do is to add a condition to use item.insert instead of item.replace in the snippet @archseer linked.

[aral]

@pascalkuthe @archseer When would this ever be the desired behaviour? It results in corrupted syntax. I’d highly recommend making insert the default behaviour. I don’t remember once ending up with corrupted syntax while using VSCode (and I’m so glad I don’t have to use VSCode anymore for a multitude of other reasons).

Also, note that the replacement isn’t being carried out perfectly either (the selection goes up to but does not include the final “t”). That might be a separate bug, thought.

[pascalkuthe]

Simple let's say you have two functions foo_bar and foo_rab.
Right now your code is test.foo_rab() but you want to change it to test.foo_bar so you can just start the autcompletion at foo_ and select foo_bar and the _rab is automatically removed. The replace behavior is valid and has it's place. There is good reason that both options are included in the LSP spec.

Offering an option definitely makes sense. I also prefer the insert behavior (seems more consistent for insert mode, I usually delete in normal mode instead of insert mode) but both have their merit.

[pascalkuthe]

Duplicate of #4380