fix: comment creation should not be possible for renamed and moved files

[harrisoncramer]

Makes it not possible to leave comments on renamed files

[harrisoncramer]

@jakubbortlik I believe this resolves your issues with renamed files.

You are able to leave comments on renamed files with these changes, so long as they have modifications. Normal, unmodified but renamed files will throw an error as those are not supported. Please let me know if this resolves your issues.

[harrisoncramer]

Include the old path when calculating the diff, since the new file may have a different path

[jakubbortlik]

Hi Harrison, thanks for doing this! It works in that commenting on renamed files is disabled, but for some default commenting keybindings, more errors are shown than intended:

The normal mode keybinding ss and the visual mode keybinding s show this error:

Error executing vim.schedule lua callback: ....../gitlab.nvim/lua/gitlab/actions/comment.lua:365: attempt to index field 'comment_popup' (a nil value)
stack traceback:                                                                                                                                                                                                                                                                                                                
        ....../gitlab.nvim/lua/gitlab/actions/comment.lua:365: in function <....../gitlab.nvim/lua/gitlab/actions/comment.lua:363>                                                                                                                                                                          
Press ENTER or type command to continue

This can be fixed by returning early from create_comment_suggestion:

M lua/gitlab/actions/comment.lua
@@ -359,6 +359,8 @@ M.create_comment_suggestion = function()
   local layout = M.create_comment_layout({ ranged = range_length > 0, unlinked = false })
   if layout ~= nil then
     layout:mount()
+  else
+    return
   end
   vim.schedule(function()
     if suggestion_lines then

The keybindings cc, ss for commenting/making a suggestion on the current line and combinations c<motion> and s<motion also show this message:

E5108: Error executing lua ....../gitlab.nvim/lua/gitlab/reviewer/init.lua:292: Vim:gitlab.nvim: Commenting on (unchanged) renamed or moved files is not supported                                                                                                                                                  
stack traceback:                                                                                                                                                                                                                                                                                                                
        [C]: in function 'nvim_cmd'                                                                                                                                                                                                                                                                                             
        ....../gitlab.nvim/lua/gitlab/reviewer/init.lua:292: in function <....../gitlab.nvim/lua/gitlab/reviewer/init.lua:290>                                                                                                                                                                          
Press ENTER or type command to continue

This will possibly require some fixes of how the callback is executed for the operator mappings. I haven't found a trivial fix when briefly investigating this.

Also, interestingly when the cc mapping fails, nvim stays in visual mode, while for ss it stays in gets back to normal mode but cursor moves to the end of the intermediate visual selection. This should be fixed so that ideally the visual mode is never even entered (lua/gitlab/reviewer/init.lua line 291: vim.api.nvim_cmd({ cmd = "normal", bang = true, args = { "'[V']" } }, {})) with these mappings.

[jakubbortlik]

A possible solution for the error E5108 would be to change the notification level in lua/gitlab/actions/comment.lua line 185 from ERROR to WARN. This level is already used e.g. when checking visual mode for making suggestions in lua/gitlab/utils/init.lua line 643.

[harrisoncramer]

I've fixed the first issue you pointed out, using your provided solution.

I'm not seeing the second issue at all, ranged comments or suggestions are just giving me the single error message.

[jakubbortlik]

I'm not seeing the second issue at all, ranged comments or suggestions are just giving me the single error message.

Interesting. I'll find out what the problem is and let you know.

[jakubbortlik]

Hi @harrisoncramer, you should be able to reproduce the issue with this config:

-- This is a minimal init.lua that is used for debugging
local root = vim.fn.stdpath("run") .. "/nvim/mininit"
vim.fn.mkdir(root, "p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    lazypath,
  })
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
  {
    "harrisoncramer/gitlab.nvim",
    dir = "~/projects/gitlab.nvim/",
    dependencies = {
      "MunifTanjim/nui.nvim",
      "nvim-lua/plenary.nvim",
      "sindrets/diffview.nvim",
      "stevearc/dressing.nvim",   -- Recommended but not required. Better UI for pickers.
      "nvim-tree/nvim-web-devicons" -- Recommended but not required. Icons in discussion tree.
    },
    enabled = true,
    build = function() require("gitlab.server").build(true) end, -- Builds the Go binary
    config = function()
      require("gitlab").setup({
        colors = {
          discussion_tree = {
            mention = "Keyword",  -- Overrides to get rid of errors due to missing colors...
            directory = "Keyword",
            resolved = "Keyword",
            unresolved = "Keyword",
            draft = "Keyword"
          },
        },
      })
    end,
  },
}

require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

The way I understand this is that the problem is in lines 292-295 in lua/gitlab/reviewer/init.lua:

    vim.api.nvim_cmd(
      { cmd = "lua", args = { ("require'gitlab'.%s()"):format(callback) }, mods = { lockmarks = true } },
      {}
    )

The docs on vim.api.nvim_cmd say: On execution error: fails with Vimscript error, updates v:errmsg. I think line 185 in lua/gitlab/actions/comment.lua: u.notify("Commenting on (unchanged) renamed or moved files is not supported", vim.log.levels.ERROR) causes this execution error and hence the verbose error.

I've looked at your nvim config and you seem to be using rcarriga/nvim-notify which works around this problem.

[jakubbortlik]

I still haven't figured out why for cc nvim stays in visual mode while for ss it turns back to normal mode.

This is because gitlab.actions.comment.create_comment_suggestion calls gitlab.actions.comment.build_suggestion which calls u.get_visual_selection_boundaries which calls vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes("<Esc>", false, true, true), "nx", false) that exits visual mode.

u.get_sivual_selection_boundaries is ultimately called in create-comment_layout too, but that's only after checking for renamed/moved files, so it doesn't happen for create_multiline_comment (triggered by cc).

[jakubbortlik]

I still haven't figured out why for cc nvim stays in visual mode while for ss it turns back to normal mode.

This is because gitlab.actions.comment.create_comment_suggestion calls gitlab.actions.comment.build_suggestion which calls u.get_visual_selection_boundaries which calls vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes("<Esc>", false, true, true), "nx", false) that exits visual mode.

u.get_sivual_selection_boundaries is ultimately called in create-comment_layout too, but that's only after checking for renamed/moved files, so it doesn't happen for create_multiline_comment (triggered by cc).

Maybe the cursor-restoration logic could be moved from operatorfunc callback (gitlab/reviewer/init.lua line 296) to the gitlab.actions.comment.create_multiline_comment and gitlab.actions.comment.create_comment_suggestion functions so that the cursor is restored also when running the functions from a non-default mapping or from the command line.

[harrisoncramer]

Feel free to change the cursor restoration logic in a separate MR if you'd like, it's not really a top priority for me and you seem to understand the issue quite well. I'm just going to merge this MR as-is, since it resolves the original issue here.

Thanks for digging into this so deeply and helping me resolve those edge cases.