j-hui / fidget.nvim

💫 Extensible UI for Neovim notifications and LSP progress messages.
MIT License
1.97k stars 57 forks source link

E523 when using clever-f #68

Closed rockyzhang24 closed 2 years ago

rockyzhang24 commented 2 years ago

Hello, I am using clever-f and I got this issue

image

It happens during the initialization of LSP server and the issue is gone after that. To reproduce, open any lua file (lua server needs longer time to initialize, so it's easier to reproduce), then before the server initialization finish, press f plus some char, then errors pop up.

Thank you very much.

wookayin commented 2 years ago

@rockyzhang24 it looks like your figet.nvim is outdated, because L216 does not read like vim.schedule. Are you sure it is on the latest version? Which commit/version are you on? and neovim versions?

rockyzhang24 commented 2 years ago

Thanks. No, it is the latest fidget. I update my plugins everyday. My neovim is the latest nightly (NVIM v0.7.0-dev+1439-gda31e953b)

rockyzhang24 commented 2 years ago

Btw, the error info is not the same for different filetype.

In a go file, I got

image
wookayin commented 2 years ago

Sorry, I misread the stacktrace. You are right -- I can also reproduce the bug.

Indeed vim.schedule is from L386 (the bottommost line) and L216 reads

api.nvim_win_set_option(self.winid, "winblend", options.window.blend)

The problem is that the "deferred" fidget:fmt() call can still be executed by the event loop during the execution of getchar() (see https://github.com/vim/vim/issues/2811 and https://github.com/neovim/neovim/issues/16278). I am looking into why.

rockyzhang24 commented 2 years ago

@wookayin Thank you so much. I am not sure whether or not this issue is caused by clever-f. I already opened an issue there (https://github.com/rhysd/clever-f.vim/issues/65).

wookayin commented 2 years ago

Also worth checking out a comment by @kevinhwang91 (https://www.reddit.com/r/neovim/comments/tzz5v7/share_a_ff_highlight_plugin/i42cy0d/) -- You might want to switch to another similar plugin as a workaround. I use a similar plugin that maps f, namely quick-scope. Or @kevinhwang91's nvim-fFHighlight might be another alternative.

It's a bug that clever-f needs to fix, or neovim might be responsible for scheduling as vim.schedule is supposed to defer the execution when it is available.

rockyzhang24 commented 2 years ago

Okay. Thank you for your suggestions. I know this newborn plugin nvim-fFHighlight but it does not support tT highlight. For quick-scope, I tried it shortly before but I am worried about its performance as it analyzes each word in a line, not just the char I want to jump to.

wookayin commented 2 years ago

So the problem is while UI is blocking during the execution of getchar(), neovim executes all the events (and so is render_fidgets(), etc.). I tried nvim_get_mode() but it gives blocking = False, mode = "n" (normal mode). Operator pending modes are other such cases, but getchar() is a tricky case.

We will need to find a way to tell whether neovim is currently waiting for some input or blocking, or it is safe to do some buffer manipulation (E523 error).

rockyzhang24 commented 2 years ago

@wookayin Thank you so much for your investigation. Both fidget and clever-f are my must-have. Hope this can be fixed soon. Thanks again.

wookayin commented 2 years ago

@rockyzhang24 Try out PR #69.

rockyzhang24 commented 2 years ago

@wookayin Thank you. I already tried and no errors any more. Just a quick question, does ignoring this E523 error have any side effects? Fidget will work as normal, right?

wookayin commented 2 years ago

Yes, it is perfectly fine. Because E523 errors happens due to a 'textlock' (see :h textlock) and it only pertains to drawing of the fidget windows. Any internal states and data will remain intact. As you could test out, the fidget windows will simply won't be updated nor be spinning while it is in the operator-pending mode.

rockyzhang24 commented 2 years ago

@wookayin Okay, thank you for explaining it in detail. I just tested it, and I found another glitch.

To reproduce it:

  1. Open a lua file (because lua LSP takes longer to initialize)
  2. Press d followed by f, now we are entering Operator Pending mode
  3. Wait here without giving a char to f. We can see that fidget progress update and spinning pause as expect. Keep waiting, and we can see fidget left some ghost chars even though it has finished in background.
Screen Shot 2022-04-12 at 00 37 57
  1. Then if we press <ESC> immediately after the ghost chars appear. The ghost will still be there and won't be removed any more. However, if we press <ESC> after a while, the error shows again
image

Thank you!

wookayin commented 2 years ago

@rockyzhang24 Good catch. Can you try again in the updated PR?

kevinhwang91 commented 2 years ago

I tried nvim_get_mode() but it gives c = False, mode = "n" (normal mode).

Because nvim_get_mode is only for RPC API, blocking = false is always for Lua.

I don't think it's appropriate for fidget to fix this bug. There're maybe a lot of other async functions to encounter this issue. From the upstream is too hard to fix. From the clever-f avoid using <expr> must be refactored the cost is too expensive.

wookayin commented 2 years ago

@kevinhwang91 Thanks for the additional input. Yes, I think neovim should provide some API to tell whether a 'lock' is held, and this is just a workaround. But this error happens quite aggressively with clever-f as well as other similar plugins (e.g., yours and quickscope) which we don't have much control over.

rockyzhang24 commented 2 years ago

@wookayin Tried but still see the issue: ghost chars and errors. Thank you.

kevinhwang91 commented 2 years ago

@kevinhwang91 Thanks for the additional input. Yes, I think neovim should provide some API to tell whether a 'lock' is held, and this is just a workaround. But this error happens quite aggressively with clever-f as well as other similar plugins (e.g., yours and quickscope) which we don't have much control over.

I'm not sure whether quickscope has remaped with <expr>, this issue only happens for the function map with <expr> + getchar + other async function. For fFHighlight, I use FFI to hack ,and no need to care about the cursor issue caused by getchar.

wookayin commented 2 years ago

Again, the problem is that a function deferred by vim.schedule(..) will still be executed while an operator is pending. We could safely skipping rendering, but when it comes to kill or closing, this workaround doesn't seem to work because it will lead to resource leakage or weird race condition. Fidget cannot easily tell when it is really available, despite the use of vim.schedule. This is a problem of not only fidget, but any other plugins that uses asynchronous callbacks (and do some buffer/window management).

I think this requires an upstream fix, so no good solution for now. For the time being, I think #69 can be still merged to avoid most of E523 errors, and we can later improve the remaining case of the E523 error when fidget is trying to close.

@j-hui Any thoughts?

wookayin commented 2 years ago

@rockyzhang24 I rewrote the logic (#69, ef09f43) and we can ignore the error when fidgets would be closed as well. This will make the annoying error not happen any more. However it cannot 100% eliminate "ghost chars" as I explained earlier; but we have some workaround like :FidgetClose so this should be usable enough.

rockyzhang24 commented 2 years ago

@wookayin Great. Thank you so much for digging deeper into this issue and trying to come up with solutions. It looks like we have to wait for the upstream fix to make this issue solved completely. Could you please open an issue upstream? If I do, I'm afraid I cannot describe it clearly.