Multiline comment and suggestion (#66)

This MR adds the ability to leave multi-line comments and suggested changes to an MR. The features are only supported for `diffview` because we plan to deprecate `delta` as a reviewer soon.
2023-10-31 02:58:53 +01:00
parent f853c2f940
commit 3a67424fec
11 changed files with 689 additions and 161 deletions
--- a/lua/gitlab/reviewer/delta.lua
+++ b/lua/gitlab/reviewer/delta.lua
@@ -1,16 +1,16 @@
 -- This Module contains all of the code specific to the Delta reviewer.
-local state                   = require("gitlab.state")
-local u                       = require("gitlab.utils")
+local state = require("gitlab.state")
+local u = require("gitlab.utils")

-local M                       = {
-  bufnr = nil
+local M = {
+  bufnr = nil,
 }

 -- Public Functions
 -- These functions are exposed externally and are used
 -- when the reviewer is consumed by other code. They must follow the specification
 -- outlined in the reviewer/init.lua file
-M.open                        = function()
+M.open = function()
  local current_buf = vim.api.nvim_get_current_buf()
  if current_buf == state.discussion_buf then
    vim.api.nvim_command("wincmd w")
@@ -23,20 +23,22 @@ M.open                        = function()
  end

  local term_command_template =
-  "GIT_PAGER='delta --hunk-header-style omit --line-numbers --paging never --file-added-label %s --file-removed-label %s --file-modified-label %s' git diff %s...HEAD"
+    "GIT_PAGER='delta --hunk-header-style omit --line-numbers --paging never --file-added-label %s --file-removed-label %s --file-modified-label %s' git diff %s...HEAD"

-  local term_command = string.format(term_command_template,
+  local term_command = string.format(
+    term_command_template,
    state.settings.review_pane.delta.added_file,
    state.settings.review_pane.delta.removed_file,
    state.settings.review_pane.delta.modified_file,
-    state.INFO.target_branch)
+    state.INFO.target_branch
+  )

  vim.fn.termopen(term_command) -- Calls delta and sends the output to the currently blank buffer
  M.bufnr = vim.api.nvim_get_current_buf()
  M.winnr = vim.api.nvim_get_current_win()
 end

-M.jump                        = function(file_name, new_line, old_line)
+M.jump = function(file_name, new_line, old_line)
  local linnr, error = M.get_jump_location(file_name, new_line, old_line)
  if error ~= nil then
    vim.notify(error, vim.log.levels.ERROR)
@@ -47,25 +49,47 @@ M.jump                        = function(file_name, new_line, old_line)
  u.jump_to_buffer(M.bufnr, linnr)
 end

-M.get_location                = function()
-  if M.bufnr == nil then return nil, nil, "Delta reviewer must be initialized first" end
+---Get the location of a line within the delta buffer. If range is specified, then also the location
+---of the lines in range.
+---@param range LineRange | nil Line range to get location for
+---@return ReviewerInfo | nil nil is returned only if error was encountered
+M.get_location = function(range)
+  if M.bufnr == nil then
+    vim.notify("Delta reviewer must be initialized first", vim.log.levels.ERROR)
+    return
+  end
+
+  if range then
+    vim.notify("Multiline comments are not yet supported for delta reviewer", vim.log.levels.ERROR)
+    return
+  end

  local bufnr = vim.api.nvim_get_current_buf()
-  if bufnr ~= M.bufnr then return nil, nil, "Line location can only be determined within reviewer window" end
+  if bufnr ~= M.bufnr then
+    vim.notify("Line location can only be determined within reviewer window")
+    return
+  end

  local line_num = u.get_current_line_number()
  local file_name = M.get_file_from_review_buffer(u.get_current_line_number())

-  local range, error = M.get_review_buffer_range(file_name)
+  local review_range, error = M.get_review_buffer_range(file_name)

-  if error ~= nil then return nil, nil, error end
-  if range == nil then return nil, nil, "Review buffer range could not be identified" end
+  if error ~= nil then
+    vim.notify(error, vim.log.levels.ERROR)
+    return
+  end
+
+  if review_range == nil then
+    vim.notify("Review buffer range could not be identified", vim.log.levels.ERROR)
+    return
+  end

  -- In case the comment is left on a line without change information, we
  -- iterate backward until we find it within the range of the changes
  local current_line_changes = nil
  local num = line_num
-  while range ~= nil and num >= range[1] and current_line_changes == nil do
+  while review_range ~= nil and num >= review_range[1] and current_line_changes == nil do
    local content = u.get_line_content(M.bufnr, num)
    local change_nums = M.get_change_nums(content)
    current_line_changes = change_nums
@@ -73,12 +97,13 @@ M.get_location                = function()
  end

  if current_line_changes == nil then
-    return nil, nil, "Could not find current line change information"
+    vim.notify("Could not find current line change information", vim.log.levels.ERROR)
+    return
  end

  local new_line_num = line_num + 1
  local next_line_changes = nil
-  while range ~= nil and new_line_num <= range[2] and next_line_changes == nil do
+  while review_range ~= nil and new_line_num <= review_range[2] and next_line_changes == nil do
    local content = u.get_line_content(M.bufnr, new_line_num)
    local change_nums = M.get_change_nums(content)
    next_line_changes = change_nums
@@ -86,41 +111,53 @@ M.get_location                = function()
  end

  if next_line_changes == nil then
-    return nil, nil, "Could not find next line change information"
+    vim.notify("Could not find next line change information", vim.log.levels.ERROR)
+    return
  end

+  local result = { file_name = file_name }
  -- This is actually a modified line if these conditions are met
-  if (current_line_changes.old_line and not current_line_changes.new_line and not next_line_changes.old_line and next_line_changes.new_line) then
-    do
-      current_line_changes = {
-        old_line = current_line_changes.old,
-        new_line = next_line_changes.new_line
-      }
-    end
+  if
+    current_line_changes.old_line
+    and not current_line_changes.new_line
+    and not next_line_changes.old_line
+    and next_line_changes.new_line
+  then
+    result.old_line = current_line_changes.old
+    result.new_line = next_line_changes.new_line
+  else
+    vim.notify("Could not determine line location", vim.log.levels.ERROR)
+    return
  end

-  return file_name, current_line_changes
+  return result
 end

 -- Helper Functions 🤝
 -- These functions are not exported and should be private
 -- to the delta reviewer, they are used to support the public functions
-M.get_jump_location           = function(file_name, new_line, old_line)
+M.get_jump_location = function(file_name, new_line, old_line)
  local range, error = M.get_review_buffer_range(file_name)
-  if error ~= nil then return nil, error end
-  if range == nil then return nil, "Review buffer range could not be identified" end
+  if error ~= nil then
+    return nil, error
+  end
+  if range == nil then
+    return nil, "Review buffer range could not be identified"
+  end

  local linnr = nil

  local lines = M.get_review_buffer_lines(range)
  for _, line in ipairs(lines) do
    local line_data = M.get_change_nums(line.line_content)
-    if old_line == line_data.old_line and new_line == line_data.new_line then
+    if line_data and old_line == line_data.old_line and new_line == line_data.new_line then
      linnr = line.line_number
      break
    end
  end
-  if linnr == nil then return nil, "Could not find matching line" end
+  if linnr == nil then
+    return nil, "Could not find matching line"
+  end
  return linnr, nil
 end

@@ -134,32 +171,39 @@ M.get_file_from_review_buffer = function(linenr)
  end
 end

-M.get_change_nums             = function(line)
+M.get_change_nums = function(line)
  local data, _ = line:match("(.-)" .. "│" .. "(.*)")
  local line_data = {}
-  if data == nil then return nil end
+  if data == nil then
+    return nil
+  end

-  if data ~= nil then
+  if data ~= nil and data ~= "" then
    local old_line = u.trim(u.get_first_chunk(data, "[^" .. "⋮" .. "]+"))
    local new_line = u.trim(u.get_last_chunk(data, "[^" .. "⋮" .. "]+"))
    line_data.new_line = tonumber(new_line)
    line_data.old_line = tonumber(old_line)
  end

-  if line_data.new_line == nil and line_data.old_line == nil then return nil end
+  if line_data.new_line == nil and line_data.old_line == nil then
+    return nil
+  end

  return line_data
 end

-
 M.get_review_buffer_range = function(file_name)
-  if M.bufnr == nil then return nil, "Delta reviewer must be initialized first" end
+  if M.bufnr == nil then
+    return nil, "Delta reviewer must be initialized first"
+  end
  local lines = vim.api.nvim_buf_get_lines(M.bufnr, 0, -1, false)
  local start = nil
  local stop = nil

  for i, line in ipairs(lines) do
-    if start ~= nil and stop ~= nil then return { start, stop } end
+    if start ~= nil and stop ~= nil then
+      return { start, stop }
+    end
    if M.starts_with_file_symbol(line) then
      -- Check if the file name matches the node name
      local delta_file_name = u.get_last_chunk(line)
@@ -173,7 +217,9 @@ M.get_review_buffer_range = function(file_name)

  -- We've reached the end of the file, set "stop" in case we already found start
  stop = #lines
-  if start ~= nil and stop ~= nil then return { start, stop } end
+  if start ~= nil and stop ~= nil then
+    return { start, stop }
+  end
 end

 M.starts_with_file_symbol = function(line)
@@ -200,4 +246,12 @@ M.get_review_buffer_lines = function(review_buffer_range)
  return lines
 end

+---Return content between start_line and end_line
+---@param start_line integer
+---@param end_line integer
+---@return string[] | nil
+M.get_lines = function(start_line, end_line)
+  vim.notify("Getting lines in delta is not supported yet", vim.log.levels.ERROR)
+  return nil
+end
 return M
--- a/lua/gitlab/reviewer/diffview.lua
+++ b/lua/gitlab/reviewer/diffview.lua
@@ -1,22 +1,23 @@
 -- This Module contains all of the code specific to the Diffview reviewer.
-local state           = require("gitlab.state")
+local u = require("gitlab.utils")
+local state = require("gitlab.state")
 local async_ok, async = pcall(require, "diffview.async")

-local M               = {
+local M = {
  bufnr = nil,
-  tabnr = nil
+  tabnr = nil,
 }

 -- Public Functions
 -- These functions are exposed externally and are used
 -- when the reviewer is consumed by other code. They must follow the specification
 -- outlined in the reviewer/init.lua file
-M.open                = function()
+M.open = function()
  vim.api.nvim_command(string.format("DiffviewOpen %s", state.INFO.target_branch))
  M.tabnr = vim.api.nvim_get_current_tabpage()
 end

-M.jump                = function(file_name, new_line, old_line)
+M.jump = function(file_name, new_line, old_line)
  if M.tabnr == nil then
    vim.notify("Can't jump to Diffvew. Is it open?", vim.log.levels.ERROR)
    return
@@ -49,12 +50,25 @@ M.jump                = function(file_name, new_line, old_line)
  end
 end

-M.get_location        = function()
-  if M.tabnr == nil then return nil, nil, "Diffview reviewer must be initialized first" end
+---Get the location of a line within the diffview. If range is specified, then also the location
+---of the lines in range.
+---@param range LineRange | nil Line range to get location for
+---@return ReviewerInfo | nil nil is returned only if error was encountered
+M.get_location = function(range)
+  if M.tabnr == nil then
+    vim.notify("Diffview reviewer must be initialized first")
+    return
+  end
  local bufnr = vim.api.nvim_get_current_buf()
+  local current_line = vim.api.nvim_win_get_cursor(0)[1]
+
  -- check if we are in the diffview tab
  local tabnr = vim.api.nvim_get_current_tabpage()
-  if tabnr ~= M.tabnr then return nil, nil, "Line location can only be determined within reviewer window" end
+  if tabnr ~= M.tabnr then
+    vim.notify("Line location can only be determined within reviewer window")
+    return
+  end
+
  -- check if we are in the diffview buffer
  local view = require("diffview.lib").get_current_view()
  if view == nil then
@@ -62,18 +76,81 @@ M.get_location        = function()
    return
  end
  local layout = view.cur_layout
-  local file_name = nil
-  local current_line_changes = nil
+  local result = {}
+  local type
+  local is_new
  if layout.a.file.bufnr == bufnr then
-    file_name = layout.a.file.path
-    current_line_changes = { new_line = nil, old_line = vim.api.nvim_win_get_cursor(0)[1] }
-    return file_name, current_line_changes
+    result.file_name = layout.a.file.path
+    result.old_line = current_line
+    type = "old"
+    is_new = false
  elseif layout.b.file.bufnr == bufnr then
-    file_name = layout.b.file.path
-    current_line_changes = { new_line = vim.api.nvim_win_get_cursor(0)[1], old_line = nil }
-    return file_name, current_line_changes
+    result.file_name = layout.b.file.path
+    result.new_line = current_line
+    type = "new"
+    is_new = true
+  else
+    vim.notify("Line location can only be determined within reviewer window")
+    return
  end
-  return nil, nil, "Line location can only be determined within reviewer window"
+
+  local hunks = u.parse_hunk_headers(result.file_name, state.INFO.target_branch)
+  if hunks == nil then
+    vim.notify("Could not parse hunks", vim.log.levels.ERROR)
+    return
+  end
+
+  local current_line_info
+  if is_new then
+    current_line_info = u.get_lines_from_hunks(hunks, result.new_line, is_new)
+  else
+    current_line_info = u.get_lines_from_hunks(hunks, result.old_line, is_new)
+  end
+
+  -- If single line comment is outside of changed lines then we need to specify both new line and old line
+  -- otherwise the API returns error.
+  -- https://docs.gitlab.com/ee/api/discussions.html#create-a-new-thread-in-the-merge-request-diff
+  if not current_line_info.in_hunk then
+    result.old_line = current_line_info.old_line
+    result.new_line = current_line_info.new_line
+  end
+
+  if range == nil then
+    return result
+  end
+
+  result.range_info = { start = {}, ["end"] = {} }
+  if current_line == range.start_line then
+    result.range_info.start.old_line = current_line_info.old_line
+    result.range_info.start.new_line = current_line_info.new_line
+    result.range_info.start.type = type
+  else
+    local start_line_info = u.get_lines_from_hunks(hunks, range.start_line, is_new)
+    result.range_info.start.old_line = start_line_info.old_line
+    result.range_info.start.new_line = start_line_info.new_line
+    result.range_info.start.type = type
+  end
+
+  if current_line == range.end_line then
+    result.range_info["end"].old_line = current_line_info.old_line
+    result.range_info["end"].new_line = current_line_info.new_line
+    result.range_info["end"].type = type
+  else
+    local end_line_info = u.get_lines_from_hunks(hunks, range.end_line, is_new)
+    result.range_info["end"].old_line = end_line_info.old_line
+    result.range_info["end"].new_line = end_line_info.new_line
+    result.range_info["end"].type = type
+  end
+
+  return result
+end
+
+---Return content between start_line and end_line
+---@param start_line integer
+---@param end_line integer
+---@return string[]
+M.get_lines = function(start_line, end_line)
+  return vim.api.nvim_buf_get_lines(0, start_line - 1, end_line, false)
 end

 return M
--- a/lua/gitlab/reviewer/init.lua
+++ b/lua/gitlab/reviewer/init.lua
@@ -10,7 +10,7 @@ local M = {

 local reviewer_map = {
  delta = delta,
-  diffview = diffview
+  diffview = diffview,
 }

 M.init = function()
@@ -31,9 +31,12 @@ M.init = function()
  --   • {interval}  The old_line of the change

  M.get_location = reviewer.get_location
-  -- Returns the current location (based on cursor) from the reviewer window in format:
-  -- file_name, {new_line, old_line}, error
+  -- Parameters:
+  --   • {range}  LineRange if function was triggered from visual selection
+  -- Returns the current location (based on cursor) from the reviewer window as ReviewerInfo class
+
+  M.get_lines = reviewer.get_lines
+  -- Returns the content of the file in the current location in the reviewer window
 end

-
 return M