folke / noice.nvim

💥 Highly experimental plugin that completely replaces the UI for messages, cmdline and the popupmenu.
Apache License 2.0
4.42k stars 102 forks source link

bug: Repeated display “nui.nvim: Invalid window id” error #636

Closed nanozuki closed 8 months ago

nanozuki commented 1 year ago

Did you check docs and existing issues?

I found some "Invalid window id" issue. It looks a little different, I'm not quite sure if it's a duplicate, and if it is, I'm sorry!

Neovim version (nvim -v)

NVIM v0.9.4 Build type: Release LuaJIT 2.1.1693350652

Operating system/version

MacOS 14.0

Describe the bug

telegram-cloud-photo-size-1-5021882462535527376-y

After I closed a windows which I used noice, every time I use noice, this error will be display.

Steps To Reproduce

  1. nvim -u repro.lua to open minimal config, and use <Ctrl>W V to vsplit window.
  2. Close this window by :q (use noice, and close window)
  3. type : to open noice's cmdline popup window, and the error comes.

Expected Behavior

No error message.

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',
  {
    'folke/noice.nvim',
    event = 'VeryLazy',
    opts = {
      views = {
        cmdline_popup = {
          position = { row = 0, col = -2 },
          relative = 'cursor',
        },
      },
      lsp = {
        override = {
          ['vim.lsp.util.convert_input_to_markdown_lines'] = true,
          ['vim.lsp.util.stylize_markdown'] = true,
          ['cmp.entry.get_documentation'] = true,
        },
      },
      presets = {
        bottom_search = true, -- use a classic bottom cmdline for search
        command_palette = true, -- position the cmdline and popupmenu together
        long_message_to_split = true, -- long messages will be sent to a split
        inc_rename = false, -- enables an input dialog for inc-rename.nvim
        lsp_doc_border = false, -- add a border to hover docs and signature help
      },
    },
    dependencies = {
      'MunifTanjim/nui.nvim',
      'rcarriga/nvim-notify',
    },
  },
  -- add any other plugins here
}
require('lazy').setup(plugins, {
  root = root .. '/plugins',
})

vim.opt.background = 'dark'
vim.cmd.colorscheme('tokyonight')
-- add anything else here
jackielii commented 9 months ago

I get this very often. I have habit of using <C-w>c to close a window. If I do this on a noice split window, I get this error, once. The following ones seems to be swallowed by router deduplication.

Floating window doesn't seem to have this issue...

pysan3 commented 8 months ago

This is the root cause.

https://github.com/MunifTanjim/nui.nvim/discussions/335

Thesola10 commented 8 months ago

this seems to be related to reusing the same widget instance? Like, when a new split is created, its window ID is updated but the split being deleted means the instance is now linked to a nonexistent window ID? That's the way I understand it since I'm using (or trying to use) relative = "win" on cmdline_popup, it opens over the last opened window, except when changing types (e.g. with :lua or :!) the "new" popup appears over the actual current window.

Maybe the nui widget needs to be discarded and re-created every time?

pysan3 commented 8 months ago

Reusing the same instance is not a problem. On NuiSplit:mount(), nui will either focus the existing window or generate a new window if not exists.

Upon creating a window, nuisplit remembers from which window it was spawned. This is essentially with relative = win, so it can respawn at the same location the second time.

FYI if you want to always spawn relative to a valid window, call split:update_layout() every time before :mount / :show. (It uses previous window if it's valid and current win if not)

But when split is meant to be spawned relative to "editor", we don't care from which window it was spawned last time, but still, nui tries to spawn from the last position.

This becomes a problem when the linked window is already dead. The error message we see here is this dead window's ID, and nui complaining that it doesn't know where he should split from.

With relative = "editor", the resulting position is the same regardless of where it was spawned from, so he actually doesn't need to remember that previous window and we just want him to split from anywhere (so current win (0) is the easiest winid to specify).

This is the change I'm proposing over on the discussion I mentioned above.

Thesola10 commented 8 months ago

okay, then is there a way to discard the widget every time, so I can have cmdline_popup open over the currently focused window every time? Configuration-wise I mean.

Because from what I can gather, clearly this issue (and the invalid position with relative = "win") stems from reusing the same instance, as the window ID won't update unless unmounted.

pysan3 commented 8 months ago

I don't think your question belongs to this issue.

Theoretically it would be opts.cmdline_popup.relative = "win", but I doubt noice supports relative = "win" in the first place since I don't see it mentioned anywhere in the docs or code.

That'll be a separate feature request (or a bug report), to make all nui related views (split, popup and all others that uses these two underneath) to support relative = "win".

I'm not a maintainer of this project, so these are just my opinion.

pysan3 commented 8 months ago

@Thesola10

A theoretical fix would be to add

-- ...
-- `self` = NuiSplit or NuiPopup somewhere in the code.

self:layout_update({
  relative = {
    type = opts.<view>.relative,
    -- win = vim.api.nvim_get_current_win() -- UPDATE: INVALID
    winid = vim.api.nvim_get_current_win(), -- UPDATE: CORRECT
  }
})

-- ...
-- before self:mount() and self:show()

To force the window follow the current window. But this will add additional unnecessary computation for all users using relative = "editor" which is the default option, so I don't know if it is worth supporting relative = "win" option.

pysan3 commented 8 months ago

This should solve this issue, but I don't have the permission to close this issue here.

And please double check if this fix does indeed fix this issue. @ maintainers

nanozuki commented 8 months ago

I can still reproduce this issue, after I update the latest "nui.nvim". The line of error changed to utils/init.lua:29.

Should I change my config in noice?

pysan3 commented 8 months ago

You might be right. I'll look into it once again. Thanks for testing @nanozuki

nanozuki commented 8 months ago

@pysan3 I saw the PR has merged. I tried the latest version of "nui.nvim," and this bug is fixed! Thanks!