fix: get diffview layout dynamically

lua/gitlab/actions/comment.lua

@@ -32,7 +32,11 @@ M.new_location_from_reviewer = function()
   end
   local diff_hunks =
     hunks.get_hunks(reviewer_data.old_sha, reviewer_data.new_sha, reviewer_data.old_file_name, reviewer_data.file_name)
-  return Location.new(reviewer_data, diff_hunks)
+  local location = Location.new(reviewer_data, diff_hunks)
+  if location == nil then
+    u.notify("Error getting location information", vim.log.levels.ERROR)
+  end
+  return location
 end
 
 ---Fire the API to send the comment data to the Go server.
@@ -340,37 +344,41 @@ M.can_create_comment = function(must_be_visual)
   end
 
   -- Check that we are in a valid buffer
-  if not M.sha_exists() then
+  if not M.file_exists() then
     return false
   end
 
-  -- Check that there aren't saved modifications
-  local file = reviewer.get_current_file_path()
-  if file == nil then
+  -- Check we're in visual mode for code suggestions and multiline comments
+  if must_be_visual and not u.check_visual_mode() then
     return false
   end
-  local has_changes, err = git.has_changes(file)
-  if err ~= nil then
+
+  if M.location == nil then
     return false
   end
+
   -- Check that there aren't unsaved modifications
   local is_modified = vim.bo[0].modified
-  if state.settings.reviewer_settings.diffview.imply_local and (is_modified or has_changes) then
+  if state.settings.reviewer_settings.diffview.imply_local and is_modified then
     u.notify("Cannot leave comments on changed files, please stash or commit and push", vim.log.levels.ERROR)
     return false
   end
 
-  if not git.check_current_branch_up_to_date_on_remote(vim.log.levels.ERROR) then
+  -- Check that there aren't saved modifications
+  local file = reviewer.get_current_file_path()
+  if file == nil then
     return false
   end
-
-  -- Check we're in visual mode for code suggestions and multiline comments
-  if must_be_visual and not u.check_visual_mode() then
+  local has_changes, err = git.has_changes(file)
+  if err ~= nil then
+    return false
+  end
+  if state.settings.reviewer_settings.diffview.imply_local and has_changes then
+    u.notify("Cannot leave comments on changed files, please stash or commit and push", vim.log.levels.ERROR)
     return false
   end
 
-  if M.location == nil then
-    u.notify("Error getting location information", vim.log.levels.ERROR)
+  if not git.check_current_branch_up_to_date_on_remote(vim.log.levels.ERROR) then
     return false
   end
 
@@ -380,7 +388,7 @@ end
 ---Check whether user is commenting on a valid buffer.
 ---Tightly coupled to how the Diffview plugin names non-existent buffers!
 ---@return boolean
-M.sha_exists = function()
+M.file_exists = function()
   if vim.fn.expand("%") == "diffview://null" then
     u.notify("This file does not exist, please comment on the other buffer", vim.log.levels.ERROR)
     return false

lua/gitlab/actions/discussions/init.lua

@@ -98,14 +98,12 @@ M.initialize_discussions = function()
   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()
     M.refresh_diagnostics()
   end)
   reviewer.set_callback_for_buf_read(function(args)
     vim.api.nvim_set_option_value("modifiable", false, { buf = args.buf })
-    reviewer.update_winid_for_buffer(args.buf)
     reviewer.set_keymaps(args.buf)
     reviewer.set_reviewer_autocommands(args.buf)
   end)

lua/gitlab/init.lua

@@ -40,6 +40,8 @@ local function setup(args)
   server.build()
   state.set_global_keymaps()
   require("gitlab.colors") -- Sets colors
+  -- FIXME: The plugin should be set up more lazily: All the work in
+  -- initialize_discussions should be done only when a review is actually opened
   discussions.initialize_discussions()
 
   local is_healthy = health.check(true)

lua/gitlab/reviewer/init.lua

@@ -14,7 +14,6 @@ local M = {
   bufnr = nil,
   tabid = nil,
   stored_win = nil,
-  buf_winids = {},
 }
 
 -- Open the reviewer windows.
@@ -60,7 +59,6 @@ M.open = function()
     u.notify("Could not find Diffview view", vim.log.levels.ERROR)
     return
   end
-  M.diffview_layout = M.diffview.cur_layout --[[@as Diff4]]
   M.tabid = vim.api.nvim_get_current_tabpage()
 
   if state.settings.discussion_diagnostic ~= nil or state.settings.discussion_sign ~= nil then
@@ -110,6 +108,17 @@ M.close = function(opts)
   end
 end
 
+---Return the current diffview layout.
+---@return Diff4?
+M.get_layout = function()
+  local layout = M.diffview and M.diffview.cur_layout
+  if layout == nil then
+    u.notify("Could not find Diffview view", vim.log.levels.ERROR)
+    return
+  end
+  return layout --[[@as Diff4]]
+end
+
 ---Load new INFO state from Gitlab. Then, if diffview.api is available, apply the new
 ---diff refs to the existing diffview, otherwise close and re-open the reviewer.
 M.reload = function()
@@ -163,13 +172,17 @@ M.jump = function(file_name, old_file_name, linenr, new_buffer)
   end
   async.await(M.diffview:set_file(file))
 
+  local diffview_layout = M.get_layout()
+  if diffview_layout == nil then
+    return
+  end
   local number_of_lines
   if new_buffer then
-    M.diffview_layout.b:focus()
-    number_of_lines = u.get_buffer_length(M.diffview_layout.b.file.bufnr)
+    diffview_layout.b:focus()
+    number_of_lines = u.get_buffer_length(diffview_layout.b.file.bufnr)
   else
-    M.diffview_layout.a:focus()
-    number_of_lines = u.get_buffer_length(M.diffview_layout.a.file.bufnr)
+    diffview_layout.a:focus()
+    number_of_lines = u.get_buffer_length(diffview_layout.a.file.bufnr)
   end
   if linenr > number_of_lines then
     u.notify("Diagnostic position outside buffer. Jumping to last line instead.", vim.log.levels.WARN)
@@ -195,7 +208,8 @@ end
 ---Get the data from the reviewer: file names, line information, and cursor focus.
 ---@return ReviewerData?
 M.get_reviewer_data = function()
-  if M.diffview_layout == nil then
+  local diffview_layout = M.get_layout()
+  if diffview_layout == nil then
     return
   end
 
@@ -204,8 +218,8 @@ M.get_reviewer_data = function()
   local diff_refs = state.INFO.diff_refs
 
   return {
-    old_file_name = M.is_file_renamed() and M.diffview_layout.a.file.path or M.diffview_layout.b.file.path,
-    file_name = M.diffview_layout.b.file.path,
+    old_file_name = M.is_file_renamed() and diffview_layout.a.file.path or diffview_layout.b.file.path,
+    file_name = diffview_layout.b.file.path,
     old_sha = diff_refs.base_sha,
     new_sha = diff_refs.head_sha,
     start_line = start_line,
@@ -218,8 +232,12 @@ end
 ---@param current_win integer The ID of the currently focused window
 ---@return boolean
 M.is_new_file_focused = function(current_win)
-  local b_win = u.get_window_id_by_buffer_id(M.diffview_layout.b.file.bufnr)
-  local a_win = u.get_window_id_by_buffer_id(M.diffview_layout.a.file.bufnr)
+  local diffview_layout = M.get_layout()
+  if diffview_layout == nil then
+    return false
+  end
+  local b_win = u.get_window_id_by_buffer_id(diffview_layout.b.file.bufnr)
+  local a_win = u.get_window_id_by_buffer_id(diffview_layout.a.file.bufnr)
   if a_win ~= current_win and b_win ~= current_win then
     current_win = M.stored_win
     M.stored_win = nil
@@ -372,7 +390,6 @@ M.set_keymaps = function(bufnr)
   if bufnr == nil or not vim.api.nvim_buf_is_loaded(bufnr) then
     return
   end
-  -- Require keymaps only after user settings have been merged with defaults
   local keymaps = require("gitlab.state").settings.keymaps
   if keymaps.disable_all or keymaps.reviewer.disable_all then
     return
@@ -460,7 +477,6 @@ local del_keymaps = function(bufnr)
   if bufnr == nil or not vim.api.nvim_buf_is_loaded(bufnr) then
     return
   end
-  -- Require keymaps only after user settings have been merged with defaults
   local keymaps = require("gitlab.state").settings.keymaps
   if keymaps.disable_all or keymaps.reviewer.disable_all then
     return
@@ -478,18 +494,33 @@ local del_keymaps = function(bufnr)
 end
 
 ---Set up autocommands to set and unset buffer-local options and keymaps.
+---The buffer is set up when entering it as part of the reviewer layout, and reset when
+---entering the buffer in any other window.
 M.set_reviewer_autocommands = function(bufnr)
+  local diffview_layout = M.get_layout()
+  if diffview_layout == nil then
+    return false
+  end
+  local is_new_file = bufnr == diffview_layout.b.file.bufnr
+
   local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.win_enter." .. bufnr, {})
-  vim.api.nvim_create_autocmd({ "WinEnter", "BufWinEnter" }, {
+  vim.api.nvim_create_autocmd({
+    "WinEnter", -- Fires when focusing window that already is displaying buffer (e.g., when navigating to buffer in another tab)
+    "BufWinEnter", -- Fires when buffer enters focused window (e.g., when cycling diffview files)
+  }, {
     buffer = bufnr,
     callback = function()
-      if vim.api.nvim_get_current_win() == M.buf_winids[bufnr] then
+      local _diffview_layout = M.get_layout()
+      if _diffview_layout == nil then
+        return false
+      end
+      if vim.api.nvim_get_current_win() == (is_new_file and _diffview_layout.b.id or _diffview_layout.a.id) then
         M.stored_win = vim.api.nvim_get_current_win()
         u.switch_can_edit_buf(bufnr, false)
         M.set_keymaps(bufnr)
       else
-        -- Only make the local file modifiable, not the diffview buffer for the old revision
-        if M.diffview_layout.b.id == M.buf_winids[bufnr] then
+        -- Only make the local file modifiable, not the diffview buffers
+        if vim.bo[bufnr].buftype ~= "nowrite" then
           u.switch_can_edit_buf(bufnr, true)
         end
         del_keymaps(bufnr)
@@ -500,13 +531,4 @@ M.set_reviewer_autocommands = function(bufnr)
   })
 end
 
----Update the stored winid for a given reviewer buffer.
----This is necessary for the M.set_reviewer_autocommands function to work correctly in
----cases like when the user closes one of the original reviewer windows and Diffview
----automatically creates a new pair of reviewer windows or the user wipes out a buffer
----and Diffview reloads it with a different ID.
-M.update_winid_for_buffer = function(bufnr)
-  M.buf_winids[bufnr] = vim.fn.bufwinid(bufnr)
-end
-
 return M