feat: replace plenary Job with vim.system (#556)

README.md

@@ -44,7 +44,6 @@ With <a href="https://github.com/folke/lazy.nvim">Lazy</a>:
   "harrisoncramer/gitlab.nvim",
   dependencies = {
     "MunifTanjim/nui.nvim",
-    "nvim-lua/plenary.nvim",
     "dlyongemallo/diffview-plus.nvim", -- Maintained fork of "sindrets/diffview.nvim".
     "stevearc/dressing.nvim", -- Recommended but not required. Better UI for pickers.
     "nvim-tree/nvim-web-devicons", -- Recommended but not required. Icons in discussion tree.
@@ -63,7 +62,6 @@ And with <a href="https://github.com/lewis6991/pckr.nvim">pckr.nvim</a>:
   "harrisoncramer/gitlab.nvim",
   requires = {
     "MunifTanjim/nui.nvim",
-    "nvim-lua/plenary.nvim",
     "dlyongemallo/diffview-plus.nvim", -- Maintained fork of "sindrets/diffview.nvim".
     "stevearc/dressing.nvim", -- Recommended but not required. Better UI for pickers.
     "nvim-tree/nvim-web-devicons", -- Recommended but not required. Icons in discussion tree.

doc/gitlab.nvim.txt

@@ -69,7 +69,6 @@ With Lazy:
       "harrisoncramer/gitlab.nvim",
       dependencies = {
         "MunifTanjim/nui.nvim",
-        "nvim-lua/plenary.nvim",
         "dlyongemallo/diffview-plus.nvim", -- Maintained fork of "sindrets/diffview.nvim".
         "stevearc/dressing.nvim", -- Recommended but not required. Better UI for pickers.
         "nvim-tree/nvim-web-devicons", -- Recommended but not required. Icons in discussion tree.
@@ -86,7 +85,6 @@ And with pckr.nvim:
       "harrisoncramer/gitlab.nvim",
       requires = {
         "MunifTanjim/nui.nvim",
-        "nvim-lua/plenary.nvim",
         "dlyongemallo/diffview-plus.nvim", -- Maintained fork of "sindrets/diffview.nvim".
         "stevearc/dressing.nvim", -- Recommended but not required. Better UI for pickers.
         "nvim-tree/nvim-web-devicons", -- Recommended but not required. Icons in discussion tree.

lua-test.sh

@@ -9,7 +9,6 @@ set -euo pipefail
 PLUGINS_FOLDER="tests/plugins"
 PLUGINS=(
   "https://github.com/MunifTanjim/nui.nvim"
-  "https://github.com/nvim-lua/plenary.nvim"
   "https://github.com/dlyongemallo/diffview-plus.nvim"
 )
 

lua/gitlab/git.lua

@@ -234,4 +234,25 @@ M.check_mr_in_good_condition = function()
   end
 end
 
+---Returns the full diff between the local working tree relative to the named `base_sha`, for the
+---given file(s).
+---@param base_sha string Base sha to diff against
+---@param old_path string? Old file name
+---@param new_path string? New file name - relevant for renamed files, ignored if same as old_path
+---@return string|nil diff, string|nil err
+M.diff_files = function(base_sha, old_path, new_path)
+  return run_system({
+    "git",
+    "diff",
+    "--minimal",
+    "--unified=0",
+    "--no-color",
+    "--no-ext-diff",
+    base_sha,
+    "--",
+    old_path,
+    new_path,
+  })
+end
+
 return M

lua/gitlab/health.lua

@@ -26,10 +26,6 @@ M.check = function(return_results)
       name = "MunifTanjim/nui.nvim",
       package = "nui.popup",
     },
-    {
-      name = "nvim-lua/plenary.nvim",
-      package = "plenary",
-    },
     {
       name = "dlyongemallo/diffview-plus.nvim",
       package = "diffview",

lua/gitlab/hunks.lua

@@ -14,7 +14,7 @@ local M = {}
 ---@field all_diff_output table The data from the git diff command
 
 ---Turn hunk line into Lua table
----@param line table
+---@param line string
 ---@return Hunk|nil
 M.parse_possible_hunk_headers = function(line)
   if line:sub(1, 2) == "@@" then
@@ -34,7 +34,7 @@ M.parse_possible_hunk_headers = function(line)
 end
 ---@param linnr number
 ---@param hunk Hunk
----@param all_diff_output table
+---@param all_diff_output string[]
 ---@return boolean
 local line_was_removed = function(linnr, hunk, all_diff_output)
   for matching_line_index, line in ipairs(all_diff_output) do
@@ -58,7 +58,7 @@ end
 
 ---@param linnr number
 ---@param hunk Hunk
----@param all_diff_output table
+---@param all_diff_output string[]
 ---@return boolean
 local line_was_added = function(linnr, hunk, all_diff_output)
   for matching_line_index, line in ipairs(all_diff_output) do
@@ -96,46 +96,26 @@ local parse_hunks_and_diff = function(base_sha)
   local hunks = {}
   local all_diff_output = {}
 
+  local git = require("gitlab.git")
   local reviewer = require("gitlab.reviewer")
-  local cmd = {
-    "diff",
-    "--minimal",
-    "--unified=0",
-    "--no-color",
-    "--no-ext-diff",
-    base_sha,
-    "--",
-    reviewer.get_current_file_oldpath(),
-    reviewer.get_current_file_path(),
-  }
 
-  local Job = require("plenary.job")
-  local diff_job = Job:new({
-    command = "git",
-    args = cmd,
-    on_exit = function(j, return_code)
-      if return_code == 0 then
-        all_diff_output = j:result()
-        for _, line in ipairs(all_diff_output) do
-          local hunk = M.parse_possible_hunk_headers(line)
-          if hunk ~= nil then
-            table.insert(hunks, hunk)
-          end
-        end
-      else
-        M.notify("Failed to get git diff: " .. j:stderr(), vim.log.levels.WARN)
+  local diff, _ = git.diff_files(base_sha, reviewer.get_current_file_oldpath(), reviewer.get_current_file_path())
+  if diff ~= nil then
+    for line in diff:gmatch("[^\r\n]+") do
+      table.insert(all_diff_output, line)
+      local hunk = M.parse_possible_hunk_headers(line)
+      if hunk ~= nil then
+        table.insert(hunks, hunk)
       end
-    end,
-  })
-
-  diff_job:sync()
+    end
+  end
 
   return { hunks = hunks, all_diff_output = all_diff_output }
 end
 
 -- Parses the lines from a diff and returns the
 -- index of the next hunk, when provided an initial index
----@param lines table
+---@param lines string[]
 ---@param i integer
 ---@return integer|nil
 local next_hunk_index = function(lines, i)

lua/gitlab/job.lua

@@ -1,43 +1,56 @@
 -- This module is responsible for making API calls to the Go server and
 -- running the callbacks associated with those jobs when the JSON is returned
-local Job = require("plenary.job")
 local u = require("gitlab.utils")
 local M = {}
 
+---Send a request to the Go server
+---@param endpoint string The endpoint path on the server.
+---@param method string The HTTP rquest method.
+---@param callback fun(data: table) The function to run on the decoded JSON response data if the response contains no error details.
+---@param on_error_callback? fun(data: table) The function to run on the decoded JSON response data in case the response contains error details.
 M.run_job = function(endpoint, method, body, callback, on_error_callback)
   local state = require("gitlab.state")
   local port = state.settings.server and state.settings.server.port
-  local args =
-    { "--noproxy", "localhost", "-s", "-X", (method or "POST"), string.format("localhost:%s%s", port, endpoint) }
+  local cmd = {
+    "curl",
+    "--noproxy",
+    "localhost",
+    "-s",
+    "-X",
+    (method or "POST"),
+    string.format("localhost:%s%s", port, endpoint),
+  }
 
   if body ~= nil then
     local encoded_body = vim.json.encode(body)
-    table.insert(args, 1, "-d")
-    table.insert(args, 2, encoded_body)
+    table.insert(cmd, 2, "-d")
+    table.insert(cmd, 3, encoded_body)
   end
 
   -- This handler will handle all responses from the Go server. Anything with a successful
   -- status will call the callback (if it is supplied for the job). Otherwise, it will print out the
   -- success message or error message and details from the Go server and run the on_error_callback
   -- (if supplied for the job).
-  local stderr = {}
-  Job:new({
-    command = "curl",
-    args = args,
-    on_stdout = function(_, output)
-      vim.defer_fn(function()
-        if output == nil then
-          return
-        end
-        local data_ok, data = pcall(vim.json.decode, output)
+  vim.system(cmd, { text = true }, function(out)
+    vim.schedule(function()
+      if out.code ~= 0 then
+        u.notify(string.format("Go server exited with non-zero code: %d", out.code), vim.log.levels.ERROR)
+      end
 
+      if out.stderr ~= "" then
+        u.notify(string.format("Could not run command `%s`! Stderr was:", table.concat(cmd, " ")), vim.log.levels.ERROR)
+        u.notify(vim.trim(out.stderr), vim.log.levels.ERROR)
+      end
+
+      if out.stdout ~= "" then
+        local data_ok, data = pcall(vim.json.decode, out.stdout)
         -- Failing to unmarshal JSON
         if not data_ok then
           local msg = string.format("Failed to parse JSON from %s endpoint", endpoint)
-          if type(output) == "string" then
-            msg = string.format(msg .. ", got: '%s'", output)
+          if type(out.stdout) == "string" then
+            msg = string.format(msg .. ", got: '%s'", out.stdout)
           end
-          u.notify(string.format(msg, endpoint, output), vim.log.levels.WARN)
+          u.notify(string.format(msg, endpoint, out.stdout), vim.log.levels.WARN)
           return
         end
 
@@ -60,28 +73,9 @@ M.run_job = function(endpoint, method, body, callback, on_error_callback)
             on_error_callback(data)
           end
         end
-      end, 0)
-    end,
-    on_stderr = function(_, data)
-      if data then
-        table.insert(stderr, data)
       end
-    end,
-    on_exit = function(code, status)
-      vim.defer_fn(function()
-        if #stderr ~= 0 then
-          u.notify(
-            string.format("Could not run command `%s %s`! Stderr was:", code.command, table.concat(code.args, " ")),
-            vim.log.levels.ERROR
-          )
-          vim.notify(string.format("%s", table.concat(stderr, "\n")), vim.log.levels.ERROR)
-        end
-        if status ~= 0 then
-          u.notify(string.format("Go server exited with non-zero code: %d", status), vim.log.levels.ERROR)
-        end
-      end, 0)
-    end,
-  }):start()
+    end)
+  end)
 end
 
 return M

lua/gitlab/server.lua

@@ -4,7 +4,6 @@
 local state = require("gitlab.state")
 local u = require("gitlab.utils")
 local job = require("gitlab.job")
-local Job = require("plenary.job")
 local M = {}
 
 -- Builds the binary if it doesn't exist, and starts the server. If the pre-existing binary has an older
@@ -62,8 +61,6 @@ M.start = function(callback)
 
   local settings = vim.json.encode(go_server_settings)
 
-  local stderr_buf = ""
-
   local ok, err = pcall(vim.system, { state.settings.server.binary, settings }, {
     stdout = function(_, data)
       if data == nil or parsed_port ~= nil then
@@ -85,18 +82,12 @@ M.start = function(callback)
         end
       end
     end,
-    stderr = function(_, data)
-      if data == nil or data == "" then
-        return
-      end
-      stderr_buf = stderr_buf .. data
-    end,
   }, function(out)
     if out.code ~= 0 then
       vim.schedule(function()
         local msg = "Golang gitlab server exited: code: " .. out.code .. ", signal: " .. (out.signal or 0)
-        if stderr_buf ~= "" then
-          msg = msg .. ", msg: " .. vim.trim(stderr_buf)
+        if out.stderr ~= "" then
+          msg = msg .. ", msg: " .. vim.trim(out.stderr)
         end
         u.notify(msg, vim.log.levels.ERROR)
       end)
@@ -214,44 +205,34 @@ M.restart = function(cb)
   end)
 end
 
--- Returns the version (git tag) that was baked into the binary when it was last built
+-- Runs `callback` with the version (git tag) that was baked into the binary when it was last built.
 M.get_version = function(callback)
   if not state.go_server_running then
     u.notify("Gitlab server not running", vim.log.levels.ERROR)
-    return nil
+    return
   end
   local parent_dir = u.get_root_path()
 
   local version_output = vim.system({ "git", "describe", "--tags", "--always" }, { cwd = parent_dir }):wait()
   local plugin_version = version_output.code == 0 and vim.trim(version_output.stdout) or "unknown"
 
-  local args =
-    { "--noproxy", "localhost", "-s", "-X", "GET", string.format("localhost:%s/version", state.settings.server.port) }
-
-  -- We call the "/version" endpoint here instead of through the regular jobs pattern because earlier versions of the plugin
-  -- may not have it. We handle a 404 as an "unknown" version error.
-  Job:new({
-    command = "curl",
-    args = args,
-    on_stdout = function(_, output)
-      vim.defer_fn(function()
-        if output == nil then
-          callback({ plugin_version = plugin_version, binary_version = "unknown" })
-          return
-        end
-
-        local data_ok, data = pcall(vim.json.decode, output)
-        if not data_ok or data == nil or data.version == nil then
-          callback({ plugin_version = plugin_version, binary_version = "unknown" })
-          return
-        end
+  -- We call the "/version" endpoint here instead of through the regular run_job pattern because
+  -- earlier versions of the plugin may not have the endpoint. We handle a 404 as an "unknown"
+  -- version error.
+  local cmd = {
+    "curl",
+    "--noproxy",
+    "localhost",
+    "-s",
+    "-X",
+    "GET",
+    string.format("localhost:%s/version", state.settings.server.port),
+  }
+  local result = vim.system(cmd):wait()
+  local _, data = pcall(vim.json.decode, result.stdout)
+  local binary_version = data and data.version and data.version or "unknown"
 
-        callback({ plugin_version = plugin_version, binary_version = data.version })
-      end, 0)
-    end,
-    on_stderr = function() end,
-    on_exit = function() end,
-  }):start()
+  callback({ plugin_version = plugin_version, binary_version = binary_version })
 end
 
 return M