Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snippet Priority #268

Closed
L3MON4D3 opened this issue Jan 19, 2022 · 9 comments
Closed

Snippet Priority #268

L3MON4D3 opened this issue Jan 19, 2022 · 9 comments

Comments

@L3MON4D3
Copy link
Owner

There is no explicit way to give snippets a priority, only implicitly through the order they are added to the ls.snippets.<filetype>-table (scanned sequentially, return on first match).
As there are some limitations on ordering snippets as well (adding lua-snippets behind those loaded from a snippet-package (see #267), no defined order of snippets loaded from different packages) we should properly implement priority.

  • The priority should probably be per-snippet
  • Allow passing prio to load-functions to set it on the whole collection (allows overriding snippets from something like friendly-snippet with custom ones)
  • Add functions for adding multiple lua-snippets with a given priority

That's all the requirements I can think of, feel free to suggest more :)

@L3MON4D3
Copy link
Owner Author

#334 implements a first attempt at allowing snippet-priorities, priorities can only be used manually for now, but that will be improved.

@leiserfg
Copy link
Contributor

leiserfg commented Mar 13, 2022

IMHO user-added snippets should have a higher priority than loaders by default (so they don't get overridden by external ones), and loaders should write directly to the inner structure to avoid the meta-table (that last one is maybe an overoptimization but measuring is the only way to know).

@L3MON4D3
Copy link
Owner Author

IMHO user-added snippets should have a higher priority than loaders by default (so by they don't get overridden by external ones)

I agree, but I'm not sure how we can separate them. Loaded snippets may also be user-added and not from some snippet-library.

loaders should write directly to the inner structure to avoid the meta-table (that last one is maybe an overoptimization but measuring is the only way to know).

Right, that's something to keep in mind if just setting them turns out to be too slow👍

@leiserfg
Copy link
Contributor

Maybe setting the snippets added to the snippets table a high prio if not specified, and a default lower one to the to the loaders, plus adding the ability to pass the prio to the loaders.

@L3MON4D3
Copy link
Owner Author

Mhmm maybe, that would definitely be smoother as long as snippets are mainly set via lua.
The loaders aren't able to exclude a specific path, right?
I think that would be nice to have for loading all snippets from libraries with one, and all personal snippets with another priority (otherwise one would have to specify the path to eg. friendly-snippets, which isn't that nice imo)

@leiserfg
Copy link
Contributor

I can think of two options:

  • Extend the package.json to contain the priority of the collection (this only work for vscode)
  • Add a priority arg to the load function that is (lets say) 500 by default and that can take two kind of values, a number that will be assigned to all loaded collections, a function that takes the collection path as argument and returns the number for the prio to assign.

@L3MON4D3
Copy link
Owner Author

Extending the package.json sounds like a good idea, maybe we can get friendly-snippets (or other collections) to set theirs to 500.
I'm not that convinced by the function for prio, that would become more cumbersome with more parameters that can be applied to load, eg loading one collection with one set of parameters and another with a different set of parameters would mean lots of long functions that are much less readable than two loads including one path and one set of parameters each.

I'd also extend the current luasnip-field inside the vscode-snippets with a priority, that would allow setting the priority for a single snippet only.
What do you think of (as params to load) override_prio (applied to all snippets, regardless of snippet-prio) and default_prio (only applied to snippets without default-priority)?

@L3MON4D3
Copy link
Owner Author

Okay, priority can now be set in the call to add_snippets or in the snippet-constructor (DOC.md has some details).

What's missing is using priority in loaders, I'm merging the patches without it because I want to get the breaking changes out.

@L3MON4D3
Copy link
Owner Author

L3MON4D3 commented May 15, 2022

Done (see here) 🎉 :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants