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
redraw a specific floating window ignoring other pending screen updates #28510
Comments
This sounds nice but there is no controlling when
What makes you say this? Have you tried it out? The "before doing 'cmdpreview'" happens on each key press while in cmdline mode. So a cmdline window can be updated, then the ephemeral 'cmdpreview' state is pushed to the screen and stays there until the next key press. I don't think there is a need for "re-updating the cmdline afterwards". |
This comment was marked as resolved.
This comment was marked as resolved.
Anecdotally I was toying with a command line highlighting plugin a while back and ran into all of these issues. Trying to open a floating window in the command line area caused a bunch of problems similar to those tracked in folke/noice.nvim#6. I ended up doing a symbol name lookup with I mention this because it's kind of the implementation that I had mind: a function similar to |
We had a lot of such ad-hoc mechanims in the past, which had be painfully kept in sync and maintained along with any improvement to We could add a The direction we have already chosen and should continue on is to make any fragile state which not must be overdrawn by accident into real stateful decorations which in fact can be safely redrawn, like all other stuff on the screen. thus just calling |
It could be that this works for the external cmdline case and may be solved by adding a higher priority redraw type, to be updated in isolation by
I think that may be overestimated; could even be done in Lua. And as bfredl mentions, is what we should aim to do. |
Shall we use this issue to track that? |
Ah, I see what you mean. Again, I can anecdotally say that my approach was working as intended, but that's because I wasn't using Regardless, I see the points everyone is making and completely agree. Getting rid of ephemeral screen state is a much more "treat the root cause"-type of solution than the approach I suggested.
Agreed. I suppose this approach would solve many more issues (in addition to those I mentioned), like the fact that cmdpreview is cleared when resizing the editor or when |
I would be willing to work on this. I think the priority is addressing this for cmdpreview specifically, I don't know if people care about the visual selection thing. Anyway I can imagine a pretty simple solution for the visual highlight on the plugin side if there isn't a sufficient need/desire to merge this into master. The only method that I can think of that maintains the cmdpreview state across redraws would be to re-invoke the associated preview callback when appropriate. Is this the proper approach? My first thought was to store the window/buffer changes applied by the preview callback and reapply them when needed. But this assumes the preview callback's changes are independent of things like the visible region in the buffer, making it impossible for cmdpreview callbacks to, say, lazily only apply changes in the visible viewport (which, if I'm not mistaken, is done in |
No the point is to get rid of artificially maintaining cmdpreview state. What we want to do is instead of |
That's also something I considered, and is definitely the method that is most aligned with the "getting rid of fragile state" idea. However wouldn't it be detrimental to performance? The benefit of having cmdpreview handled differently and it being intrinsically tied to redrawing is that it allows lazily showing changes based on what's visible. Like I mentioned for I'm not against this approach though. Definitely seems like it'd be easier to implement and would remove a lot of edge cases. Maybe explicit support for handling these kinds of redraws to a preview buffer could be added, or like another user-supplied callback that can decide if the preview callback should be invoked again. |
I don't see a reason why the cmdpreview buffer in this approach can't still contain only the visible lines. The point is just to keep that window around until cmdpreview ends. But yes, it looks like manipulating the buffer content in the cmdpreview window should happen more frequently, at which point the window dimensions could also be updated. Resizing Nvim currently also removes the cmdpreview window until the next keypress. |
It could, but a preview callback applying changes lazily like this would need to be re-invoked when the visible lines change. The editor can be resized at any time. E.g. suppose I have a buffer with 100 lines of the text EDIT: didn't see you updated your comment before I posted this, sorry. So you see my point? For any redraw that changes what's actually visible on the affected buffer/window, the cmdpreview callback would either have to be re-invoked to support lazy previews, or support for such optimizations would have to be dropped. If the former is the chosen path, then in the worst case we'd have to invoke the callback on every redraw, which is essentially what I was proposing. |
Well yes but like I said the resize issue is separate from replacing the ephemeral window with a persistent one, and is already present in the status quo. If anything a persistent window should make resolving that easier. |
Right, I don't disagree. I guess I misunderstood your comment then. The aspect of the approach I'm discussing is agnostic to window persistence. Obv for Although I don't see why a new window would have to be opened for |
I don't quite follow what you mean with "lazily applying the substitution preview". I think the command should (and currently does) just execute on the entire preview buffer. What is redrawn is probably cheap in comparison and I don't think keeping the cmdpreview state around will be detrimental to performance but only improve it.
I don't see why "the cmdpreview callback(
Yes that sounds right, sorry for the confusion. Floating windows and moving the implementation to Lua were mentioned recently when discussing this on Matrix but I don't think it's actually relevant. If you're willing to work on this I would suggest just trying to do |
Sorry, I was using that term pretty loosely. I specifically meant either the user-supplied lua preview callback or the one associated to a builtin command (
It doesn't. The Lines 3517 to 3527 in 435dee7
Hopefully that explains why I was saying the cmdpreview callback would potentially have to be re-invoked on every redraw (assuming every redraw is a resize or scroll). I think the above snippet makes it clear that resizing/scrolling a window that is showing a
Cool, that's what I was thinking. |
I see, my bad(not actually familiar with the cmdpreview code, I just read some of the code in ex_getln.c). I guess that breaks down with wrapped lines when the command changes the line height.
Not sure why we're assuming every redraw is a resize or scroll(why would a scroll happen during cmdpreview?) but yes, in that case the cmdpreview call back would have to be re-invoked to sync the viewport (perhaps also recursively when the command changes the line height to account for a new botline/topline). But is that a reasonable performance concern? I don't think scrolling or resizing happens frequently during cmdpreview. I would consider that a reasonable/necessary fix for a bug that's also present with the current ephemeral preview. |
I was assuming the "worst" case. But yes that is an unrealistic assumption, I was just using that to make my point about the necessity to re-invoke the preview callback.
No its not, I agree. My concern with respect to performance wasn't related to redrawing. It's no longer a concern though, I brought it up earlier when you responded (in #28510 (comment)) to my initial implementation proposal but I think I made my point since then. I meant that your response, as I understood it, forced command-preview callbacks to draw on the entire buffer because e.g. the callback would not be re-invoked between redraws even if e.g. the viewport changes. So the perf concern was more about preview implementors having to drop the assumption they could previously make that they can lazily only show changes on visible lines (see the |
Problem
There is no way to redraw and update a specific window without updating other windows that "need redrawing". Individual windows can be marked for redraw, but updates are only ever flushed to the UI via
update_screen()
which will go through all windows with pending updates. In most cases this is either necessary (e.g. for splits, resizing, etc) or it's not a problem.However, there is much work being done in PR #27855 (tracking issue #27811) to externalize messages and the command line for the TUI via floating windows (also noice.nvim). The current high-level redrawing logic for the command line/message grid is orthogonal to that of windows, which has the following advantages:
:'<,'>[command]...
this is great because you can see what the ex-cmd is (presumably) operating on. Any call to:redraw
, opening a new float, etc. will clear the visual selection highlight.inccommand
. command line can be redrawn independently, without being blocked (#9777, #9783, #20463) or clearing the preview.Neither can be achieved via regular window redrawing, so if the cmdline/msg grid is externalized with a floating window we lose both of these benefits (I think #27950 somewhat addresses point (2) but still wouldn't allow properly re-updating the cmdline afterwards). Afaik this also applies to the native popup wildmenu which has its own seperate redrawing codepath.
In brief, externalized ui component providers in the TUI (specifically
ext_cmdline
andext_messages
, also likelyext_popumenu
forwildmenu
) have to use hacks and workarounds to maintain ephemeral screen state (like visual selection in cmdline, or cmdpreview) because there is no publicly exposed mechanism to redraw a window without screwing up the dirty screen state (which should be kept dirty). Some PRs have addressed this problem but in an ad-hoc/hacky manner.Expected behavior
Either:
The text was updated successfully, but these errors were encountered: