Snippet Priority.

[L3MON4D3]

Allow giving snippets a priority.
For now, only the groundwork is implemented, I'm thinking of adding functions like add_snippets(filetype, snippets, opts), where opts can be used to specify priority, autosnippets and similar, but it'll still be possible to set snippets with a default-priority (1000, for now) directly via ls.snippets.
This PR will also include patches for passing a priority to (lazy_)load

[L3MON4D3]

One rather subtle problem: the current approach to lazy loading lua-snippets (__index on ls.snippets, now ls.session.by_prio.snippets[1000]) won't work completely: if snippets with a higher priority (<>1000) are added in __index, they won't show up on the first expansion because they are only added after these tables were already searched.
Also, it'd suck because the table we're iterating over (ls.session.by_prio.snippets.order) is appended during iteration, and I'm not sure how lua handles that.

[leiserfg]

That's because our loaders are a hack on top of the original implementation that was thought to only have lua written snippets 😉. What do you think of adding a different table for the loaders and keeping ls.snippets only for user config, and integrate that one after the first call to list the snippets? With the tradeoff of having to notify the guys calling luasnippet to list plugins (like cmp_luasnip) to adopt the now API, but I prefer to change the API than changing the "UI".

[leiserfg]

I mean, using the __index keeps the UI and API but is a bit hacky and can easily become a footgun in performance and behavior.

[L3MON4D3]

Oh, I meant the other __index-hack xD
It currently loads high-prio snippets too late during the first expansion.

I think having all snippets in one table is a bit nicer, but you're right about the downside of heavily utilizing __index.
I'm considering phasing out adding snippets via direct access to ls.snippets (or any table), only allowing it by calling some function.
That would make everything work pretty nicely (no more breaking changes due to the changing the internal snippet-storage) from now on, we could even do optimizations like sorting the snippets by last trigger-char, or building multiple views on all snippets (eg. ls.session.by_filetype, which would be much more useful to list-plugins).
What do you think?

[leiserfg]

The idea seems nice, we could provide a function that takes a table just like the one we have now so users can replace do:

ls.snippets = { ... }

with

ls.define_snippets{...}  -- or any other name

And maybe a __newindex in ls to complain if someone tries to set ls.snippets

[L3MON4D3]

Great, it won't look as nice though, we'll still need some arguments, I'm currency thinking of add_snippets(ft, snippets, opts), where opts only has prio, for now.

The best way to introduce that change would probably be to deprecate direct-adding soon while introducing the new functions everywhere (and maybe printing a depreciation-warning via __newindex), then after two weeks (I think that should be enough) we can make adding via functions the only possible way of adding snippets.

[L3MON4D3]

Looks like using add_snippets, at least for honza/vim-snippets, is even slightly faster, I'm getting 68ms -> 61ms
pre:

post:

There's probably a slight slowdown with adding lua-snippets (against just setting ls.snippets), haven't tested that yet.

[L3MON4D3]

Before this can be merged, we'll have to switch to only adding snippets via a function, at least that will make the merge much smoother and allow us a deprecation-period where plugins that depend on luasnip can adapt to the changes.

[Isos9]

hi, just a question (might be a bit related to this PR), do you think it would be possible to implement a "noremap" option to a trigger, meaning that it will override the others snippets loaded from external sources for exemple, and not "coexist" feeling like it's duplicated because it has the same trigger.
Btw thanks for the plugin, it has a lot of potential!

[L3MON4D3]

Yeah that could be doable. The underlying problem is that duplicate trigger lead to duplicate entries in eg. nvim-cmp, right?

[Isos9]

Yes, so there's two cases which cause that, first it's when I create my own snippet, for exemple if have my own console.log snippet with same trigger as the one loaded from vscode snippets, there's a duplicate, and the second one is from extended_types which can often bring duplicates.

[L3MON4D3]

Would changing the label in cmp to the snippets name (which can be changed already, it just defaults to the trigger) work for you?

[Isos9]

What do you mean ? do you have an example ?

[L3MON4D3]

Currently cmp_luasnip shows the snippets' trigger in the menu, showing the name (which can be different) might make more sense because that can easily be changed:

s({trig = "trigger", name = "name"}, {...})

the name just defaults to the trigger, but for your purposes it might be the easiest fix.

[Isos9]

Doesn't really help because I don't check the description of it, usually I just expend the first directly, here's an example, and I set a different name, but same trig.

[L3MON4D3]

Oh, with menu I meant the PUM in the screenshot. One snippet could show up as eg. my-log or log (lua) and the other as log (c).

Duplicate snippets from loaders can be prevented by iterating over all snippets and removing the duplicates, but that won't be possible with snippets from ft_extended filetypes because the table/snippets used via extend_ft are the same ones that are used normally for that filetype, so snippets in these tables cannot be removed for one filetype only.

[L3MON4D3]

I probably won't be able to properly resolve all merge conflicts today, so no Breaking Changes for now :(

[L3MON4D3]

This PR will not yet add priority to the loaders, only to add_snippets. The loaders would require some more restructuring, and I don't have much time at the moment, so this will probably be implemented later.

[Isos9]

Hi, sorry I forgot to answer you, but the priority feature actually did the job, and also used filetype_set instead of extend.
Thanks for your help