From a4d81a800293f05790025f43033de11c7bd05a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Linse?= Date: Sun, 25 Jul 2021 14:33:28 +0200 Subject: [PATCH] refactor(decorations): merge the two different code paths for virt_text test(bufhl): CHANGE of tested behaviour (inb4 a proper priority mechanism) test(decoration): change of test; previous behavior was buggy (ghost buffer text) --- src/nvim/api/buffer.c | 14 +-- src/nvim/api/private/helpers.c | 6 +- src/nvim/decoration.c | 26 ++---- src/nvim/decoration.h | 13 +-- src/nvim/screen.c | 112 ++++++++---------------- test/functional/ui/bufhl_spec.lua | 22 ++--- test/functional/ui/decorations_spec.lua | 2 +- 7 files changed, 78 insertions(+), 117 deletions(-) diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c index 47216996b4..56c9cfc517 100644 --- a/src/nvim/api/buffer.c +++ b/src/nvim/api/buffer.c @@ -1566,7 +1566,8 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id, "virt_text is not an Array"); goto error; } - decor.virt_text = parse_virt_text(v->data.array, err); + decor.virt_text = parse_virt_text(v->data.array, err, + &decor.virt_text_width); if (ERROR_SET(err)) { goto error; } @@ -1949,23 +1950,26 @@ Integer nvim_buf_set_virtual_text(Buffer buffer, } uint64_t ns_id = src2ns(&src_id); + int width; - VirtText virt_text = parse_virt_text(chunks, err); + VirtText virt_text = parse_virt_text(chunks, err, &width); if (ERROR_SET(err)) { return 0; } - VirtText *existing = decor_find_virttext(buf, (int)line, ns_id); + Decoration *existing = decor_find_virttext(buf, (int)line, ns_id); if (existing) { - clear_virttext(existing); - *existing = virt_text; + clear_virttext(&existing->virt_text); + existing->virt_text = virt_text; + existing->virt_text_width = width; return src_id; } Decoration *decor = xcalloc(1, sizeof(*decor)); decor->virt_text = virt_text; + decor->virt_text_width = width; extmark_set(buf, ns_id, 0, (int)line, 0, -1, -1, decor, true, false, kExtmarkNoUndo); diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c index e78bd2ea9a..e9a436d3e3 100644 --- a/src/nvim/api/private/helpers.c +++ b/src/nvim/api/private/helpers.c @@ -1592,9 +1592,10 @@ bool extmark_get_index_from_obj(buf_T *buf, Integer ns_id, Object obj, int } } -VirtText parse_virt_text(Array chunks, Error *err) +VirtText parse_virt_text(Array chunks, Error *err, int *width) { VirtText virt_text = KV_INITIAL_VALUE; + int w = 0; for (size_t i = 0; i < chunks.size; i++) { if (chunks.items[i].type != kObjectTypeArray) { api_set_error(err, kErrorTypeValidation, "Chunk is not an array"); @@ -1635,9 +1636,12 @@ VirtText parse_virt_text(Array chunks, Error *err) } char *text = transstr(str.size > 0 ? str.data : ""); // allocates + w += (int)mb_string2cells((char_u *)text); + kv_push(virt_text, ((VirtTextChunk){ .text = text, .hl_id = hl_id })); } + *width = w; return virt_text; free_exit: diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c index 9d969ada89..74cb9a26b7 100644 --- a/src/nvim/decoration.c +++ b/src/nvim/decoration.c @@ -119,7 +119,7 @@ void clear_virttext(VirtText *text) *text = (VirtText)KV_INITIAL_VALUE; } -VirtText *decor_find_virttext(buf_T *buf, int row, uint64_t ns_id) +Decoration *decor_find_virttext(buf_T *buf, int row, uint64_t ns_id) { MarkTreeIter itr[1] = { 0 }; marktree_itr_get(buf->b_marktree, row, 0, itr); @@ -132,7 +132,7 @@ VirtText *decor_find_virttext(buf_T *buf, int row, uint64_t ns_id) mark.id, false); if (item && (ns_id == 0 || ns_id == item->ns_id) && item->decor && kv_size(item->decor->virt_text)) { - return &item->decor->virt_text; + return item->decor; } marktree_itr_next(buf->b_marktree, itr); } @@ -218,6 +218,7 @@ bool decor_redraw_line(buf_T *buf, int row, DecorState *state) } state->row = row; state->col_until = -1; + state->eol_col = -1; return true; // TODO(bfredl): be more precise } @@ -230,10 +231,6 @@ static void decor_add(DecorState *state, int start_row, int start_col, *decor, attr_id, kv_size(decor->virt_text) && owned, -1 }; - if (decor->virt_text_pos == kVTEndOfLine) { - range.win_col = -2; // handled separately - } - kv_pushp(state->active); size_t index; for (index = kv_size(state->active)-1; index > 0; index--) { @@ -345,29 +342,22 @@ void decor_redraw_end(DecorState *state) state->buf = NULL; } -VirtText decor_redraw_eol(buf_T *buf, DecorState *state, int *eol_attr, - bool *aligned) +bool decor_redraw_eol(buf_T *buf, DecorState *state, int *eol_attr, int eol_col) { decor_redraw_col(buf, MAXCOL, MAXCOL, false, state); - VirtText text = VIRTTEXT_EMPTY; + state->eol_col = eol_col; + bool has_virttext = false; for (size_t i = 0; i < kv_size(state->active); i++) { DecorRange item = kv_A(state->active, i); if (item.start_row == state->row && kv_size(item.decor.virt_text)) { - if (!kv_size(text) && item.decor.virt_text_pos == kVTEndOfLine) { - text = item.decor.virt_text; - } else if (item.decor.virt_text_pos == kVTRightAlign - || item.decor.virt_text_pos == kVTWinCol) { - *aligned = true; - } + has_virttext = true; } - if (item.decor.hl_eol && item.start_row <= state->row) { *eol_attr = hl_combine_attr(*eol_attr, item.attr_id); } } - - return text; + return has_virttext; } void decor_add_ephemeral(int start_row, int start_col, int end_row, int end_col, diff --git a/src/nvim/decoration.h b/src/nvim/decoration.h index 4cebc0b731..28dabeeada 100644 --- a/src/nvim/decoration.h +++ b/src/nvim/decoration.h @@ -34,19 +34,20 @@ typedef enum { struct Decoration { - int hl_id; // highlight group VirtText virt_text; + int hl_id; // highlight group VirtTextPos virt_text_pos; - bool virt_text_hide; HlMode hl_mode; + bool virt_text_hide; bool hl_eol; + bool shared; // shared decoration, don't free // TODO(bfredl): style, signs, etc DecorPriority priority; - bool shared; // shared decoration, don't free int col; // fixed col value, like win_col + int virt_text_width; // width of virt_text }; -#define DECORATION_INIT { 0, KV_INITIAL_VALUE, kVTEndOfLine, false, \ - kHlModeUnknown, false, DECOR_PRIORITY_BASE, false, 0 } +#define DECORATION_INIT { KV_INITIAL_VALUE, 0, kVTEndOfLine, kHlModeUnknown, \ + false, false, false, DECOR_PRIORITY_BASE, 0, 0 } typedef struct { int start_row; @@ -67,6 +68,8 @@ typedef struct { int row; int col_until; int current; + + int eol_col; VirtText *virt_text; } DecorState; diff --git a/src/nvim/screen.c b/src/nvim/screen.c index 586aeff1e2..d8f5f1077c 100644 --- a/src/nvim/screen.c +++ b/src/nvim/screen.c @@ -165,7 +165,7 @@ static bool resizing = false; #endif #define SEARCH_HL_PRIORITY 0 -static char * provider_first_error = NULL; +static char * provider_err = NULL; static bool provider_invoke(NS ns_id, const char *name, LuaRef ref, Array args, bool default_true) @@ -187,10 +187,10 @@ static bool provider_invoke(NS ns_id, const char *name, LuaRef ref, const char *ns_name = describe_ns(ns_id); ELOG("error in provider %s:%s: %s", ns_name, name, err.msg); bool verbose_errs = true; // TODO(bfredl): - if (verbose_errs && provider_first_error == NULL) { + if (verbose_errs && provider_err == NULL) { static char errbuf[IOSIZE]; snprintf(errbuf, sizeof errbuf, "%s: %s", ns_name, err.msg); - provider_first_error = xstrdup(errbuf); + provider_err = xstrdup(errbuf); } } @@ -2103,7 +2103,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool search_attr_from_match = false; // if search_attr is from :match bool has_decor = false; // this buffer has decoration - bool do_virttext = false; // draw virtual text for this line int win_col_offset = 0; // offset for window columns char_u buf_fold[FOLD_TEXT_LEN + 1]; // Hold value returned by get_foldtext @@ -2148,8 +2147,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, row = startrow; - char *err_text = NULL; - buf_T *buf = wp->w_buffer; if (!number_only) { @@ -2194,14 +2191,20 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, } } - if (has_decor) { - extra_check = true; + if (provider_err) { + Decoration err_decor = DECORATION_INIT; + int hl_err = syn_check_group((char_u *)S_LEN("ErrorMsg")); + kv_push(err_decor.virt_text, + ((VirtTextChunk){ .text = provider_err, + .hl_id = hl_err })); + err_decor.virt_text_width = mb_string2cells((char_u *)provider_err); + decor_add_ephemeral(lnum-1, 0, lnum-1, 0, &err_decor); + provider_err = NULL; + has_decor = true; } - if (provider_first_error) { - err_text = provider_first_error; - provider_first_error = NULL; - do_virttext = true; + if (has_decor) { + extra_check = true; } // Check for columns to display for 'colorcolumn'. @@ -3953,19 +3956,15 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, if (draw_color_col) draw_color_col = advance_color_col(VCOL_HLC, &color_cols); - VirtText virt_text = KV_INITIAL_VALUE; - bool has_aligned = false; - if (err_text) { - int hl_err = syn_check_group((char_u *)S_LEN("ErrorMsg")); - kv_push(virt_text, ((VirtTextChunk){ .text = err_text, - .hl_id = hl_err })); - do_virttext = true; - } else if (has_decor) { - virt_text = decor_redraw_eol(wp->w_buffer, &decor_state, &line_attr, - &has_aligned); - if (kv_size(virt_text)) { - do_virttext = true; - } + bool has_virttext = false; + // Make sure alignment is the same regardless + // if listchars=eol:X is used or not. + int eol_skip = (wp->w_p_lcs_chars.eol == lcs_eol_one && eol_hl_off == 0 + ? 1 : 0); + + if (has_decor) { + has_virttext = decor_redraw_eol(wp->w_buffer, &decor_state, &line_attr, + col + eol_skip); } if (((wp->w_p_cuc @@ -3974,20 +3973,10 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, grid->Columns * (row - startrow + 1) + v && lnum != wp->w_cursor.lnum) || draw_color_col || line_attr_lowprio || line_attr - || diff_hlf != (hlf_T)0 || do_virttext - || has_aligned)) { + || diff_hlf != (hlf_T)0 || has_virttext)) { int rightmost_vcol = 0; int i; - size_t virt_pos = 0; - LineState s = LINE_STATE(""); - int virt_attr = 0; - - // Make sure alignment is the same regardless - // if listchars=eol:X is used or not. - bool delay_virttext = wp->w_p_lcs_chars.eol == lcs_eol_one - && eol_hl_off == 0; - if (wp->w_p_cuc) { rightmost_vcol = wp->w_virtcol; } @@ -4013,38 +4002,15 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, } int base_attr = hl_combine_attr(line_attr_lowprio, diff_attr); - if (base_attr || line_attr || has_aligned) { + if (base_attr || line_attr || has_virttext) { rightmost_vcol = INT_MAX; } int col_stride = wp->w_p_rl ? -1 : 1; while (wp->w_p_rl ? col >= 0 : col < grid->Columns) { - int cells = -1; - // TODO: integrate this whith the other virt_text impl already - if (do_virttext && !delay_virttext) { - if (*s.p == NUL) { - if (virt_pos < virt_text.size) { - s.p = kv_A(virt_text, virt_pos).text; - int hl_id = kv_A(virt_text, virt_pos).hl_id; - virt_attr = hl_id > 0 ? syn_id2attr(hl_id) : 0; - virt_pos++; - } else { - do_virttext = false; - } - } - if (*s.p != NUL) { - cells = line_putchar(&s, &linebuf_char[off], grid->Columns - col, - false); - } - } - delay_virttext = false; - - if (cells == -1) { - schar_from_ascii(linebuf_char[off], ' '); - cells = 1; - } - col += cells * col_stride; + schar_from_ascii(linebuf_char[off], ' '); + col += col_stride; if (draw_color_col) { draw_color_col = advance_color_col(VCOL_HLC, &color_cols); } @@ -4057,24 +4023,16 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, col_attr = mc_attr; } - if (do_virttext) { - col_attr = hl_combine_attr(col_attr, virt_attr); - } - col_attr = hl_combine_attr(col_attr, line_attr); linebuf_attr[off] = col_attr; - if (cells == 2) { - linebuf_attr[off+1] = col_attr; - } - off += cells * col_stride; + off += col_stride; - if (VCOL_HLC >= rightmost_vcol && *s.p == NUL - && virt_pos >= virt_text.size) { + if (VCOL_HLC >= rightmost_vcol) { break; } - vcol += cells; + vcol += 1; } } @@ -4393,7 +4351,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, } xfree(p_extra_free); - xfree(err_text); return row; } @@ -4401,13 +4358,17 @@ void draw_virt_text(buf_T *buf, int col_off, int *end_col, int max_col) { DecorState *state = &decor_state; int right_pos = max_col; + bool do_eol = state->eol_col > -1; for (size_t i = 0; i < kv_size(state->active); i++) { DecorRange *item = &kv_A(state->active, i); if (item->start_row == state->row && kv_size(item->decor.virt_text)) { if (item->win_col == -1) { if (item->decor.virt_text_pos == kVTRightAlign) { - right_pos -= item->decor.col; + right_pos -= item->decor.virt_text_width; item->win_col = right_pos; + } else if (item->decor.virt_text_pos == kVTEndOfLine && do_eol) { + item->win_col = state->eol_col; + state->eol_col += item->decor.virt_text_width; } else if (item->decor.virt_text_pos == kVTWinCol) { item->win_col = MAX(item->decor.col+col_off, 0); } @@ -4436,7 +4397,6 @@ void draw_virt_text(buf_T *buf, int col_off, int *end_col, int max_col) hl_id > 0 ? syn_id2attr(hl_id) : 0); virt_pos++; } while (!s.p && virt_pos < kv_size(vt)); - // TODO: y w0t m8 if (!s.p) { break; } diff --git a/test/functional/ui/bufhl_spec.lua b/test/functional/ui/bufhl_spec.lua index af709cd521..16ed3b9486 100644 --- a/test/functional/ui/bufhl_spec.lua +++ b/test/functional/ui/bufhl_spec.lua @@ -858,8 +858,8 @@ describe('Buffer highlighting', function() it('works with cursorline', function() command("set cursorline") - screen:expect([[ - {14:^1 + 2 }{15:=}{16: 3}{14: }| + screen:expect{grid=[[ + {14:^1 + 2 }{3:=}{2: 3}{14: }| 3 + {11:ERROR:} invalid syntax | 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5| , 5, 5, 5, 5, 5, 5, Lorem ipsum dolor s| @@ -867,32 +867,32 @@ describe('Buffer highlighting', function() {1:~ }| {1:~ }| | - ]]) + ]]} feed('j') - screen:expect([[ + screen:expect{grid=[[ 1 + 2 {3:=}{2: 3} | - {14:^3 + }{11:ERROR:}{14: invalid syntax }| + {14:^3 + }{11:ERROR:} invalid syntax{14: }| 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5| , 5, 5, 5, 5, 5, 5, Lorem ipsum dolor s| x = 4 | {1:~ }| {1:~ }| | - ]]) + ]]} feed('j') - screen:expect([[ + screen:expect{grid=[[ 1 + 2 {3:=}{2: 3} | 3 + {11:ERROR:} invalid syntax | {14:^5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5}| - {14:, 5, 5, 5, 5, 5, 5, Lorem ipsum dolor s}| + {14:, 5, 5, 5, 5, 5, 5, }Lorem ipsum dolor s| x = 4 | {1:~ }| {1:~ }| | - ]]) + ]]} end) it('works with color column', function() @@ -910,11 +910,11 @@ describe('Buffer highlighting', function() command("set colorcolumn=9") screen:expect{grid=[[ - ^1 + 2 {3:=}{2: }{17:3} | + ^1 + 2 {3:=}{2: 3} | 3 + {11:ERROR:} invalid syntax | 5, 5, 5,{18: }5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5| , 5, 5, 5, 5, 5, 5, Lorem ipsum dolor s| - x = 4 {12:暗}{19:x}{12:事} | + x = 4 {12:暗x事} | {1:~ }| {1:~ }| | diff --git a/test/functional/ui/decorations_spec.lua b/test/functional/ui/decorations_spec.lua index 96d9eab065..4373d17890 100644 --- a/test/functional/ui/decorations_spec.lua +++ b/test/functional/ui/decorations_spec.lua @@ -562,7 +562,7 @@ end]] {5:l}{8:blen}{7:dy}{10:e}{7:text}{10:h}{7:-}{10:_}{7:here}ell, count = unpack(item) | {5:i}{12:c}{11:ombining color} {13:nil} {5:then} | {11:replacing color}d_cell | - {5:e}{8:bl}{14:endy}{15:i}{14:text}{15:o}{14:-}{15:o}{14:h}{7:ere} | + {5:e}{8:bl}{7:endy}{10: }{7:text}{10: }{7:-}{10: }{7:here} | {5:f}{12:co}{11:mbini}{16:n}{11:g color}t {5:or} {13:1}) {5:do} | {11:replacing color} line[colpos] | cell.text = text |