Cursor screenline public #4933

chrisbra · 2019-09-13T22:03:00Z

As requested in #4693 (comment) amend the cursorlineopt setting and allow a 'screenline' value. The value both has been aliased to the option number,line which is now the new default. To set the value do :set culopt=screenline or :set culopt=screenline,number. Setting both,screenline or line,screenline will be an error.

I have been conscious in adjusting the win_line() function. It needed quite a bit additional code to handle all different special cases, like when 'list' is set or when a multibyte character does not fit on the screen and gets wrapped and handling are different drawing states. Also, it might cause a small performance penalty, because the caching using w_last_cursorline does not work, since we are only highlighting parts of the file.

BTW: I noticed that the default cursorline highlighting does also underline the sbr setting (which is a bit strange, since it does not actually contain to the line content). I almost fixed it, but a test started to fail, so I left it as is (and is only a minor annoyance).

To that end, I added quite a few screen tests, that should make sure that most of the special cases are correctly handled.

I think most of the special cases are explained in the corresponding commits. I left those commits there and did not squash them to allow for easier review, but I can squash if needed.

No configuration yet, just always do screen line highlighting

and also force redraw in change_common if 'culopt' starts with "s"

If the character continues on the next line, need to correctly reset the character highlighting so that screenline cursorline is not applied to the wrapped part of the character. So save and store the previous char_attr before applying the cursorline highlighting to it. Once the line wrapps, we need to test if wrapping occurs and a screen line cursorline highlighting is needed and apply the stored highlighting rather than the actual char_attr to the next chars. Also: correctly handle diff mode Make sure to use the line highlighting and combine it with the cursorline highlighting attribute. In addition, correctly highlight parts of previous line that have wrapped

cache values for determining the margins

First of all, we need to add the cursorline screen highlighting to special items on a line (e.g. like special multibyte characters like `<200d>` zero-width joiner or to a tab when list mode is set `^I`. However this might pose a problem, if this character spans over a line and is wrapped. Then the cursorline highlighting will be extended on the next screen line. To prevent this, save and restore the extra_attr before applying the cursorline highlighting to it and restore it after a line has been wrapped. Also add screenline cursorline highlighting in a couple of more places

After a partly wrapped multibyte char when the start of the line is being drawn, make sure to add the cursorline highlighting to that partly character at the beginning of the line

Store the actual culopt option value as a flag inside the window structure, so we can actual OR the different values. Remove the global p_culopt_values as it is not needed anymore.

replace the test of *wp->w_p_culopt == 'l' etc by making use of the parsed flags available in wp->w_p_culopt_flags.

Update option values test for culopt and add screen tests

dpelle · 2019-09-13T22:51:59Z

src/option.c

+
+#ifdef FEAT_SYN_HL
+/*
+ * This is called when 'culopt' is change


is change → is changed

dpelle · 2019-09-13T22:58:18Z

runtime/doc/options.txt

 			CursorLineNr |hl-CursorLineNr|.
-	    "both"	Highlight as both "line" and "number" are set.
+	Special value:
+	"both"		Alias for the values "line,number".


"both" is a strange name, now that 'culopt' has more than 2 options. But I understand it's needed for backward compatibility.

dpelle · 2019-09-13T22:59:35Z

src/screen.c

@@ -158,6 +158,9 @@ static void win_redr_custom(win_T *wp, int draw_ruler);
 #ifdef FEAT_CMDL_INFO
 static void win_redr_ruler(win_T *wp, int always, int ignore_pum);
 #endif
+#ifdef FEAT_SYN_HL
+static void margin_columns_win(win_T *wp, int *lmargin, int *rmargin);
+ #endif


Indentation is wrong here.

dpelle · 2019-09-13T23:00:57Z

src/screen.c

+#ifdef FEAT_SYN_HL
+			if (need_cul_screenline)
+			    extra_attr = hl_combine_attr(extra_attr, HL_ATTR(HLF_CUL));
+ #endif


Indentation not aligned with #ifdef

Fix a couple of warnings, indent #ifdefs appropriately fix comment Make bool operator logic in change.c more obvious Fix wrong initialization in margin_columns_win

codecov-io · 2019-09-14T00:10:00Z

Codecov Report

Merging #4933 into master will decrease coverage by 0.05%.
The diff coverage is 82.05%.

@@            Coverage Diff             @@
##           master    #4933      +/-   ##
==========================================
- Coverage   81.44%   81.39%   -0.06%     
==========================================
  Files         126      126              
  Lines      146272   146381     +109     
==========================================
+ Hits       119134   119143       +9     
- Misses      27138    27238     +100

Impacted Files	Coverage Δ
src/main.c	`72.37% <100%> (-0.08%)`	⬇️
src/change.c	`79.44% <100%> (+0.02%)`	⬆️
src/screen.c	`82.91% <77.77%> (+0.16%)`	⬆️
src/option.c	`82.68% <93.1%> (+0.09%)`	⬆️
src/libvterm/src/vterm_internal.h	`50% <0%> (-50%)`	⬇️
src/libvterm/src/rect.h	`79.31% <0%> (-17.25%)`	⬇️
src/libvterm/include/vterm.h	`25% <0%> (-12.5%)`	⬇️
src/libvterm/src/mouse.c	`91.07% <0%> (-5.36%)`	⬇️
src/libvterm/src/unicode.c	`84.09% <0%> (-5.04%)`	⬇️
src/libvterm/src/encoding.c	`72.91% <0%> (-3.13%)`	⬇️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57da698...2674dde. Read the comment docs.

brammool · 2019-09-14T13:58:52Z

Christian wrote:

As requested in #4693 (comment) amend the cursorlineopt setting and allow a 'screenline' value. The value `both` has been aliased to the option `number,line` which is now the new default. To set the value do `:set culopt=screenline` or `:set culopt=screenline,number`. Setting `both,screenline` or `line,screenline` will be an error. I have been conscious in adjusting the `win_line()` function. It needed quite a bit additional code to handle all different special cases, like when 'list' is set or when a multibyte character does not fit on the screen and gets wrapped and handling are different drawing states. Also, it might cause a small performance penalty, because the caching using `w_last_cursorline` does not work, since we are only highlighting parts of the file.

I was going to include this, but with a simple test the highlighting is very wrong. Try editing src/main.c with: hi CursorLine cterm=underline With 'cursorlineopt' set to "both" the number column isn't highlighted. With set to "screenline" lots of highlighting is missing.

…

-- If VIM were a woman, I'd marry her. Slim, organized, helpful and beautiful; what's not to like? --David A. Rogers /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Problem: Cannot control 'cursorline' highlighting well. Solution: Add "screenline". (Christian Brabandt, closes vim#4933)

Vim version v8.1.2029 [1] added the `underline` attribute for the `CursorLineNr` group to `cterm` based on vim/vim#4933 [2]. This change results in gutter line numbers being underlined which has now been reverted back to Nord's style by explicitly setting the attribute for the group to `NONE`. [1]: vim/vim@d9b0d83...017ba07#diff-80fffb3e9c20e93e5b2328a9a20e19c9 [2]: vim/vim#4933 GH-174

Vim version 8.1.2029 [1] added the `underline` attribute for the `CursorLineNr` group to `cterm` [2] based on vim/vim#4933 [3]. This change resulted in gutter line numbers being underlined which has now been reverted back to Nord's style by explicitly setting the attribute for the group to `NONE`. [1]: https://github.com/vim/vim/releases/tag/v8.1.2029 [2]: vim/vim@d9b0d83...017ba07#diff-80fffb3e9c20e93e5b2328a9a20e19c9 [3]: vim/vim#4933 Resolves GH-174

- load local.vimrc if existed - vimplug: update plugins - vimplug: load colorscheme from repo + Do not use default because of annoying CursorLineNr underline in commit vim/vim@017ba07fa2 issue vim/vim#4933 - consider solarzied_light and tomorrow_night on konsole - set thesaurus if available - open veritcal split when using termdebug - some netrw settings - fzf.vim: ctrl-t jump to tab instead of open new - fzf.vim: add git grep and modify ripgrep - modify search function with optional argument - zz when jumping to tag - modify mapping

* adds coc error gutter support * remove duplicated entries * Update colors/nord.vim Co-Authored-By: Arctic Ice Studio <development@arcticicestudio.com> * Fix typo line 13 * Uniform status lines config for bundled airline and lightline themes (nordtheme#169) The included theme bundles have not supported the "uniform status line" feature (nordthemeGH-58), which allows to change the background color of the status bar (StatusLine) group to `nord3`. Related to nordthemeGH-58 Resolves nordthemeGH-168 * Plugin support for `vim-startify` (nordtheme#176) Added custom highlight groups of the `vim-startify` (1) plugin to adapt to Nord's style. References: (1) https://github.com/mhinz/vim-startify Resolves nordthemeGH-159 * Remove underline from gutter line numbers (nordtheme#185) Vim version 8.1.2029 [1] added the `underline` attribute for the `CursorLineNr` group to `cterm` [2] based on vim/vim#4933 [3]. This change resulted in gutter line numbers being underlined which has now been reverted back to Nord's style by explicitly setting the attribute for the group to `NONE`. [1]: https://github.com/vim/vim/releases/tag/v8.1.2029 [2]: vim/vim@d9b0d83...017ba07#diff-80fffb3e9c20e93e5b2328a9a20e19c9 [3]: vim/vim#4933 Resolves nordthemeGH-174 * Prepare release version 0.13.0 * adds coc error gutter support * Add nvim-lsp support (nordtheme#198) Added highlighting support for the build-in Neovim language server [1] using the coc.nvim [2] groups as reference. [1]: https://github.com/neovim/nvim-lsp [2]: https://github.com/neoclide/coc.nvim * Consistent `Error` and MoreMsg highlight group consistent between console and GUI modes. (nordtheme#202) Consistent `Error` and `MoreMsg` highligh. in term and GUI mode (nordtheme#202) Before the `Error` group in GUI mode used `nord0` as foreground color instead of `nord4` resulting in a bad contrast. Also after checking ( links to it) Also since there was also no color defined for terminal mode for the `MoreMsg` group (see `:help MoreMsg` that link to `:help more-prompt`) Vim used the default color which was some kind of green. To ensure it matches Nord's style it has now been changed to use `nord8` (main accent color) for both terminal and GUI mode. This can be tested by running `:echon "MESSAGE\n"` taht produces a lot of lines that won't fit on the current screen space anymore. Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: Sven Greb <development@svengreb.de> * Use transparent background for gutter line number in GUI mode (nordtheme#204) The `LineNr` and `CursorLineNr` highlight groups now have a transparent background in GUI mode. Before it was set to `nord0_gui` which worked fine in most cases. However, some plugins use these highlight groups to render their content in a popup window which can potentially have a different background color. This caused some issues e.g. for the fuzzy search plugin LeaderF [1]. The compatibility with the `g:nord_cursor_line_number_background` theme configuration has been verified to work as expected in both modes when it is set to `0` or `1`. This change is not related to the terminal mode or when using `set notermguicolors` since `ctermbg` for `LineNr` and `CursorLineNr` is set to `NONE` by default. [1]: https://github.com/Yggdroot/LeaderF * Release version 0.14.0 * adds coc error gutter support * Add nvim-lsp support (nordtheme#198) Added highlighting support for the build-in Neovim language server [1] using the coc.nvim [2] groups as reference. [1]: https://github.com/neovim/nvim-lsp [2]: https://github.com/neoclide/coc.nvim * Add basic TypeScript and improve TSX support (nordtheme#208) Basic highlighting support for TypeScript & TSX Added basic support to highlight TypeScript & TSX syntax more consistently through the HerringtonDarkholme/yats.vim plugin [1]. This includes improvements to highlight... 1. ...TypeScript interface an class names using `nord7` as foreground, where interfaces also use the bold attribute, to match with structs/classes. 2. ...global methods like e.g. `setTimeout` with `nord8` using the italic attribute to mark it kind of static. 3. ...regular expressions with `nord13` as foreground color instead of the normal color for quoted strings (`nord14`) to make it easier to differ between both. 4. ...global objects like `Error`, `JSON` and `console` with `nord7`. 5. ...primitive/builtin types like `string` with `nord9`. 6. ...TypeScript type references with `nord7`. 7. ...TypeScript specific characters like for type annotations (`:`) and member optionality (`?`) as operator with `nord9`. This also includes improvements for "vanilla" JavaScript elements. [1]: https://github.com/HerringtonDarkholme/yats.vim Resolves nordthemeGH-208 Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: Sven Greb <development@svengreb.de> * Add support for vim-clap (nordtheme#178) Added basic support for vim-clap [1], a modern and performant generic finder and dispatcher for Vim and NeoVim. [1]: https://github.com/liuchengxu/vim-clap nordthemeGH-178 Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: Sven Greb <development@svengreb.de> * Release version 0.15.0 Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: john.hennessey <john.hennessey@ef.com> Co-authored-by: John Hennessey <jghennes@gmail.com> Co-authored-by: Radu Vasilescu <vasilescur@gmail.com> Co-authored-by: Jose M. Murinello <jm.murinello@gmail.com> Co-authored-by: Alexander Jeurissen <1220084+alexanderjeurissen@users.noreply.github.com> Co-authored-by: xulongwu4 <xulongwu4@gmail.com> Co-authored-by: Sven Greb <development@svengreb.de> Co-authored-by: iamdi <helloiamdi@gmail.com> Co-authored-by: Johan <meck@users.noreply.github.com>

Problem: Cannot control 'cursorline' highlighting well. Solution: Add "screenline". (Christian Brabandt, closes vim/vim#4933) vim/vim@017ba07

chrisbra added 7 commits September 13, 2019 23:52

Allow screenline as value for cursorlineopt

8286ef4

Initial implementation for screenline highlighting

48a28e5

No configuration yet, just always do screen line highlighting

Force cursorline updating in main_loop and change.c

e070313

and also force redraw in change_common if 'culopt' starts with "s"

Only calculate margin columns once

77b04f6

cache values for determining the margins

Re-apply screenline cursorline highlight after wrapped multibyte char

97d3f84

After a partly wrapped multibyte char when the start of the line is being drawn, make sure to add the cursorline highlighting to that partly character at the beginning of the line

chrisbra force-pushed the cursor_screenline_public branch 2 times, most recently from 8cfff7c to 60828ce Compare September 13, 2019 22:23

chrisbra added 4 commits September 14, 2019 00:49

refactor parsing of culopt option

73ddaff

Store the actual culopt option value as a flag inside the window structure, so we can actual OR the different values. Remove the global p_culopt_values as it is not needed anymore.

Make use of cursorlineopt flags

8fd610c

replace the test of *wp->w_p_culopt == 'l' etc by making use of the parsed flags available in wp->w_p_culopt_flags.

cursorline highlighting: add tests

297f509

Update option values test for culopt and add screen tests

Document changed values for the cursorlineopt setting

5b30665

chrisbra force-pushed the cursor_screenline_public branch from 60828ce to 5b30665 Compare September 13, 2019 22:49

dpelle reviewed Sep 13, 2019

View reviewed changes

Review

2674dde

Fix a couple of warnings, indent #ifdefs appropriately fix comment Make bool operator logic in change.c more obvious Fix wrong initialization in margin_columns_win

chrisbra force-pushed the cursor_screenline_public branch from b194efd to 2674dde Compare September 13, 2019 23:34

brammool closed this in 017ba07 Sep 14, 2019

chrisbra deleted the cursor_screenline_public branch September 16, 2019 07:50

manuelschiller pushed a commit to manuelschiller/vim that referenced this pull request Nov 10, 2019

patch 8.1.2029: cannot control 'cursorline' highlighting well

7b14cec

Problem: Cannot control 'cursorline' highlighting well. Solution: Add "screenline". (Christian Brabandt, closes vim#4933)

arcticicestudio mentioned this pull request Dec 17, 2019

Remove underline from gutter line numbers nordtheme/vim#185

Merged

zeertzjq mentioned this pull request Jul 31, 2021

vim-patch:8.1.{2029,2117,2214},8.2.3204 neovim/neovim#15226

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cursor screenline public #4933

Cursor screenline public #4933

Cursor screenline public #4933

Cursor screenline public #4933

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Codecov Report