fix: only check Go version when building the server#572
Conversation
|
|
||
| M.is_go_valid = function() | ||
| local go_version = io.popen("go version"):read("*a") | ||
| local go_version = io.popen("go version 2>&1"):read("*a") |
There was a problem hiding this comment.
Hi @snejugal, maybe instead you could modify the health check (health.lua module) to only call check_go_version when the state.settings.server.binary_provided option is nil.
There was a problem hiding this comment.
I can see that asking the user to install Go is unnecessary when they provide a prebuilt binary. Still, if the user is missing both of those, this function will still be called, and non-captured stderr causes the editor to misrender slightly. Compare:
:checkhealth when stderr is not captured
Screencast_20260701_181330.mp4
:checkhealth when stderr is captured
Screencast_20260701_181647.mp4
so I believe a patch to capture stderr here is still needed.
There was a problem hiding this comment.
I see, maybe we could use vim.system instead od io.popen, as we do in most other places. I'll have a look next week when I'm back from vacation.
There was a problem hiding this comment.
Just tried to tinker with vim.system myself, definitely seems to be a better option since no shell is spawned and no command not found message is produced in the first place. Pushed d5fa4d4, please take a look when you have the time.
|
|
||
| local go_version = go:wait().stdout; | ||
| local major, minor, _ = go_version:match("(%d+)%.(%d+)%.?(%d*)") | ||
| if major and tonumber(major) >= 1 and tonumber(minor) >= 25 then |
There was a problem hiding this comment.
I've just remembered, there's a more robust way to compare versions in Neovim: https://neovim.io/doc/user/lua/#vim.version. Could you please try and replace the current way?
I use NixOS and prefer to keep my environment clean. As such, I prebuilt the plugin's server binary using Nix and specified its path in
setup. However,setupforcibly checks Go version and aborts if I don't have it installed, which doesn't make sense because Go is only required to build the server and I already have one. Thus I moved the check toserver.buildfunction when it actually needs to build the server. I also touchedversion.luato remove the annoyinggo: command not foundmessage when starting Neovim (I'm not sure if this is a good way to fix it since I'm not a Lua programmer, so I'm open to suggestions).