folke / trouble.nvim

🚦 A pretty diagnostics, references, telescope results, quickfix and location list to help you solve all the trouble your code is causing.
Apache License 2.0
5.11k stars 173 forks source link

The async logic does not respect the `api.open { new = false }` calls #492

Closed NGPONG closed 3 weeks ago

NGPONG commented 3 weeks ago

Did you check docs and existing issues?

Neovim version (nvim -v)

0.11

Operating system/version

ubuntu22.04

Describe the bug

Hello, after several days of deep usage, I feel that although the V3 version has undergone significant restructuring compared to the previous version, it is still very much worth updating. Thank you very much for your efforts @folke.

During usage, I found that sources like lsp_xxx do not respect the decision of api.open({ new = false }). This issue is particularly important for some slower LSPs. It is very difficult to clearly and completely describe an issue, so I recorded a video to demonstrate the problem I encountered (I will add any more info in the comments if needed). At the beginning of the video, I show that I have bound api.open to the de key and set new = false when calling api.open. Then, immediately after I first entered the editor, I quickly pressed the de key four times in succession. There was no response at this time, which I understand is because the LSP was too slow, so I started to wait. After a few seconds, three trouble windows popped up in succession, along with an additional warning message with no source found.

https://github.com/folke/trouble.nvim/assets/38415769/e9651926-e491-4a1f-9fbd-c9b4063a5eae

I'm not certain if this is the intended behavior, as my in-depth research revealed that the code only considers the open = true scenario. When open is initially called, a temporary view is stored in trouble.view._views until any data (potentially fetched asynchronously) fills this view and it gets displayed (if open_no_results = true). In my opinion, it’s necessary to include this intermediate state view within the scope of api.open({ new = false }). Should clarify that this is just my personal opinion, and I will respect any perspectives on your intended design.

https://github.com/folke/trouble.nvim/blob/806c50491078b66daf13c408042f2e74da46d0ff/lua/trouble/api.lua#L47-L49

https://github.com/folke/trouble.nvim/blob/806c50491078b66daf13c408042f2e74da46d0ff/lua/trouble/api.lua#L23

Anyway, at least until this is properly "fixed", I am compensating for the logic of api.open({ new = false }) on the user side by hijacking the trouble.view.refresh function to broadcast whether the specified trouble mode is open. This is undoubtedly a very ugly and hacky solution.

Steps To Reproduce

see above

Expected Behavior

see above

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/trouble.nvim",
  -- add any other plugins here
}
require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

vim.cmd.colorscheme("tokyonight")
-- add anything else here
folke commented 3 weeks ago

Should be fixed now. Thank you for reporting!