feat: open discussions immediately load data afterwards

Main features:
- opens discussion tree immediately and loads data asynchronously
- improves restoring cursor position after rebuilding trees

Minor improvements:
- moves view type switching from winbar to discussions
- adds protection from invalid line number
- replaces deprecated vim.api function

Author: jakubbortlik

doc/gitlab.nvim.txt

@@ -264,6 +264,7 @@ you call this function with no values the defaults will be used:
         },
         spinner_chars = { "/", "|", "\\", "-" }, -- Characters for the refresh animation
         auto_open = true, -- Automatically open when the reviewer is opened
+        enter_on_open = true, -- Automatically enter the discussion tree when it is opened
         default_view = "discussions", -- Show "discussions" or "notes" by default
         blacklist = {}, -- List of usernames to remove from tree (bots, CI, etc)
         sort_by = "latest_reply", -- Sort discussion tree by the "latest_reply", or by "original_comment", see `:h gitlab.nvim.toggle_sort_method`

lua/gitlab/actions/discussions/init.lua

@@ -89,7 +89,7 @@ M.initialize_discussions = function()
     M.refresh_diagnostics()
   end)
   reviewer.set_callback_for_buf_read(function(args)
-    vim.api.nvim_buf_set_option(args.buf, "modifiable", false)
+    vim.api.nvim_set_option_value("modifiable", false, { buf = args.buf })
     reviewer.set_keymaps(args.buf)
     reviewer.set_reviewer_autocommands(args.buf)
   end)
@@ -112,46 +112,67 @@ end
 ---@param callback function?
 ---@param view_type "discussions"|"notes" Defines the view type to select (useful for overriding the default view type when jumping to discussion tree when it's closed).
 M.open = function(callback, view_type)
-  view_type = view_type and view_type or state.settings.discussion_tree.default_view
+  local original_window = vim.api.nvim_get_current_win() -- The window from which ther user called M.open
+
+  M.current_view_type = view_type and view_type or state.settings.discussion_tree.default_view
+  state.DISCUSSION_DATA = u.ensure_table(state.DISCUSSION_DATA)
   state.DISCUSSION_DATA.discussions = u.ensure_table(state.DISCUSSION_DATA.discussions)
   state.DISCUSSION_DATA.unlinked_discussions = u.ensure_table(state.DISCUSSION_DATA.unlinked_discussions)
   state.DRAFT_NOTES = u.ensure_table(state.DRAFT_NOTES)
 
-  -- Make buffers, get and set buffer numbers, set filetypes
+  -- Make discussion split window and buffers, store buffer numbers
   local split, linked_bufnr, unlinked_bufnr = M.create_split_and_bufs()
   M.split = split
   M.linked_bufnr = linked_bufnr
   M.unlinked_bufnr = unlinked_bufnr
+  M.split_visible = true
+  split:mount()
 
+  -- Set window and buffer local options to discussion tree split after mounting the split
   for opt, val in pairs(state.settings.discussion_tree.winopts) do
     vim.api.nvim_set_option_value(opt, val, { win = M.split.winid })
   end
 
   vim.api.nvim_set_option_value("filetype", "gitlab", { buf = M.linked_bufnr })
   vim.api.nvim_set_option_value("filetype", "gitlab", { buf = M.unlinked_bufnr })
 
-  M.split = split
-  M.split_visible = true
-  split:mount()
+  -- Set autocmds to clean up state when discussions buffers are deleted manually
+  vim.api.nvim_create_autocmd("BufWipeout", {
+    buffer = M.linked_bufnr,
+    callback = function()
+      M.linked_bufnr = nil
+    end,
+  })
+  vim.api.nvim_create_autocmd("BufWipeout", {
+    buffer = M.unlinked_bufnr,
+    callback = function()
+      M.unlinked_bufnr = nil
+    end,
+  })
 
-  -- Initialize winbar module with data from buffers
-  winbar.start_timer()
-  winbar.set_buffers(M.linked_bufnr, M.unlinked_bufnr)
-  winbar.switch_view_type(view_type)
+  -- Set autocmd to clean up state when discussions split is closed manually
+  vim.api.nvim_create_autocmd("WinClosed", {
+    pattern = tostring(M.split.winid),
+    callback = M.close,
+  })
 
-  local current_window = vim.api.nvim_get_current_win() -- Save user's current window in case they switched while content was loading
-  vim.api.nvim_set_current_win(M.split.winid)
+  -- Initialize winbar
+  winbar.start_timer()
 
-  common.switch_can_edit_bufs(true, M.linked_bufnr, M.unlinked_bufnr)
-  M.rebuild_discussion_tree()
+  -- Rebuild trees in order to set keymaps and make buffers protected
+  M.switch_view_type(M.current_view_type)
   M.rebuild_unlinked_discussion_tree()
+  M.rebuild_discussion_tree()
 
-  -- Set default buffer
-  local default_buffer = winbar.bufnr_map[view_type]
-  vim.api.nvim_set_current_buf(default_buffer)
-  common.switch_can_edit_bufs(false, M.linked_bufnr, M.unlinked_bufnr)
+  -- Focus the correct window
+  local win_to_enter = not state.settings.discussion_tree.enter_on_open and original_window or M.split.winid
+  if vim.api.nvim_win_is_valid(win_to_enter) then
+    vim.api.nvim_set_current_win(win_to_enter)
+  end
+
+  -- Relooad data
+  draft_notes.rebuild_view(false, true)
 
-  vim.api.nvim_set_current_win(current_window)
   if type(callback) == "function" then
     callback()
   end
@@ -195,7 +216,7 @@ M.move_to_discussion_tree = function()
       end
       M.discussion_tree:render()
       vim.api.nvim_set_current_win(M.split.winid)
-      winbar.switch_view_type("discussions")
+      M.switch_view_type("discussions")
       vim.api.nvim_win_set_cursor(M.split.winid, { line_number, 0 })
     end
 
@@ -441,6 +462,7 @@ M.rebuild_discussion_tree = function()
   end
 
   local current_node = discussions_tree.get_node_at_cursor(M.discussion_tree, M.last_node_at_cursor)
+  local current_cursor_column = vim.api.nvim_win_get_cursor(0)[2]
 
   local expanded_node_ids = M.gather_expanded_node_ids(M.discussion_tree)
   common.switch_can_edit_bufs(true, M.linked_bufnr, M.unlinked_bufnr)
@@ -463,7 +485,7 @@ M.rebuild_discussion_tree = function()
     tree_utils.open_node_by_id(discussion_tree, id)
   end
   discussion_tree:render()
-  discussions_tree.restore_cursor_position(M.split.winid, discussion_tree, current_node)
+  discussions_tree.restore_cursor_position(M.split.winid, discussion_tree, current_cursor_column, current_node, nil)
 
   M.set_tree_keymaps(discussion_tree, M.linked_bufnr, false)
   M.discussion_tree = discussion_tree
@@ -479,6 +501,7 @@ M.rebuild_unlinked_discussion_tree = function()
   end
 
   local current_node = discussions_tree.get_node_at_cursor(M.unlinked_discussion_tree, M.last_node_at_cursor)
+  local current_cursor_column = vim.api.nvim_win_get_cursor(0)[2]
 
   local expanded_node_ids = M.gather_expanded_node_ids(M.unlinked_discussion_tree)
   common.switch_can_edit_bufs(true, M.linked_bufnr, M.unlinked_bufnr)
@@ -501,7 +524,7 @@ M.rebuild_unlinked_discussion_tree = function()
     tree_utils.open_node_by_id(unlinked_discussion_tree, id)
   end
   unlinked_discussion_tree:render()
-  discussions_tree.restore_cursor_position(M.split.winid, unlinked_discussion_tree, current_node)
+  discussions_tree.restore_cursor_position(M.split.winid, unlinked_discussion_tree, current_cursor_column, current_node)
 
   M.set_tree_keymaps(unlinked_discussion_tree, M.unlinked_bufnr, true)
   M.unlinked_discussion_tree = unlinked_discussion_tree
@@ -681,7 +704,7 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked)
 
   if keymaps.discussion_tree.toggle_node then
     vim.keymap.set("n", keymaps.discussion_tree.toggle_node, function()
-      tree_utils.toggle_node(tree)
+      tree_utils.toggle_node(M.split.winid, tree)
     end, { buffer = bufnr, desc = "Toggle node", nowait = keymaps.discussion_tree.toggle_node_nowait })
   end
 
@@ -737,7 +760,7 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked)
 
   if keymaps.discussion_tree.switch_view then
     vim.keymap.set("n", keymaps.discussion_tree.switch_view, function()
-      winbar.switch_view_type()
+      M.switch_view_type()
     end, {
       buffer = bufnr,
       desc = "Change view type between discussions and notes",
@@ -804,6 +827,21 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked)
   emoji.init_popup(tree, bufnr)
 end
 
+---Toggles the current view type (or sets it to `override`) and then updates the view.
+---@param override? "discussions"|"notes" The view type to select.
+M.switch_view_type = function(override)
+  vim.api.nvim_set_option_value("winfixbuf", false, { win = M.split.winid })
+  if override == "discussions" or M.current_view_type == "notes" then
+    M.current_view_type = "discussions"
+    vim.api.nvim_set_current_buf(M.linked_bufnr)
+  elseif override == "notes" or M.current_view_type == "discussions" then
+    M.current_view_type = "notes"
+    vim.api.nvim_set_current_buf(M.unlinked_bufnr)
+  end
+  vim.api.nvim_set_option_value("winfixbuf", true, { win = M.split.winid })
+  winbar.update_winbar()
+end
+
 ---Toggle comments tree type between "simple" and "by_file_name"
 M.toggle_tree_type = function()
   if state.settings.discussion_tree.tree_type == "simple" then

lua/gitlab/actions/discussions/tree.lua

@@ -256,29 +256,26 @@ M.create_node_list_by_file_name = function(node_list)
   return discussion_by_file_name
 end
 
-local attach_uuid = function(str)
-  return { text = str, id = u.uuid() }
-end
-
 ---Build note node body
 ---@param note Note|DraftNote
 ---@param resolve_info table?
 ---@return string
 ---@return NuiTree.Node[]
 local function build_note_body(note, resolve_info)
   local text_nodes = {}
-  for bodyLine in u.split_by_new_lines(note.body or note.note) do
-    local line = attach_uuid(bodyLine)
+  local i = 0
+  for body_line in u.split_by_new_lines(note.body or note.note) do
     table.insert(
       text_nodes,
       NuiTree.Node({
         new_line = (type(note.position) == "table" and note.position.new_line),
         old_line = (type(note.position) == "table" and note.position.old_line),
-        text = line.text,
-        id = line.id,
+        text = body_line,
+        id = string.format("%d:%d", note.id, i),
         type = "note_body",
       }, {})
     )
+    i = i + 1
   end
 
   local symbol = ""
@@ -377,8 +374,8 @@ end
 ---@field keep_current_open boolean Whether to keep the current discussion open even if it should otherwise be closed.
 
 ---This function expands/collapses all nodes and their children according to the opts.
----@param tree NuiTree
 ---@param winid integer
+---@param tree NuiTree
 ---@param unlinked boolean
 ---@param opts ToggleNodesOptions
 M.toggle_nodes = function(winid, tree, unlinked, opts)
@@ -387,6 +384,7 @@ M.toggle_nodes = function(winid, tree, unlinked, opts)
     return
   end
   local root_node = common.get_root_node(tree, current_node)
+  local current_cursor_column = vim.api.nvim_win_get_cursor(winid)[2]
   for _, node in ipairs(tree:get_nodes()) do
     if opts.toggle_resolved then
       if
@@ -426,7 +424,7 @@ M.toggle_nodes = function(winid, tree, unlinked, opts)
     end
   end
   tree:render()
-  M.restore_cursor_position(winid, tree, current_node, root_node)
+  M.restore_cursor_position(winid, tree, current_cursor_column, current_node, root_node)
 end
 
 -- Get current node for restoring cursor position
@@ -446,9 +444,10 @@ end
 ---Restore cursor position to the original node if possible
 ---@param winid integer Window number of the discussions split
 ---@param tree NuiTree The inline discussion tree or the unlinked discussion tree
+---@param cursor_column integer The original column of the cursor
 ---@param original_node NuiTree.Node|nil The last node with the cursor
 ---@param root_node NuiTree.Node|nil The root node of the last node with the cursor
-M.restore_cursor_position = function(winid, tree, original_node, root_node)
+M.restore_cursor_position = function(winid, tree, cursor_column, original_node, root_node)
   if original_node == nil or tree == nil then
     return
   end
@@ -460,10 +459,9 @@ M.restore_cursor_position = function(winid, tree, original_node, root_node)
       _, line_number = tree:get_node("-" .. tostring(root_node.id))
     end
   end
-  if line_number ~= nil then
-    if vim.api.nvim_win_is_valid(winid) then
-      vim.api.nvim_win_set_cursor(winid, { line_number, 0 })
-    end
+  if line_number ~= nil and winid and vim.api.nvim_win_is_valid(winid) then
+    local last_line = vim.fn.line("$")
+    vim.api.nvim_win_set_cursor(winid, { math.min(line_number, last_line), cursor_column or 0 })
   end
 end
 
@@ -518,24 +516,22 @@ M.open_node_by_id = function(tree, id)
 end
 
 -- This function (settings.keymaps.discussion_tree.toggle_node) expands/collapses the current node and its children
-M.toggle_node = function(tree)
+---@param winid integer The id if the tree split.
+---@param tree NuiTree The current discussion tree.
+M.toggle_node = function(winid, tree)
   local node = tree:get_node()
-  if node == nil then
-    return
-  end
+  local current_cursor_column = vim.api.nvim_win_get_cursor(winid)[2]
 
   -- Switch to the "note" node from "note_body" nodes to enable toggling discussions inside comments
-  if node.type == "note_body" then
+  if node ~= nil and node.type == "note_body" then
     node = tree:get_node(node:get_parent_id())
   end
   if node == nil then
     return
   end
 
   local children = node:get_child_ids()
-  if node == nil then
-    return
-  end
+
   if node:is_expanded() then
     node:collapse()
     if common.is_node_note(node) then
@@ -553,6 +549,7 @@ M.toggle_node = function(tree)
   end
 
   tree:render()
+  M.restore_cursor_position(winid, tree, current_cursor_column, node, common.get_root_node(tree, node))
 end
 
 return M

lua/gitlab/actions/discussions/winbar.lua

@@ -2,20 +2,7 @@ local u = require("gitlab.utils")
 local List = require("gitlab.utils.list")
 local state = require("gitlab.state")
 
-local M = {
-  bufnr_map = {
-    discussions = nil,
-    notes = nil,
-  },
-  current_view_type = state.settings.discussion_tree.default_view,
-}
-
-M.set_buffers = function(linked_bufnr, unlinked_bufnr)
-  M.bufnr_map = {
-    discussions = linked_bufnr,
-    notes = unlinked_bufnr,
-  }
-end
+local M = {}
 
 ---@param nodes Discussion[]|UnlinkedDiscussion[]|nil
 ---@return number, number, number
@@ -158,7 +145,7 @@ end
 
 ---@param t WinbarTable
 M.make_winbar = function(t)
-  local discussions_focused = M.current_view_type == "discussions"
+  local discussions_focused = require("gitlab.actions.discussions").current_view_type == "discussions"
   local discussion_text = add_drafts_and_resolvable(
     "Comments:",
     t.resolvable_discussions,
@@ -269,23 +256,6 @@ M.get_ahead_behind = function(ahead, behind)
   return a .. "↑ " .. b .. "↓"
 end
 
----Toggles the current view type (or sets it to `override`) and then updates the view.
----@param override "discussions"|"notes" Defines the view type to select.
-M.switch_view_type = function(override)
-  if override then
-    M.current_view_type = override
-  else
-    if M.current_view_type == "discussions" then
-      M.current_view_type = "notes"
-    elseif M.current_view_type == "notes" then
-      M.current_view_type = "discussions"
-    end
-  end
-
-  vim.api.nvim_set_current_buf(M.bufnr_map[M.current_view_type])
-  M.update_winbar()
-end
-
 ---Set up a timer to update the winbar periodically
 M.start_timer = function()
   M.cleanup_timer()

lua/gitlab/annotations.lua

@@ -300,6 +300,7 @@
 ---@class DiscussionSettings: table
 ---@field expanders? ExpanderOpts -- Customize the expander icons in the discussion tree
 ---@field auto_open? boolean -- Automatically open when the reviewer is opened
+---@field enter_on_open? boolean -- Automatically enter the discussion tree when it is opened
 ---@field default_view? string - Show "discussions" or "notes" by default
 ---@field blacklist? table<string> -- List of usernames to remove from tree (bots, CI, etc)
 ---@field keep_current_open? boolean -- If true, current discussion stays open even if it should otherwise be closed when toggling

lua/gitlab/indicators/diagnostics.lua

@@ -113,6 +113,9 @@ end
 ---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 vim.api.nvim_buf_is_valid(bufnr) then
+    return
+  end
   if not state.settings.discussion_signs.enabled then
     return
   end