search

hrsh7th / nvim-cmp

A completion plugin for neovim coded in Lua.
MIT License
7.78k stars 385 forks source link

Sources/Completion item: Store non-copyable data. #851

Open L3MON4D3 opened 2 years ago

L3MON4D3 commented 2 years ago

Hi!

It looks like the data-fiels of completion-items is copied at some place, which can cause issues if data contains complicated structures eg. recursive tables.

The specific issue is with cmp_luasnip, where we currently (because storing a reference doesn't work due to copying) have to store what amounts to an iterator, which might be invalidated (see here), making it necessary to rebuild the entire cache, even though no items were changed.

I can see thre ways to solve this:

  1. preserve references and recursive tables while copying
  2. Don't copy/provide a way to store non-copied values in data
  3. Do some more work on luasnips side to provide keys that can be resolved to snippets

IMO 2 would be pretty nice, because there's no need to copy the snippets, at least in cmp_luasnip case. I wouldn't be opposed to 3, but it would only solve this problem for cmp_luasnip, not for other sources (which might be enough, I'm not sure if any other source has a similar problem). 1 doesn't sound good, I don't think there's any cases where big, complicated structures should be copied, and it would probably worsen copy for simple tables.

I'm not sure if it's possible to do already (didn't find anything about it if it is :sweat_smile:)

Thank you for your work on nvim-cmp :heart:

hrsh7th commented 2 years ago

I think the data field is expected as serializable and copied data (because the original protocol is LSP that protocol is communicate the server and client via TCP socket or studio.

Sorry. I might not understand the problem. Could you explain more easily?

L3MON4D3 commented 2 years ago

Sorry. I might not understand the problem. Could you explain more easily?

Yeah, of course. cmp_luasnip is the source for luasnip, so snippets have to be expanded in source:execute, which means we need some way to find out which snippet a completion-item refers to. The snippets are not serializable, so we currently pass an index into some table in the data-field of the completion item, but this is going to become more cumbersome soon (due to some changes in luasnip) + wasn't that nice to start with.

We basically need some way to reference a non-serializable snippet in the completion-item, which proves to be kind of hard because all of the completion-item seems to be copied.

I assumed that data is the only place in the completion-item where completion-source specific values can be stored, is that even correct?

smjonas commented 2 years ago

I assumed that data is the only place in the completion-item where completion-source specific values can be stored, is that even correct?

I think this is not correct, e.g. for cmp-nvim-ultisnips we use a custom snippet attribute for the completion item and it's working fine. So should be able to use arbitrary keys. I am not sure if that helps you for actual problem though ^^ No idea if everything in the item table is copied or only parts of it..

L3MON4D3 commented 2 years ago

I just tried it (which I could've done like 5 hours ago xD), seems like the entire completion-item is copied :/

I'm not certain if this really is something useful to completion-sources other than cmp_luasnip. If it isn't, and if it isn't trivial to add to cmp, we'll just implement some snippet -> key, key -> snippet-mechanism in luasnip and store keys.