-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
[RDY] UI grid protocol revision: line based updates and semantic highlights #8221
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
Conversation
7912dd6
to
1b85857
Compare
Cleaned up the branch a little. Concretely, here are a sketch new and changed events in the revised grid protocol. Names and such are meant to be bikeshedded (and not 100% consistent with the branches either).
The following events are redundant with these additions and will not be sent in the new mode: |
Would this also be the right time to separate the "pen" from the "edit cursor", i.e. disambiguate |
@justinmk That would be done by never using The lazyredraw-y approach would be to keep track of largest changed region of each line, and transmit that only at |
Though then |
6b2a2db
to
7aed5c6
Compare
@justinmk This is now 85% done. In addition, |
e40d27f
to
158d351
Compare
Now stateful cursor should work. Though one place it works slightly different is with ext_cmdline, it now didn't move the cursor to the cmdline row in some cases, but I think this is an actual improvement. I'd try make sure this new behavior is consistent, even with redrawing screen in cmdline mode. At least |
Also the behavior of Messages are more tricky, as messages do treat the screen as a "dumb" terminal semantically. But I think we can live with that for now. (it is still not one line update per char, more like one per logical item) |
a185ad4
to
a3fbf63
Compare
src/nvim/api/vim.c
Outdated
@@ -1562,3 +1563,15 @@ Object nvim_get_proc(Integer pid, Error *err) | |||
#endif | |||
return rvobj; | |||
} | |||
|
|||
Array nvim__inspect(Integer row, Integer col) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
though this is "private", it could still benefit from a more intuitive name, maybe nvim__screen_inspect
or nvim__ui_inspect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently it is mostly for my own debugging, it will be made redundant or replaced with something better.
Scrolling is now stateless. Stateful scrolling regions was never actually used for any good, and more an arbitrary artefact of how the code in |
Add ext_newgrid and ext_hlstate extensions. These use predefined highlights and line-segment based updates, for efficiency and simplicity.. The ext_hlstate extension in addition allows semantic identification of builtin and syntax highlights. Reimplement the old char-based updates in the remote UI layer, for compatibility. For the moment, this is still the default. The bulitin TUI uses the new line-based protocol. cmdline uses curwin cursor position when ext_cmdline is active.
I am seeing a segfault together with Neomake due to this: 989b585#commitcomment-29838744 |
Yeah a null check is needed, tough one was needed even before this PR (for anyone clearing the cursorline syntax group). Or maybe it should just be refactored to per value already, there is nothing wrong with returning a struct. |
I think I'm seeing what @blueyed is seeing too. Here's my backtrace: https://gist.githubusercontent.com/aktau/8b97030d5385087ea5ee249d9082a2b5/raw/3d8595dbd8be585ef78019366880731419b085ab/backtrace.log. Which looks similar to @blueyed's backtrace, although mine is much (much) larger (because of what some script doest). I'm not sure which script is causing it. It's got something to do with split screens and signs/columns I think. |
#8789 might fix it. I will try to reproduce tomorrow, at least something similar ( |
Make sure cmdline updates will receive highlight specifications the same way as screen cells. This is controlled by the ext_newgrid option so nothing is changed by default (as screen cells are also not changed by default). This was already done for the cmdline itself in neovim#8221, this extends it to cmdline_block. Which currently doesn't store highlights, but the placeholder should be one that makes sense for future use.
Make sure cmdline updates will receive highlight specifications the same way as screen cells. This is controlled by the ext_newgrid option so nothing is changed by default (as screen cells are also not changed by default). This was already done for the cmdline itself in neovim#8221, this extends it to cmdline_block. Which currently doesn't store highlights, but the placeholder should be one that makes sense for future use.
The old code could lead to a memory error in the following situation: 0. The previous cursor position was row 50 since before, on a grid larger than 50 rows. 1. grid_resize changes the grid height to 40, and invalidly assumes the resize moved the physical cursor to row 0 2. Some event used a operation that could move the cursor (such as clear), and then reset the cursor to the "true" position row 50 (pointless after neovim#8221, but I forgot to remove it) 3. raw_line/cheap_to_print is invoked, and tries to inspect the grid at row 50 (memory error) 4. grid_cursor_goto would have been called at this point, and set a valid cursor position 0-39.
Besides the "visible" improvements, this release features numerous internal improvements to the UI/screen code and test infrastructure. Numerous patches were merged from Vim, which are not mentioned below. FEATURES: 07ad5d7 clipboard: Support custom VimL functions neovim#9304 725da1f neovim#9401 win/TUI: Improve terminal/console support 7a8dadb neovim#9077 startup: Use $XDG_CONFIG_DIRS/nvim/sysinit.vim if it exists feec926 neovim#9299 support <cmd> mapping in more places 0653ed6 neovim#9028 diff/highlight: Show underline for low-priority CursorLine bddcbbb signs: Add "numhl" argument neovim#9113 05f9c7c clipboard: support Wayland (neovim#9230) 14ae394 neovim#9052 TUI: add support for undercurl and underline color 4fa3492 neovim#9023 man.vim: soft (dynamic) wrap neovim#9023 API: 8b39e4e neovim#6920 API: implement object namespaces b1aaa0a API: Implement nvim_win_set_buf() neovim#9100 8de87c7 neovim#8180 API: virtual text annotations (nvim_buf_set_virtual_text) 2b9fc9a neovim#8660 API: add nvim_buf_is_loaded() API: buf_get_lines, buf_line_count handle unloaded buffers 88f77c2 API: nvim_buf_get_offset_for_line 94841e5 API/UI: neovim#8221 ext_newgrid, ext_hlstate (use line-based rather than char-based updates) UI b5cfac0 neovim#8806 TUI: use BCE again more often, (smoother resizes/scrolling) 77b5e9a neovim#9315 screen: add missing status redraw when redraw_later(CLEAR) was used 5f15788 TUI: clip invalid regions on resize (neovim#8779), fixes neovim#8774 c936ae0 neovim#9193 TUI: improvements for scrolling and clearing f204274 neovim#9143 UI: disable clearing almost everywhere f4b2b66 neovim#9079 TUI: always use safe cursor movement after resize d36afaf neovim#9211 ui_options: also send when starting or from OptionSet 67f80d4 TUI: Avoid reset_cursor_color in old VTE neovim#9191 e55ebae neovim#9021 don't erase screen on `:hi Normal` during startup c5790d9 neovim#8915 TUI: Hint wrapped lines to terminals. FIXES: 231de72 RPC: turn errors from async calls into notifications 907ad92 TUI: Restore terminal title via "title stacking" (neovim#9407) cb76a8a genappimage: Unset $ARGV0 at invocation neovim#9376 b48efd9 neovim#9347 TUI: FreeBSD: Improve support for BSD vt console c16529a TUI: Konsole 18.07.70 supports DECSCUSR (neovim#9364) aec096f os/lang: use the correct LC_NUMERIC also for OS X 5fee0be provider: improve error message (neovim#9344) 3c42d7a TUI: alacritty supports set_cursor_color neovim#9353 7bff9a5 TUI: Alacritty supports DECSCUSR (neovim#9048) 57acfce macOS: infer primary language if $LANG is empty neovim#9345 bc132ae runtime/syntax: Fix highlighting of augroup contents (neovim#9328) 715fdfe neovim#9297 VimL/confirm(): Show dialog even if :silent 799d9c3 clipboard: Prefer xclip (neovim#9302) 6dae777 provider/nodejs: fix npm,yarn detection 16bc1e9 neovim#9218 channel: avoid buffering output when only terminal and no callbacks are active 72fecad neovim#8804 Fix crash in lang_init() on macOS if lang_region = NULL d581398 ruby: detect rbenv shims for other versions (neovim#8733) e568ac7 neovim#9123 third-party/unibilium: Fix parsing of extended capability entries c4c74c3 jobstart(): Fix hang on non-executable cwd neovim#9204 1cf50cb provider/nodejs: Simultaneously query npm and yarn neovim#9054 6c496db undo: Fix infinite loop if undo_read_byte returns EOF neovim#2880 f8f8357 neovim#9034 'swapfile: always show dialog' CHANGES: c236e80 neovim#9024 --embed: wait for UI unless --headless 180b50d neovim#9248 python: 'neovim' module was renamed to 'pynvim' 2000b6a neovim#8589 VimL: Remove legacy aliases "v:errmsg", "v:shell_error", "v:this_session" deb18a0 defaults: background=dark neovim#2894 (neovim#9205) c1187d4 defaults: win: 'shellpipe' for cmd.exe (neovim#8827)
Besides the "visible" improvements, this release features numerous internal improvements to the UI/screen code and test infrastructure. Numerous patches were merged from Vim, which are not mentioned below. FEATURES: 07ad5d7 clipboard: Support custom VimL functions neovim#9304 725da1f neovim#9401 win/TUI: Improve terminal/console support 7a8dadb neovim#9077 startup: Use $XDG_CONFIG_DIRS/nvim/sysinit.vim if it exists feec926 neovim#9299 support <cmd> mapping in more places 0653ed6 neovim#9028 diff/highlight: Show underline for low-priority CursorLine bddcbbb signs: Add "numhl" argument neovim#9113 05f9c7c clipboard: support Wayland (neovim#9230) 14ae394 neovim#9052 TUI: add support for undercurl and underline color 4fa3492 neovim#9023 man.vim: soft (dynamic) wrap neovim#9023 API: 8b39e4e neovim#6920 API: implement object namespaces b1aaa0a API: Implement nvim_win_set_buf() neovim#9100 8de87c7 neovim#8180 API: virtual text annotations (nvim_buf_set_virtual_text) 2b9fc9a neovim#8660 API: add nvim_buf_is_loaded() API: buf_get_lines, buf_line_count handle unloaded buffers 88f77c2 API: nvim_buf_get_offset_for_line 94841e5 API/UI: neovim#8221 ext_newgrid, ext_hlstate (use line-based rather than char-based updates) UI b5cfac0 neovim#8806 TUI: use BCE again more often, (smoother resizes/scrolling) 77b5e9a neovim#9315 screen: add missing status redraw when redraw_later(CLEAR) was used 5f15788 TUI: clip invalid regions on resize (neovim#8779), fixes neovim#8774 c936ae0 neovim#9193 TUI: improvements for scrolling and clearing f204274 neovim#9143 UI: disable clearing almost everywhere f4b2b66 neovim#9079 TUI: always use safe cursor movement after resize d36afaf neovim#9211 ui_options: also send when starting or from OptionSet 67f80d4 TUI: Avoid reset_cursor_color in old VTE neovim#9191 e55ebae neovim#9021 don't erase screen on `:hi Normal` during startup c5790d9 neovim#8915 TUI: Hint wrapped lines to terminals. FIXES: 231de72 RPC: turn errors from async calls into notifications 907ad92 TUI: Restore terminal title via "title stacking" (neovim#9407) cb76a8a genappimage: Unset $ARGV0 at invocation neovim#9376 b48efd9 neovim#9347 TUI: FreeBSD: Improve support for BSD vt console c16529a TUI: Konsole 18.07.70 supports DECSCUSR (neovim#9364) aec096f os/lang: use the correct LC_NUMERIC also for OS X 5fee0be provider: improve error message (neovim#9344) 3c42d7a TUI: alacritty supports set_cursor_color neovim#9353 7bff9a5 TUI: Alacritty supports DECSCUSR (neovim#9048) 57acfce macOS: infer primary language if $LANG is empty neovim#9345 bc132ae runtime/syntax: Fix highlighting of augroup contents (neovim#9328) 715fdfe neovim#9297 VimL/confirm(): Show dialog even if :silent 799d9c3 clipboard: Prefer xclip (neovim#9302) 6dae777 provider/nodejs: fix npm,yarn detection 16bc1e9 neovim#9218 channel: avoid buffering output when only terminal and no callbacks are active 72fecad neovim#8804 Fix crash in lang_init() on macOS if lang_region = NULL d581398 ruby: detect rbenv shims for other versions (neovim#8733) e568ac7 neovim#9123 third-party/unibilium: Fix parsing of extended capability entries c4c74c3 jobstart(): Fix hang on non-executable cwd neovim#9204 1cf50cb provider/nodejs: Simultaneously query npm and yarn neovim#9054 6c496db undo: Fix infinite loop if undo_read_byte returns EOF neovim#2880 f8f8357 neovim#9034 'swapfile: always show dialog' CHANGES: c236e80 neovim#9024 --embed: wait for UI unless --headless 180b50d neovim#9248 python: 'neovim' module was renamed to 'pynvim' 2000b6a neovim#8589 VimL: Remove legacy aliases "v:errmsg", "v:shell_error", "v:this_session" deb18a0 defaults: background=dark neovim#2894 (neovim#9205) c1187d4 defaults: win: 'shellpipe' for cmd.exe (neovim#8827)
PR neovim#8221 took a short-cut when implemententing the tests: screen.lua would translate the linegrid highlight ids back into the old per-cell attribute description. Apart from cleaning up technichal dept, this enables to check both rgb and cterm colors in the same expect(), which previously was needlessly restricted to ext_hlstate tests only.
PR neovim#8221 took a short-cut when implementing the tests: screen.lua would translate the linegrid highlight ids back into the old per-cell attribute description. Apart from cleaning up technical dept, this enables to check both rgb and cterm colors in the same expect(), which previously was needlessly restricted to ext_hlstate tests only.
PR neovim#8221 took a short-cut when implementing the tests: screen.lua would translate the linegrid highlight ids back into the old per-cell attribute description. Apart from cleaning up technical debt, this enables to check both rgb and cterm colors in the same expect(), which previously was needlessly restricted to ext_hlstate tests only.
PR neovim#8221 took a short-cut when implementing the tests: screen.lua would translate the linegrid highlight ids back into the old per-cell attribute description. Apart from cleaning up technical debt, this enables to check both rgb and cterm colors in the same expect(), which previously was needlessly restricted to ext_hlstate tests only.
Some changes I think it will make sense to group together as a new revision of the "grid protocol". This refers mostly to the
ui-grid
section ofui.txt
, other parts of the ui protocol is not expected to change much.This branch is currently a mess, but I prefer presenting messy code than vaporwave... It will be cleaned up and test added etc after some discussion this is the direction we want to move in.
For backwards compatibility
api/ui.c
should translate back to the present protocol, unless clients opt-in. All internal structures such as bridge, TUI, the "compositor" in #6619 should use the new format.update_fg
,eol_clear
etc.ext_float
flag, will be the first)eol_clear
).rgb
flag thus has no effect.Number
will be identified as such, and UIs need not guess it from color values or positions. (current representation is very detailed, we might not want this level of detail, but I wanted to see how far it could go...)Normal
color shouldn't cause clearing the screen. The UI already has all necessary information to repaint the screen. Not a very impactful optimization as such, but this convention will be used later to support efficient highlighting overlays.It probably makes sense to mark this revision as experimental/unstable in the first PR that introduces it and use some unwieldy name as
ext_multigrid_experimental
, and recommend most external UIs to stick with the present format for now. A good reference point for stabilizing it is the same time as merging the floats PR #6619, as then there will be a strong reason for clients to actually use it. We should see this an opportunity to fix the issues/limitations of the UI protocol that has accumulated over time. Moving forward we only need to support the current format, and this new revision when we are happy with it.Depends on: #7992