Register readable prefix for grouped keymap settings

[ayamir]

IMO it's better to register prefix for which-key when the keymappings are binded, i.e., the time invoking the bind.nvim_load_mapping.

[Cyberczy]

It's up to you. I'm just making a suggestion.

[ayamir]

It's up to you. I'm just making a suggestion.

OK. The reason why current approach doesn't work for keymappings binded to buffer may causes from the absense of the buffer info in the section defined in prefix.lua.

[Cheny-chui]

I have made some improvements to this pr based on ayamir's suggestion, can you please give me modification access to this pr? @Cyberczy (I'm not sure if I need your action, but I really can't push to your repository). Or maybe need the maintainer's action @ayamir .

[Cyberczy]

我根据ayamir的建议对这个pr做了一些改进，你能给我这个pr的修改权限吗？@Cyberczy（我不确定是否需要您的操作，但我确实无法自动到达您的存储库）。或者可能维护者的操作@ayamir。

I don't have the authority, lol

[ayamir]

It seems better that @Cyberczy give the authority of your fork to @Cheny-chui as the GitHub Docs says: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork

[Cyberczy]

Ok, I've just given you modification access. @Cheny-chui

[ayamir]

@CharlesChiuGit Could you please review this PR?

[CharlesChiuGit]

ok, let me take a look.

[ayamir]

Current prefix settings are not enough and needed to be added manually. I'm thinking automatically setting whether is a better idea. Though this requires keymap setup to keep align with it. Additionally, maybe we can refactor current keymap settings through this PR, such as: <leader>D -> <leader>hd <leader><leader>D -> <leader>hD What do you think? @CharlesChiuGit @Jint-lzxy

[vabatta]

What is the general rationale behind the current key mapping? Is it quick commands, frequent commands or "grouped" commands (like LazyVim which maps into groups like <leader>w for windows, <leader>b for buffers, ...)?

[ayamir]

What is the general rationale behind the current key mapping? Is it quick commands, frequent commands or "grouped" commands (like LazyVim which maps into groups like <leader>w for windows, <leader>b for buffers, ...)?

Current keymap settings are mixed but a little confusing due to history reasons.

[CharlesChiuGit]

lua\keymap\prefix.lua

--- Need to expand by hand
local prefix_desc = {
    ["<leader>b"] = icons.ui_sep.Buffer .. "Buffer",
    ["<leader>c"] = icons.ui_sep.Character .. "Character",
    ["<leader>d"] = icons.ui_sep.Bug .. "Debug",
    ["<leader>f"] = icons.ui_sep.Telescope .. "Fuzzy Find",
    ["<leader>h"] = icons.ui_sep.Git .. "Git Hunk",
    ["<leader>l"] = icons.ui_sep.Lsp .. "Lsp",
    ["<leader>p"] = icons.ui_sep.Package .. "Package",
    ["<leader>n"] = icons.ui_sep.FolderOpen .. "Nvim Tree",
    ["<leader>s"] = icons.ui_sep.Tmux .. "Session",
    ["<leader>t"] = icons.ui_sep.Lsp .. "Lsp",
    ["<leader><space>"] = icons.ui_sep.Git .. "Git: Close diff",
}

After pressing leader key one time, some prefix still can be shown, hmmm.

[Cyberczy]

lua\keymap\prefix.lua

--- Need to expand by hand
local prefix_desc = {
  ["<leader>b"] = icons.ui_sep.Buffer .. "Buffer",
  ["<leader>c"] = icons.ui_sep.Character .. "Character",
  ["<leader>d"] = icons.ui_sep.Bug .. "Debug",
  ["<leader>f"] = icons.ui_sep.Telescope .. "Fuzzy Find",
  ["<leader>h"] = icons.ui_sep.Git .. "Git Hunk",
  ["<leader>l"] = icons.ui_sep.Lsp .. "Lsp",
  ["<leader>p"] = icons.ui_sep.Package .. "Package",
  ["<leader>n"] = icons.ui_sep.FolderOpen .. "Nvim Tree",
  ["<leader>s"] = icons.ui_sep.Tmux .. "Session",
  ["<leader>t"] = icons.ui_sep.Lsp .. "Lsp",
  ["<leader><space>"] = icons.ui_sep.Git .. "Git: Close diff",
}

按一次leader键后，仍然可以显示一些前缀，嗯。

<leader><space> seems to have no effect.

[CharlesChiuGit]

<leader><space> seems to have no effect.

yeah, im not sure how to set it. i have tried <leader><space> or <leader><leader> both r not working. also, <leader>h and <leader>l r not valid too.

[CharlesChiuGit]

@ayamir @Jint-lzxy since our keymap group is a bit confusing right now, our which-key prefix nerdfont hit might be a bit misleading too.

I wonder how u guys think about current layout.

for example:

[ayamir]

IMO it's better to refactor it.

[ayamir]

It looks tidy now.

[Cyberczy]

LGTM!!!

[Cyberczy]

The right half of which-key seems a bit empty. Perhaps we can add a bit of padding.

[ayamir]

The right half of which-key seems a bit empty. Perhaps we can add a bit of padding.

IMO it is unnecessary.

[Cyberczy]

The right half of which-key seems a bit empty. Perhaps we can add a bit of padding.

IMO it is unnecessary.

Alright, it's just a suggestion

[Jint-lzxy]

Sorry for the delay! My sched's been a bit tight recently. But I'll probably review this in the next day or two lol

[ayamir]

Made improvement for startup time. Before: Now:

[ayamir]

(idk if this is related) But I'm wondering if the real issue might lie in the implementation of which-key.nvim, as I used a different approach to handle keymap registration, but it seems that (only recently) which-key sometimes displays +prefix even for named groups (like <leader>h, see the screenshot below):

I didn't view settings about <leader>h for group name in https://github.com/Jint-lzxy/nvimconfig/blob/f5e4e72cb7ab884c7afef4274c64a991ad850713/lua/modules/configs/tool/which-key.lua#L34-L145. I tend to just merge it, notify user about the BREAKING CHANGE of keymap and make keybinding-related changes in wiki.

[Jint-lzxy]

I didn't view settings about <leader>h for group name ...

Ah, I think this probably isn't related. Sorry for the noise here! (I didn't notice that our impl is kinda like an event loop 🙈) Here's a similar issue with more details: https://github.com/folke/which-key.nvim/issues/482.

@ayamir With that being said, I'm still a bit puzzled as to why we need to register these mappings dynamically. I feel like just statically registering them using require("which-key").register({}) would be enough, since which-key should do the grouping work (correct me if I'm misunderstanding something lmao). Registering a callback that gets invoked every 300 milliseconds seems a bit inefficient to me.

[ayamir]

With that being said, I'm still a bit puzzled as to why we need to register these mappings dynamically. I feel like just statically registering them using require("which-key").register({}) would be enough, since which-key should do the grouping work (correct me if I'm misunderstanding something lmao).

It's b/c the statical way only registers the group name once when init and doesn't work for our lsp-related keymaps are binded with buffer after LspAttach event. But after the keymaps were refactored, this problem is addressed now (b/c trouble.nvim-related keymaps were merged into the <leader>l group). Indeed, now we can use the statical way to avoid unnecessary complexity lol.

return function()
    local icons = {
        ui = require("modules.utils.icons").get("ui"),
        misc = require("modules.utils.icons").get("misc"),
        git = require("modules.utils.icons").get("git", true),
        cmp = require("modules.utils.icons").get("cmp", true),
    }

    require("which-key").register({
        ["<leader>"] = {
            b = {
                name = icons.ui.Buffer .. " Buffer",
            },
            d = {
                name = icons.ui.Bug .. " Debug",
            },
            f = {
                name = icons.ui.Telescope .. " Fuzzy Find",
            },
            g = {
                name = icons.git.Git .. "Git",
            },
            l = {
                name = icons.misc.LspAvailable .. " Lsp",
            },
            n = {
                name = icons.ui.FolderOpen .. " Nvim Tree",
            },
            p = {
                name = icons.ui.Package .. " Package",
            },
            s = {
                name = icons.cmp.tmux .. "Session",
            },
        },
    })

    require("modules.utils").load_plugin("which-key", {
        plugins = {
            presets = {
                operators = false,
                motions = false,
                text_objects = false,
                windows = false,
                nav = false,
                z = true,
                g = true,
            },
        },

        icons = {
            breadcrumb = icons.ui.Separator,
            separator = icons.misc.Vbar,
            group = "",
        },

        window = {
            border = "none",
            position = "bottom",
            margin = { 1, 0, 1, 0 },
            padding = { 1, 1, 1, 1 },
            winblend = 0,
        },
    })
end

[Cyberczy]

So, should we stick with statically registering them, or should we maintain the current state by dynamically registering them?

[ayamir]

It seems to me now that the statical way is better.