Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot parse which-key mappings #81

Open
sjclayton opened this issue Jan 11, 2024 · 21 comments · Fixed by #82
Open

Cannot parse which-key mappings #81

sjclayton opened this issue Jan 11, 2024 · 21 comments · Fixed by #82
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@sjclayton
Copy link

sjclayton commented Jan 11, 2024

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
Copy link
Owner

tris203 commented Jan 11, 2024

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

@sjclayton
Copy link
Author

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

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

@tris203
Copy link
Owner

tris203 commented Jan 11, 2024

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
Copy link
Author

sjclayton commented Jan 11, 2024

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. 💯

@tris203
Copy link
Owner

tris203 commented Jan 11, 2024

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 tris203 added the bug Something isn't working label Jan 11, 2024
@tris203
Copy link
Owner

tris203 commented Jan 11, 2024

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

issue81

@sjclayton
Copy link
Author

@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
Copy link
Owner

tris203 commented Jan 12, 2024

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
Copy link
Author

sjclayton commented Jan 12, 2024

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
Copy link
Owner

tris203 commented Jan 12, 2024

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

@sjclayton
Copy link
Author

sjclayton commented Jan 12, 2024

@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
Copy link
Owner

tris203 commented Jan 12, 2024

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
Copy link
Owner

tris203 commented Jan 12, 2024

That's really helpful

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

@sjclayton
Copy link
Author

@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
Copy link
Owner

tris203 commented Jan 12, 2024

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
Copy link
Author

@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
Copy link
Owner

tris203 commented Jan 12, 2024

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 tris203 added the help wanted Extra attention is needed label Jan 12, 2024
@tris203
Copy link
Owner

tris203 commented Jan 14, 2024

@sjclayton can you retry with the issue81 branch?

@sjclayton
Copy link
Author

@sjclayton can you retry with the issue81 branch?

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

@tris203
Copy link
Owner

tris203 commented Jan 14, 2024

@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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants