doom-neovim / doom-nvim

A Neovim configuration for the advanced martian hacker
GNU General Public License v2.0
1.01k stars 107 forks source link

[Question] Custom Which-key #104

Closed pcapiod closed 2 years ago

pcapiod commented 3 years ago

Hi,

Thanks again for the beautiful work, loving all the things about doom-nvim and I keep improving it on my own. Though, I encountered a weird issue with the 3.1.0. I added some which-key in the doom-whichkey.lua, for instance, I like to have a telescope in specific folders or since I heavily use Latex, I added some quick commands for compilation with VimTex. However custom which-key are not recognized, despite doing PackerCompile after the changes. Looking my added which-keys using <leader>db, they do not appear in the list. The commands works perfectly (they were working in 3.0.X).

Best

NTBBloodbath commented 3 years ago

Hey, thank you for such words!

This is a bit strange for me since I didn't changed too much stuff regarding to which-key afair but I did one that maybe is breaking some stuff (it's the only thing that I think can be causing problems right now), can you please try applying this patch to our init.lua file? You should be able to git apply file.diff

diff --git i/init.lua w/init.lua
index 22527fc..e26d509 100644
--- i/init.lua
+++ w/init.lua
@@ -32,7 +32,7 @@ vim.defer_fn(function()
   local load_modules = require("doom.utils").load_modules

   -- Load Doom stuff (core, modules, extras)
-  load_modules("doom", { "core", "modules", "extras.autocmds" })
+  load_modules("doom", { "core", "modules", "extras.autocmds", "extras.keybindings" })

   -- If the dashboard plugin is already installed and the packer_compiled.lua
   -- file exists so we can make sure that the dashboard have been loaded.
@@ -60,14 +60,7 @@ vim.defer_fn(function()
     filetype on
     filetype plugin indent on
     PackerLoad nvim-treesitter
+    PackerLoad which-key.nvim
+    silent! bufdo e
   ]])
-
-  -- Load keybindings module at the end because the keybindings module cost is high
-  vim.defer_fn(function()
-    load_modules("doom.extras", { "keybindings" })
-    vim.cmd([[
-      PackerLoad which-key.nvim
-      silent! bufdo e
-    ]])
-  end, 20)
 end, 0)

Regards

pcapiod commented 3 years ago

Alright I applied the patch above but it didn't work. However, I could reproducibly make the which-keys work by manually loading the custom config <leader>dl. Here what I did:

What is bugging me is that if I remap the same command to let's say .th, it works directly, no need to manually reload. Is there anything wrong with the <leader> keybind?

NTBBloodbath commented 3 years ago

Now that you mention this I think there's a problem, yeah.

Did you added a which-key section for t and h in lua/doom/modules/config/doom-whichkey.lua? Basically which-key will gracefully fail if you do SPC - t and there's no config for t section and so on.

If you did and still no results then please share a diff like the one that I sent so I can try it locally, you can generate them by doing git diff file1 ... > my-diff.diff

pcapiod commented 3 years ago

Hi, sorry for the late answer, I, indeed, added config sections in the doom-whichkey.lua file. Please find here the diff file:

diff --git a/lua/doom/modules/config/doom-whichkey.lua b/lua/doom/modules/config/doom-whichkey.lua
index 8f5492e..e3d5538 100644
--- a/lua/doom/modules/config/doom-whichkey.lua
+++ b/lua/doom/modules/config/doom-whichkey.lua
@@ -92,9 +92,9 @@ return function()
       ["r"] = { "Run current file" },
       ["b"] = { "Compile project" },
       ["c"] = { "Compile and run project" },
-      ["h"] = {
-        "Run restclient on the line that the cursor is currently on",
-      },
+           ["p"] = { "Compile in Python" },
+      ["g"] = { "Compile in Gnuplot" },
+      ["h"] = { "Run restclient on the line that the cursor is currently on"},
       ["d"] = {
         name = "+debug",
         ["e"] = { "Evaluate word under cursor" },
@@ -119,6 +119,8 @@ return function()
       ["R"] = { "Create crash report" },
       ["s"] = { "Change colorscheme" },
       ["l"] = { "Reload user custom settings" },
+      ["m"] = { "Commands" },
+      ["v"] = { "Vim Options" },
     },
     ["f"] = {
       name = "+file",
@@ -145,6 +147,11 @@ return function()
       ["B"] = { "Branches" },
       ["c"] = { "Commits" },
     },
+       ['l'] = {
+           name = "+latex",
+           ["l"] = { "Compile" },
+           ["v"] = { "View" },
+       },
     ["p"] = {
       name = "+plugins",
       ["c"] = { "Clean disabled or unused plugins" },
@@ -168,6 +175,11 @@ return function()
       ["h"] = { "Command history" },
       ["m"] = { "Jump to mark" },
     },
+       ["t"] = {
+           name = "+Telescope to...",
+           ["c"] = { "Nextcloud - Files" },
+           ["h"] = { "Home - Files" },
+       },
     ["w"] = {
       name = "+windows",
       ["w"] = { "Other window" },
osamuaoki commented 2 years ago

Look at how I added new leader keys in https://github.com/NTBBloodbath/doom-nvim/pull/130

Specifically: https://github.com/NTBBloodbath/doom-nvim/pull/130/commits/c20e8df4866dd19122150797257e1512ff03fd5e

At least 2 files need to be changed. If you call function, that needs to be created somewhere. That's why I have 3 files changed.

osamuaoki commented 2 years ago

Current state

I have to admit nvim-mapper generated telescope search screen is great. I like it too.

But I have to say the whole design of nvim-mapper looks flawed and it only focus on telescope and does mapping on its side job. For which-key, we need to generate table manually and independently in sync.

More over, nvim-mapper scans the configuration source tree with multi-line regex matching with unique identifier list stored in vim.g.mapper_records sandwiched between "(" and ")". Very inefficient, to say the least.

Thought for future

Since this is too much work, I am not keen to do but here is what I thought as an feasible path forward to improve situation.

If there is a single Lua data table covering for all

things will be much simpler. (With this ind of basic design, extending this to support user override will be doable and clean. Now user configuration only does mapping (no which-key, no telescope) as I glanced the code.

If /init.lua generated global Lua variable can be stored to vim.g.foo or on running nvim memory and it can be read by telescope extension, there should be no need to crawl through many files. If this is not possible, dump the Lua data to file before exiting init.lua, I suppose. If I am not mistaken, lua can make global variable.

Roughly, storing configuration data should be like:

M.[key_sequence][mode_list] = {mapping_flag, data1, data2, ...}

Until someone creates this kind of clean data infrastructure, editing non-configuration files and managing to merge changes to upstream is the option. Creating more ad-hoc mechanism to address some popular concern for keyboard remapping including the current one doesn't seem to me a good idea.

Just a thought....

osamuaoki commented 2 years ago

Although I don't like the way rg is used, now I see some rationale behind this (like getting definition file location. Now I know I should avoid using unique string even in the comment).

As long as this package is loaded on-demand via opt, rg doesn't seem to be run when booting. All we need seems to be having access to map(mode, keys, cmd, options, category, unique_identifier, description) related functions accessible during initial boot process. Rest can be done when telescope window is started.

If we parse the generated map data (generated by utils.map which calls mapper.map) and use the current category definition in it for which key, we may be able to use the current map data as common base data for almost all key bindings. I see some minor differences such as the category is capitalized but which-key is not and defining its group entry by the lower cased category name prefixed with +. Then exposing mapping to user configuration file may be simpler.

NTBBloodbath commented 2 years ago

Hi, sorry for the late reply on this!

Maybe we could move this to another separated issue related to keybindings? Not completely necessary but should allow us to have a better management :p

So while I was reading your toes I thought "Hey why don't we do something like nest.nvim does"? nest.nvim basically declares keybindings in a very clean and understandable way by using a Lua table. Perhaps with a similar syntax we could be able to

  1. Set keybindings in a nicer way
  2. Set which-key menu entries in an easier way without having to modify two files

I think this idea is pretty similar with yours, obviously mine is just a vague idea (still don't know if this would work) and we can combine both in some aspects, maybe something very good will end up happening haha.

If I am not mistaken, which-key only applies to user-defined normal mode command.

which-key can also set other modes keybindings like visual mode ones, however I think I disabled help for these modes, it was a bit annoying to press v and a big popup being raised IMO.


Regarding to mapper telescope extension huh this is a bit muddy territory, since mapper is a telescope extension we end up in a hard decision regarding to loading order. If you lazy-load telescope like doom does but you initialize a telescope extension earlier then telescope will not be lazy-loaded as expected, however I completely agree with what you're telling me.

Maybe we could try lazy loading telescope in a different way like with event = "VimEnter"? In that way rg should be ran when we enter Neovim because telescope and mapper will be loaded.

This could increase our startup time but I don't really care about increasing our startup time by around 10-20ms if we need to do it for having a better experience while using Doom. Everything below 200ms is pretty fine IMO and to me the startup time is around 90-100ms right now (with some extra plugins that runs on launch).

osamuaoki commented 2 years ago

Yes, this is getting out of original bug report.

For me, if you merge "Tweak", I am happy for now. It's a question of cost of cleaning up code base and aesthetic code quality. When we find a time, let's open another discussion.

(I also need take care my other project. https://github.com/osamuaoki/imediff After working on indentation changes in this Doom-nvim, I have appetite for updating imediff.)

NTBBloodbath commented 2 years ago

Merged it, thank you so much per your awesome work! :)

Good luck with imediff btw :)

connorgmeehan commented 2 years ago

Closing due to inactivity + feature is handled in the https://github.com/NTBBloodbath/doom-nvim/tree/next next branch.