Diffview integration uses key_bindings instead of keymaps

[asmodeus812]

Description

Diffview's latest api uses keymaps property instead of key_bindings, which seems to cause issues deep cloning keymaps, would it be possible to update the integration to use the correct property from the api.

Neovim version

0.8.1 NVIM Release github download

Operating system and version

Ubuntu 22

Steps to reproduce

Configure diffview with the keymaps property Configure neogit with diffview integration Observe that only neogit's keymaps are present for diffview's buffers

Expected behavior

Keymaps are properly merged / inherited from the diff view config.

Actual behavior

The keymaps from the config are not correctly merged due to the fact that the key_bindings property is obsolete and the internal diffview implementation does not provide it back when doing get_config(), so neogit merges with empty map and calls diffview setup, only setting up the neogit's keybinds. Here is the relevant piece of code: https://github.dev/sindrets/diffview.nvim/blob/dc77f487b292c4a89dd437d80331e3aacbe3aaed/lua/diffview/config.lua#L469

Minimal config

vim.cmd([[set runtimepath=$VIMRUNTIME]])
vim.cmd([[set packpath=/tmp/nvim/site]])
local package_root = "/tmp/nvim/site/pack"
local install_path = package_root .. "/packer/start/packer.nvim"
local function load_plugins()
  require("packer").startup({
    {
      "wbthomason/packer.nvim",
      {
        "TimUntersberger/neogit",
        requires = {
          { "nvim-lua/plenary.nvim" },
          { "sindrets/diffview.nvim" },
        },
        config = function()
          print("loaded neogit")
          require("neogit").setup({
 integrations = {
            diffview = true
        }
})
        end,
      },
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. "/plugin/packer_compiled.lua",
      display = { non_interactive = true },
    },
  })
end
if vim.fn.isdirectory(install_path) == 0 then
  print("Installing neogit and dependencies.")
  vim.fn.system({ "git", "clone", "--depth=1", "https://github.com/wbthomason/packer.nvim", install_path })
end
load_plugins()
require("packer").sync()

[asmodeus812]

@ten3roberts maybe i spoke too soon, but it seems like this is not as easy as just changing the property. See, when you call get_config() you are not getting the user space config passed in to diffivew, you get some sort of internal representation, where the keymaps are transformed and are are table arrays, so deep extend here, won't work, it does not know how to merge table arrays, what it does is replace the [view] key's table array value with neogit passes in (with the 2 default maps for closing it). Seems like a more involved process, than that.

Here is (just a small excerpt from my config) what i get if i run get_config() on my diffview config. It is not the same as the one the user passes in. Seems to be transformed into an array of values

view = { { "n", "q", "<cmd>DiffviewClose<cr>", {
          nowait = true,
          silent = true
        } }, { "n", "<leader>x", "<cmd>diffget<cr>", {
          nowait = true,
          silent = true
        } }, ..............

Here is what i pass in

view = {
                ["q"] = "<cmd>DiffviewClose<cr>",
                ["<leader>X"] = "<cmd>diffput<cr>",
                ...................

How important is it to override the config from neogit with a couple of close bindings, anyway, if this is too complex to implement, (i.e merge the table array) maybe a better solution is to skip this step of passing these custom bindings, and use whatever diffview was already configured with. Or somehow know if it was customized by the user and skip calling setup all together not sure.

[ten3roberts]

I also suspected that was the case.

I agree, and find it confusing and unintuitive that using diffview from neogit has different bindings than when invoking it manually.

I'd be more than happy to review a PR which removes the "custom" setup and keybindings from the diffview integration, as it seems you are more knowledged in that area than I am.