neovide / neovide

No Nonsense Neovim Client in Rust
https://neovide.dev
MIT License
12.71k stars 515 forks source link

Neovide not playing nice with noice #1751

Closed StephanRoemer closed 1 year ago

StephanRoemer commented 1 year ago

Neovide crashes when using noice.nvim, when calling the command line. Any way to fix this? https://github.com/folke/noice.nvim/issues/17

shaunsingh commented 1 year ago

From what I understand noice also uses multigrid which interferes with our --multigrid flag, I believe it should work if you disable multigrid, we'll check if we can get the --multigrid flag to play well with noice

folke commented 1 year ago

Noice does not enable multigrid, but it does enable ext_cmdline, ext_messages and ext_popupmenu.

This means that Neovide will possible also receive events it didn't subscribe to (that's how this works in Neovim). It should just drop these events.

shaunsingh commented 1 year ago

Thanks, we'll drop those events then

StephanRoemer commented 1 year ago

Amazing, thanks guys for the exchange and a possible fix!

Ciel-MC commented 1 year ago

After adding noice, NeoVide seems to crash whenever I delete more than 2 lines (i.e. d2k) and it doesn't delete the entire buffer. The crash has the message:

ERROR [neovide::error_handling] Could not parse event from neovim: invalid string format []
Neovide panicked with the message 'Could not parse event from neovim: invalid string format []'. (File: src/error_handling.rs; Line: 5, Column: 5)
This is a bug and we would love for it to be reported to https://github.com/neovide/neovide/issues

Backtrace saved to neovide_backtraces.log!
ERROR [neovide::bridge] Error joining IO loop: 'task 33 panicked'
ryo33 commented 1 year ago

I'm working on fixing this these days.

ryo33 commented 1 year ago

Really weird redraw event. I'm reading the Neovim code

[
  "msg_show",
  [
    [],
    [
      false
    ],
    [
      "flush",
      []
    ]
  ]
]
ryo33 commented 1 year ago

Please use this patch to debugprint the event. https://github.com/ryo33/neovide/commit/1c0ef4dc1d1d2f7c319c62247569277cea9f0dd1

Ciel-MC commented 1 year ago

Please use this patch to debugprint the event. https://github.com/ryo33/neovide/commit/1c0ef4dc1d1d2f7c319c62247569277cea9f0dd1

Are you saying I use this and trigger the crash for more info?

Ciel-MC commented 1 year ago

Please use this patch to debugprint the event. ryo33@1c0ef4d

Here's the backtrace: neovide_backtraces.log

Ciel-MC commented 1 year ago

@ryo33 I cloned the repo, checked out noice and built the binary and got the backtrace above, hopefully I did it right.

ryo33 commented 1 year ago

@Ciel-MC Thanks! The printed event is almost the same as mine. I'm finding where the event comes from.

folke commented 1 year ago

@ryo33 Not sure if this helps, but since Noice is running in TUI, I sometimes need to trigger redraw events while handling ui messages. This is needed during when the ui is normally bocking, since I need to render the cmdline etc.

When doing redraws, Neovim will always sends redraw events and resend all the current msg_show events as well which is rather annoying. I simply drop duplicate events on my end, but maybe that causes issues in other attached GUIs

ryo33 commented 1 year ago

I'm getting the idea that the cause of the problem is perhaps not neovide's problem, so I'm wondering where to continue this investigation. I could go back to https://github.com/folke/noice.nvim/issues/17 or go elsewhere. Neovide can ignore (and log) the ill-formed redraw events for now. I'll open a pull request for it. Then this issue could be closed. I suspect that one of the following is true:

  1. Noice or other plugin causes ill-formed redraw events through notify or messages (especially messages) (but how?)
  2. Neovim has a race condition to send redraw events to multiple subscribers.
  3. The implementation of RPC or MessagePack that neovide uses are broken.

I'll continue the investigation in the above order but It could take longer because I just started learning the Neovim internals and its API's.

folke commented 1 year ago

Since Noice is running inside the lua main thread, there's nothing much I could do, so invalid messages are probably due to bugs in Neovim. Maybe open an issue there?

Apart from that, noice does seem to work in other GUIs that I tested.

For 2., I'm pretty sure messages are sent to each subscriber one by one, but I'm not sure how that would work if one of the subscribers (Noice in this case) triggers a new event while still handling the other events.

Opening a Neovim issue with your findings probbaly makes most sense, so we can have @bfredl's input as well

ryo33 commented 1 year ago

I have one thing to check briefly. After then, I'll open a Neovim issue.

ryo33 commented 1 year ago

Thanks to @folke, I opened https://github.com/neovim/neovim/issues/22344.

cifvts commented 1 year ago

I have a similar problem, as soon as I try to use the command line having noice installed, neovide crash:

2023-07-06 08:22:45 - Neovide panicked with the message 'Could not parse event from neovim: invalid event format for event 'cmdline_show' - [Array([Integer(PosInt(0))]), String(Utf8String { s: Ok("\0") }), String(Utf8String { s: Ok("") }), Integer(PosInt(0)), Integer(PosInt(1)), Array([String(Utf8String { s: Ok("flush") }), Array([])])] - invalid array format 0'. (File: src/error_handling.rs; Line: 5, Column: 5)
folke commented 1 year ago

I've just pushed a new release of Noice where I'm pretty confident I fixed the incompatibility. Should also work fine now with --multigrid

StephanRoemer commented 1 year ago

Wow, amazing Folke, thank you! Giving it a spin for the next few days!

fredizzimo commented 1 year ago

Thank you @folke. I think I can close this issue now.

StephanRoemer commented 1 year ago

Yep, no issues so far! The only thing I still get is the warning that I'm using multigrid which isn't compatible with noice. I guess this can be removed?

folke commented 1 year ago

I think I actually already removed that warning no?

StephanRoemer commented 1 year ago

The general warning that there might be a problem with noice and neovide was removed, yes. But there is a second one:

image

folke commented 1 year ago

right, will remove that one too

folke commented 1 year ago

done!

StephanRoemer commented 1 year ago

You are blazing fast, thank you! Installed and works :)