Fix: Restore buffer local settings outside reviewer (#446)

Author: jakubbortlik

lua/gitlab/actions/discussions/init.lua

@@ -15,7 +15,6 @@ local List = require("gitlab.utils.list")
 local tree_utils = require("gitlab.actions.discussions.tree")
 local discussions_tree = require("gitlab.actions.discussions.tree")
 local draft_notes = require("gitlab.actions.draft_notes")
-local diffview_lib = require("diffview.lib")
 local signs = require("gitlab.indicators.signs")
 local diagnostics = require("gitlab.indicators.diagnostics")
 local winbar = require("gitlab.actions.discussions.winbar")
@@ -74,36 +73,23 @@ end
 M.initialize_discussions = function()
   state.discussion_tree.last_updated = os.time()
   signs.setup_signs()
-  reviewer.set_callback_for_file_changed(function()
-    M.refresh_diagnostics()
-    M.modifiable(false)
-    reviewer.set_reviewer_keymaps()
+  reviewer.set_callback_for_file_changed(function(args)
+    reviewer.update_winid_for_buffer(args.buf)
   end)
   reviewer.set_callback_for_reviewer_enter(function()
-    M.modifiable(false)
+    M.refresh_diagnostics()
+  end)
+  reviewer.set_callback_for_buf_read(function(args)
+    vim.api.nvim_buf_set_option(args.buf, "modifiable", false)
+    reviewer.set_keymaps(args.buf)
+    reviewer.set_reviewer_autocommands(args.buf)
   end)
   reviewer.set_callback_for_reviewer_leave(function()
     signs.clear_signs()
     diagnostics.clear_diagnostics()
-    M.modifiable(true)
-    reviewer.del_reviewer_keymaps()
   end)
 end
 
---- Ensures that the both buffers in the reviewer are/not modifiable. Relevant if the user is using
---- the --imply-local setting
-M.modifiable = function(bool)
-  local view = diffview_lib.get_current_view()
-  local a = view.cur_layout.a.file.bufnr
-  local b = view.cur_layout.b.file.bufnr
-  if a ~= nil and vim.api.nvim_buf_is_loaded(a) then
-    vim.api.nvim_buf_set_option(a, "modifiable", bool)
-  end
-  if b ~= nil and vim.api.nvim_buf_is_loaded(b) then
-    vim.api.nvim_buf_set_option(b, "modifiable", bool)
-  end
-end
-
 --- Take existing data and refresh the diagnostics, the winbar, and the signs
 M.refresh_diagnostics = function()
   if state.settings.discussion_signs.enabled then

lua/gitlab/reviewer/init.lua

@@ -16,6 +16,7 @@ local M = {
   bufnr = nil,
   tabnr = nil,
   stored_win = nil,
+  buf_winids = {},
 }
 
 -- Checks for legacy installations, only Diffview is supported.
@@ -62,6 +63,8 @@ M.open = function()
   vim.api.nvim_command(string.format("%s %s..%s", diffview_open_command, diff_refs.base_sha, diff_refs.head_sha))
 
   M.is_open = true
+  local cur_view = diffview_lib.get_current_view()
+  M.diffview_layout = cur_view.cur_layout
   M.tabnr = vim.api.nvim_get_current_tabpage()
 
   if state.settings.discussion_diagnostic ~= nil or state.settings.discussion_sign ~= nil then
@@ -77,9 +80,11 @@ M.open = function()
       M.tabnr = nil
     end
   end
-  require("diffview.config").user_emitter:on("view_closed", function(_, ...)
-    M.is_open = false
-    on_diffview_closed(...)
+  require("diffview.config").user_emitter:on("view_closed", function(_, args)
+    if M.tabnr == args.tabpage then
+      M.is_open = false
+      on_diffview_closed(args)
+    end
   end)
 
   if state.settings.discussion_tree.auto_open then
@@ -248,44 +253,63 @@ M.does_file_have_changes = function()
   return file_data.stats.additions > 0 or file_data.stats.deletions > 0
 end
 
----Diffview exposes events which can be used to setup autocommands.
+---Run callback every time the buffer in one of the two reviewer windows changes.
 ---@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd
 M.set_callback_for_file_changed = function(callback)
   local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.file_changed", {})
   vim.api.nvim_create_autocmd("User", {
     pattern = { "DiffviewDiffBufWinEnter" },
     group = group,
     callback = function(...)
-      M.stored_win = vim.api.nvim_get_current_win()
       if M.tabnr == vim.api.nvim_get_current_tabpage() then
         callback(...)
       end
     end,
   })
 end
 
----Diffview exposes events which can be used to setup autocommands.
+---Run callback the first time a new diff buffer is created and loaded into a window.
+---@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd
+M.set_callback_for_buf_read = function(callback)
+  local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.buf_read", {})
+  vim.api.nvim_create_autocmd("User", {
+    pattern = { "DiffviewDiffBufRead" },
+    group = group,
+    callback = function(...)
+      if vim.api.nvim_get_current_tabpage() == M.tabnr then
+        callback(...)
+      end
+    end,
+  })
+end
+
+---Run callback when the reviewer is closed or the user switches to another tab.
 ---@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd
 M.set_callback_for_reviewer_leave = function(callback)
   local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.leave", {})
   vim.api.nvim_create_autocmd("User", {
     pattern = { "DiffviewViewLeave", "DiffviewViewClosed" },
     group = group,
     callback = function(...)
-      if M.tabnr == vim.api.nvim_get_current_tabpage() then
+      if vim.api.nvim_get_current_tabpage() == M.tabnr then
         callback(...)
       end
     end,
   })
 end
 
+---Run callback when the reviewer is opened for the first time or the view is entered from another
+---tab page.
+---@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd
 M.set_callback_for_reviewer_enter = function(callback)
   local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.enter", {})
   vim.api.nvim_create_autocmd("User", {
-    pattern = { "DiffviewViewOpened" },
+    pattern = { "DiffviewViewEnter", "DiffviewViewOpened" },
     group = group,
     callback = function(...)
-      callback(...)
+      if vim.api.nvim_get_current_tabpage() == M.tabnr then
+        callback(...)
+      end
     end,
   })
 end
@@ -325,8 +349,16 @@ end
 
 ---Set keymaps for creating comments, suggestions and for jumping to discussion tree.
 ---@param bufnr integer Number of the buffer for which the keybindings will be created.
----@param keymaps table The settings keymaps table.
-local set_keymaps = function(bufnr, keymaps)
+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
+  end
+
   -- Set mappings for creating comments
   if keymaps.reviewer.create_comment ~= false then
     -- Set keymap for repeated operator keybinding
@@ -399,29 +431,17 @@ local set_keymaps = function(bufnr, keymaps)
   end
 end
 
---- Sets up keymaps for both buffers in the reviewer.
-M.set_reviewer_keymaps = function()
+---Delete keymaps from reviewer buffers.
+---@param bufnr integer Number of the buffer from which the keybindings will be removed.
+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
   end
-
-  local view = diffview_lib.get_current_view()
-  local a = view.cur_layout.a.file.bufnr
-  local b = view.cur_layout.b.file.bufnr
-  if a ~= nil and vim.api.nvim_buf_is_loaded(a) then
-    set_keymaps(a, keymaps)
-  end
-  if b ~= nil and vim.api.nvim_buf_is_loaded(b) then
-    set_keymaps(b, keymaps)
-  end
-end
-
----Delete keymaps from reviewer buffers.
----@param bufnr integer Number of the buffer from which the keybindings will be removed.
----@param keymaps table The settings keymaps table.
-local del_keymaps = function(bufnr, keymaps)
   for _, func in ipairs({ "create_comment", "create_suggestion" }) do
     if keymaps.reviewer[func] ~= false then
       for _, mode in ipairs({ "n", "o", "v" }) do
@@ -434,23 +454,33 @@ local del_keymaps = function(bufnr, keymaps)
   end
 end
 
---- Deletes keymaps from both buffers in the reviewer.
-M.del_reviewer_keymaps = function()
-  -- 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
-  end
+--- Set up autocaommands that will take care of setting and unsetting buffer-local options and keymaps
+M.set_reviewer_autocommands = function(bufnr)
+  local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.win_enter." .. bufnr, {})
+  vim.api.nvim_create_autocmd({ "WinEnter", "BufWinEnter" }, {
+    group = group,
+    buffer = bufnr,
+    callback = function()
+      if vim.api.nvim_get_current_win() == M.buf_winids[bufnr] then
+        M.stored_win = vim.api.nvim_get_current_win()
+        vim.api.nvim_buf_set_option(bufnr, "modifiable", false)
+        M.set_keymaps(bufnr)
+      else
+        if M.diffview_layout.b.id == M.buf_winids[bufnr] then
+          vim.api.nvim_buf_set_option(bufnr, "modifiable", true)
+        end
+        del_keymaps(bufnr)
+      end
+    end,
+  })
+end
 
-  local view = diffview_lib.get_current_view()
-  local a = view.cur_layout.a.file.bufnr
-  local b = view.cur_layout.b.file.bufnr
-  if a ~= nil and vim.api.nvim_buf_is_loaded(a) then
-    del_keymaps(a, keymaps)
-  end
-  if b ~= nil and vim.api.nvim_buf_is_loaded(b) then
-    del_keymaps(b, keymaps)
-  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