Support for rust-analyzer's SnippetTextEdits

[augustocdias]

As requested for vim-snip (hrsh7th/vim-vsnip#225), I think it should be nice to be supported in LuaSnip as well...

Reference: https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/lsp-extensions.md#snippet-textedit Reference: hrsh7th/nvim-cmp#353 (comment) Reference: simrat39/rust-tools.nvim#74

[L3MON4D3]

What exactly would have to be added to luasnip to enable snippet-textedits? We have the capability to expand lsp-style snippets at the current cursor position via luasnip.lsp_expand(snippet_body), is that enough?

[augustocdias]

@simrat39 would that be enough to add this to rust-tools?

[simrat39]

umm not really, rust analyzer's snippet text edits are more like normal lsp text edits, they send a location and stuff as well, but I afaik most snippet plugins only support snippet expansion at the current cursor position, so it wouldn't work. It would need a completely separate implementation of TextEdits.

[L3MON4D3]

Expanding at any position shouldn't be a problem, I could add that. I wouldn't implement lsp-communication in luasnip, but that part could be handled by rust-tools?

[augustocdias]

I believe that would be the end goal. Providing the API to call from lsp handlers.

[simrat39]

@L3MON4D3 yes the lsp communication will be handled by rust-tools. We override vim.lsp.utils.apply_text_edit, not ideal but it works so it's whatever, just need the snippet plugin to apply the snippets

[L3MON4D3]

I added functionality to expand lsp-snippets at any position, check here. Does that work for you?

[L3MON4D3]

Just realized that link doesn't actually get you anywhere, just scroll all the way down and look for lsp_expand

[simrat39]

i'll play around with it, thanks for the support

[simrat39]

This works mostly, but one thing missing is that TextEdits give back a range, which can be used for example replacing multiple lines of text with something else, idk if it's in the scope of this plugin to support that but we would need that.

This is how neovim does it https://github.com/neovim/neovim/blob/master/runtime/lua/vim/lsp/util.lua#L329

[L3MON4D3]

Ah, no I wouldn't like to implement that in Luasnip, srry.

[simrat39]

that's fair, i'll try to see what I can do on rust-tools' side then

[anstadnik]

Hi @simrat39, is there a follow-up to this? I found simrat39/rust-tools.nvim#74 which is closed, does it mean that you won't be able to support the feature?

P.S. I love the amount of quality profile pics in this issue

[L3MON4D3]

Ah, no I wouldn't like to implement that in Luasnip, srry.

I'm going to backtrack on my stance here, it doesn't make sense to require each plugin that wants to utilize luasnip for snippetTextEdits to write their own conversion from snippetTextEdit to luasnip-expand-call.

I'll add an implementation for applying a single snippetTextEdit, would that be enough @simrat39? (Applying textEdits interleaved with snippetTextEdits seems pretty complicated, is that required?)

[L3MON4D3]

(Taking a closer look, the main problem with applying multiple textEdits is that applying them in any order may cause the lines in the textEdit to no longer correspond to the correct position, but I think I know how to make that work properly)

[L3MON4D3]

Implemented a first version in branch snippet_text_edits, call as require("luasnip.extras.lsp").apply_text_edits(edits, bufnr, offset_encoding, apply_text_edit_func) (with apply_text_edit_func probably vim.lsp.util.apply_text_edits)

[simrat39]

Thanks for this, I'll check it out and point out issues if any! Would be nice if we finally get this into luasnip and rust-tools

[simrat39]

Ok so this works pretty good, thanks for the work. One small issue though, for some reason rust-analyzer sends multiple snippet text edits in one call, even though their docs say that they won't do that, but the extra ones they sent all had empty newText, which is also weird. I think I'll bring this issue upstream and see what the rust-analyzer people say.

[simrat39]

Btw i pushed a separate branch if anyone wants to test it out. https://github.com/simrat39/rust-tools.nvim/commit/95a6f90b4ba290fa07662a660286cde60cc35831

Use it like this:

rt.setup({
  tools = {
    snippet_func = function(edits, bufnr, offset_encoding, old_func)
      require("luasnip.extras.lsp").apply_text_edits(
        edits,
        bufnr,
        offset_encoding,
        old_func
      )
    end,
...

[L3MON4D3]

Ok so this works pretty good, thanks for the work.

Nice, you're welcome :D

One small issue though, for some reason rust-analyzer sends multiple snippet text edits in one call, even though their docs say that they won't do that, but the extra ones they sent all had empty newText, which is also weird.

Mhmmm that's annoying, I guess we could filter it here, but would be nicer to accommodate for languageserver-quirks in their respective plugins imo. Wdyt?

I think I'll bring this issue upstream and see what the rust-analyzer people say.

Sounds good👍

[L3MON4D3]

Oh we could also just handle multiple snippetTextEdits by inserting the into the jump list one after the other, and then jumping back into the first placeholder of the first snippet. The main problem then becomes determining the order of the snippets

[simrat39]

Mhmmm that's annoying, I guess we could filter it here, but would be nicer to accommodate for languageserver-quirks in their respective plugins imo. Wdyt?

Yeah for sure, but I still think it should be clarified upstream first, their docs and their implementation should match.

[L3MON4D3]

Oh yeah, no question there :+1:

[hrsh7th]

For the reference, the rust-analyzer's VSCode client implements applySnippetTextEdit.

https://github.com/rust-lang/rust-analyzer/blob/master/editors/code/src/snippets.ts#L38

[L3MON4D3]

Oh, thank you for sharing! Looks like they expect snippets to only contain $0, so I'll assume langugage-servers will limit themselves to just that :/

But at least they'll allow multiple snippetTextEdits, I'll look into accomodating that here. Chaining multiple snippets together sounds cool+is definitely feasible :D

[hrsh7th]

The VSCode seems to now support SnippetTextEdit in the SnippetSession.

https://github.com/microsoft/vscode/blob/main/src/vs/editor/contrib/snippet/browser/snippetController2.ts#L91 https://github.com/microsoft/vscode/blob/main/src/vs/editor/contrib/snippet/browser/snippetSession.ts#L526

I think the time has come to support these functionality in the snippet engine side.

[L3MON4D3]

True! Are you aware of a formal specification of the snippetTextEdits vscode expects, and how it handles them? I'm not at all familiar with vscode's way of handling snippets, so I can't infer much from just the code, unfortunately (also, might still be a while until I can really get into this :/)

[L3MON4D3]

@simrat39 Would you test #577 again? Multiple snippetTextEdits in one should now work. To test, try

require("luasnip.extras.lsp").apply_text_edits(
{ {
    insertTextFormat = 2,
    newText = "${1: lolo} adsffff ${2: lele}",
    range = {
      ["end"] = {
        character = 16,
        line = 2
      },
      start = {
        character = 0,
        line = 2
      }
    }
  }, {
    insertTextFormat = 2,
    newText = "${1: lolo} adsffff ${2: lele}",
    range = {
      ["end"] = {
        character = 0,
        line = 3
      },
      start = {
        character = 16,
        line = 2
      }
    }
  }, {
    insertTextFormat = 2,
    newText = "\n\nimpl std::fmt::Debug for Bruh {\n    $0fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {\n        f.debug_struct(\"Bruh\").field(\"a\", &self.a).finish()\n    }\n}",
    range = {
      ["end"] = {
        character = 1,
        line = 5
      },
      start = {
        character = 1,
        line = 5
      }
    }
  } }, 1, "utf-8", vim.lsp.util.apply_text_edits)

(stolen from that issue you opened :D)

One problem is: if the server sends a snippetTextEdit with newText="", we will just insert a i(0), which causes one jump to end up there, which does not seem expected. We probably need to do some special handling of those :(

[L3MON4D3]

To test, try

Oh, or the call you have in your plugin, that should still work

[IndianBoy42]

So now 3.18 has brought SnippetTextEdit into the core lsp protocol. I wonder what is the best way to make sure LuaSnip is always handling them?

[DropDemBits]

One problem is: if the server sends a snippetTextEdit with newText="", we will just insert a i(0), which causes one jump to end up there, which does not seem expected. We probably need to do some special handling of those :(

I think it's okay to consider this situation as a server bug and just treat it like a regular text edit? In rust-analyzer's case, that came from always making every TextEdit into a SnippetTextEdit if there were any text edits in the document that used snippets. This isn't the case anymore thankfully, as rust-analyzer now applies the insertTextMode more granularly. 😀

So now 3.18 has brought SnippetTextEdit into the core lsp protocol. I wonder what is the best way to make sure LuaSnip is always handling them?

3.18 represents SnippetTextEdit in a different way than the rust-analyzer extension (SnippetTextEdit has a snippet field which is a StringValue, StringValue has kind and value which is snippet, and the text to insert respectively). I don't imagine it'd be too hard to also handle this representation too.

[L3MON4D3]

So now 3.18 has brought SnippetTextEdit into the core lsp protocol

Oh cool, thanks for letting me know!

I think it's okay to consider this situation as a server bug and just treat it like a regular text edit?

Mhmm, yeah that seems reasonable.. just look for $0, if it doesn't exist -> regular text edit :)

I can look into handling the new format, but it'll take a while :/

[mrcjkb]

Hey :wave:

I've just ported the changes from the rust-tools branch (which has been archived) to rustaceanvim: https://github.com/mrcjkb/rustaceanvim/pull/420

My initial impression is that it works well. Please let me know if there's anything else you would like me to test :smile:

[mrcjkb]

3.18 represents SnippetTextEdit in a different way than the rust-analyzer extension (SnippetTextEdit has a snippet field which is a StringValue, StringValue has kind and value which is snippet, and the text to insert respectively). I don't imagine it'd be too hard to also handle this representation too.

Since the rust-analyzer implementation is experimental, it's likely that it will be replaced to the upstream one soon: https://github.com/rust-lang/rust-analyzer/issues/16604

[L3MON4D3]

Oh, great! I'm completely OOTL on nvim-lsp right now, so: does neovim already support these/is there a PR for it? We should adopt that/agree on one so switching from luasnip<->nvim-builtin snippets is as painless as it is currently in nvim-cmp, where only that one callback needs to be defined.

[mrcjkb]

Afaict, Neovim doesn't support them yet and there's no issues or PRs.

We should adopt that/agree on one so switching from luasnip<->nvim-builtin snippets is as painless as it is currently in nvim-cmp

Agreed.

@MariaSolOs I hope you don't mind me tagging you, since you implemented snippets in core and might be interested 😃

[mrcjkb]

Related: https://github.com/neovim/neovim/issues/25696

[MariaSolOs]

Related: neovim/neovim#25696

@mrcjkb I wouldn't say that this is exactly part of the snippet roadmap. I haven't read the entire discussion but I believe all we need is Neovim being able to handle snippet workspace edits as documented in https://github.com/microsoft/vscode-languageserver-node/pull/1343. If you want, I would suggest creating a separate issue in Neovim to track the work.

To do this we would need to update the client's capabilities and expand snippets when returned as the workspace edit of a code action. As long as rust-analyzer sends legit LSP snippets, I don't think supporting this will be too hard in core.

[mrcjkb]

@MariaSolOs

I wouldn't say that this is exactly part of the snippet roadmap

That's not what I was implying 😃 I just thought you might be interested because you've been involved in snippet support in core in general. And we could probably reuse much of your prior work.

I will look into opening an issue in Nvim later. Thanks for the input.

As long as rust-analyzer sends legit LSP snippets

Its snippets are "legit", but the current implementation is off-spec. I think it would be best for Neovim and LuaSnip to implement an interface that supports the upstream spec only, and for rustaceanvim to provide an adapter between rust-analyzer's implementation and the upstream interface (at least until rust-analyzer switches to the upstream version).

[MariaSolOs]

I just thought you might be interested because you've been involved in snippet support in core in general. And we could probably reuse much of your prior work.

Got it, I didn't mean my reply to sound snappy <3

I think it would be best for Neovim and LuaSnip to implement an interface that supports the upstream spec only, and for rustaceanvim to provide an adapter between rust-analyzer's implementation and the upstream interface (at least until rust-analyzer switches to the upstream version).

Absolutely. Sticking to the LSP as much as possible and avoiding extension or server specific hacks will definitely makes everyone's jobs much easier.