tris203 / hawtkeys.nvim

⌨️🔥 Suggest new easy to hit keymaps and find issues with your current key map configurations
MIT License
215 stars 5 forks source link

Cannot parse which-key mappings #81

Open sjclayton opened 8 months ago

sjclayton commented 8 months ago

I have my which-key mappings table setup like the Primary example in the which-key README

Similar to this...

wk.register({
  f = {
    name = "file", -- optional group name
    f = { "<cmd>Telescope find_files<cr>", "Find File" }, -- create a binding with label
    r = { "<cmd>Telescope oldfiles<cr>", "Open Recent File", noremap=false, buffer = 123 }, -- additional options for creating the keymap
    n = { "New File" }, -- just a label. don't create any mapping
    e = "Edit File", -- same as above
    ["1"] = "which_key_ignore",  -- special label to hide it in the popup
    b = { function() print("bar") end, "Foobar" } -- you can also pass functions!
  },
}, { prefix = "<leader>" })

Using prefix at the end... and I get an error when I run HawtkeysAll that says "Error parsing which-key table"

I have this as part of my config opts for hawtkeys

-- If you use whichkey.register with an alias eg wk.register
        ["wk.register"] = {
            method = "which_key",
        },
tris203 commented 8 months ago

Can you confirm versions of hawtkeys, whichkey and neovim pls @sjclayton

sjclayton commented 8 months ago

Can you confirm versions of hawtkeys, whichkey and neovim pls @sjclayton

Neovim is latest nightly hawtkeys is ^HEAD which-key is ^HEAD

tris203 commented 8 months ago

Can you confirm versions of hawtkeys, whichkey and neovim pls @sjclayton

Neovim is latest nightly hawtkeys is ^HEAD which-key is ^HEAD

I think this is a format in whichkey I didn't consider

Let me recreate this in a test

This is the format we test for

https://github.com/tris203/hawtkeys.nvim/blob/main/tests%2Fhawtkeys%2Fexample_configs%2Fwhich-key.register_keymap.lua

sjclayton commented 8 months ago

Can you confirm versions of hawtkeys, whichkey and neovim pls @sjclayton

Neovim is latest nightly hawtkeys is ^HEAD which-key is ^HEAD

I think this is a format in whichkey I didn't consider

Let me recreate this in a test

This is the format we test for

https://github.com/tris203/hawtkeys.nvim/blob/main/tests%2Fhawtkeys%2Fexample_configs%2Fwhich-key.register_keymap.lua

The way shown in the primary example in the documentation I believe is the default way in which most configs I've seen have been implemented, so it might be good to add it, so things don't break if someone followed the which-key docs. :100:

tris203 commented 8 months ago

The way shown in the primary example in the documentation I believe is the default way in which most configs I've seen have been implemented, so it might be good to add it.

Fully agree with you

I would like to support all methods

tris203 commented 8 months ago

@sjclayton please could you try with the branch I have created for this

issue81

sjclayton commented 8 months ago

@sjclayton please could you try with the branch I have created for this

issue81

No go, still giving the same error when I try on that branch.

tris203 commented 8 months ago

Are your dotfiles public? I might need to see the example

As we pass all of the examples in the readme for whichkey on that branch

sjclayton commented 8 months ago

It would appear the which-key mappings are being populated in the list now, but the error is still being thrown even so...

Question, can your plugin be effectively lazy loaded or loaded after which-key and still work?

Also you can check here -- keymap config

tris203 commented 8 months ago

I have also just pushed another commit to that branch that may give a more meaningful error

sjclayton commented 8 months ago

@tris203 I get the which-key table back in the error now

{
  b = {
    name = icons.ui.Files .. 'Buffers',
    j = { '<Plug>(cokeline-pick-focus)', 'Jump to buffer', noremap = true },
    C = {
      '<Plug>(cokeline-pick-close)',
      'Pick buffer to close',
      noremap = true,
    },
    n = { '<Plug>(cokeline-focus-next)', 'Cycle next buffer', noremap = true },
    p = { '<Plug>(cokeline-focus-prev)', 'Cycle previous buffer', noremap = true },
  },
  c = {
    name = icons.ui.Code .. 'Code',
    c = {
      name = icons.kinds.Package .. 'Rust Crates',
    },
  },
  d = {
    name = icons.ui.Bug .. 'Debug',
  },
  f = {
    name = icons.kinds.File .. 'File',
  },
  n = {
    name = icons.ui.Notes .. 'Notes',
    f = { '<CMD>Telescope frecency workspace=notes<CR>', 'Recent notes', noremap = true },
    j = { '<CMD>ObsidianToday<CR>', 'Journal - Today', noremap = true },
    y = { '<CMD>ObsidianYesterday<CR>', 'Journal - Yesterday', noremap = true },
    n = {
      function()
        vim.ui.input({ prompt = 'Note title:' }, function(input)
          if input ~= nil then
            vim.cmd('ObsidianNew ' .. input)
          else
            return
          end
        end)
      end,
      'New note',
      noremap = true,
    },
    r = {
      function()
        local current_name = vim.fn.expand('%:t:r')
        vim.ui.input({ prompt = 'Rename note: ', default = current_name }, function(input)
          if input ~= nil then
            vim.cmd('ObsidianRename ' .. input)
          else
            return
          end
        end)
      end,
      'Rename note',
      noremap = true,
    },
    s = { '<CMD>ObsidianSearch<CR>', 'Search notes', noremap = true },
    t = { '<CMD>ObsidianTemplate<CR>', 'Insert template', noremap = true },
  },
  t = {
    name = icons.ui.Telescope .. 'Telescope',
  },
  l = {
    i = { '<CMD>LspInfo<CR>', 'LSP Info', noremap = true },
    r = { '<CMD>LspRestart<CR>', 'Restart LSP', noremap = true },
  },
  u = {
    name = icons.ui.UI .. 'UI/Utils',
    c = {
      function()
        helper.toggle('Listchars', { enable = 'set list', disable = 'set nolist' })
      end,
      'Toggle listchars',
      noremap = true,
    },
    n = {
      function()
        helper.number()
      end,
      'Toggle line numbers',
      noremap = true,
    },
    N = {
      function()
        helper.toggle('Relative numbers', { enable = 'set norelativenumber', disable = 'set relativenumber' })
      end,
      'Toggle relative line numbers',
      noremap = true,
    },
    l = { '<CMD>Lazy<CR>', 'Open Lazy', noremap = true },
    s = {
      function()
        helper.toggle('Spell check', { enable = 'set spell', disable = 'set nospell' })
      end,
      'Toggle spell check',
      noremap = true,
    },
    w = {
      function()
        helper.toggle('Word wrap', { enable = 'set wrap', disable = 'set nowrap' })
      end,
      'Toggle word wrap',
      noremap = true,
    },
  },
  x = {
    name = icons.diagnostics.Warn .. 'Diagnostics',
  },
  ['<space>'] = { '<C-^>', 'Jump to alternate file', noremap = true },
}, { prefix = '<leader>' }

The above is the complete output, nothing removed or added.

tris203 commented 8 months ago

It would appear the which-key mappings are being populated in the list now, but the error is still being thrown even so...

Question, can your plugin be effectively lazy loaded or loaded after which-key and still work?

Also you can check here -- keymap config

Yes. You can lazy load on cmd. The parsing is all done on UI open

So you can re-open as you edit the config and it should update in real time

Do you get a different message with the latest commit I just pushed?

tris203 commented 8 months ago

That's really helpful

I'm headed to bed now. But will take another look tomorrow and hopefully get this sorted for you

sjclayton commented 8 months ago

@tris203 Just noticed something else... like I said the which-key mapping are showing up now, even though the error is still getting thrown.

But in the list they are being labeled as being Vim Defaults

I'm not sure if this is intended behavior, because I see that the Lazy key mappings are labeled as being configured from Lazy.

Just thought I'd mention that.

tris203 commented 8 months ago

I see what the problem is i think. It is because of the subset of icons.ui.*

we evaluate the string to turn it back into a table, but that fails I assume due to the assigned var.

let me think about how we are best to handle this...

sjclayton commented 8 months ago

@tris203 Is there no way of filtering out the name field completely? as technically in and of itself it isn't actually part of a mapping and should be ignored anyways..

So if you get a table with an object that contains that field you return the same table/object with the field stripped?

tris203 commented 8 months ago

yes @sjclayton, but the problem isn't just limited to the name field, it could be used for the function definition itself.

as it is in the test case I've written in pr #84

for which key mapping is 1) Extract everything inside the function call as a string 2) return it through loadstring() to make it into a table 3) pass it through the actual whichkey mapping parser (this allows us to support everything which-key supports with little overhead)

We fall over at step 2, in this case, as the string can't be return'd into a lua table as variables inside the string are undefined.

You could rewrite each var to be standalone eg instead of icons.ui.* you would use require('icons).ui.* but that is unreasonable and one of the goals of this project is to require 0 alterations to your config to be compatible.

tris203 commented 8 months ago

@sjclayton can you retry with the issue81 branch?

sjclayton commented 8 months ago

@sjclayton can you retry with the issue81 branch?

Just updated, still a no go... same error.

tris203 commented 8 months ago

@sjclayton can you retry with the issue81 branch?

Just updated, still a no go... same error.

I'm narrowing in on the issue, it works for variables that are out of scope and included in the function But it returns a table, and you concatenate onto them as strings.

Thanks for the issue, this one is definately a brain tickler

sjclayton commented 8 months ago

@tris203 Glad to see it's makin' ya think 😉