Register readable prefix for grouped keymap settings #1134

Cyberczy · 2024-01-07T14:40:51Z

No description provided.

ayamir · 2024-01-08T12:39:46Z

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 · 2024-01-08T12:41:34Z

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

ayamir · 2024-01-08T12:54:38Z

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 · 2024-01-11T15:52:32Z

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 · 2024-01-11T15:58:28Z

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

I don't have the authority, lol

ayamir · 2024-01-12T02:04:59Z

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 · 2024-01-12T04:07:04Z

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

…font

ayamir · 2024-01-12T07:35:57Z

@CharlesChiuGit Could you please review this PR?

CharlesChiuGit · 2024-01-12T08:54:33Z

ok, let me take a look.

CharlesChiuGit

plz remove lazy-lock

lazy-lock.json

ayamir · 2024-01-12T13:34:01Z

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 · 2024-01-14T19:14:12Z

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, ...)?

more test still need to be done

Signed-off-by: ayamir <[email protected]>

ayamir · 2024-01-15T09:30:42Z

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 · 2024-01-15T09:41:41Z

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.

CharlesChiuGit · 2024-01-15T10:17:36Z

@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 · 2024-01-15T11:33:30Z

IMO it's better to refactor it.

…tup time.

ayamir · 2024-01-17T08:34:12Z

It looks tidy now.

Cyberczy · 2024-01-17T08:35:19Z

LGTM!!!

Cyberczy · 2024-01-17T08:58:57Z

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

ayamir · 2024-01-17T14:55:09Z

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

IMO it is unnecessary.

Cyberczy · 2024-01-17T14:57:30Z

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 · 2024-01-17T15:22:35Z

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

CharlesChiuGit

LGTM

Jint-lzxy

LGTM! This PR is awesome lmao

(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):

Then I did a git bisect, and the issue seems to originate from this commit (strangely tho, this means that which-key thinks some of my keymap groups don't have a valid name, but obviously I did give them names). For some reason, in this call, the (to be processed) group still has a name, but immediately in the next line, that name disappears. Based on this, IMO a simpler solution 'might' be to revert 8503c0 (I can create a fork just like we made a temporary fix for lsp-signature), and then wait for folke's response (I'll open an issue for this); or just merge this PR. What do you guys think @ayamir @CharlesChiuGit?

Signed-off-by: ayamir <[email protected]>

ayamir · 2024-01-20T10:03:12Z

Made improvement for startup time.
Before:

Now:

ayamir · 2024-01-20T10:19:18Z

(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 · 2024-01-21T12:16:05Z

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: folke/which-key.nvim#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 · 2024-01-21T13:14:17Z

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 · 2024-01-22T12:00:18Z

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

ayamir · 2024-01-22T14:52:05Z

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

Signed-off-by: ayamir <[email protected]>

* Use Nerd Font to replace the +prefix (It's just a demo) * Move insert registration into bind. * Fix unexpected shadow. * format code. * For tab instead of space. * chore(ci): fix linting * fix: works for buffer-related keymap by registering prefix immediately. Signed-off-by: ayamir <[email protected]> * chore(which-key): add more nerdfont for which-key prefix * fix: works for <space> as prefix. * refactor: update keymaps and defer register operation to shorten startup time. * clean up. * fixup: remove redundant code. * perf: reduce number of register queue insertion. Signed-off-by: ayamir <[email protected]> * refactor: use the statical way to set group names. * clean files. Signed-off-by: ayamir <[email protected]> --------- Signed-off-by: ayamir <[email protected]> Co-authored-by: Chui <[email protected]> Co-authored-by: CharlesChiuGit <[email protected]> Co-authored-by: ayamir <[email protected]>

Use Nerd Font to replace the +prefix (It's just a demo)

50f52f9

Merge branch 'ayamir:main' into main

9d008ba

Merge branch 'ayamir:main' into main

db37c39

Cheny-chui added 5 commits January 12, 2024 14:39

Move insert registration into bind.

0f5deda

Merge branch 'main' of https://github.com/Cyberczy/nvimdots into nerd…

48aa6cc

…font

Fix unexpected shadow.

714e646

format code.

071100a

For tab instead of space.

88f00be

CharlesChiuGit requested changes Jan 12, 2024

View reviewed changes

lazy-lock.json Outdated Show resolved Hide resolved

Cyberczy requested a review from CharlesChiuGit January 12, 2024 10:07

Merge branch 'ayamir:main' into main

1816e0a

chore(ci): fix linting

65d140d

CharlesChiuGit previously approved these changes Jan 15, 2024

View reviewed changes

fix: works for buffer-related keymap by registering prefix immediately.

dda5da9

Signed-off-by: ayamir <[email protected]>

ayamir changed the title ~~Use Nerd Font to replace the +prefix (It's just a demo)~~ Register readable prefix to grouped keymap settings Jan 15, 2024

ayamir changed the title ~~Register readable prefix to grouped keymap settings~~ Register readable prefix for grouped keymap settings Jan 15, 2024

chore(which-key): add more nerdfont for which-key prefix

4513ec3

fix: works for <space> as prefix.

3c6de70

CharlesChiuGit mentioned this pull request Jan 16, 2024

refactor: organized keymaps group #1153

Closed

ayamir added 3 commits January 17, 2024 12:55

refactor: update keymaps and defer register operation to shorten star…

a9bdd12

…tup time.

clean up.

9adaa36

fixup: remove redundant code.

87183d4

CharlesChiuGit approved these changes Jan 18, 2024

View reviewed changes

Jint-lzxy approved these changes Jan 19, 2024

View reviewed changes

perf: reduce number of register queue insertion.

4bbae8f

Signed-off-by: ayamir <[email protected]>

ayamir added 2 commits January 22, 2024 23:10

refactor: use the statical way to set group names.

725ea2e

clean files.

454c785

Signed-off-by: ayamir <[email protected]>

ayamir approved these changes Jan 22, 2024

View reviewed changes

ayamir merged commit de99f14 into ayamir:main Jan 22, 2024
2 checks passed

TonyWu20 mentioned this pull request Jan 29, 2024

ayamir main TonyWu20/nvimdots#37

Merged

TonyWu20 mentioned this pull request Feb 16, 2024

ayamir main TonyWu20/nvimdots#38

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Register readable prefix for grouped keymap settings #1134

Register readable prefix for grouped keymap settings #1134

Cyberczy commented Jan 7, 2024

ayamir commented Jan 8, 2024 •

edited

Loading

Cyberczy commented Jan 8, 2024

ayamir commented Jan 8, 2024

Cheny-chui commented Jan 11, 2024

Cyberczy commented Jan 11, 2024

ayamir commented Jan 12, 2024

Cyberczy commented Jan 12, 2024

ayamir commented Jan 12, 2024

CharlesChiuGit commented Jan 12, 2024

CharlesChiuGit left a comment

ayamir commented Jan 12, 2024 •

edited

Loading

vabatta commented Jan 14, 2024

ayamir commented Jan 15, 2024

CharlesChiuGit commented Jan 15, 2024 •

edited

Loading

CharlesChiuGit commented Jan 15, 2024 •

edited

Loading

ayamir commented Jan 15, 2024

ayamir commented Jan 17, 2024

Cyberczy commented Jan 17, 2024

Cyberczy commented Jan 17, 2024

ayamir commented Jan 17, 2024

Cyberczy commented Jan 17, 2024

Jint-lzxy commented Jan 17, 2024

CharlesChiuGit left a comment

Jint-lzxy left a comment

ayamir commented Jan 20, 2024

ayamir commented Jan 20, 2024 •

edited

Loading

Jint-lzxy commented Jan 21, 2024

ayamir commented Jan 21, 2024 •

edited

Loading

Cyberczy commented Jan 22, 2024

ayamir commented Jan 22, 2024

Register readable prefix for grouped keymap settings #1134

Register readable prefix for grouped keymap settings #1134

Conversation

Cyberczy commented Jan 7, 2024

ayamir commented Jan 8, 2024 • edited Loading

Cyberczy commented Jan 8, 2024

ayamir commented Jan 8, 2024

Cheny-chui commented Jan 11, 2024

Cyberczy commented Jan 11, 2024

ayamir commented Jan 12, 2024

Cyberczy commented Jan 12, 2024

ayamir commented Jan 12, 2024

CharlesChiuGit commented Jan 12, 2024

CharlesChiuGit left a comment

Choose a reason for hiding this comment

ayamir commented Jan 12, 2024 • edited Loading

vabatta commented Jan 14, 2024

ayamir commented Jan 15, 2024

CharlesChiuGit commented Jan 15, 2024 • edited Loading

CharlesChiuGit commented Jan 15, 2024 • edited Loading

ayamir commented Jan 15, 2024

ayamir commented Jan 17, 2024

Cyberczy commented Jan 17, 2024

Cyberczy commented Jan 17, 2024

ayamir commented Jan 17, 2024

Cyberczy commented Jan 17, 2024

Jint-lzxy commented Jan 17, 2024

CharlesChiuGit left a comment

Choose a reason for hiding this comment

Jint-lzxy left a comment

Choose a reason for hiding this comment

ayamir commented Jan 20, 2024

ayamir commented Jan 20, 2024 • edited Loading

Jint-lzxy commented Jan 21, 2024

ayamir commented Jan 21, 2024 • edited Loading

Cyberczy commented Jan 22, 2024

ayamir commented Jan 22, 2024

ayamir commented Jan 8, 2024 •

edited

Loading

ayamir commented Jan 12, 2024 •

edited

Loading

CharlesChiuGit commented Jan 15, 2024 •

edited

Loading

CharlesChiuGit commented Jan 15, 2024 •

edited

Loading

ayamir commented Jan 20, 2024 •

edited

Loading

ayamir commented Jan 21, 2024 •

edited

Loading