LuaSnip
LuaSnip copied to clipboard
feat: add fallback if no paths are currently loaded
This PR adds a fallback if no files are found in the current cache. This allows configs like the following:
require("luasnip.loaders.from_snipmate").lazy_load()
vim.api.nvim_create_user_command("LuaSnipEdit",
function()
require("luasnip.loaders").edit_snippet_files {
fallback = function(ft, edit)
if #ft > 0 then
local filename = "~/.config/nvim/snippets/" .. ft .. ".snippets"
print("Creating new file: " .. filename)
edit(filename)
else
print("Canceling")
end
end
}
end, {})
Pretty minor change, should be self explanatory - basically it calls the fallback function, passing in the chosen filetype and the edit function. Main use case is to add file for current filetype if it does not already exist, but could be used for other things as well.
I made this because there was no easy way to tell if a file already exists and add it if it doesn't (at least not that I could tell). If I'm mistaken about this, I'd be interested in what the current preferred method is for this.
If I need to add docs or anything, just let me know!
Hey, thank you for participating, always happy to see some PRs from new ppl :)
Creating new snippet-files via :LuaSnipEdit is a great idea, should improve that workflow :+1:
One downside I can see with this implementation is that it is not possible to add a personal snippet-file if there already is one for this filetype, provided by some collection.
What do you think of, instead of fallback, providing a more general extend-callback, which gets a list of the files found by luasnip (ft_paths), and can return additional ones (as a list of {item_name, path}-pairs), which will then also be displayed in the second vim.ui.select-dialog?
For creating a new file, one could check the passed paths, if there is no item from the personal snippet-collection, an item for creating that file can be returned, maybe even with a hint that this selection creates a new file.
How does this look?
One thing I would say to note is that line 58 in loaders/init.lua will result in an error if the user-provided function does not return a table (e.g. it returns nil). This could be changed by adding ... or {} to the end of the line, but I thought it was probably best to leave it as is rather than force ignoring the type-error. But I'm open to your ideas about this.
BTW, this is an example of how I'm using it for reference:
vim.api.nvim_create_user_command("LuaSnipEdit", function()
require("luasnip.loaders").edit_snippet_files {
extend = function(ft, paths)
if #paths == 0 then
return {
{ "$CONFIG/" .. ft .. ".snippets",
string.format("%s/%s.snippets", pc_path, ft) }
}
end
return {}
end
}
end, {})
How does this look?
Looks great, I've no complaints :+1:
Could you add a description of what extend does (and maybe an example of what it can be used for) here, in DOC.md (oh, and update that line stating that opts only contains one setting :sweat_smile:)?
One thing I would say to note is that line 58 in loaders/init.lua will result in an error if the user-provided function does not return a table (e.g. it returns nil). This could be changed by adding
... or {}to the end of the line, but I thought it was probably best to leave it as is rather than force ignoring the type-error. But I'm open to your ideas about this.
It's okay, as long as the doc specifies that extend should return a table. You could add an assert that extended actually is "table" though, directly point out misuse.
Sounds good. I won't be able to get to this for a while (probably at least a week and a half), just so you're aware why there isn't any progress.
Take your time :+1:
Hey, will you have time to finish this feature in the near future? (No worries if not, I'll take over and complete this, I think it's a nice addition)
Hey, will you have time to finish this feature in the near future? (No worries if not, I'll take over and complete this, I think it's a nice addition)
It's still my intention to get to it eventually, but I'm not sure when. I was on vacation in August, and life's been busy since getting back. TBH I haven't turned my workstation computer on since I got back.
So yes, but I'm not sure I can give you a timeline. I won't be offended if you just take over now, but I'm still willing to get to it eventually if you don't.
Completely up to you.
Ahh, alright :D I'm in no real hurry to get this in, so you can continue as soon as you find time for it (just wanted to make sure you're still up for finishing this :sweat_smile:)
Updated docs, added assert for extended. Is there anything else?
Ah nice :+1: I'll have look
How's this?
Very nice, great work :+1: Could you squash the commits?
(oh, and maybe rebase, that would be nice :D)
Lol, I don't actually know how to do that...I'm still learning how to use git properly. I will google it and get back with you.
Or you're welcome to tell me how to do it too, that way I know for sure I'm doing the right thing. Up to you.
I'm always squashing via git rebase -i HEAD~<numOfCommits> (important that the number isn't too big) and then interactively deciding to squash (maybe dropping the auto gen docs commits to avoid merge conflicts on the doc/luasnip.txt afterwards (your changes regarding the docs are in Doc.md and shouldn't get lost by that)).
Regarding the rebase on the master branch, I'd recommend to read on that (but from a users perspective it's not that different from a merge git rebase master -> fix the merge-conflicts).
Important:
Just be aware that yo're rewriting the commit history with these methods (rebase). That means in order to push afterwards, you'll need a force push -> git -f push and that you might mess up the history, so maybe you want to make a backup of your local repo (the folder with .git in it) in advance just to be sure.
(I'd do that stuff myself, but if I rebase, the commits would seem to be made by me, not you)
Ok, finally got around to this. Sorry for the wait (again 👀).
Did I do everything right? Is there anything else you need before merging?
Oh, I think something went wrong, theres some difference between this branch and master 250 commits ago, which should not be the case. I can take a look at this tomorrow
I think that the merge commit is breaking the history, it will be cleaner to remake the branch starting from the current master and cherry-pick the commit with the wanted changes only.
That did it :+1: I also reworded the doc a bit, @pianocomposer321 if you're happy with that I'll go ahead and squash->merge
Looks good to me 👍
I think that the merge commit is breaking the history, it will be cleaner to remake the branch starting from the current master and cherry-pick the commit with the wanted changes only.
Also sorry about this. Glad you figured it out ;-).
Wonderful! Took a while, but we're there now :D Thank you!
Awesome! Sorry again for the long waiting periods and for the noobishness lol. Thanks for bearing with me...this was the first major project I've contributed to, and it was a great first experience. I learned a lot. Thanks again!
Oh wow, thank you, that's great to hear :D Don't worry about noobishness (and waiting periods especially, we're all doing this in our free time, some have more, some less), the actual code was pretty much perfect after your first iteration, it's just the organizational (documentation+git) stuff you were "noobish" at all in. I'm certain especially those will get easier with more experience :) Looking forward to more contributions from you, don't hesitate if you have an idea/are interested in helping out on issues