wbthomason / packer.nvim

A use-package inspired plugin manager for Neovim. Uses native packages, supports Luarocks dependencies, written in Lua, allows for expressive config
MIT License
7.72k stars 262 forks source link

feat(compile): use nvim_create_user_command to make lazy-load commands #1149

Closed wbthomason closed 1 year ago

wbthomason commented 1 year ago

This is a slight performance improvement over the old style of command creation, and moves packer toward the modern Neovim APIs

Not immediately merging because I need to track down the Neovim version when nvim_create_user_command was released - if it's v0.8.0, I'll want to hold off on this change since the release was quite recent.

I could add logic to test for the function's existence and fall back to the old method if it's not present, and might. But for now, I'd prefer to avoid adding that complexity.

lewis6991 commented 1 year ago

I think it was added in v0.6: https://github.com/neovim/neovim/pull/16752

folke commented 1 year ago

@wbthomason you can get completion to work for lazy loaded commands as well. Check https://github.com/folke/lazy.nvim/blob/6affae64545da6efd2940589bff8253c08bc7252/lua/lazy/core/handler.lua#L169

wbthomason commented 1 year ago

@folke Thanks, that's a neat trick!

wbthomason commented 1 year ago

I think it was added in v0.6: neovim/neovim#16752

Yeah, I think you're right. Thanks for tracking down the relevant PR; I was too lazy to try before sleeping yesterday.

wbthomason commented 1 year ago

Going to merge this because it hasn't seemed to cause problems on my setup with a few days of use. Please @ me if you start experiencing bugs due to this change, and I'll revert it.

kevinhwang91 commented 1 year ago

Error in packer_compiled: ...vim/site/pack/packer/opt/packer.nvim/lua/packer/load.lua:84: Vim:E477: No ! allowed Please check your config for correctness

wbthomason commented 1 year ago

@kevinhwang91 Did this happen on Neovim start, or when using a particular command?

wbthomason commented 1 year ago

Pushed a temporary reversion in 1d8babf

kevinhwang91 commented 1 year ago

using a particular command?

Not sure, but Flog in vim-flog with ! throw this error.

wbthomason commented 1 year ago

Did it happen when you just started neovim, though, or when you tried to use Flog? Or tab-complete Flog, maybe?

kevinhwang91 commented 1 year ago
        use {
            'rbong/vim-flog', branch = 'v2',
            cmd = {'Flog', 'Flogsplit'},
            requires = 'tpope/vim-fugitive',
            config = conf('plugs.flog')
        }

Type :Flog in the command will reproduce it.

kevinhwang91 commented 1 year ago

I found that it only throws E477 without calling PackerCompile after this PR. After calling PackerCompile, no errors anymore.

wbthomason commented 1 year ago

Oh, I think I know what the problem is then (because I also cannot reproduce with a recompile). I'll add a patch to load.lua, test that I can't reproduce after that, and re-merge. Thanks!

wbthomason commented 1 year ago

@kevinhwang91 Note also that :Flog does not support bang, so :Flog! should throw an error (per https://github.com/rbong/vim-flog/blob/master/plugin/flog.vim#L299)

This was a legitimate bug though, since :Flog alone also throws the error and should not.

kevinhwang91 commented 1 year ago

Yes, look like the issue is caused by the synchronization between packer_compiled.lua and load.lua

wbthomason commented 1 year ago

Ok, feature re-merged with my fix (that I tested to remove the issue @kevinhwang91 noted - thanks for the easy reproducer)

kevinhwang91 commented 1 year ago

It's a bit off-topic, looks like it's a core's bug.....

vim.api.nvim_create_user_command('SayHello', 'echo "Hello world!"', {bang = true})
vim.cmd('verb com SayHello')
print(vim.inspect(vim.api.nvim_parse_cmd('SayHello', {})))

image I guess the bang in core is false, that's why PackerCompile can ignore E477.

wbthomason commented 1 year ago

No, I believe that what happened was this:

That's why my patch fixes the issue by explicitly checking and handling the type of cause.bang.

kevinhwang91 commented 1 year ago

I know your explanation.

The compiled file after this PR:

pcall(vim.api.nvim_create_user_command, 'Flog', function(cmdargs)
    -- print(vim.inspect(cmdargs)) -- uncomment me

          require('packer.load')({'vim-flog'}, { cmd = 'Flog', l1 = cmdargs.line1, l2 = cmdargs.line2, bang = cmdargs.bang, args = cmdargs.args, mods = cmdargs.mods }, _G.packer_plugins)
        end,
        {nargs = '*', range = true, bang = true, complete = function()
          require('packer.load')({'vim-flog'}, { cmd = 'Flog' }, _G.packer_plugins)
          vim.api.nvim_input('<space><bs><tab>')
      end})

bang = true is passed to nvim_create_user_command, but you can uncomment print(vim.inspect(cmdargs)) to inspect the result.