Fix: Checking whether comment can be created (#434)

Author: jakubbortlik

lua/gitlab/actions/comment.lua

@@ -3,7 +3,6 @@
 --- to this module the data required to make the API calls
 local Popup = require("nui.popup")
 local Layout = require("nui.layout")
-local diffview_lib = require("diffview.lib")
 local state = require("gitlab.state")
 local job = require("gitlab.job")
 local u = require("gitlab.utils")
@@ -47,6 +46,18 @@ local confirm_create_comment = function(text, visual_range, unlinked, discussion
     return
   end
 
+  -- Creating a draft reply, in response to a discussion ID
+  if discussion_id ~= nil and is_draft then
+    local body = { comment = text, discussion_id = discussion_id }
+    job.run_job("/mr/draft_notes/", "POST", body, function()
+      u.notify("Draft reply created!", vim.log.levels.INFO)
+      draft_notes.load_draft_notes(function()
+        discussions.rebuild_view(unlinked)
+      end)
+    end)
+    return
+  end
+
   -- Creating a note (unlinked comment)
   if unlinked and discussion_id == nil then
     local body = { comment = text }
@@ -90,18 +101,6 @@ local confirm_create_comment = function(text, visual_range, unlinked, discussion
     line_range = location_data.line_range,
   }
 
-  -- Creating a draft reply, in response to a discussion ID
-  if discussion_id ~= nil and is_draft then
-    local body = { comment = text, discussion_id = discussion_id, position = position_data }
-    job.run_job("/mr/draft_notes/", "POST", body, function()
-      u.notify("Draft reply created!", vim.log.levels.INFO)
-      draft_notes.load_draft_notes(function()
-        discussions.rebuild_view(unlinked)
-      end)
-    end)
-    return
-  end
-
   -- Creating a new comment (linked to specific changes)
   local body = u.merge({ type = "text", comment = text }, position_data)
   local endpoint = is_draft and "/mr/draft_notes/" or "/mr/comment"
@@ -158,46 +157,8 @@ end
 ---multi-line comment. It also sets up the basic keybindings for switching between
 ---window panes, and for the non-primary sections.
 ---@param opts LayoutOpts
----@return NuiLayout|nil
+---@return NuiLayout
 M.create_comment_layout = function(opts)
-  if opts.unlinked ~= true and opts.discussion_id == nil then
-    -- Check that diffview is initialized
-    if reviewer.tabnr == nil then
-      u.notify("Reviewer must be initialized first", vim.log.levels.ERROR)
-      return
-    end
-
-    -- Check that Diffview is the current view
-    local view = diffview_lib.get_current_view()
-    if view == nil and not opts.reply then
-      u.notify("Comments should be left in the reviewer pane", vim.log.levels.ERROR)
-      return
-    end
-
-    -- Check that we are in the diffview tab
-    local tabnr = vim.api.nvim_get_current_tabpage()
-    if tabnr ~= reviewer.tabnr then
-      u.notify("Line location can only be determined within reviewer window", vim.log.levels.ERROR)
-      return
-    end
-
-    -- Check that the file has not been renamed
-    if reviewer.is_file_renamed() and not reviewer.does_file_have_changes() then
-      u.notify("Commenting on (unchanged) renamed or moved files is not supported", vim.log.levels.WARN)
-      return
-    end
-
-    -- Check that we are hovering over the code
-    local filetype = vim.bo[0].filetype
-    if not opts.reply and (filetype == "DiffviewFiles" or filetype == "gitlab") then
-      u.notify(
-        "Comments can only be left on the code. To leave unlinked comments, use gitlab.create_note() instead",
-        vim.log.levels.ERROR
-      )
-      return
-    end
-  end
-
   local popup_settings = state.settings.popup
   local title
   local user_settings
@@ -262,53 +223,31 @@ 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()
-  local has_clean_tree, err = git.has_clean_tree()
-  if err ~= nil then
-    return
-  end
-
-  local is_modified = vim.bo[0].modified
-  if state.settings.reviewer_settings.diffview.imply_local and (is_modified or not has_clean_tree) then
-    u.notify(
-      "Cannot leave comments on changed files. \n Please stash all local changes or push them to the feature branch.",
-      vim.log.levels.WARN
-    )
-    return
-  end
-
-  if not M.sha_exists() then
+  if not M.can_create_comment(false) then
     return
   end
 
   local layout = M.create_comment_layout({ ranged = false, unlinked = false })
-  if layout ~= nil then
-    layout:mount()
-  end
+  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()
-  if not u.check_visual_mode() then
-    return
-  end
-  if not M.sha_exists() then
+  if not M.can_create_comment(true) then
+    u.press_escape()
     return
   end
 
   local layout = M.create_comment_layout({ ranged = true, unlinked = false })
-  if layout ~= nil then
-    layout:mount()
-  end
+  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 })
-  if layout ~= nil then
-    layout:mount()
-  end
+  layout:mount()
 end
 
 ---Given the current visually selected area of text, builds text to fill in the
@@ -353,28 +292,85 @@ end
 --- 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()
-  if not u.check_visual_mode() then
-    return
-  end
-  if not M.sha_exists() then
+  if not M.can_create_comment(true) then
+    u.press_escape()
     return
   end
 
   local suggestion_lines, range_length = build_suggestion()
 
   local layout = M.create_comment_layout({ ranged = range_length > 0, unlinked = false })
-  if layout ~= nil then
-    layout:mount()
-  else
-    return -- Failure in creating the comment layout
-  end
+  layout:mount()
+
   vim.schedule(function()
     if suggestion_lines then
       vim.api.nvim_buf_set_lines(M.comment_popup.bufnr, 0, -1, false, suggestion_lines)
     end
   end)
 end
 
+---Returns true if it's possible to create an Inline Comment
+---@param must_be_visual boolean True if current mode must be visual
+---@return boolean
+M.can_create_comment = function(must_be_visual)
+  -- Check that diffview is initialized
+  if reviewer.tabnr == nil then
+    u.notify("Reviewer must be initialized first", vim.log.levels.ERROR)
+    return false
+  end
+
+  -- Check that we are in the Diffview tab
+  local tabnr = vim.api.nvim_get_current_tabpage()
+  if tabnr ~= reviewer.tabnr then
+    u.notify("Comments can only be left in the reviewer pane", vim.log.levels.ERROR)
+    return false
+  end
+
+  -- Check that we are hovering over the code
+  local filetype = vim.bo[0].filetype
+  if filetype == "DiffviewFiles" or filetype == "gitlab" then
+    u.notify(
+      "Comments can only be left on the code. To leave unlinked comments, use gitlab.create_note() instead",
+      vim.log.levels.ERROR
+    )
+    return false
+  end
+
+  -- Check that the file has not been renamed
+  if reviewer.is_file_renamed() and not reviewer.does_file_have_changes() then
+    u.notify("Commenting on (unchanged) renamed or moved files is not supported", vim.log.levels.ERROR)
+    return false
+  end
+
+  -- Check that we are in a valid buffer
+  if not M.sha_exists() then
+    return false
+  end
+
+  -- Check that there aren't saved modifications
+  local file = reviewer.get_current_file_path()
+  if file == nil then
+    return false
+  end
+  local has_changes, err = git.has_changes(file)
+  if err ~= 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
+    u.notify("Cannot leave comments on changed files, please stash or commit and push", vim.log.levels.ERROR)
+    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
+    return false
+  end
+
+  return true
+end
+
 ---Checks to see whether you are commenting on a valid buffer. The Diffview plugin names non-existent
 ---buffers as 'null'
 ---@return boolean

lua/gitlab/actions/discussions/init.lua

@@ -253,9 +253,7 @@ M.reply = function(tree)
     reply = true,
   })
 
-  if layout then
-    layout:mount()
-  end
+  layout:mount()
 end
 
 -- This function (settings.keymaps.discussion_tree.delete_comment) will trigger a popup prompting you to delete the current comment

lua/gitlab/actions/merge_requests.lua

@@ -12,14 +12,6 @@ local M = {}
 ---Opens up a select menu that lets you choose a different merge request.
 ---@param opts ChooseMergeRequestOptions|nil
 M.choose_merge_request = function(opts)
-  local has_clean_tree, clean_tree_err = git.has_clean_tree()
-  if clean_tree_err ~= nil then
-    return
-  elseif has_clean_tree ~= "" then
-    u.notify("Your local branch has changes, please stash or commit and push", vim.log.levels.ERROR)
-    return
-  end
-
   if opts == nil then
     opts = state.settings.choose_merge_request
   end
@@ -38,6 +30,19 @@ M.choose_merge_request = function(opts)
       reviewer.close()
     end
 
+    if choice.source_branch ~= git.get_current_branch() then
+      local has_clean_tree, clean_tree_err = git.has_clean_tree()
+      if clean_tree_err ~= nil then
+        return
+      elseif not has_clean_tree then
+        u.notify(
+          "Cannot switch branch when working tree has changes, please stash or commit and push",
+          vim.log.levels.ERROR
+        )
+        return
+      end
+    end
+
     vim.schedule(function()
       local _, branch_switch_err = git.switch_branch(choice.source_branch)
       if branch_switch_err ~= nil then

lua/gitlab/git.lua

@@ -25,10 +25,19 @@ M.branches = function(args)
   return run_system(u.combine({ "git", "branch" }, args or {}))
 end
 
----Checks whether the tree has any changes that haven't been pushed to the remote
----@return string|nil, string|nil
+---Returns true if the working tree hasn't got any changes that haven't been commited
+---@return boolean, string|nil
 M.has_clean_tree = function()
-  return run_system({ "git", "status", "--short", "--untracked-files=no" })
+  local changes, err = run_system({ "git", "status", "--short", "--untracked-files=no" })
+  return changes == "", err
+end
+
+---Returns true if the `file` has got any uncommitted changes
+---@param file string File to check for changes
+---@return boolean, string|nil
+M.has_changes = function(file)
+  local changes, err = run_system({ "git", "status", "--short", "--untracked-files=no", "--", file })
+  return changes ~= "", err
 end
 
 ---Gets the base directory of the current project

lua/gitlab/reviewer/init.lua

@@ -42,26 +42,28 @@ M.open = function()
   end
 
   local diffview_open_command = "DiffviewOpen"
-  local has_clean_tree, err = git.has_clean_tree()
-  if err ~= nil then
-    return
-  end
-  if state.settings.reviewer_settings.diffview.imply_local and has_clean_tree then
-    diffview_open_command = diffview_open_command .. " --imply-local"
+
+  if state.settings.reviewer_settings.diffview.imply_local then
+    local has_clean_tree, err = git.has_clean_tree()
+    if err ~= nil then
+      return
+    end
+    if has_clean_tree then
+      diffview_open_command = diffview_open_command .. " --imply-local"
+    else
+      u.notify(
+        "Your working tree has changes, cannot use 'imply_local' setting for gitlab reviews.\n Stash or commit all changes to use.",
+        vim.log.levels.WARN
+      )
+      state.settings.reviewer_settings.diffview.imply_local = false
+    end
   end
 
   vim.api.nvim_command(string.format("%s %s..%s", diffview_open_command, diff_refs.base_sha, diff_refs.head_sha))
 
   M.is_open = true
   M.tabnr = vim.api.nvim_get_current_tabpage()
 
-  if state.settings.reviewer_settings.diffview.imply_local and not has_clean_tree then
-    u.notify(
-      "There are uncommited changes in the working tree, cannot use 'imply_local' setting for gitlab reviews.\n Stash or commit all changes to use.",
-      vim.log.levels.WARN
-    )
-  end
-
   if state.settings.discussion_diagnostic ~= nil or state.settings.discussion_sign ~= nil then
     u.notify(
       "Diagnostics are now configured as settings.discussion_signs, see :h gitlab.nvim.signs-and-diagnostics",
@@ -289,12 +291,16 @@ end
 M.execute_callback = function(callback)
   return function()
     vim.api.nvim_cmd({ cmd = "normal", bang = true, args = { "'[V']" } }, {})
-    vim.api.nvim_cmd(
+    local _, err = pcall(
+      vim.api.nvim_cmd,
       { cmd = "lua", args = { ("require'gitlab'.%s()"):format(callback) }, mods = { lockmarks = true } },
       {}
     )
     vim.api.nvim_win_set_cursor(M.old_winnr, M.old_cursor_position)
     vim.opt.operatorfunc = M.old_opfunc
+    if err ~= "" then
+      u.notify_vim_error(err, vim.log.levels.ERROR)
+    end
   end
 end