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

[BUG] keymaps: mapping telescope's undo extension breaks custom keymaps #1493

Closed
1 task done
PowerUser64 opened this issue May 6, 2024 · 5 comments · Fixed by #1544
Closed
1 task done

[BUG] keymaps: mapping telescope's undo extension breaks custom keymaps #1493

PowerUser64 opened this issue May 6, 2024 · 5 comments · Fixed by #1544
Labels
bug Something isn't working

Comments

@PowerUser64
Copy link

Field Description
Plugin telescope
Nixpkgs 63c3a29ca82437c87573e4c6919b09a24ea61b0f
Home Manager 9036fe9ef8e15a819fa76f47a8b1f287903fb848
  • I have read the FAQ and my bug is not listed there.

Description

Binding a key to Telescope's undo plugin causes custom keymaps to stop working.

Minimal, Reproducible Example

programs.nixvim = {
  plugins.which-key.enable = true; # helps see what's happening
  plugins.telescope = {
    enable = true;
    keymaps = {

        ###### the problematic line - comment this to "fix" the bug
        "<leader>fu" = { action = "undo";         options.desc = "Telescope Undo"; };

        # These should work fine
        "<leader>ff" = { action = "builtin";      options.desc = "Telescope Builtin"; };
        "<leader>fg" = { action = "live_grep";    options.desc = "Telescope Live Grep"; };
    };
    # NOTE: the undo plugin doesn't have to be enabled
    #extensions.undo.enable = true;
  };

  # test keymaps
  globals.mapleader = " ";
  keymaps = [
    {
      action = "<cmd>help<CR>";
      key = "gh";
      mode = [ "n" ];
    }
    {
      action = "<cmd>help<CR>";
      key = "yu";
      mode = [ "n" ];
    }
    {
      action = "<cmd>help<CR>";
      key = "<leader>g";
      mode = [ "n" ];
    }
  ];
}

Steps to reproduce

  1. load nixvim with the config above
  2. try using any of the custom keymaps (gh, yu, <leader>g)
    • they don't work (help page doesn't open)
    • note that <leader>ff and the other telescope.keymaps options work (as they should)
  3. comment line 8 (the problematic line)
  4. try using the custom keymaps from before
    • help page opens, they work
    • all other telescope.keymaps options still work

it took me 2 days to find this 😭

@PowerUser64 PowerUser64 added the bug Something isn't working label May 6, 2024
@PowerUser64 PowerUser64 changed the title [BUG] keymapping telescope's undo extension breaks custom keymaps [BUG] keymaps: mapping telescope's undo extension breaks custom keymaps May 6, 2024
@MattSturgeon
Copy link
Collaborator

I can't see how the keymap code could lead to one keymap effecting others...

keymaps = mapAttrsToList (
key: mapping:
let
actionStr = if isString mapping then mapping else mapping.action;
in
{
mode = mapping.mode or "n";
inherit key;
action.__raw = "require('telescope.builtin').${actionStr}";
options = {
silent = cfg.keymapsSilent;
} // (mapping.options or { });
}
) cfg.keymaps;

So this is a bit of a head scratchier 🤔

Is there anything suspicious in the generated init.lua?

Using nix repl, is there anything suspicious when you inspect the resolved config options keymaps and plugins.telescope.keymaps?

All the telescope keymaps should get added to the "main" keymaps options, with their action converted to a __raw lua string.

@PowerUser64
Copy link
Author

PowerUser64 commented May 18, 2024

Here are the generated files: generated-inits.zip.

Here's the result of running diff on them:

$ diff generated-broken-init.lua generated-working-init.lua
57,62d56
<         {
<             ["action"] = require("telescope.builtin").fdsafsda,
<             ["key"] = "<leader>fu",
<             ["mode"] = "n",
<             ["options"] = { ["desc"] = "Telescope Undo" },
<         },

The only difference between the inputs is that the problematic line is uncommented.

It turns out it doesn't matter what the action it gets bound to. Here, I'm binding it to fdsafsda and I get the same result as when I use undo. Again, this is regardless of whether extensions.undo.enable = true; is set. This would seem to indicate that setting extensions.undo.enable = true; isn't enabling telescope undo like it's supposed to.

It might also be worth noting the error that is generated by neovim when the MRE is loaded with the problematic line uncommented.

Error detected while processing /nix/store/fqx9ngcpbvz390804l1nav113iwql4xf-init.lua:
E5113: Error while calling lua chunk: vim/keymap.lua:0: rhs: expected string|function, got nil
stack traceback:
        [C]: in function 'error'
        vim/shared.lua: in function 'validate'
        vim/keymap.lua: in function 'set'
        /nix/store/fqx9ngcpbvz390804l1nav113iwql4xf-init.lua:68: in main chunk

@MattSturgeon
Copy link
Collaborator

rhs: expected string|function, got nil

That's saying the keymap's "right hand side" (action) should be a string or a lua function. Instead it was nil.

That's the expected result for fdsafsda, and anything else that doesn't exist in the table returned from require("telescope.builtin").

Undo is not a telescope built-in (it's a 3rd party extension), so it won't be present in telescope.builtins. Maybe NixVim should handle this better... 🤔

The extension's readme suggests it can be involved as follows:

" Directly by calling it through Telescope's extension interface:

" using lua
:lua require("telescope").extensions.undo.undo()

" using custom options for just this call
:lua require("telescope").extensions.undo.undo({ side_by_side = true })

" using legacy Vim script
:Telescope undo

" Using the optional mapping:

" using lua
:lua vim.keymap.set("n", "<leader>u", "<cmd>Telescope undo<cr>")
" using legacy Vim script
:nmap <leader>u <cmd>Telescope undo<cr>

So, in this case the action should be require("telescope").extensions.undo.undo (raw string).

"<leader>fu" = {
 action.__raw = "require("telescope").extensions.undo.undo";
 options.desc = "Telescope Undo";
};

@GaetanLepage should we be using require("telescope").extensions instead of require("telescope.builtin") here? Or perhaps "<cmd>Telescope ${actionStr}<cr>"?

@GaetanLepage
Copy link
Collaborator

Thanks @MattSturgeon for the precise troubleshooting on this.

@GaetanLepage should we be using require("telescope").extensions instead of require("telescope.builtin") here? Or perhaps "<cmd>Telescope ${actionStr}<cr>"?

Hmm, are the "builtin" actions also part of require('telescope').extensions ?

@MattSturgeon
Copy link
Collaborator

MattSturgeon commented May 20, 2024

Hmm, are the "builtin" actions also part of require('telescope').extensions ?

Not sure, but even if they are it appears the actual functions are nested in the table; e.g. extensions.undo.undo().

I think <cmd>Telescope ${actionStr}<cr> is the way to go (that command supports built-in and third-party extensions), unless there's any reason to prefer calling the lua functions directly?

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

Successfully merging a pull request may close this issue.

3 participants