saadparwaiz1 / cmp_luasnip

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

Add snippet completion for injected languages #28

Closed ner0-m closed 2 years ago

ner0-m commented 2 years ago

Hi, I've been playing with neorg lately and at the same time writing a lot of latex stuff. But if I'm in a code block such as

@code latex
... some latex code
@end

I only get snippets for the file type of the file (norg) and all.

I thought it would be cool to use treesitter to find the language at the cursor position and add those snippets as well. What do you think? :-) Any feedback is appreciated!

saadparwaiz1 commented 2 years ago

Hi, this seems like a great feature. I am a bit conscious about the perf impact. Maybe make this an optional feature that people can opt-in?

Pinging @L3MON4D3 as well in case he has any specific comments

L3MON4D3 commented 2 years ago

Oohh that's really cool! What do you think about implementing it in Luasnip instead? That way even regular expansion could be treesitter-aware.

ner0-m commented 2 years ago

I am a bit conscious about the perf impact

I've had the same thought, Do you have any idea on how I could test that?

I'm very happy to implement it in luasnip. I hope, I have time tomorrow.

One question that I just thought about. Do you expect the get snippets for the filetype of the file as well as for the injected code, or just for one of them. So in my case do you expect to get snippets for latex, norg and all, or just for latex and all?

L3MON4D3 commented 2 years ago

IMO treesitter ft-only sounds right, but I could imagine there being cases where one would want snippets for both, so a per-ft-setting should be best.

saadparwaiz1 commented 2 years ago

Should this be closed now?

L3MON4D3 commented 2 years ago

Yeah I think so, cmp_luasnip should (haven't actually tried it yet :D) the filetype for injected languages from get_snippet_filetypes. One thing, but minor, would be not passing params.context.filetype there as it doesn't do anything now.

saadparwaiz1 commented 2 years ago

@ner0-m do you want to update this PR to just remove the unused arguments :) or I can create a new pr

saadparwaiz1 commented 2 years ago

closing as I haven't heard anything :)

ner0-m commented 2 years ago

Sorry for not getting back, absolutely fine to close it. I might be able to do the PR to remove the default argument, but I can't really tell when I'll find time.

saadparwaiz1 commented 2 years ago

@ner0-m no worries have updated the plugin now :)