-
Notifications
You must be signed in to change notification settings - Fork 43.4k
Fix bug when server not explicitely defined in configuration but installed #376
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
|
Thanks for the fix! @tjdevries @mech-a can anyone validate this approach? Not so much for the fix itself as that seems valid to my eye but the approach he's taking to allowing additional filetypes to attach to a given LSP? I really should have asked for wider input on the initial change but got excited and merged it without enough testing, so I'm asking now :) Thanks folks! |
|
I think you could do something like this:
|
|
I personally would prefer if Kickstart would only call setup() on LSP servers I have defined in the servers variable above. I think that would cause the config to behave in a more predictable way since you would always have an explicit map of all the LSP's you have enabled right in your init.lua. The downside would be that everything would work less automatic for the people who do not modify their config at all and install LSP's through Mason manually without relying on the ensure_installed function. It probably is a bit of a coin toss which approach would be most useful for most people so I understand that its hard to have a perfect solution for everyone. @tjdevries |
|
@Numkil It's a tricky thing to balance the preferences of our myriad users in a project like this :) For every user who comments there are a hundred just quietly git clone-ing and moving on with their lives. Thanks for understanding. I tested @tjdevries proposed fix locally and it works, so if you could indeed update your PR we'll get this merged and everyone will be (mostly?) happy :) |
|
@feoh done! I might change the approach in my own fork since thats the spirit of this project haha 👍 |
|
Abso-freaking-lutely! One tip I wasn't aware of with forks: If you're encountering all kinds of crazy pulling down changes from upstream and you're using rebase, try merge, it will do the git auto-resolve magic much more effectively. I was having a hell of a time maintaining mine but now that I've switched from rebase to merge I only get dinged when there's an ACTUAL conflict. If you like maybe put a pointer to your refactor here once you're done. Always worth keeping an open mind and seeing how others are doing things! Thanks everyone for your comments and help with this. I feel like my Neovim Lua education is ongoing :) |
Fix bug when server not explicitely defined in configuration but installed
Fix bug when server not explicitely defined in configuration but installed
Fix bug when server not explicitely defined in configuration but installed
Fix bug when server not explicitely defined in configuration but installed
Fix bug when server not explicitely defined in configuration but installed
Fix bug when server not explicitely defined in configuration but installed
Fix bug when server not explicitely defined in configuration but installed
Fix bug when server not explicitely defined in configuration but installed
hi @feoh
The problem was that kickstart initializes all LSP's that are installed and not only the ones explicitly defined in your servers variable. I locally had not done this so I did not realize this at the time.
Here I add a conditional check to see if servers[server_name] is explicitly defined in servers and if not to just set nil on the filetypes variable
This fixes #375