fix: make cycling reviewed files faster (#474)

Author: jakubbortlik

doc/gitlab.nvim.txt

@@ -316,6 +316,7 @@ you call this function with no values the defaults will be used:
           comment = "→|",
           range = " |",
         },
+        skip_old_revision_discussion = false, -- Don't show diagnostics for discussions that were created for earlier MR revisions
       },
       pipeline = {
         created = "",

lua/gitlab/actions/common.lua

@@ -265,7 +265,7 @@ M.get_line_number_from_node = function(root_node)
 end
 
 -- This function (settings.keymaps.discussion_tree.jump_to_reviewer) will jump the cursor to the reviewer's location associated with the note. The implementation depends on the reviewer
-M.jump_to_reviewer = function(tree, callback)
+M.jump_to_reviewer = function(tree)
   local node = tree:get_node()
   local root_node = M.get_root_node(tree, node)
   if root_node == nil then
@@ -278,7 +278,6 @@ M.jump_to_reviewer = function(tree, callback)
     return
   end
   reviewer.jump(root_node.file_name, line_number, is_new_sha)
-  callback()
 end
 
 -- This function (settings.keymaps.discussion_tree.jump_to_file) will jump to the file changed in a new tab

lua/gitlab/actions/discussions/init.lua

@@ -74,6 +74,7 @@ M.initialize_discussions = function()
   state.discussion_tree.last_updated = os.time()
   signs.setup_signs()
   reviewer.set_callback_for_file_changed(function(args)
+    diagnostics.place_diagnostics(args.buf)
     reviewer.update_winid_for_buffer(args.buf)
   end)
   reviewer.set_callback_for_reviewer_enter(function()
@@ -90,7 +91,7 @@ M.initialize_discussions = function()
   end)
 end
 
---- Take existing data and refresh the diagnostics, the winbar, and the signs
+--- Take existing data and refresh the diagnostics and the signs
 M.refresh_diagnostics = function()
   if state.settings.discussion_signs.enabled then
     diagnostics.refresh_diagnostics()
@@ -573,7 +574,7 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked)
     if keymaps.discussion_tree.jump_to_reviewer then
       vim.keymap.set("n", keymaps.discussion_tree.jump_to_reviewer, function()
         if M.is_current_node_note(tree) then
-          common.jump_to_reviewer(tree, M.refresh_diagnostics)
+          common.jump_to_reviewer(tree)
         end
       end, { buffer = bufnr, desc = "Jump to reviewer", nowait = keymaps.discussion_tree.jump_to_reviewer_nowait })
     end
@@ -743,6 +744,16 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked)
     })
   end
 
+  if keymaps.discussion_tree.print_node then
+    vim.keymap.set("n", keymaps.discussion_tree.print_node, function()
+      common.print_node(tree)
+    end, {
+      buffer = bufnr,
+      desc = "Print current node (for debugging)",
+      nowait = keymaps.discussion_tree.print_node_nowait,
+    })
+  end
+
   if keymaps.discussion_tree.add_emoji then
     vim.keymap.set("n", keymaps.discussion_tree.add_emoji, function()
       M.add_emoji_to_note(tree, unlinked)

lua/gitlab/annotations.lua

@@ -346,3 +346,15 @@
 ---@field perform_action? string -- Once in normal mode, does action (like saving comment or applying description edit, etc)
 ---@field perform_linewise_action? string -- Once in normal mode, does the linewise action (see logs for this job, etc)
 ---@field discard_changes? string -- Quit the popup discarding changes, the popup content is not? saved to the `temp_registers` (see `:h gitlab.nvim.temp-registers`)
+
+---@class List The base class for all list objects
+---@field new function -- Creates a new List from a table
+---@field map function -- Mutates a given list
+---@field filter function -- Filters a given list
+---@field partition function -- Partitions a given list into two lists
+---@field reduce function -- Applies a function to reduce the list to a single value
+---@field sort function -- Sorts the list in place based on a comparator function
+---@field find function -- Returns the first element that satisfies the callback
+---@field slice function -- Returns a portion of the list between start and end indices
+---@field includes function -- Returns true if any of the elements can satisfy the callback
+---@field values function -- Returns an iterator over the list's values

lua/gitlab/indicators/common.lua

@@ -1,6 +1,5 @@
 local u = require("gitlab.utils")
 local state = require("gitlab.state")
-local reviewer = require("gitlab.reviewer")
 local List = require("gitlab.utils.list")
 
 local M = {}
@@ -11,24 +10,25 @@ local M = {}
 ---@field resolved boolean|nil
 ---@field created_at string|nil
 
+---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, file)
+local filter_discussions_and_notes = function(note)
   ---Do not include unlinked notes
   return note.position ~= nil
-    and (note.position.new_path == file or note.position.old_path == file)
     ---Skip resolved discussions if user wants to
     and not (state.settings.discussion_signs.skip_resolved_discussion and note.resolvable and note.resolved)
     ---Skip discussions from old revisions
     and not (
       state.settings.discussion_signs.skip_old_revision_discussion
+      and note.created_at ~= nil
       and u.from_iso_format_date_to_timestamp(note.created_at)
         <= u.from_iso_format_date_to_timestamp(state.MR_REVISIONS[1].created_at)
     )
 end
 
----Filter all discussions which are relevant for currently visible signs and diagnostics.
+---Filter all discussions and drafts which have placeable signs and diagnostics.
 ---@return Discussion|DraftNote[]
 M.filter_placeable_discussions = function()
   local discussions = u.ensure_table(state.DISCUSSION_DATA and state.DISCUSSION_DATA.discussions or {})
@@ -41,18 +41,13 @@ M.filter_placeable_discussions = function()
     draft_notes = {}
   end
 
-  local file = reviewer.get_current_file_path()
-  if not file then
-    return {}
-  end
-
   local filtered_discussions = List.new(discussions):filter(function(discussion)
     local first_note = discussion.notes[1]
-    return type(first_note.position) == "table" and filter_discussions_and_notes(first_note, file)
+    return type(first_note.position) == "table" and filter_discussions_and_notes(first_note)
   end)
 
   local filtered_draft_notes = List.new(draft_notes):filter(function(note)
-    return filter_discussions_and_notes(note, file)
+    return filter_discussions_and_notes(note)
   end)
 
   return u.join(filtered_discussions, filtered_draft_notes)

lua/gitlab/indicators/diagnostics.lua

@@ -85,47 +85,64 @@ local create_multiline_diagnostic = function(d_or_n)
   }, d_or_n)
 end
 
----Set diagnostics in currently new SHA.
+---Set diagnostics in the given buffer.
 ---@param namespace number namespace for diagnostics
+---@param bufnr number the bufnr for placing the diagnostics
 ---@param diagnostics table see :h vim.diagnostic.set
 ---@param opts table? see :h vim.diagnostic.set
-local set_diagnostics_in_new_sha = function(namespace, diagnostics, opts)
+local set_diagnostics = function(namespace, bufnr, diagnostics, opts)
+  vim.diagnostic.set(namespace, bufnr, diagnostics, opts)
+  require("gitlab.indicators.signs").set_signs(diagnostics, bufnr)
+end
+
+---Refresh the diagnostics for all the reviewed files, and place diagnostics for the currently
+---visible buffers.
+M.refresh_diagnostics = function()
+  require("gitlab.indicators.signs").clear_signs()
+  M.clear_diagnostics()
+  M.placeable_discussions = indicators_common.filter_placeable_discussions()
+
   local view = diffview_lib.get_current_view()
-  if not view then
+  if view == nil then
+    u.notify("Could not find Diffview view", vim.log.levels.ERROR)
     return
   end
-  vim.diagnostic.set(namespace, view.cur_layout.b.file.bufnr, diagnostics, opts)
-  require("gitlab.indicators.signs").set_signs(diagnostics, view.cur_layout.b.file.bufnr)
+  M.place_diagnostics(view.cur_layout.a.file.bufnr)
+  M.place_diagnostics(view.cur_layout.b.file.bufnr)
 end
 
----Set diagnostics in old SHA.
----@param namespace number namespace for diagnostics
----@param diagnostics table see :h vim.diagnostic.set
----@param opts table? see :h vim.diagnostic.set
-local set_diagnostics_in_old_sha = function(namespace, diagnostics, opts)
+---Filter and place the diagnostics for the given buffer.
+---@param bufnr number The number of the buffer for placing diagnostics.
+M.place_diagnostics = function(bufnr)
+  if not state.settings.discussion_signs.enabled then
+    return
+  end
   local view = diffview_lib.get_current_view()
-  if not view then
+  if view == nil then
+    u.notify("Could not find Diffview view", vim.log.levels.ERROR)
     return
   end
-  vim.diagnostic.set(namespace, view.cur_layout.a.file.bufnr, diagnostics, opts)
-  require("gitlab.indicators.signs").set_signs(diagnostics, view.cur_layout.a.file.bufnr)
-end
 
----Refresh the diagnostics for the currently reviewed file
-M.refresh_diagnostics = function()
   local ok, err = pcall(function()
-    require("gitlab.indicators.signs").clear_signs()
-    M.clear_diagnostics()
-    local filtered_discussions = indicators_common.filter_placeable_discussions()
-    if filtered_discussions == nil then
+    local file_discussions = List.new(M.placeable_discussions):filter(function(discussion_or_note)
+      local note = discussion_or_note.notes and discussion_or_note.notes[1] or discussion_or_note
+      -- Surprisingly, the following line works even if I'd expect it should match new/old paths
+      -- with a/b buffers the other way round!
+      return note.position.new_path == view.cur_layout.a.file.path
+        or note.position.old_path == view.cur_layout.b.file.path
+    end)
+
+    if #file_discussions == 0 then
       return
     end
 
-    local new_diagnostics = M.parse_new_diagnostics(filtered_discussions)
-    set_diagnostics_in_new_sha(diagnostics_namespace, new_diagnostics, create_display_opts())
+    local new_diagnostics, old_diagnostics = List.new(file_discussions):partition(indicators_common.is_new_sha)
 
-    local old_diagnostics = M.parse_old_diagnostics(filtered_discussions)
-    set_diagnostics_in_old_sha(diagnostics_namespace, old_diagnostics, create_display_opts())
+    if bufnr == view.cur_layout.a.file.bufnr then
+      set_diagnostics(diagnostics_namespace, bufnr, M.parse_diagnostics(old_diagnostics), create_display_opts())
+    elseif bufnr == view.cur_layout.b.file.bufnr then
+      set_diagnostics(diagnostics_namespace, bufnr, M.parse_diagnostics(new_diagnostics), create_display_opts())
+    end
   end)
 
   if not ok then
@@ -134,24 +151,13 @@ M.refresh_diagnostics = function()
 end
 
 ---Iterates over each discussion and returns a list of tables with sign
----data, for instance group, priority, line number etc for the new SHA
----@param discussions Discussion[]
----@return DiagnosticTable[]
-M.parse_new_diagnostics = function(discussions)
-  local new_diagnostics = List.new(discussions):filter(indicators_common.is_new_sha)
-  local single_line = new_diagnostics:filter(indicators_common.is_single_line):map(create_single_line_diagnostic)
-  local multi_line = new_diagnostics:filter(indicators_common.is_multi_line):map(create_multiline_diagnostic)
-  return u.combine(single_line, multi_line)
-end
-
----Iterates over each discussion and returns a list of tables with sign
----data, for instance group, priority, line number etc for the old SHA
----@param discussions Discussion[]
+---data, for instance group, priority, line number etc
+---@param discussions List
 ---@return DiagnosticTable[]
-M.parse_old_diagnostics = function(discussions)
-  local old_diagnostics = List.new(discussions):filter(indicators_common.is_old_sha)
-  local single_line = old_diagnostics:filter(indicators_common.is_single_line):map(create_single_line_diagnostic)
-  local multi_line = old_diagnostics:filter(indicators_common.is_multi_line):map(create_multiline_diagnostic)
+M.parse_diagnostics = function(discussions)
+  local single_line, multi_line = discussions:partition(indicators_common.is_single_line)
+  single_line = single_line:map(create_single_line_diagnostic)
+  multi_line = multi_line:map(create_multiline_diagnostic)
   return u.combine(single_line, multi_line)
 end