Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add interface for applying snippetTextEdits #577

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

L3MON4D3
Copy link
Owner

@L3MON4D3 L3MON4D3 commented Sep 4, 2022

This can be useful for rust-analyzer and other lsp-servers implementing their extension.
Not 100% sure luasnip is really the best place for this, but it makes more sense than each languageserver-extension wanting to expand these snippets implementing it themselves.
(nvim also doesn't really fit since it's just an extension to lsp, and not official)

@L3MON4D3 L3MON4D3 linked an issue Sep 4, 2022 that may be closed by this pull request
@leiserfg
Copy link
Contributor

leiserfg commented Sep 4, 2022

Should not this also expose the companion lsp-capability that should be passed to the lsp-server? I mean, similar to what require('cmp_nvim_lsp').update_capabilities does.

@L3MON4D3
Copy link
Owner Author

L3MON4D3 commented Sep 4, 2022

Oh, yeah that makes sense.
Is there some standard for how the capabilities are updated? Or would one have to call update_capabilities(caps) manually?

@leiserfg
Copy link
Contributor

leiserfg commented Sep 4, 2022

Oh, yeah that makes sense. Is there some standard for how the capabilities are updated? Or would one have to call update_capabilities(caps) manually?

Yes, it has to be manually, but it's something that those developing plugins that require it could do, like in this case the guys of rust-tools.

If jumplist_insert is configurable, we should clean this up a bit.
IMO it might be expected that the links between a snippets and its i(0),
i(-1) exist, so we'll just add them.

If this is not desired, they can still be overridden easily.
This might break some previously-working jump_into_funcs.
Explained in one of the comments, essentially we want the
snippetTextEdits to be visited one after the other, beginning with the
first.
This is not the default-behaviour, and thus needs some extra work.
…ions.

In particular, snip.next.next of a i(0), or snip.prev.prev of a i(-1) is
often assumed to be the next/previous snippet, which is not the case if
we do the modifications from the parent.

Instead: just modify behaviour of i(-1), such that it is just ignored
while jumping.
@L3MON4D3
Copy link
Owner Author

L3MON4D3 commented Feb 2, 2023

Thanks for taking a look, I had some more stuff to add though :D
Applying snippetTextEdits should work pretty well now, but we have some more complexity too:
There's a new option for snip_expand, jumplist_insert_func, which can link up snippets in a different way. Problem is, this is subject to some restrictions, and should be done carefully, so I'll have to add some pretty thorough documentation for it (or not document it at all, which would go a bit against having everything in extras not depend on internals)
In a similar vein, there's a lot of stuff that needs API for this to be really well-made, so far it's more of a POC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for rust-analyzer's SnippetTextEdits
2 participants