Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[RDY] hl-Syntax should override cursorline and cursorcolumn #6380

Closed
wants to merge 1 commit into from
Closed

[RDY] hl-Syntax should override cursorline and cursorcolumn #6380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 28 additions & 27 deletions src/nvim/screen.c
View file
Open in desktop
Expand Up @@ -2207,6 +2207,8 @@ win_line (
int prev_c1 = 0; /* first composing char for prev_c */
int did_line_attr = 0;

int line_attr_low_priority = 0; // attribute for current line
// with very low priority
bool search_attr_from_match = false; // if search_attr is from :match
bool has_bufhl = false; // this buffer has highlight matches
int bufhl_attr = 0; // attributes desired by bufhl
Expand Down Expand Up @@ -2421,10 +2423,18 @@ win_line (
filler_lines = wp->w_topfill;
filler_todo = filler_lines;

/* If this line has a sign with line highlighting set line_attr. */
// Cursor line highlighting for 'cursorline' in the current window. Not
// when Visual mode is active, because it's not clear what is selected
// then.
if (wp->w_p_cul && lnum == wp->w_cursor.lnum
&& !(wp == curwin && VIsual_active)) {
line_attr_low_priority = win_hl_attr(wp, HLF_CUL);
}

v = buf_getsigntype(wp->w_buffer, lnum, SIGN_LINEHL);
if (v != 0)
line_attr = sign_get_attr((int)v, TRUE);
if (v != 0) {
line_attr = sign_get_attr((int)v, true);
}

// Highlight the current line in the quickfix window.
if (bt_quickfix(wp->w_buffer) && qf_current_entry(wp) == lnum) {
Expand All @@ -2435,7 +2445,7 @@ win_line (
line_attr = hl_combine_attr(wp->w_hl_attr_normal, line_attr);
}

if (line_attr != 0) {
if (line_attr_low_priority || line_attr) {
area_highlighting = true;
}

Expand Down Expand Up @@ -2653,20 +2663,6 @@ win_line (
cur = cur->next;
}

/* Cursor line highlighting for 'cursorline' in the current window. Not
* when Visual mode is active, because it's not clear what is selected
* then. */
if (wp->w_p_cul && lnum == wp->w_cursor.lnum
&& !(wp == curwin && VIsual_active)) {
if (line_attr != 0 && !(State & INSERT) && bt_quickfix(wp->w_buffer)
&& qf_current_entry(wp) == lnum) {
line_attr = hl_combine_attr(win_hl_attr(wp, HLF_CUL), line_attr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this branch needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old one basically says if we are in quickfix window, then we use HLF_CUL to complement current line_attr, otherwise it will directly set line_attr to HLF_CUL. Setting line_attr to HLF_CUL is not what we want, since it incorrectly override some highlights under some condition (the logic is complex here)....

In this patch, I split line_attr into line_attr and line_attr_low_priority. Now line_attr_low_priority only takes care of cursorline, and it will be merged into char_attr with lowest priority. For other line_attr, it will still use the old logic. Since line_attr_low_priority already has lowest priority, I believe these lines are not necessary.

} else {
line_attr = win_hl_attr(wp, HLF_CUL);
}
area_highlighting = true;
}

off = (unsigned)(current_ScreenLine - ScreenLines);
col = 0;
if (wp->w_p_rl) {
Expand Down Expand Up @@ -3580,13 +3576,14 @@ win_line (
// character if the line break is included.
// For a diff line the highlighting continues after the
// "$".
if (diff_hlf == (hlf_T)0 && line_attr == 0) {
/* In virtualedit, visual selections may extend
* beyond end of line. */
if (diff_hlf == (hlf_T)0
&& line_attr == 0
&& line_attr_low_priority == 0) {
// In virtualedit, visual selections may extend beyond end of line
if (area_highlighting && virtual_active()
&& tocol != MAXCOL && vcol < tocol)
&& tocol != MAXCOL && vcol < tocol) {
n_extra = 0;
else {
} else {
p_extra = at_end_str;
n_extra = 1;
c_extra = NUL;
Expand Down Expand Up @@ -3644,7 +3641,7 @@ win_line (
(col < wp->w_width))) {
c = ' ';
ptr--; // put it back at the NUL
} else if ((diff_hlf != (hlf_T)0 || line_attr != 0)
} else if ((diff_hlf != (hlf_T)0 || line_attr_low_priority || line_attr)
&& (wp->w_p_rl
? (col >= 0)
: (col - boguscols < wp->w_width))) {
Expand All @@ -3656,7 +3653,8 @@ win_line (
did_line_attr++;

// don't do search HL for the rest of the line
if (line_attr != 0 && char_attr == search_attr && col > 0) {
if ((line_attr_low_priority || line_attr)
&& char_attr == search_attr && col > 0) {
char_attr = line_attr;
}
if (diff_hlf == HLF_TXD) {
Expand Down Expand Up @@ -4018,13 +4016,16 @@ win_line (
if (wp->w_p_cuc && VCOL_HLC == (long)wp->w_virtcol
&& lnum != wp->w_cursor.lnum) {
vcol_save_attr = char_attr;
char_attr = hl_combine_attr(char_attr, win_hl_attr(wp, HLF_CUC));
char_attr = hl_combine_attr(win_hl_attr(wp, HLF_CUC), char_attr);
} else if (draw_color_col && VCOL_HLC == *color_cols) {
vcol_save_attr = char_attr;
char_attr = hl_combine_attr(char_attr, win_hl_attr(wp, HLF_MC));
char_attr = hl_combine_attr(win_hl_attr(wp, HLF_MC), char_attr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a separate change, right? And it's much lower-risk than the line_attr_low_priority change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. They can fix the problem for cursorcolumn and colorcolumn.

}
}

// apply line attr with lowest priority so that everthing can override it
char_attr = hl_combine_attr(line_attr_low_priority, char_attr);

/*
* Store character to be displayed.
* Skip characters that are left of the screen for 'nowrap'.
Expand Down
6 changes: 3 additions & 3 deletions test/functional/ui/highlight_spec.lua
View file
Open in desktop
Expand Up @@ -518,15 +518,15 @@ describe("'listchars' highlight", function()
]])
feed_command('set cursorline')
screen:expect([[
{2:^>-------.}{1:abcd}{2:.}{1:Lorem}{4:>}|
{2:^>-------.}{1:abcd}{2:.}{1:Lorem}{3:>}|
{5:>-------.}abcd{5:*}{4:¬} |
{4:¬} |
{4:~ }|
:set cursorline |
]])
feed('$')
screen:expect([[
{4:<}{1:r}{2:.}{1:sit}{2:.}{1:ame^t}{3:¬}{1: }|
{3:<}{1:r}{2:.}{1:sit}{2:.}{1:ame^t}{3:¬}{1: }|
{4:<} |
{4:<} |
{4:~ }|
Expand Down Expand Up @@ -607,7 +607,7 @@ describe("'listchars' highlight", function()
feed('<esc>$')
screen:expect([[
{4:<} |
{4:<}{1:r}{2:.}{1:sit}{2:.}{1:ame^t}{3:¬}{1: }|
{3:<}{1:r}{2:.}{1:sit}{2:.}{1:ame^t}{3:¬}{1: }|
{4:<} |
{4:~ }|
|
Expand Down