fix(server): use vim.system instead of jobstart to bypass shell

[JonLD]

Summary

Switch the server launch from vim.fn.jobstart to vim.system, passing the binary and JSON settings as a list. Neovim spawns the process directly instead of going through the user's shell, removing the Windows-specific "-escaping hack that only worked for POSIX-style shells.

Why

On Windows with a non-POSIX shell (e.g. Nushell) the previous string-form jobstart call was passed through &shell &shellcmdflag, and the quoting/escaping assumptions broke, preventing the Go server from starting. vim.system skips the shell entirely, so argument boundaries and JSON quoting are preserved verbatim regardless of shell.

Test plan

• Start the plugin on Windows with shell=nu and confirm the server launches
• Start the plugin on Windows with default cmd.exe/PowerShell
• Start the plugin on Linux/macOS to confirm no regression

[jakubbortlik]

Hi @JonLD, thanks for the contribution! Have you considered replacing vim.fn.jobstart with vim.system as recommended by the Neovim docs? The vim.system function was introduced in Nvim v 0.10, that is after this piece of code had been written so it's worth revising it more thoroughly.

[JonLD]

Hi @jakubbortlik, thanks for the suggestion! I've gone ahead and pushed a change to use vim.system instead of vim.fn.jobstart. It provides the same fix via a nicer mechanism.

[jakubbortlik]

Hi @JonLD, can you please rebase and update the PR description.

[JonLD]

Hi @jakubbortlik, rebased and updated PR description.

[jakubbortlik]

Hi @JonLD, I'va had Claude review this and it says:

  Previously stderr was accumulated with List.reduce and shown as one message. Now each
  chunk fires a separate u.notify.

  If the Go server writes a multi-line error across two chunks, the user sees two separate
   notifications. For a long stacktrace this can be noisy. Simple fix: concatenate into a
  module-scoped buffer and flush on nil:

  local stderr_buf = ""
  stderr = function(_, data)
    if data == nil then
      if stderr_buf ~= "" then
        local msg = stderr_buf
        stderr_buf = ""
        vim.schedule(function() u.notify(msg, vim.log.levels.ERROR) end)
      end
      return
    end
    stderr_buf = stderr_buf .. data
  end,

Could you please verify that your modification is not a regression from the previous behaviour? I can confirm that now when I for example mess up the server settings, I'm getting two messages:

gitlab.nvim: Failure parsing plugin settings: invalid character 'B' after top-level value
gitlab.nvim: Golang gitlab server exited: code: 1, signal: 0

With the following patch, I get only one message (gitlab.nvim: Golang gitlab server exited: code: 1, signal: 0, msg: Failure parsing plugin settings: invalid character 'B' after top-level value) which I would prefer to see, if it's just one problem:

diff --git a/lua/gitlab/server.lua b/lua/gitlab/server.lua
index b06c5060..9d2e6a4e 100644
--- a/lua/gitlab/server.lua
+++ b/lua/gitlab/server.lua
@@ -62,6 +62,7 @@ M.start = function(callback)
 
   local settings = vim.json.encode(go_server_settings)
 
+  local stderr_buf, msg = "", ""
   local ok, err = pcall(vim.system, { state.settings.server.binary, settings }, {
     stdout = function(_, data)
       if data == nil or parsed_port ~= nil then
@@ -84,18 +85,20 @@ M.start = function(callback)
       end
     end,
     stderr = function(_, data)
-      if data == nil or data == "" then
+      if data == nil then
+        if stderr_buf ~= "" then
+          msg = stderr_buf
+          stderr_buf = ""
+        end
         return
       end
-      vim.schedule(function()
-        u.notify(data, vim.log.levels.ERROR)
-      end)
+      stderr_buf = stderr_buf .. data
     end,
   }, function(out)
     if out.code ~= 0 then
       vim.schedule(function()
         u.notify(
-          "Golang gitlab server exited: code: " .. out.code .. ", signal: " .. (out.signal or 0),
+          "Golang gitlab server exited: code: " .. out.code .. ", signal: " .. (out.signal or 0) .. ", msg: " .. msg,
           vim.log.levels.ERROR
         )
       end)

[JonLD]

Hi @jakubbortlik, nice spot. I don't think this is a regression with my changes as on the head of develop you still get separate notifications for "Golang gitlab server exited" and the failure message. That said it does make sense to just combine them into one notification so I've gone ahead and made the change. It's slightly different to the diff you suggested but achieves the same.

[jakubbortlik]

I think the point of the "regression" was that your original stderr implementation would have notified for each "chunk of data":

    stderr = function(_, data)
      if data == nil or data == "" then
        return
      end
      vim.schedule(function()
        u.notify(data, vim.log.levels.ERROR)
      end)
    end,

The fact that both stderr and on_exit notified separately was a separate (and indeed pre-existing) issue.

Now your solutio looks much cleaner than my suggestion! The only thing I've noticed is that now the message can contain a trailing new line character that e.g., in Snacks notification history show as blank lines:

Could you please trim the final new lines?

[JonLD]

Ahh I see your point with the regression, didn't notice that in any testing.

Trailing newline trimmed now.

[jakubbortlik]

Thanks for the PR!