-
Notifications
You must be signed in to change notification settings - Fork 34.5k
Upgrade folke/neodev (sunsetting) to folke/lazydev #961
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
Conversation
Since this is a dependency of the lsp would this not be loaded along with the lsp config regardless of filetype? |
-- used for completion, annotations and signatures of Neovim apis | ||
{ 'folke/neodev.nvim', opts = {} }, | ||
{ 'folke/lazydev.nvim', ft = 'lua', opts = {} }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid an "undefined field fs_stat
" LSP error on
Line 207 in 20fa60a
if not vim.uv.fs_stat(lazypath) then |
perhaps this should use the configuration suggested in https://github.com/folke/lazydev.nvim/tree/6184ebbbc8045d70077659b7d30c705a588dc62f#-installation
{ 'folke/lazydev.nvim', ft = 'lua', opts = {} }, | |
{ | |
'folke/lazydev.nvim', | |
ft = 'lua', | |
opts = { | |
library = { | |
-- Load luvit types when the `vim.uv` word is found | |
{ path = 'luvit-meta/library', words = { 'vim%.uv' } }, | |
}, | |
}, | |
}, | |
{ 'Bilal2453/luvit-meta', lazy = true }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got the sense that these are optional configs for those who want specific vim.uv LSP typings. Not everyone will need/want them. However, if there's no downside and or most people need them, I see no problem with it.
At the risk of bloat perhaps someone else can chime in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like, for a kickstart config, the suggested would be the preferred aproach. Anyone who doens't want this will propably be more aware of how to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it stands, on master
using folke/neodev.nvim
we don't see an "undefined field fs_stat
" LSP error. But with the change in this PR, we do see that error (unless we apply the suggestion above). That's why I think we might want to follow the configuration suggested in https://github.com/folke/lazydev.nvim/tree/6184ebbbc8045d70077659b7d30c705a588dc62f#-installation - Otherwise, merging this PR as-is will leave the fresh kickstart.nvim
installation with an LSP error in init.lua
, which IMO degrades the desired "first timer" experience.
Has this PR been abandoned? It seems like merging it will leave us with errors which is no bueno. Could we either fix or close? Thanks a bunch! |
@feoh This PR was cherry-picked into #936, which was just merged, so this PR can be closed. But this also means the LSP error discussed above is present in |
Thank you so much for being so incredibly thorough. I really appreciate having the links ready at hand so I can read up on the context and understand the nature of the fix and why it's important. Closing this, merging that :) |
Address issue #960