Add basic support for LSP snippets

[Urgau]

This PR adds a basic support for LSP completion snippet based on https://github.com/helix-editor/helix/tree/snippets.

A snippet is a bit different from a normal completion in that it contains tab stops/placeholders (ie $1, $2 or ${3:foo} and $0 who is always the last one). A complete definition of the syntax can be found [here(https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#snippet_syntax). So when creating the transaction we parse the snippet input, create the output text with the correct indentation and create the selections for the placeholders.

This PR brings the support for snippet in a good state where only variable, placeholder removal and Tab/S-Tab support isn't implemented for the moment, those should be feasible but I didn't wanted to do them in this PR.

I've tested multiple snippet with rust-analyzer, I recommend the macro_rules snippet for testing as it uses multiple tab stops and is multi line.

[filipdutescu]

Hey there, thanks a lot for submitting this PR, I really miss snippets at the moment, but it always seemed like a hard thing to implement. I hope you don't mind me leaving a comment, thought it might be of interest.

[archseer]

Note: we have a full snippet parser here: https://github.com/helix-editor/helix/tree/snippets It's just not tied into the LSP stack yet.

[Urgau]

Note: we have a full snippet parser here: https://github.com/helix-editor/helix/tree/snippets It's just not tied into the LSP stack yet.

Do you want me to close the PR and wait or the full snippet parser to land ?

[archseer]

You could base your PR on top of that branch and use the snippet parser rather than a custom parser. It would be a good enough stopgap until full snippet support is developed

[archseer]

Actually, @the-mikedavis looks like your branch already matches this PR's behaviour? (apart from jumping to the first tabstop and correctly indenting)

[Urgau]

Actually, @the-mikedavis looks like your branch already matches this PR's behaviour? (apart from jumping to the first tabstop and correctly indenting)

I think it already jumps to the first tab stop here. As for indentation, I can add it.

[Urgau]

I have now rebased this PR with the changes from @the-mikedavis. 🥳

On top of the rebase I added the auto-indentation, sorting and merging of tabstops, removed the need for regex when parsing the snippets and added the deletion of the placeholder when inserting a character (edit: removed for now as it wasn't right).

The support for snippet is now in a good state where only variable and Tab/S-Tab support isn't implemented.

[pascalkuthe]

The changes in 1c548d8 heavily regress many core editing operating in helix like appendmode (and also handle other details incorrectly like using byte indexing instead of char indexing). You should not be changing the core editing operations. Instead the placeholders should be stored on the Document and specical cased in apply_impl.

[Urgau]

The changes in 1c548d8 heavily regress many core editing operating in helix like appendmode (and also handle other details incorrectly like using byte indexing instead of char indexing). You should not be changing the core editing operations. Instead the placeholders should be stored on the Document and specical cased in apply_impl.

My bad, I didn't test it enough. I'm still new to the code base or even helix. I will try to do the special cased as said this week. For the moment, I've removed the offending commit.

[archseer]

I'd implement this the simplest way possible for now (jump to first tabstop and drop all the snippet state). The idea is to use marks in the future (#3720) to keep track of each tabstop locations even as text edits occur.

[Urgau]

The pull request already jumps the cursor to the first tabstop. So I think it's ready for review.

The idea is to use marks in the future (#3720) to keep track of each tabstop locations even as text edits occur.

That seems exactly what tabstops should be map to. Looking forward to see this being merged.

[pascalkuthe]

The pull request already jumps the cursor to the first tabstop. So I think it's ready for review.

The idea is to use marks in the future (#3720) to keep track of each tabstop locations even as text edits occur.

That seems exactly what tabstops should be map to. Looking forward to see this being merged.

I think removing the placeholder text for the first tabstop would be required for a MVP, deleting that would be quite annoying. Maybe if we want to keep this PR really simple you could simply remove the placeholder for the first tabstop by generating a second transaction that deletes it and apply it immedietly. That would probably be the easiest workaround and just take 5 lines of code.

[David-Else]

I tested this and it unlocks all the basic features of the HTML language server that didn't work before. You now get auto suggestions and auto tag completion :) I remember it was an issue in Neovim that you needed a special snippet plugin to solve. This PR adds a lot of value as it is.

[H4ckint0sh]

I tested this and it unlocks all the basic features of the HTML language server that didn't work before. You now get auto suggestions and auto tag completion :) I remember it was an issue in Neovim that you needed a special snippet plugin to solve. This PR adds a lot of value as it is.

@David-Else Do I need to do some additional steps beside merging this PR to get auto suggestion and auto completion working? Cuz I have build from src merging this PR but it seems that those are not present.

[David-Else]

@H4ckint0sh I assume you have:

Configured language server: vscode-html-language-server
Binary for language server: /usr/local/bin/vscode-html-language-server

I just switched to this PR using GitHub CLI, rebuilt Helix and it worked right away. You need to be in a valid HTML file for it to work, here is what I was using to test:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Document</title>
  </head>
  <body>
    <div>Make another div, you should get suggestions for span, div etc, then start to close it and it suggests the right closing tag</div>
  </body>
</html>

[Urgau]

@pascalkuthe Here's what I hacked for deleting the first tabstop, I had to do a bit of range manipulation since the selection was too big at the right and was eating the next character. Do you have a better things in mind or should I push this ?

[archseer]

Seems a bit odd that you would have to modify the ranges after the fact, it's probably because the returned ranges are end inclusive?

[archseer]

Or we could do it the other way around (\cc @pascalkuthe)

[pascalkuthe]

@archseer I think it's probably better to merge this PR first since some of the fixes from #5728 need to be applied to the logic from this PR (see #5864 (comment)). It's probably easier if I just do that in #5728 myself

[the-mikedavis]

This looks good 🚀

I left some minor comments

[leonqadirie]

@leonqadirie: the fix is in Urgau#6

Thank you very much for the fix/improvement!
Indentation for ElixirLS' do completion is still a bit off on my end but this might be unrelated.

[andriigrynenko]

Thank you very much for the fix/improvement! Indentation for ElixirLS' do completion is still a bit off on my end but this might be unrelated.

Thank you for flagging this! At first it seemed like an easy issue to fix, but I ended up uncovering some more subtle bugs in the current implementation. Urgau#7 is the fix.

[archseer]

We'll postpone Urgau#7 until this PR + Pascal's PR land.

I wanted to say, thank you @Urgau and @andriigrynenko for working on this! It's great to see basic snippet expansion finally land :)

[AddictArts]

Apologies, I've read this through more than once and my queston isn't answered, thus I'll ask.

How do you test macro-rules if Tab and S-Tab are not implemented? It is hard to see the feature actually working. The news video doesn't highlight or show what actually is the snippet when completing std::fs::create_dir_all(.

I think the idea is it hits the first tab stop path. Then that is it; However @Urgau mentions using macro_rules, I'm guessing it has multiple tab stops, but you can't actually more through them. I think the issue here is from the video I can't tell what is different.

[noahfraiture]

Hello, I didn't find anything about it, is it now possible to tab to next position ?

[danillos]

@noahfraiture you're looking for this PR #9801