Fix: Store reviewer data before creating comment popup (#476)

Author: jakubbortlik

lua/gitlab/actions/comment.lua

@@ -15,7 +15,6 @@ local reviewer = require("gitlab.reviewer")
 local Location = require("gitlab.reviewer.location")
 
 local M = {
-  current_win = nil,
   start_line = nil,
   end_line = nil,
   draft_popup = nil,
@@ -25,10 +24,9 @@ local M = {
 ---Fires the API that sends the comment data to the Go server, called when you "confirm" creation
 ---via the M.settings.keymaps.popup.perform_action keybinding
 ---@param text string comment text
----@param visual_range LineRange | nil range of visual selection or nil
 ---@param unlinked boolean if true, the comment is not linked to a line
 ---@param discussion_id string | nil The ID of the discussion to which the reply is responding, nil if not a reply
-local confirm_create_comment = function(text, visual_range, unlinked, discussion_id)
+local confirm_create_comment = function(text, unlinked, discussion_id)
   if text == nil then
     u.notify("Reviewer did not provide text of change", vim.log.levels.ERROR)
     return
@@ -75,30 +73,16 @@ local confirm_create_comment = function(text, visual_range, unlinked, discussion
     return
   end
 
-  local reviewer_data = reviewer.get_reviewer_data()
-  if reviewer_data == nil then
-    u.notify("Error getting reviewer data", vim.log.levels.ERROR)
-    return
-  end
-
-  local location = Location.new(reviewer_data, visual_range)
-  location:build_location_data()
-  local location_data = location.location_data
-  if location_data == nil then
-    u.notify("Error getting location information", vim.log.levels.ERROR)
-    return
-  end
-
   local revision = state.MR_REVISIONS[1]
   local position_data = {
-    file_name = reviewer_data.file_name,
-    old_file_name = reviewer_data.old_file_name,
+    file_name = M.location.reviewer_data.file_name,
+    old_file_name = M.location.reviewer_data.old_file_name,
     base_commit_sha = revision.base_commit_sha,
     start_commit_sha = revision.start_commit_sha,
     head_commit_sha = revision.head_commit_sha,
-    old_line = location_data.old_line,
-    new_line = location_data.new_line,
-    line_range = location_data.line_range,
+    old_line = M.location.location_data.old_line,
+    new_line = M.location.location_data.new_line,
+    line_range = M.location.location_data.line_range,
   }
 
   -- Creating a new comment (linked to specific changes)
@@ -148,10 +132,10 @@ M.confirm_edit_comment = function(discussion_id, note_id, unlinked)
 end
 
 ---@class LayoutOpts
----@field ranged boolean
 ---@field unlinked boolean
 ---@field discussion_id string|nil
 ---@field reply boolean|nil
+---@field file_name string|nil
 
 ---This function sets up the layout and popups needed to create a comment, note and
 ---multi-line comment. It also sets up the basic keybindings for switching between
@@ -163,21 +147,24 @@ M.create_comment_layout = function(opts)
   local title
   local user_settings
   if opts.discussion_id ~= nil then
-    title = "Reply"
+    title = "Reply" .. (opts.file_name and string.format(" [%s]", opts.file_name) or "")
     user_settings = popup_settings.reply
   elseif opts.unlinked then
     title = "Note"
     user_settings = popup_settings.note
   else
-    title = "Comment"
+    -- TODO: investigate why `old_file_name` is in fact the new name for renamed files!
+    local file_name = M.location.reviewer_data.old_file_name ~= "" and M.location.reviewer_data.old_file_name
+      or M.location.reviewer_data.file_name
+    title =
+      popup.create_title("Comment", file_name, M.location.visual_range.start_line, M.location.visual_range.end_line)
     user_settings = popup_settings.comment
   end
   local settings = u.merge(popup_settings, user_settings or {})
 
-  M.current_win = vim.api.nvim_get_current_win()
+  local current_win = vim.api.nvim_get_current_win()
   M.comment_popup = Popup(popup.create_popup_state(title, settings))
   M.draft_popup = Popup(popup.create_box_popup_state("Draft", false, settings))
-  M.start_line, M.end_line = u.get_visual_selection_boundaries()
 
   local internal_layout = Layout.Box({
     Layout.Box(M.comment_popup, { grow = 1 }),
@@ -194,22 +181,21 @@ M.create_comment_layout = function(opts)
   }, internal_layout)
 
   popup.set_cycle_popups_keymaps({ M.comment_popup, M.draft_popup })
-  popup.set_up_autocommands(M.comment_popup, layout, M.current_win)
+  popup.set_up_autocommands(M.comment_popup, layout, current_win)
 
-  local range = opts.ranged and { start_line = M.start_line, end_line = M.end_line } or nil
   local unlinked = opts.unlinked or false
 
   ---Keybinding for focus on draft section
   popup.set_popup_keymaps(M.draft_popup, function()
     local text = u.get_buffer_text(M.comment_popup.bufnr)
-    confirm_create_comment(text, range, unlinked, opts.discussion_id)
-    vim.api.nvim_set_current_win(M.current_win)
+    confirm_create_comment(text, unlinked, opts.discussion_id)
+    vim.api.nvim_set_current_win(current_win)
   end, miscellaneous.toggle_bool, popup.non_editable_popup_opts)
 
   ---Keybinding for focus on text section
   popup.set_popup_keymaps(M.comment_popup, function(text)
-    confirm_create_comment(text, range, unlinked, opts.discussion_id)
-    vim.api.nvim_set_current_win(M.current_win)
+    confirm_create_comment(text, unlinked, opts.discussion_id)
+    vim.api.nvim_set_current_win(current_win)
   end, miscellaneous.attach_file, popup.editable_popup_opts)
 
   vim.schedule(function()
@@ -223,44 +209,43 @@ end
 --- This function will open a comment popup in order to create a comment on the changed/updated
 --- line in the current MR
 M.create_comment = function()
+  M.location = Location.new()
   if not M.can_create_comment(false) then
     return
   end
 
-  local layout = M.create_comment_layout({ ranged = false, unlinked = false })
+  local layout = M.create_comment_layout({ unlinked = false })
   layout:mount()
 end
 
 --- This function will open a multi-line comment popup in order to create a multi-line comment
 --- on the changed/updated line in the current MR
 M.create_multiline_comment = function()
+  M.location = Location.new()
   if not M.can_create_comment(true) then
     u.press_escape()
     return
   end
 
-  local layout = M.create_comment_layout({ ranged = true, unlinked = false })
+  local layout = M.create_comment_layout({ unlinked = false })
   layout:mount()
 end
 
 --- This function will open a a popup to create a "note" (e.g. unlinked comment)
 --- on the changed/updated line in the current MR
 M.create_note = function()
-  local layout = M.create_comment_layout({ ranged = false, unlinked = true })
+  local layout = M.create_comment_layout({ unlinked = true })
   layout:mount()
 end
 
 ---Given the current visually selected area of text, builds text to fill in the
 ---comment popup with a suggested change
 ---@return LineRange|nil
----@return integer
 local build_suggestion = function()
   local current_line = vim.api.nvim_win_get_cursor(0)[1]
-  M.start_line, M.end_line = u.get_visual_selection_boundaries()
-
-  local range_length = M.end_line - M.start_line
+  local range_length = M.location.visual_range.end_line - M.location.visual_range.start_line
   local backticks = "```"
-  local selected_lines = u.get_lines(M.start_line, M.end_line)
+  local selected_lines = u.get_lines(M.location.visual_range.start_line, M.location.visual_range.end_line)
 
   for _, line in ipairs(selected_lines) do
     if string.match(line, "^```%S*$") then
@@ -270,36 +255,37 @@ local build_suggestion = function()
   end
 
   local suggestion_start
-  if M.start_line == current_line then
+  if M.location.visual_range.start_line == current_line then
     suggestion_start = backticks .. "suggestion:-0+" .. range_length
-  elseif M.end_line == current_line then
+  elseif M.location.visual_range.end_line == current_line then
     suggestion_start = backticks .. "suggestion:-" .. range_length .. "+0"
   else
     --- This should never happen afaik
     u.notify("Unexpected suggestion position", vim.log.levels.ERROR)
-    return nil, 0
+    return nil
   end
   suggestion_start = suggestion_start
   local suggestion_lines = {}
   table.insert(suggestion_lines, suggestion_start)
   vim.list_extend(suggestion_lines, selected_lines)
   table.insert(suggestion_lines, backticks)
 
-  return suggestion_lines, range_length
+  return suggestion_lines
 end
 
 --- This function will open a a popup to create a suggestion comment
 --- on the changed/updated line in the current MR
 --- See: https://docs.gitlab.com/ee/user/project/merge_requests/reviews/suggestions.html
 M.create_comment_suggestion = function()
+  M.location = Location.new()
   if not M.can_create_comment(true) then
     u.press_escape()
     return
   end
 
-  local suggestion_lines, range_length = build_suggestion()
+  local suggestion_lines = build_suggestion()
 
-  local layout = M.create_comment_layout({ ranged = range_length > 0, unlinked = false })
+  local layout = M.create_comment_layout({ unlinked = false })
   layout:mount()
 
   vim.schedule(function()
@@ -368,6 +354,11 @@ M.can_create_comment = function(must_be_visual)
     return false
   end
 
+  if M.location == nil or M.location.location_data == nil then
+    u.notify("Error getting location information", vim.log.levels.ERROR)
+    return false
+  end
+
   return true
 end
 

lua/gitlab/actions/discussions/init.lua

@@ -237,10 +237,10 @@ M.reply = function(tree)
   local comment = require("gitlab.actions.comment")
   local unlinked = tree.bufnr == M.unlinked_bufnr
   local layout = comment.create_comment_layout({
-    ranged = false,
     discussion_id = discussion_id,
     unlinked = unlinked,
     reply = true,
+    file_name = discussion_node.file_name,
   })
 
   layout:mount()
@@ -274,14 +274,16 @@ end
 
 -- This function (settings.keymaps.discussion_tree.edit_comment) will open the edit popup for the current comment in the discussion tree
 M.edit_comment = function(tree, unlinked)
-  local edit_popup = Popup(popup.create_popup_state("Edit Comment", state.settings.popup.edit))
   local current_node = tree:get_node()
   local note_node = common.get_note_node(tree, current_node)
   local root_node = common.get_root_node(tree, current_node)
   if note_node == nil or root_node == nil then
     u.notify("Could not get root or note node", vim.log.levels.ERROR)
     return
   end
+  local title = "Edit Comment"
+  title = root_node.file_name ~= nil and string.format("%s [%s]", title, root_node.file_name) or title
+  local edit_popup = Popup(popup.create_popup_state(title, state.settings.popup.edit))
 
   popup.set_up_autocommands(edit_popup, nil, vim.api.nvim_get_current_win())
 

lua/gitlab/annotations.lua

@@ -120,11 +120,11 @@
 ---Relevant for renamed files only, the name of the file in the previous commit
 ---@field old_file_name string
 ---@field current_bufnr integer
----@field new_sha_win_id integer
----@field old_sha_win_id integer
 ---@field opposite_bufnr integer
 ---@field new_line_from_buf integer
 ---@field old_line_from_buf integer
+---@field new_sha_focused boolean
+---@field current_win_id integer
 
 ---@class LocationData
 ---@field old_line integer | nil

lua/gitlab/hunks.lua

@@ -230,17 +230,17 @@ end
 ---This is in order to build the payload for Gitlab correctly by setting the old line and new line.
 ---@param old_line number|nil
 ---@param new_line number|nil
----@param is_current_sha_focused boolean
+---@param new_sha_focused boolean
 ---@return string|nil
-function M.get_modification_type(old_line, new_line, is_current_sha_focused)
+function M.get_modification_type(old_line, new_line, new_sha_focused)
   local hunk_and_diff_data = parse_hunks_and_diff(state.INFO.diff_refs.base_sha)
   if hunk_and_diff_data.hunks == nil then
     return
   end
 
   local hunks = hunk_and_diff_data.hunks
   local all_diff_output = hunk_and_diff_data.all_diff_output
-  return is_current_sha_focused and get_modification_type_from_new_sha(new_line, hunks, all_diff_output)
+  return new_sha_focused and get_modification_type_from_new_sha(new_line, hunks, all_diff_output)
     or get_modification_type_from_old_sha(old_line, new_line, hunks, all_diff_output)
 end
 

lua/gitlab/indicators/common.lua

@@ -12,7 +12,6 @@ local M = {}
 
 ---Return true if discussion has a placeable diagnostic, false otherwise.
 ---@param note NoteWithValues
----@param file string
 ---@return boolean
 local filter_discussions_and_notes = function(note)
   ---Do not include unlinked notes

lua/gitlab/popup.lua

@@ -236,4 +236,16 @@ M.set_cycle_popups_keymaps = function(popups)
   end
 end
 
+---Create the title for the comment popup.
+---@param title string The main title, e.g., "Comment".
+---@param file_name string Name of file for which comment is created.
+---@param start_line integer Start of the line range.
+---@param end_line integer End of the line range.
+---@return string title The full title of the popup.
+M.create_title = function(title, file_name, start_line, end_line)
+  local range = start_line < end_line and string.format("-%s", end_line) or ""
+  local position = string.format("%s%s", start_line, range)
+  return string.format("%s [%s:%s]", title, file_name, position)
+end
+
 return M

lua/gitlab/reviewer/init.lua

@@ -154,9 +154,13 @@ end
 
 ---Get the data from diffview, such as line information and file name. May be used by
 ---other modules such as the comment module to create line codes or set diagnostics
+---@param current_win integer The ID of the currently focused window
 ---@return DiffviewInfo | nil
-M.get_reviewer_data = function()
+M.get_reviewer_data = function(current_win)
   local view = diffview_lib.get_current_view()
+  if view == nil then
+    return
+  end
   local layout = view.cur_layout
   local old_win = u.get_window_id_by_buffer_id(layout.a.file.bufnr)
   local new_win = u.get_window_id_by_buffer_id(layout.b.file.bufnr)
@@ -175,9 +179,9 @@ M.get_reviewer_data = function()
   local new_line = vim.api.nvim_win_get_cursor(new_win)[1]
   local old_line = vim.api.nvim_win_get_cursor(old_win)[1]
 
-  local is_current_sha_focused = M.is_current_sha_focused()
+  local new_sha_focused = M.is_new_sha_focused(current_win)
 
-  local modification_type = hunks.get_modification_type(old_line, new_line, is_current_sha_focused)
+  local modification_type = hunks.get_modification_type(old_line, new_line, new_sha_focused)
   if modification_type == nil then
     u.notify("Error getting modification type", vim.log.levels.ERROR)
     return
@@ -187,32 +191,32 @@ M.get_reviewer_data = function()
     u.notify("Comments on unmodified lines will be placed in the old file", vim.log.levels.WARN)
   end
 
-  local current_bufnr = is_current_sha_focused and layout.b.file.bufnr or layout.a.file.bufnr
-  local opposite_bufnr = is_current_sha_focused and layout.a.file.bufnr or layout.b.file.bufnr
-  local old_sha_win_id = u.get_window_id_by_buffer_id(layout.a.file.bufnr)
-  local new_sha_win_id = u.get_window_id_by_buffer_id(layout.b.file.bufnr)
+  local current_bufnr = new_sha_focused and layout.b.file.bufnr or layout.a.file.bufnr
+  local opposite_bufnr = new_sha_focused and layout.a.file.bufnr or layout.b.file.bufnr
 
   return {
-    old_file_name = layout.a.file.path,
-    file_name = layout.b.file.path,
+    -- TODO: swap 'a' and 'b' to fix lua/gitlab/actions/comment.lua:158, and hopefully also
+    -- lua/gitlab/indicators/diagnostics.lua:129.
+    file_name = layout.a.file.path,
+    old_file_name = M.is_file_renamed() and layout.b.file.path or "",
     old_line_from_buf = old_line,
     new_line_from_buf = new_line,
     modification_type = modification_type,
-    new_sha_win_id = new_sha_win_id,
     current_bufnr = current_bufnr,
-    old_sha_win_id = old_sha_win_id,
     opposite_bufnr = opposite_bufnr,
+    new_sha_focused = new_sha_focused,
+    current_win_id = current_win,
   }
 end
 
 ---Return whether user is focused on the new version of the file
+---@param current_win integer The ID of the currently focused window
 ---@return boolean
-M.is_current_sha_focused = function()
+M.is_new_sha_focused = function(current_win)
   local view = diffview_lib.get_current_view()
   local layout = view.cur_layout
   local b_win = u.get_window_id_by_buffer_id(layout.b.file.bufnr)
   local a_win = u.get_window_id_by_buffer_id(layout.a.file.bufnr)
-  local current_win = require("gitlab.actions.comment").current_win
   if a_win ~= current_win and b_win ~= current_win then
     current_win = M.stored_win
     M.stored_win = nil