hrsh7th / cmp-cmdline

nvim-cmp source for vim's cmdline
MIT License
505 stars 41 forks source link

`cmdheight=0`and latest nvim gives `custom_entries_view.lua:209: Expected Lua number` #64

Closed gegoune closed 1 year ago

gegoune commented 1 year ago

Hi there,

latest neovim HEAD changed the implementation of cmdheight=0 in https://github.com/neovim/neovim/commit/708bd686516b420c2b65f4bc4d2c58fe43fb945e causing nvim-cmdline to fail with

:psy
Error executing vim.schedule lua callback: ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:209: Expected Lua number
stack traceback:
        [C]: in function 'nvim_win_set_cursor'
        ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:209: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/view.lua:110: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/core.lua:323: in function 'fn'
        .../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:55: in function <.../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:53>
:psy
Error executing vim.schedule lua callback: ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:209: Expected Lua number
stack traceback:
        [C]: in function 'nvim_win_set_cursor'
        ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:209: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/view.lua:110: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/core.lua:323: in function 'fn'
        .../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:55: in function <.../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:53>
:psy

when single :psy is typed into cmd mode. Please note that I only type it once but get the above error message twice.

Once cmd line is cleared and I try to type :psy again I get different error:

Error executing vim.schedule lua callback: ...site/pack/packer/start/nvim-cmp/lua/cmp/utils/window.lua:146: 'height' key must be a positive Integer
stack traceback:
        [C]: in function 'nvim_win_set_config'
        ...site/pack/packer/start/nvim-cmp/lua/cmp/utils/window.lua:146: in function 'update'
        ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:368: in function '_select'
        ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:220: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/view.lua:110: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/core.lua:323: in function 'fn'
        .../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:55: in function <.../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:53>

(:psy is fuzzy match I use to get :PackerSync).

adelarsq commented 1 year ago

This is a bug on Neovim nightly. Reference https://github.com/neovim/neovim/issues/20243

gegoune commented 1 year ago

Perhaps https://github.com/neovim/neovim/pull/20249 will fix it.

Shougo commented 1 year ago

Yes. Please test it.

Shougo commented 1 year ago

Please test neovim latest.

gegoune commented 1 year ago

On latest HEAD (v0.8.0-dev-2603-g7a70e9587-dirty), first error seems to be gone. Still getting second one (height one) on occasion after navigating through cmp's completion window for a bit.

statusline obfuscates cmdline but it's known neovim issue I thin.

Shougo commented 1 year ago

Oh, the height error seems nvim-cmp problem.

laytan commented 1 year ago

I am still getting the following errors when completions are trying to show:

Error executing vim.schedule lua callback: ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:209: Expected Lua number
stack traceback:
        [C]: in function 'nvim_win_set_cursor'
        ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:209: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/view.lua:110: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/core.lua:323: in function 'fn'
        .../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:55: in function <.../site/pack/packer/start/nvim-cmp/lua/cmp/
utils/async.lua:53>

Both on nightly(v0.8.0-dev-1146-g7a70e9587) and the master version with the latest version of nvim-cmp and cmp-cmdline.

gegoune commented 1 year ago

There were some fixes in Neovim in last couple of days but they did not seem to have fixed that error. @Shougo do you think that requires fix on nvim-cmp? Should I open issue there?

Shougo commented 1 year ago

What is the error in the latest neovim HEAD? I think @hrsh7th 's investigation is needed.

Please upload the reproduce instructions.

Shougo commented 1 year ago

Reproduced.

self.style.height seems 0 in window.update().

Shougo commented 1 year ago
diff --git a/lua/cmp/utils/window.lua b/lua/cmp/utils/window.lua
index b150c03..349e01b 100644
--- a/lua/cmp/utils/window.lua
+++ b/lua/cmp/utils/window.lua
@@ -142,7 +142,7 @@ window.update = function(self)
         col = info.col + info.width - info.scrollbar_offset, -- info.col was already contained the scrollbar offset.
         zindex = (self.style.zindex and (self.style.zindex + 1) or 1),
       }
-      if self.sbar_win and vim.api.nvim_win_is_valid(self.sbar_win) then
+      if self.sbar_win and vim.api.nvim_win_is_valid(self.sbar_win) and style.height > 0 then
         vim.api.nvim_win_set_config(self.sbar_win, style)
       else
         style.noautocmd = true

The patch fixes the error.

@hrsh7th Can you test the patch?

gegoune commented 1 year ago

@Shougo I have just tested it. Still getting the same error

Error executing vim.schedule lua callback: ...site/pack/packer/start/nvim-cmp/lua/cmp/utils/window.lua:146: 'height' key must be a positive Integer
stack traceback:
        [C]: in function 'nvim_open_win'
        ...site/pack/packer/start/nvim-cmp/lua/cmp/utils/window.lua:146: in function 'update'
        ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:368: in function '_select'
        ...cker/start/nvim-cmp/lua/cmp/view/custom_entries_view.lua:220: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/view.lua:110: in function 'open'
        ...re/nvim/site/pack/packer/start/nvim-cmp/lua/cmp/core.lua:324: in function 'fn'
        .../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:55: in function <.../site/pack/packer/start/nvim-cmp/lua/cmp/utils/async.lua:53>

but maybe at different times now. For example, when I type :psy I don't actually see PackerSync in completion menu, only psy itself which must have accidentally got added to my command history:

Screenshot 2022-09-27 at 10 42 08
Shougo commented 1 year ago

Please test the patch.

https://github.com/hrsh7th/cmp-cmdline/issues/64#issuecomment-1259041710

gegoune commented 1 year ago

@Shougo I have tested it with that patch, my previous message was an update after applying that patch to cmp code.

Shougo commented 1 year ago

Really? It should not. Because I have added the check. Please don't use the package manager. I think the code is cached.

Shougo commented 1 year ago

My init.vim

set rtp+=~/src/nvim-cmp
set rtp+=~/src/cmp-buffer
set rtp+=~/src/cmp-nvim-lsp
set rtp+=~/src/vim-vsnip
set rtp+=~/src/cmp-vsnip
set rtp+=~/src/nvim-lspconfig
set rtp+=~/src/cmp-cmdline

set title
set titlestring="%F %r"

lua <<EOF
  -- Setup nvim-cmp.
  local cmp = require'cmp'

  cmp.setup({
    snippet = {
      expand = function(args)
        vim.fn["vsnip#anonymous"](args.body)
      end,
    },
    mapping = {
      ['<CR>'] = cmp.mapping.confirm {
        behavior = cmp.ConfirmBehavior.Replace,
        select = true,
      },
      ['<tab>'] = cmp.mapping(cmp.mapping.confirm({ select = true }), { 'i', 's', 'c' }),
      ['<C-p>'] = cmp.mapping.select_prev_item(),
      ['<C-n>'] = cmp.mapping.select_next_item(),
    },
    sources = {
      { name = 'nvim_lsp' },
      { name = 'buffer' },
    },
    completion = {
    completeopt = 'menu,menuone,noselect'
    }
  })
  cmp.setup.cmdline(':', {
    mapping = cmp.mapping.preset.cmdline(),
    sources = {
      { name = 'cmdline' }
    }
  })
EOF

lua << EOF
-- Add additional capabilities supported by nvim-cmp
local capabilities = vim.lsp.protocol.make_client_capabilities()
capabilities = require('cmp_nvim_lsp').update_capabilities(capabilities)

--require'lspconfig'.sumneko_lua.setup{}
--require'lspconfig'.cssls.setup{}
--require'lspconfig'.pylsp.setup{}
require'lspconfig'.tailwindcss.setup{}
--require'lspconfig'.gopls.setup{
--  capabilities = capabilities,
--  init_options = {
--    usePlaceholders = true,
--  },
--}
EOF

runtime! after/plugin/*.lua
gegoune commented 1 year ago

Oh, I see, fair point, apologies. I have disabled impatient and cleared its cache.

Behaviour is as follow now:

(Seems like menu gets drawn too low?)

Shougo commented 1 year ago
diff --git a/lua/cmp/utils/window.lua b/lua/cmp/utils/window.lua
index b150c03..2dbec06 100644
--- a/lua/cmp/utils/window.lua
+++ b/lua/cmp/utils/window.lua
@@ -137,7 +137,7 @@ window.update = function(self)
         relative = 'editor',
         style = 'minimal',
         width = 1,
-        height = self.style.height,
+        height = math.min(1, self.style.height),
         row = info.row,
         col = info.col + info.width - info.scrollbar_offset, -- info.col was already contained the scrollbar offset.
         zindex = (self.style.zindex and (self.style.zindex + 1) or 1),

I think the patch is better.

hit <backspace> to delete characters - this results with 'height' key must be a positive Integer error on each backspace keystroke.

I don't reproduce the error.

Shougo commented 1 year ago

Fixed.

diff --git a/lua/cmp/utils/window.lua b/lua/cmp/utils/window.lua
index b150c03..b350eae 100644
--- a/lua/cmp/utils/window.lua
+++ b/lua/cmp/utils/window.lua
@@ -137,7 +137,7 @@ window.update = function(self)
         relative = 'editor',
         style = 'minimal',
         width = 1,
-        height = self.style.height,
+        height = math.max(1, self.style.height),
         row = info.row,
         col = info.col + info.width - info.scrollbar_offset, -- info.col was already contained the scrollbar offset.
         zindex = (self.style.zindex and (self.style.zindex + 1) or 1),
diff --git a/lua/cmp/view/custom_entries_view.lua b/lua/cmp/view/custom_entries_view.lua
index 2c72fe6..50243bd 100644
--- a/lua/cmp/view/custom_entries_view.lua
+++ b/lua/cmp/view/custom_entries_view.lua
@@ -206,7 +206,9 @@ custom_entries_view.open = function(self, offset, entries)
     zindex = completion.zindex or 1001,
   })
   -- always set cursor when starting. It will be adjusted on the call to _select
-  vim.api.nvim_win_set_cursor(self.entries_win.win, { 1, 0 })
+  if self.entries_win.win then
+    vim.api.nvim_win_set_cursor(self.entries_win.win, { 1, 0 })
+  end
   if preselect_index > 0 and config.get().preselect == types.cmp.PreselectMode.Item then
     self:_select(preselect_index, { behavior = types.cmp.SelectBehavior.Select })
   elseif not string.match(config.get().completion.completeopt, 'noselect') then

self.entries_win.win may be nil. I don't know why.

gegoune commented 1 year ago

It behaves the same with that patch as well:

https://user-images.githubusercontent.com/69750637/192482249-2f29fa51-2c9a-41b3-a92d-3874c2b5c4ed.mov

gegoune commented 1 year ago

Your latest patch fixes all discussed issues, thank you!

Ah, no, sorry, still behaves as in the video above, forgot to set cmdheight=0.

Shougo commented 1 year ago

It behaves the same with that patch as well:

I don't reproduce the behavior. Please upload the real error messages.

And please don't test it with the plugin managers. I need to reproduce the issue without plugin managers.

Shougo commented 1 year ago

And the issue is for nvim-cmp not cmd-cmdline.

gegoune commented 1 year ago

Please give me some time, I will try to come up with reproducible minimal configuration as your minimal config does alleviate some of the issues, but I think I might need history for cmdline hence shada is needed (so I can't use --clean?).

Edit: I can still reproduce it with your minimal config but without --clean (with #1196 checkout on nvim-cmp):

Screenshot 2022-09-27 at 14 01 25

ctrl-n

Screenshot 2022-09-27 at 14 02 15
Shougo commented 1 year ago

Please don't use packer to reproduce the problem.

laytan commented 1 year ago

The PR on nvim-cmp fixes these errors for me!

gegoune commented 1 year ago

@Shougo Thank you for you patience. I have managed to reproduce it with your minimal config running:

nvim --clean -u min.vim

and then in nvim typing :wq.

You can then only see completion for wq with wqall being hidden behind cmdline as on my previous screenshots for psy. Hope that will allow you to replicate that particular issue.

Please not that I have checked out your PR 1196.

Shougo commented 1 year ago

Reproduced the problem. The hight seems wrong.

Shougo commented 1 year ago

Fixed the error.

gegoune commented 1 year ago

Fixed by https://github.com/hrsh7th/nvim-cmp/pull/1196

Ddystopia commented 1 month ago

Hello, issue still persists for be, but with different error:

image

Shougo commented 1 month ago

@Ddystopia I have checked the code. The line 146 is not same.

https://github.com/hrsh7th/nvim-cmp/blob/5260e5e8ecadaf13e6b82cf867a909f54e15fd07/lua/cmp/utils/window.lua#L146

I think you don't update nvim-cmp.