diff/highlight: diminish low-priority CursorLine #9028
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a9a320f to
44886ab
Compare
44886ab to
2ff5183
Compare
src/nvim/highlight.c
Outdated
| int hl_get_underline(void) | ||
| { | ||
| int hl_attrs = HL_UNDERLINE; | ||
| return hl_get_term_attr(&(HlAttrs) { |
There was a problem hiding this comment.
using kHlTerminal here. Not sure if it matters.
There was a problem hiding this comment.
should be kHlUI if the trick is used, but I think it can be avoided entirely (see other comment).
f9b851d to
b62548e
Compare
src/nvim/screen.c
Outdated
| // Overlay CursorLine onto diff-mode highlight. | ||
| if (wp->w_p_cul && lnum == wp->w_cursor.lnum) { | ||
| line_attr = 0 != line_attr_lowprio // Low-priority CursorLine | ||
| ? hl_combine_attr(line_attr, hl_get_underline()) |
There was a problem hiding this comment.
Why not hl_combine_attr(win_hl_attr(wp, HLF_CUL), line_attr) which would work for any attribute and not only underline ?
There was a problem hiding this comment.
@bfredl Thanks. That effectively hides the CursorLine with the use-case given above, but maybe we could do that + add underline. The idea is to have something that's visible without being unreadable.
There was a problem hiding this comment.
@justinmk How could hl_combine_attr(win_hl_attr(wp, HLF_CUL), line_attr) "effectively hide underline" in any way that hl_combine_attr(line_attr, hl_get_underline()) doesn't? The former will include any attribute that CursorLine contains.
There was a problem hiding this comment.
If the purpose rather is to actively special case CursorLine on diff lines, then CursorLineDiff group would be the proper and configurable option (which even can default to underline, for this behavior by default) , rather than adding ad-hoc logic for hardcoding an implicit gui=underline group, which is not done anywhere else. What is the motivation to add code to prohibit users to configure this particular case?
There was a problem hiding this comment.
I mean visually, the diff highlight still dominates. Try the command given in the top description.
There was a problem hiding this comment.
Again, that depends on the attributes used. If reverse is used, both high and low priority already works, but with this change diff lines are needlessly different.
There was a problem hiding this comment.
Yes, it depends on the attributes.
The point of this PR is to avoid a degenerate case for something like hi CursorLine ctermbg=grey. With hi DiffAdd ctermfg=black ctermbg=red, that's hard to read.
There was a problem hiding this comment.
Yes, all I'm saying is if the corner case is a result of user configuration, the override itself should be configurable (compare CursorLineNr). Otherwise it will just make some configurations better by making others worse..
There was a problem hiding this comment.
I will add CursorLineDiff if anyone complains.
But with the latest push AFAIK it won't make anything worse except if someone considers it worse to have an underline.
It's also overridable by setting foreground on CursorLine (so it's not "low priority").
There was a problem hiding this comment.
But with the latest push AFAIK it won't make anything worse except if someone considers it worse to have an underline. It's also overridable by setting foreground on CursorLine (so it's not "low priority").
@justinmk All other screen attributes added by vim/nvim are configurable by the user, moving towards a mixture of user configurable attributes and hard-coded attributes is a step backwards, and an unexpected one.
If it counts for anything, it breaks my setup (i don't use CursorLine always, but when I do), by not only unexpected underline but also unreadable text on diff lines. And yes it is "fixable" by setting guifg, but why should I or any user need to use guifg if we only want guibg (or some non-ul attribute)? Why should anyone be forced to use guifg to take control of bg and attributes, instead of having the later being controlled by guibg= and gui= like in all other UI highlights in nvim?
closes neovim#9274 ref neovim#9028 If stdin closed then read_error_exit calls preserve_exit. Handling SIGHUP during preserve_exit would cause a premature teardown, and conflicts with e.g. ui_bridge_stop which waits for TUI to teardown. Vim ignores SIGHUP in its prepare_to_exit and getout_preserve_modified routines: /* Ignore SIGHUP, because a dropped connection causes a read error, which * makes Vim exit and then handling SIGHUP causes various reentrance * problems. */ signal(SIGHUP, SIG_IGN);
9f8efaa to
59c6f17
Compare
closes neovim#9274 ref neovim#9028 If stdin closed then read_error_exit calls preserve_exit. Handling SIGHUP during preserve_exit would cause a premature teardown, and conflicts with e.g. ui_bridge_stop which waits for TUI to teardown. Vim ignores SIGHUP in its prepare_to_exit and getout_preserve_modified routines: /* Ignore SIGHUP, because a dropped connection causes a read error, which * makes Vim exit and then handling SIGHUP causes various reentrance * problems. */ signal(SIGHUP, SIG_IGN);
Rename get_term_attr_entry to hl_get_term_attr, similar to hl_get_syn_attr, hl_get_ui_attr.
closes neovim#9274 ref neovim#9028 If stdin closed then read_error_exit calls preserve_exit. Handling SIGHUP during preserve_exit would cause a premature teardown, and conflicts with e.g. ui_bridge_stop which waits for TUI to teardown. Vim ignores SIGHUP in its prepare_to_exit and getout_preserve_modified routines: /* Ignore SIGHUP, because a dropped connection causes a read error, which * makes Vim exit and then handling SIGHUP causes various reentrance * problems. */ signal(SIGHUP, SIG_IGN);
dd9cfa3 to
7fdb45e
Compare
terminal_get_line_attributes() had this bug for a long time, though it likely had no effect visible to users. ref neovim#9028 ref 60f845c
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)
Render low-priority CursorLine only as underline when the cursor is on a diff line. This is because typically a diff line has a foreground highlight, which makes text difficult to see if CursorLine has a background highlight.
CursorLine highlight with ctermfg/guifg=NONE is "low priority" in the presence of 'colorcolumn' and some other highlights. But before this PR it was not altered in a diff line.
To see the effect, change
ctermfg=whitetoctermfg=NONEin this command:ref #6380
todo:
cc @zhou13