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: Telescope fails to load properly after "lazy resolving of plugin handlers" refactor #1132

Closed
3 tasks done
marceldev89 opened this issue Oct 17, 2023 · 10 comments · Fixed by #1130
Closed
3 tasks done
Labels
bug Something isn't working

Comments

@marceldev89
Copy link

marceldev89 commented Oct 17, 2023

Did you check docs and existing issues?

  • I have read all the lazy.nvim docs
  • I have searched the existing issues of lazy.nvim
  • I have searched the existing issues of plugins related to this issue

Neovim version (nvim -v)

0.10.0 commit gc9963e2212

Operating system/version

Arch

Describe the bug

Commit 47dafae breaks Telescope's user command when doing a require("telescope.builtin") within a keys = function() config thingy.

        keys = function()
            local ts = require("telescope.builtin")
            -- vim.keymap.set("n", "<leader>ff", ts.find_files, { desc = "[F]ind [F]iles" })
            vim.keymap.set("n", "<leader>ff", ":Telescope find_files<CR>", { desc = "[F]ind [F]iles" })
        end,

Note: I'm probably doing something I shouldn't be doing anyway within the lazy spec but it worked before 😛

This gives me the following error:

Command `Telescope` not found after loading `telescope.nvim`

Reloading telescope with :Lazy reload does fix the user command

Steps To Reproduce

  1. Run with repro.lua

Expected Behavior

Telescope window opens when running with repro.lua

Repro

-- DO NOT change the paths and don't remove the colorscheme
local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({ "git", "clone", "--filter=blob:none", "https://github.com/folke/lazy.nvim.git", lazypath, })
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
  "folke/tokyonight.nvim",
  -- add any other plugins here
    {
        "nvim-telescope/telescope.nvim",
        version = false,
        dependencies = {
            "nvim-lua/plenary.nvim",
        },
        cmd = "Telescope",
        config = function()
            require("telescope").setup()
        end,
        keys = function()
            local ts = require("telescope.builtin")
            return {
                { "<leader>ff", ts.oldfiles, desc = "[F]ind [F]iles" },
            }
        end,
    }
}
require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

vim.cmd.colorscheme("tokyonight")
-- add anything else here

vim.api.nvim_create_autocmd({ "VimEnter" }, {
  group = vim.api.nvim_create_augroup("Repro", {}),
  callback = function()
    vim.cmd([[:Telescope]])
  end,
})
@marceldev89 marceldev89 added the bug Something isn't working label Oct 17, 2023
@marceldev89 marceldev89 changed the title bug: Telescope fails to load properly after 47dafaed643172f97956014ce62350769e3fb0cb bug: Telescope fails to load properly after "lazy resolving of plugin handlers" refactor Oct 17, 2023
@max397574
Copy link
Contributor

  1. send the error if you get one
  2. I think you need to return a table or string from the keys function

@marceldev89
Copy link
Author

marceldev89 commented Oct 17, 2023

  1. send the error if you get one

Added the error to the report

  1. I think you need to return a table or string from the keys function

Yes, I realized I'm doing it wrong while figuring out why it no longer works. But it worked before so maybe worth reporting :)

@max397574
Copy link
Contributor

well if you used something that wasn't documented before and it worked you "abused" a bug which is now fixed ig
ig folke will tell you if it was a feature or a bug

@marceldev89
Copy link
Author

marceldev89 commented Oct 17, 2023

I think this is a more reasonable keys config which is also broken now:

        keys = function()
            local ts = require("telescope.builtin")
            return {
                { "<leader>ff", ts.oldfiles, desc = "[F]ind [F]iles" },
            }
        end,

In the end it's still kinda weird because require-ing during the function call kinda defeats the purpose of lazy loading.. 😅

Edit: Changed the repro to this variation because it feels more reasonable to be used in the wild :)

@marceldev89
Copy link
Author

Hmm another thing of note: I have another lazy spec that's basically the same for trouble.nvim. This one doesn't break the :Trouble user commands. 🤔

  keys = function()
    local trouble = require("trouble")
    vim.keymap.set("n", "<leader>xw", function() trouble.toggle("workspace_diagnostics") end)
    vim.keymap.set("n", "<leader>xd", function() trouble.toggle("document_diagnostics") end)
  end,

@max397574
Copy link
Contributor

max397574 commented Oct 17, 2023

by looking at the error
do you perhaps just not call the setup function? Because that's where the command should be created when loading telescope
at least in the minimal config you don't

@marceldev89
Copy link
Author

The cmd = "Telescope" bit should call the default config implementation, doesn't it? But in my config I do call the setup manually and adding it to the repro doesn't change anything. I'll add it to the repro anyway, just in case.

In the meantime I fixed my weird keys configs :)

@max397574
Copy link
Contributor

🤔 not sure about that tbh
I think it will just load the plugin
and then load your config
so you should either have a config or e.g. opts = {}

@folke
Copy link
Owner

folke commented Oct 17, 2023

The cmd = "Telescope" bit should call the default config implementation, doesn't it? But in my config I do call the setup manually and adding it to the repro doesn't change anything. I'll add it to the repro anyway, just in case.

Nope. config gets only called when config=true, or opts=... are set.

Only the default config will call setup() automatically. If you specify your own config function then you need to call setup yourself.

This is regardless of lazy handlers like cmd.


Fixed that issue at the top in the meantime.
You could actually do it the way you did it. Telescope won't be loaded when adding its lazy handlers.
But performance-wise it's still better to put those calls in a closure.

@marceldev89
Copy link
Author

Nope. config gets only called when config=true, or opts=... are set.

Only the default config will call setup() automatically. If you specify your own config function then you need to call setup yourself.

This is regardless of lazy handlers like cmd.

Thanks for clarifying :)

Fixed that issue at the top in the meantime. You could actually do it the way you did it. Telescope won't be loaded when adding its lazy handlers. But performance-wise it's still better to put those calls in a closure.

Nice! Ah well I guess now that I've changed it to a "proper" way I might as well just keep it 😛

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