saadparwaiz1 / cmp_luasnip

luasnip completion source for nvim-cmp
Apache License 2.0
712 stars 32 forks source link

fix(source:complete): missing lazy-loaded items #19

Closed captainko closed 3 years ago

captainko commented 3 years ago

The reason for this PR is that there may be some snippets that don't make it into the completion result.

Only some snippet items were shown on the completion menu.

Item found

As you can see the lreq is listed in LuaSnipListAvailable command result. LuaSnipListAvailable

But it's missed from the completion result image

And this is the expected result. image

saadparwaiz1 commented 3 years ago

I'm not quite sure about this as the point of caching things was to avoid iterating over the snippets for every request. We can get the same behaviour by just removing caching?

We could possibly just empty the cache when everything is lazy loaded. Which should solve the issue?

L3MON4D3 commented 3 years ago

Luasnip could emit some event as soon as new snippets are lazy-loaded (maybe pass the filetype(s)), listening on that and refreshing/adding the new snippets should work nicely.

captainko commented 3 years ago

Luasnip could emit some event as soon as new snippets are lazy-loaded (maybe pass the filetype(s)), listening on that and refreshing/adding the new snippets should work nicely.

Yeah, that would be great. If that's possible then we no longer have to iterate over the snippets for every request.

L3MON4D3 commented 3 years ago

Check out Luasnip/load_event, User LuasnipSnippetsAdded is triggered upon lazy-loading new snippets. The filetype of the snippets that were loaded can be accessed via require("luasnip.session").latest_load_ft.

saadparwaiz1 commented 3 years ago

Check out Luasnip/load_event, LuasnipSnippetsAdded is triggered upon lazy-loading new snippets.

The filetype of the snippets that were loaded can be accessed via require("luasnip.session").latest_load_ft.

This looks great. @captainko do you want to update the PR if not I can make a new one

captainko commented 3 years ago

I will update the PR ASAP.

captainko commented 3 years ago

@L3MON4D3 could you create a hook instead of vim User Event. I could register a callback instead of relying on the require("luasnip.session").latest_load_ft. I have looked into from_vscode and wondering if LuaSnip could emit an event there too. If you agree I could create a new feat PR at LuaSnip to address this issue.

L3MON4D3 commented 3 years ago

I'm not sure if the added complexity is worth it, we'd have to register and store callbacks and call all of them vs. just doautocmd and having users access that variable. And you're right, we should only trigger the event there asload() is asynchronous, so we can't be sure that all snippets are loaded before the first call to completion (could especially happen if there's some conditional loading via packer), I'd welcome a PR to change that :D

saadparwaiz1 commented 3 years ago

imo we should just use the loader event and empty the relevant filetypes. It's a pretty simple change no need to over complicate things

captainko commented 3 years ago

imo we should just use the loader event and empty the relevant filetypes. It's a pretty simple change no need to over complicate things

Yeah, I have overcomplicated it. The main reason for that is because I want to build the completion items as soon as we received the filetype change events instead of wait for cmp to call the complete() method. Now I see there's no need for that :smile:

captainko commented 3 years ago

I'm not quite sure about this as the point of caching things was to avoid iterating over the snippets for every request. We can get the same behaviour by just removing caching?

I have updated the PR. Now we just remove the cache when we receive the LuasnipSnippetsAdded event.

captainko commented 3 years ago

Can you avoid adding the function to global name space and add it to the module instead.

Also please move the autocmd to existing augroup in after/plugin/cmp_luasnip.lua

Will do.

captainko commented 3 years ago

Resolved by author