Can not set mappings between popup and status after recent breaking change

[asmodeus812]

Description

Neogit reports that there are duplicate mappings, since instead of overriding them it tries to merge with the defaults which is bad imo. So for example i would like to have both x and X keys open two different popups, but since x is already taken by default in the status mappings map, i get this error. Please allow us to completely override the default mappings, we might want to even disable certain ones, instead of merging them with the defaults.

E5108: Error executing lua: .../packages/start/plenary.nvim/lua/plenary/async/async.lua:18: The coroutine failed with this message: vim/shared.lua:0: key found in more than one map: x
stack traceback:
[C]: in function 'error'
.../packages/start/plenary.nvim/lua/plenary/async/async.lua:18: in function 'callback_or_next'
.../packages/start/plenary.nvim/lua/plenary/async/async.lua:45: in function 'step'
.../packages/start/plenary.nvim/lua/plenary/async/async.lua:48: in function 'execute'
.../packages/start/plenary.nvim/lua/plenary/async/async.lua:108: in function 'run'
.../share/nvim/site/pack/packages/opt/neogit/lua/neogit.lua:103: in function 'open'
/home/asmodeus/.config/nvim/lua/plugin/neogit.lua:9: in function 'open_vcs_element'
/home/asmodeus/.config/nvim/lua/user/actions.lua:1049: in function 'open_vercontrol_view'
/home/asmodeus/.config/nvim/lua/user/command.lua:1959: in function </home/asmodeus/.config/nvim/lua/user/command.
lua:1958>

Neovim version

0.9.4

Operating system and version

ubuntu

Steps to reproduce

        mappings = {
            finder = {
                ["<cr>"] = "Select",
                ["<esc>"] = "Close",
                ["<c-j>"] = "Next",
                ["<c-p>"] = "Previous",
                ["<tab>"] = "MultiselectToggleNext",
                ["<s-tab>"] = "MultiselectTogglePrevious",
            },
            popup = {
                ["?"] = "HelpPopup",
                ["i"] = "IgnorePopup",
                ["b"] = "BranchPopup",
                ["p"] = "PushPopup",
                ["f"] = "FetchPopup",
                ["r"] = "RebasePopup",
                ["l"] = "LogPopup",
                ["z"] = "StashPopup",
                ["D"] = "DiffPopup",
                ["P"] = "PullPopup",
                ["c"] = "CommitPopup",
                ["C"] = "CherryPickPopup",
                ["x"] = "RevertPopup",
                ["X"] = "ResetPopup",
                ["m"] = "MergePopup",
                ["M"] = "RemotePopup",
            },
            status = {
                ["<c-r>"] = "RefreshBuffer",
                ["<c-v>"] = "VSplitOpen",
                ["<c-x>"] = "SplitOpen",
                ["<c-t>"] = "TabOpen",
                ["<esc>"] = close_git_view,
                ["<enter>"] = "GoToFile",
                ["<tab>"] = "Toggle",
                ["n"] = "InitRepo",
                ["s"] = "Stage",
                ["S"] = "StageUnstaged",
                ["u"] = "Unstage",
                ["U"] = "UnstageStaged",
                ["d"] = "DiffAtFile",
                ["R"] = "Discard",
                ["I"] = "InitRepo",
                ["L"] = "CommandHistory",
                ["}"] = "GoToNextHunkHeader",
                ["{"] = "GoToPreviousHunkHeader",
            },
        },

Expected behavior

No errors

Actual behavior

Error is observed opening the git status.

Minimal config

See above.

[CKolkey]

Cool, I knew I'd see stuff like this. The end goal is to make things more flexible for everyone, but the path to getting there is... bumpy.

Anyways, thanks for the reproduction config. I'll try and sort this out asap.

[star-szr]

I'm wondering if setting the mapping you want to override to false would help here - assuming we keep the merge approach. I currently have the following in my config to completely switch the stage mappings, but note that I am not yet on the latest revision.

mappings = {
  status = {
    ["a"] = "Stage",
    ["A"] = "StageUnstaged",
    ["s"] = false,
    ["S"] = false,
  },
},

Edit: I should add that I'm not able to test at the moment and that there is a strong chance that this may not work for your use case.

[asmodeus812]

Not working on master/latest changes, tried that before posting this issue - @star-szr

[star-szr]

Maybe a config switch to disable all built-in keybindings would help?

oil.nvim example:

use_default_keymaps = true,

Though I do think using false should work for this use case too. I haven’t had a chance to dig into the code but I’m imagining we could remove all mappings set to false from the defaults before merging.

[CKolkey]

Maybe a config switch to disable all built-in keybindings would help?

oil.nvim example:

use_default_keymaps = true,

I... I love the simplicity. Done. #926

Though I do think using false should work for this use case too. I haven’t had a chance to dig into the code but I’m imagining we could remove all mappings set to false from the defaults before merging.

Easy, too. Great ideas :)

[fdelacruz]

Hi, I'm sorry..
Did that work for anybody??

[CKolkey]

Really hope so. Is stuff broken for you?

[fdelacruz]

Yeah, suddenly i'm getting the following:

But, now that i look at it closer maybe it is not related to this issue here.
Something about Vim(sign):E155: Unknown sign: NeogitOpen:section

[CKolkey]

Looks unrelated. I'll close this, and do you wanna open a new issue with your config?