-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Fix #65 Error on closing outline when computer is running slow #66
base: main
Are you sure you want to change the base?
Conversation
Error on closing outline when computer is running slow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR.
I'm not sure why you're getting this issue, possibly a race condition. Your fix looks reasonable and for now I'm not able to troubleshoot/reproduce it so I'm fine with merging it as-is so long as it does fix the issue, just one comment.
@@ -9,13 +9,17 @@ local M = { | |||
---Clear all highlights in buffer | |||
---@param bufnr integer | |||
function M.clear_all_ns(bufnr) | |||
vim.api.nvim_buf_clear_namespace(bufnr, -1, 0, -1) | |||
if vim.api.nvim_buf_is_valid(bufnr) then | |||
pcall(function() vim.api.nvim_buf_clear_namespace(bufnr, -1, 0, -1) end) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this pcall
still needed after we've already checked the bufnr
is valid? You haven't added the pcall anywhere else. Do you still encounter this issue if we unwrap the clear_namespace
call from pcall here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Yes, as you correctly assumed it is due to race conditions. pcall
here is needed. I also found another issue with the Sidebar:update_cursor_pos
. Apparently when we quickly close the outline window right after pressing enter on the selected symbol, the Sidebar:destroy
is called which sets self.view to nil and then in the Sidebar:update_cursor_pos
we get an error due to this.
Error executing lua callback: .../site/pack/lazy/opt/outline.nvim/lua/outline/sidebar.lua:284: Invalid 'window': Expected Lua number
stack traceback:
[C]: in function 'nvim_win_set_cursor'
.../site/pack/lazy/opt/outline.nvim/lua/outline/sidebar.lua:284: in function 'update_cursor_pos'
.../site/pack/lazy/opt/outline.nvim/lua/outline/sidebar.lua:295: in function '_update_lines'
.../site/pack/lazy/opt/outline.nvim/lua/outline/sidebar.lua:707: in function '_highlight_current_item'
.../site/pack/lazy/opt/outline.nvim/lua/outline/sidebar.lua:237: in function <.../site/pack/lazy/opt/outline.nvim/lua/outline/sidebar.lua:236>
I think to tackle all these issue we should implement some sort of state machine which only destroys the objects when we are done with all the callbacks. What are your thoughts on this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, to reproduce the issue:
When the PC is under some load,
- Quickly open the symbol sidebar select a symbol (which is known to be bit lower in the file)
- Press enter on the selected symbol and quickly close the outline sidebar.
I see a lot of errors popping up when I do this. When this happens, outline auto-commands are left in some inconsistent state. Therefore on every cursor move similar errors keeps popping up. I then have to re-open the neovim to fix it.
This PR resolves issue on closing outline when computer is running slow.