Feat(autosnippets)

[atticus-sullivan]

Successor of #27 Up to this point this is just a rebase of [at]saadparwaiz1 changes on the current master master.

Tested this and it seems to be working except if the new entry in the completion menu is selected (text is inserted -> snippet gets expanded) there is an error when trying to get the documentation.

See this example. lfn is expanded automatically, but there is an error when selecting it. The error is

Error executing vim.schedule lua callback: ~/coding/cmp_luasnip/lua/cmp_luasnip/init.lua:34: attempt to index local 'snip' (a nil value)
stack traceback:
        ~/coding/cmp_luasnip/lua/cmp_luasnip/init.lua:34: in function 'get_documentation'
        ~/coding/cmp_luasnip/lua/cmp_luasnip/init.lua:139: in function 'resolve'
        ~/.config/nvim/plugged/nvim-cmp/lua/cmp/source.lua:366: in function 'resolve'
        ~/.config/nvim/plugged/nvim-cmp/lua/cmp/entry.lua:485: in function 'resolve'
        ~/.config/nvim/plugged/nvim-cmp/lua/cmp/view.lua:234: in function 'fn'
        ~/.config/nvim/plugged/nvim-cmp/lua/cmp/utils/async.lua:55: in function <~/.config/nvim/plugged/nvim-cmp/lua/cmp/utils/async.lua:53>
Press ENTER or type command to continue

I'm not quite sure where this is coming from, maybe after expanding the snippet the completion_item in source:resolve() is invalid (seems like the item_snip_id is nil). We can of course catch this case completion_item.data.snip_id, but I'm not sure if it's always the case that completion_item.data is available (attempt to index nil value on the level before otherwise). So my trouble in fixing this is, that I don't know why snip_id is unset and as long as I don't know this, I'm unsure on what values being set I can rely on.

In my test completion_item.data.auto is set (so we can skip autosnippets in the resolve function), but I'm not sure if this is/should always be the case.

Does anyone have a hint on this one?

[atticus-sullivan]

Checked if hidden works on autosnippets as well -> check

[L3MON4D3]

Oh, snip.id is just not set on the items I think, check L91 vs L109

[atticus-sullivan]

Nice, thank you. Must have overlooked that one when merging. So then this works for me and my little test. Not quite sure if @wuilliam32 will respond in the old PR (#27) and what he meant (EDIT: oh maybe this refers to tabs vs spaces, if so it should been fixed by now) Then this should be ready so far

[mathjiajia]

Is it possible to show the expandable snippets only? cf. https://github.com/quangnguyen30192/cmp-nvim-ultisnips/blob/f90ebb220306e39766ad0ec1f094e4e12bb2fdd4/lua/cmp_nvim_ultisnips/snippets.lua#L8

@L3MON4D3, or do you think it is a good idea to set show_condition to be luasnip.expandable by default?

[L3MON4D3]

@L3MON4D3, or do you think it is a good idea to set show_condition to be luasnip.expandable by default?

Slightly off-topic, but here goes: I think show_condition is the way to go here, but luasnip.expandable isn't the right function, it only returns whether any snippet is expandable at the current position. You could adapt the function used internally (here) for checking whether a snippet matches.

I do wonder: Why?? :sweat_smile: Do you have multiple snippets that are expandable, and you'd like to choose the correct one? I'd suggest using different triggers if that's the case, sounds much more direct

[mathjiajia]

I do wonder: Why?? 😅 Do you have multiple snippets that are expandable, and you'd like to choose the correct one? I'd suggest using different triggers if that's the case, sounds much more direct

The reason is, I have multiple regTrig autosnippets, and they will show-up every time even if I haven’t finished the whole trig.

(with show autosnippets enabled by the current patch).

[L3MON4D3]

Hmm okay, that can be annoying. You could also set hidden on them, if that works for you.

@atticus-sullivan @saadparwaiz1 We should (I think) hide listing autosnippets behind some option, I at least don't want them listed. Any ideas how that should be managed? setup? Or does cmp have a standard way to provide options to sources?

[mathjiajia]

Hmm okay, that can be annoying. You could also set hidden on them, if that works for you.

Thanks for the suggestion. Yes, that is what I did. But I am wondering if there is a more efficient way, as you also mentioned.

[atticus-sullivan]

already partly thoght of hidden solving this. Though having an option which keeps the "legacy" behaviour sounds reasonable. Looking at the present code, I think this would be an option similar to use_show_condition stored in the params argument. But I'm no expert on the cmp stuff either.

[saadparwaiz1]

already partly thoght of hidden solving this. Though having an option which keeps the "legacy" behaviour sounds reasonable. Looking at the present code, I think this would be an option similar to use_show_condition stored in the params argument. But I'm no expert on the cmp stuff either.

Yeah this is usually how options are passed to cmp sources. I'd go with the same approach as use_show_condition

[L3MON4D3]

Ahh yeah, that approach sounds perfect👍

[saadparwaiz1]

@atticus-sullivan let me know when you want me to do a review and I'll then merge :)

[atticus-sullivan]

Yup :+1: , now there are a few things left to do, but shouldn't be too long

[atticus-sullivan]

@mathjiajia does this change solve your problem/handle your issue?

[mathjiajia]

@mathjiajia does this change solve your problem/handle your issue?

Yes. This option works and it is useful. Thank you.

[atticus-sullivan]

Alright @saadparwaiz1 then I'm through with this so far. (Please have especially a look at the readme.md as explaining to the user is always a bit tricky)

[saadparwaiz1]

Will have a look on fridsy thanks :)

[saadparwaiz1]

LGTM. Merging :)