Skip to content

fix: only check Go version when building the server#572

Open
snejugal wants to merge 5 commits into
harrisoncramer:developfrom
snejugal:fix-go-check
Open

fix: only check Go version when building the server#572
snejugal wants to merge 5 commits into
harrisoncramer:developfrom
snejugal:fix-go-check

Conversation

@snejugal

@snejugal snejugal commented Jul 1, 2026

Copy link
Copy Markdown

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, setup forcibly 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 to server.build function when it actually needs to build the server. I also touched version.lua to remove the annoying go: command not found message 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).

Comment thread lua/gitlab/version.lua Outdated

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")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jakubbortlik jakubbortlik changed the title Fix: only check Go version when actually building the server fix: only check Go version when building the server Jul 1, 2026
Comment thread lua/gitlab/version.lua Outdated

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, see f3771da

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

Successfully merging this pull request may close these issues.

2 participants