neovim
`SafeState` or something similar could be really useful for `vim.ui_attach`
Problem
I just read up about SafeState and something like this (SafeUIState) could be really useful for vim.ui_attach when triggered:
- right before
getchar - right before
cmdpreview_may_show - right before
redraw
Especially the second one would be great, since the current work-around for this in Noice is not ideal.
When cmdpreview is active, redraws are ignored, but with vim.ui_attach this is not ideal.
User enters :%s/foo/bar. The last r will correctly trigger ui_attach and will then show the correct preview.
However, it's not possible for the cmdline ui to redraw itself (since redraws are disabled when cmdpreview is active).
My current work-around is to use nvim_feedkeys with <space><bs>.
Related issues
- [ ] https://github.com/neovim/neovim/issues/20311
- [ ] https://github.com/neovim/neovim/issues/20463
@luukvbaal maybe you have some ideas?
With all your recent changes, I was able to finally just queue the messages received in the vim.ui_attach callback and no longer need to do screen updates or redraws during.
The last r will send an ext_cmdline message with the new cmdline.
So in noice, I have the correct cmdline state, I just can't render it without my work-around
I meant that it (or a similar event) could be useful to allow ui updates at some points where it's currently not possible.
To be fair, I think I just found a better work-around specifically for cmdpreview, since CmdlineChanged is triggered right before that.
I'm not sure I understand why a dedicated event is needed, won't flushing the desired state in the appropriate places be sufficient; like in #27950?
And for cmdpreview in particular, removing the redraw restriction is the way forward: #28510.
right before redraw
I was able to finally just queue the messages received in the
vim.ui_attachcallback and no longer need to do screen updates orredrawsduring.
Care to elaborate on these? I deliberately did not look at noice's implementation and it's workarounds when working on the default ext UI(which I have postponed to work on for the time being) and instead aimed to resolve issues in the C core as I ran into them.
I'm not sure what #27950 achieves regarding my bug report?
And for cmdpreview in particular, removing the redraw restriction is the way forward: https://github.com/neovim/neovim/issues/28510.
That would be great
Care to elaborate on these?
Previously I queued messages in the ui_attach callback, and only updated the views when in a blocking event. Like right before getchar, or when the cmdline is active. This was needed, since otherwise it can happen that Neovim blocks the main loop somewhere waiting for input and I wasn't able to render views. Other view updates happen async in batches in the main loop.
With some of your recent updates, a lot of those blocking view updates I did inside the ui_attach callback started giving E565 errors, so I could no longer properly update views when needed.
Now I just queue messages and never try updating the UI in the callback (and never trigger redraws either during).
What would I need to do to get my command line floating window to redraw properly after receiving an ext_cmdline message using #27950?
fyi, how I currently solve the issue with cmdpreview:
- in a
CmdlineChangedcallback I do: - with
ffisetcmdpreview = false - trigger a
redraw - update the view (update/create buffers/windows)
- possibly trigger another redraw when needed
I'm not sure what #27950 achieves regarding my bug report?
This bug report or #20463? Like I mentioned in the last comment in that PR I think the change is sufficient to redraw updated cmdline state, did you try it out and experience otherwise?
Perhaps it's not obvious but cmdpreview is temporarily set to false on each key press, so flushing before entering "cmdpreview" state again like in that PR indeed seems sufficient to "achieve something" regarding #20463.
Yes, I meant #20463 indeed.
I tried a couple of things with your PR:
- in the
vim.ui_attachcallback, when receiving acmdlinemessage, update the ui right away:- result was that the cmdline in my floating window was always one char too late as soon as you hit the second
/in%s/foo/bar(same as without your PR) - I also had a segfault, but I assume it's simply not a good idea to update the ui from the callback?
- result was that the cmdline in my floating window was always one char too late as soon as you hit the second
To be clear, I am getting all the cmdline messages in time, but I'm just not able to create/update views due to redraw not working when cmdpreview = false
in the vim.ui_attach callback, when receiving a cmdline message, update the ui right away:
This is what I've been doing and I think it should be allowed.
This is what I've been doing and I think it should be allowed
That would be really great, but at least before all your recent work, this segfaulted Neovim in a lot of different ways.
With your PR, I get a segfault after entering :%s if I do a create/update windows in the ext_cmdline callback.
Without your PR, it still doesn't work, but doesn't segfault.
I don't know if this is helpful but before #25629 I was able to render the cmdline having cmdpreview active without issues and no workaround, by just calling redraw in the callback after setting the line in a float. On master I do observe the lagging character issue and I can't update the float window at all after the second "/".
When I created Noice and hit some of the issues with the implementation of vim.ui_attach, there were talks of maybe changing it so that callbacks would no longer be allowed to do redraws and where a new low-level API would be created to update/create windows.
Based on all the recent changes and new issues I hit with Noice, I mistakenly thought that was all in preparation of the above.
If I understand correctly now, the goal is to just make it all work without any new low-level API and where the lua callback is allowed to fully alter views during.
I also just tested #27950 again and it works great now. For Noice, this was probably the biggest issue with vim.ui_attach till now. Looking forward to having this merged :)
Closing this issue, since it no longer applies.
If I understand correctly now, the goal is to just make it all work without any new low-level API and where the lua callback is allowed to fully alter views during.
I mean this has been my goal, and so far I had not gotten the impression that there is something fundamentally wrong with that approach. If that goes against previous discussions/conclusions that is solely because I was oblivious to them. Is there a reference to those discussions?
Not really. Was mostly on matrix and a long time ago.
A lot of the underlying issues with redraw have been fixed since I released Noice. Things that also triggered segfaults unrelated to Noice.
To be clear, Noice only triggers redraws or updates/creates windows/buffers during the callback in very few cases, when I know for sure or it's possible for Neovim to block on input. Most of the time, a redraw is not needed and it was mostly those redraws that triggered segfaults. Noice also detects recursive redraws (or recursive vim.ui_attach callbacks).
The biggest new issue that popped up over the last couple of weeks were those E565 errors when I try to render stuff during the callback in those special cases. It seems that error is because of textlock and triggered by NEovim treesitter code that does a redraw that triggers vim.ui_attach events somehow. A similar situation happens with the inlay hints code.
I currently just queue updates in the callback during textlock. To check if textlock is active I have to use ffi. There's no builtin way right?
Sorry forgot to reply here @folke; short answer no I don't think there is a builtin way to check if textlock is active(other than pcall() -> check error).
I do think we should try to avoid the issue you describe but I'm not sure what's the best way to resolve it.
The issue stems from nvim__redraw() calling ui_flush() in the decor provider callback which sets textlock. Maybe we can postpone flusing (to Lua callbacks?) when textlock is active.
postponing when active is probably a good approach here. I currenty check if textlock is active with ffi and just queue the message, so receiving it later when textlock is not active would be better indeed