feat(inccommand): allow redrawing during cmdpreview

[theofabilous]

WIP

Fix #28510 and #20463. Related PR: #27950. Likely fixes other open issues but this is still a WIP so I'll try to gather more related issues/PRs once this one gets closer to being ready for review.

Still incomplete and rough around the edges, but works in many cases.

TODO

• cmdline history navigation (<Up>/<Down>) doesn't always update/clear cmdpreview
• remove cmdpreview global, replace its uses with the cpinfo->enabled field (expose it via a non-static function or smth). might even want to remove that field and just check if cpinfo->cmdpreview_type != 0
• icm=split seems to work
• add documentation/docstrings. specifically, document why preview callbacks sometimes need to be re-invoked on redraws:
  • add docstring that explains why cmdpreview_may_refresh() needs to exist
  • add comment explaining why cmdpreview_may_refresh() needs to be called in update_screen()
  • quick reference in the meantime: redraw a specific floating window ignoring other pending screen updates #28510 (comment)
• problems with cursorline/cursorcolumn if a window is split (those options are disabled during cmdpreview and restored eventually, but if a window with the option disabled in this manner is split during preview, the new window inherits the modified options and will not be restored)
• can error messages be shown now that redraws are allowed? iirc there's an issue/PR related to this, need to find it and link it here
• there's a ghost buffer that isn't loaded but saved in cmdpreview_prepare that caused crashes when trying to restore its undo state/access its changedtick (i think it being not loaded implies it lacks some normal b: variables). temporarily fixed by checking buf->b_flags & BF_NEVERLOADED but i think this points to a logic error somewhere, needs investigation
  • could this be fixed by b_locked++? like what autocommands do to prevent unloading buffers when they might still b needed
• write tests. I haven't done any manual stress testing but there's gotta be like a bazillion edge cases
• the current implementation is naive. some possible optimizations (not exhaustive):
  • save cmdline_ptr/cmdparseinfo/exarg across redraws. idk if this is realistic because execute_cmd() modifies those but if it only modifies a few of their fields then saving/restoring them might be doable
  • currently the preview will always fully reexecute when any of the windows have w_redr_type > UPD_VALID (or if must_redraw >= UPD_NOT_VALID). maybe we could check if the windows really need redrawing by inspecting what lines were modified/decorated? this also kind of assumes the cmdpreview callbacks are somewhat 'pure'
  • cmdpreview_save()/restore() could be modified to account for refreshing. currently when we refresh the preview or a new key is typed, everything is restored and subsequently saved, which is wasteful. don't need to recompute everything, e.g. the undo sequences could be kept. viewstates could maybe also be kept, but not in all cases (e.g. if a resize occurs between previews, or if a window is open)
  • ...

NOTES/IDEAS (brainstorm)

• could this be partially (entirely?) implemented via decoration providers? pretty sure decor providers can't modify buffers but maybe adding a new provider type would be worth it
• now that command previews are a little more 'persistent', we could maybe implement something like context or provide information to the callback about where we're at. for example a preview callback that has a regex could cache the compiled object in the context and have it provided back as an argument on next call
• could modify/extend the :command-preview api so that implementors can declare how their preview callback modifies the editor state
  • e.g. could specify if a callback does not only show changes within the viewport (applies decorations/changes on the entire buffer), meaning we don't have to re-execute the preview callback on every redraw

[luukvbaal]

Thanks for working on this! It seems quite strange to me to cram this into the redraw routines. If there is a need to refresh the inccommand window(s) dynamically, perhaps it makes sense to add a dedicated API for that?

EDIT: Oops, meant to add this to change in drawscreen.c. Ah I remember now the changed viewport thing(scroll/resize), is this mainly for that? Could you add a docstring to cmdpreview_may_refresh() that explains why it is needed?

[theofabilous]

Ah I remember now the changed viewport thing(scroll/resize), is this mainly for that?

yes that's precisely why I added cmdpreview_may_refresh() (for anyone else looking at this discussion, see this comment #28510 (comment)). otherwise it wouldn't be needed.

Could you add a docstring to cmdpreview_may_refresh() that explains why it is needed?

definitely worthy of a docstring. mybad if things aren't properly documented yet, still a WIP but I'll add it to the todo list in the pr description