From bc75544facbf200fc7a7d519c0963ec1becfa508 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 24 Dec 2021 08:06:26 +0800 Subject: [PATCH 1/4] fix(screen): truncate double-width character correctly The `c = '>';` is useless here, because it is not used later. `u8c` should also need to be set to '>', and `u8cc` needs to be cleared. --- src/nvim/screen.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nvim/screen.c b/src/nvim/screen.c index a938a3b062..df911a1228 100644 --- a/src/nvim/screen.c +++ b/src/nvim/screen.c @@ -5933,6 +5933,8 @@ void grid_puts_len(ScreenGrid *grid, char_u *text, int textlen, int row, int col // Only 1 cell left, but character requires 2 cells: // display a '>' in the last column to avoid wrapping. */ c = '>'; + u8c = '>'; + u8cc[0] = 0; mbyte_cells = 1; } From 28dadd5a54a67b467c08614670c65b0b5b23d247 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 24 Dec 2021 08:06:27 +0800 Subject: [PATCH 2/4] fix(screen): truncate when overwriting right half of a double-width char Unlike the code above, this truncates the character in the same grid. This is mainly for the pum scrollbar in the next commit. --- src/nvim/screen.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/nvim/screen.c b/src/nvim/screen.c index df911a1228..e62e3ca7bc 100644 --- a/src/nvim/screen.c +++ b/src/nvim/screen.c @@ -5965,6 +5965,13 @@ void grid_puts_len(ScreenGrid *grid, char_u *text, int textlen, int row, int col clear_next_cell = true; } + // When at the start of the text and overwriting the right half of a + // two-cell character in the same grid, truncate that into a '>'. + if (ptr == text && col > 0 && grid->chars[off][0] == 0) { + grid->chars[off - 1][0] = '>'; + grid->chars[off - 1][1] = 0; + } + schar_copy(grid->chars[off], buf); grid->attrs[off] = attr; if (mbyte_cells == 2) { From e6d35b9e4011023a4efd06234fe2fd2c5f7dc829 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 24 Dec 2021 08:06:27 +0800 Subject: [PATCH 3/4] fix(pum_redraw): use grid_puts_len() to truncate the text Nvim already resizes grid to the required width, so there is no need to truncate the text in pum_redraw(). What's more, truncation is currently done incorrectly because Vim patch 8.2.1995 was ported incorrectly. This nearly reverts the truncation part of Vim patch 8.2.1995, but not the part that reduces unnecessary calls to pum_redraw(). The original PR https://github.com/vim/vim/pull/7306 didn't explain much about which part of it actually reduces redraws. --- src/nvim/popupmnu.c | 13 +-- test/functional/ui/popupmenu_spec.lua | 130 ++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 11 deletions(-) diff --git a/src/nvim/popupmnu.c b/src/nvim/popupmnu.c index 606c03f838..b253077844 100644 --- a/src/nvim/popupmnu.c +++ b/src/nvim/popupmnu.c @@ -542,17 +542,8 @@ void pum_redraw(void) xfree(st); col -= width; } else { - int size = (int)STRLEN(st); - int cells = (int)mb_string2cells(st); - - // only draw the text that fits - while (size > 0 && col + cells > pum_width + pum_col) { - size--; - size -= utf_head_off(st, st + size); - cells -= utf_ptr2cells(st + size); - } - - grid_puts_len(&pum_grid, st, size, row, col, attr); + // use grid_puts_len() to truncate the text + grid_puts(&pum_grid, st, row, col, attr); xfree(st); col += width; } diff --git a/test/functional/ui/popupmenu_spec.lua b/test/functional/ui/popupmenu_spec.lua index aeba049557..4fc5c389e5 100644 --- a/test/functional/ui/popupmenu_spec.lua +++ b/test/functional/ui/popupmenu_spec.lua @@ -2213,4 +2213,134 @@ describe('builtin popupmenu', function() feed('') assert_alive() end) + + it('truncates double-width character correctly when there is no scrollbar', function() + screen:try_resize(32,8) + command('set completeopt+=menuone,noselect') + feed('i' .. string.rep(' ', 13)) + funcs.complete(14, {'哦哦哦哦哦哦哦哦哦哦'}) + screen:expect([[ + ^ | + {1:~ }{n: 哦哦哦哦哦哦哦哦哦>}| + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + {2:-- INSERT --} | + ]]) + end) + + it('truncates double-width character correctly when there is scrollbar', function() + screen:try_resize(32,8) + command('set completeopt+=noselect') + command('set pumheight=4') + feed('i' .. string.rep(' ', 12)) + local items = {} + for _ = 1, 8 do + table.insert(items, {word = '哦哦哦哦哦哦哦哦哦哦', equal = 1, dup = 1}) + end + funcs.complete(13, items) + screen:expect([[ + ^ | + {1:~ }{n: 哦哦哦哦哦哦哦哦哦>}{c: }| + {1:~ }{n: 哦哦哦哦哦哦哦哦哦>}{c: }| + {1:~ }{n: 哦哦哦哦哦哦哦哦哦>}{s: }| + {1:~ }{n: 哦哦哦哦哦哦哦哦哦>}{s: }| + {1:~ }| + {1:~ }| + {2:-- INSERT --} | + ]]) + end) +end) + +describe('builtin popupmenu with ui/ext_multigrid', function() + local screen + before_each(function() + clear() + screen = Screen.new(32, 20) + screen:attach({ext_multigrid=true}) + screen:set_default_attr_ids({ + -- popup selected item / scrollbar track + ['s'] = {background = Screen.colors.WebGray}, + -- popup non-selected item + ['n'] = {background = Screen.colors.LightMagenta}, + -- popup scrollbar knob + ['c'] = {background = Screen.colors.Grey0}, + [1] = {bold = true, foreground = Screen.colors.Blue}, + [2] = {bold = true}, + [3] = {reverse = true}, + [4] = {bold = true, reverse = true}, + [5] = {bold = true, foreground = Screen.colors.SeaGreen}, + [6] = {foreground = Screen.colors.Grey100, background = Screen.colors.Red}, + }) + end) + + it('truncates double-width character correctly when there is no scrollbar', function() + screen:try_resize(32,8) + command('set completeopt+=menuone,noselect') + feed('i' .. string.rep(' ', 13)) + funcs.complete(14, {'哦哦哦哦哦哦哦哦哦哦'}) + screen:expect({grid=[[ + ## grid 1 + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [3:--------------------------------]| + ## grid 2 + ^ | + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + ## grid 3 + {2:-- INSERT --} | + ## grid 4 + {n: 哦哦哦哦哦哦哦哦哦>}| + ]], float_pos={[4] = {{id = -1}, 'NW', 2, 1, 12, false, 100}}}) + end) + + it('truncates double-width character correctly when there is scrollbar', function() + screen:try_resize(32,8) + command('set completeopt+=noselect') + command('set pumheight=4') + feed('i' .. string.rep(' ', 12)) + local items = {} + for _ = 1, 8 do + table.insert(items, {word = '哦哦哦哦哦哦哦哦哦哦', equal = 1, dup = 1}) + end + funcs.complete(13, items) + screen:expect({grid=[[ + ## grid 1 + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [2:--------------------------------]| + [3:--------------------------------]| + ## grid 2 + ^ | + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + {1:~ }| + ## grid 3 + {2:-- INSERT --} | + ## grid 4 + {n: 哦哦哦哦哦哦哦哦哦>}{c: }| + {n: 哦哦哦哦哦哦哦哦哦>}{c: }| + {n: 哦哦哦哦哦哦哦哦哦>}{s: }| + {n: 哦哦哦哦哦哦哦哦哦>}{s: }| + ]], float_pos={[4] = {{id = -1}, 'NW', 2, 1, 11, false, 100}}}) + end) end) From 4f70b31f7aee3a34c8d6b5c63184f97b0c1dcbce Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 24 Dec 2021 08:06:27 +0800 Subject: [PATCH 4/4] refactor(pum_redraw): rename col -> grid_col This is initialized to `col_off`, while in Vim this variable `col` that is used in the same places is initialized to `pum_col`. This can cause confusion in patch porting, and it caused Vim patch 8.2.1995 to be ported incorrectly. (I reverted the incorrect part in the last commit though.) Rename it to `grid_col` to make it clear that it is different from Vim's `col` variable. --- src/nvim/popupmnu.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/nvim/popupmnu.c b/src/nvim/popupmnu.c index b253077844..da2ada791f 100644 --- a/src/nvim/popupmnu.c +++ b/src/nvim/popupmnu.c @@ -386,7 +386,7 @@ void pum_display(pumitem_T *array, int size, int selected, bool array_changed, i void pum_redraw(void) { int row = 0; - int col; + int grid_col; int attr_norm = win_hl_attr(curwin, HLF_PNI); int attr_select = win_hl_attr(curwin, HLF_PSI); int attr_scroll = win_hl_attr(curwin, HLF_PSB); @@ -479,7 +479,7 @@ void pum_redraw(void) // Display each entry, use two spaces for a Tab. // Do this 3 times: For the main text, kind and extra info - col = col_off; + grid_col = col_off; totwidth = 0; for (round = 1; round <= 3; ++round) { @@ -537,15 +537,15 @@ void pum_redraw(void) } } grid_puts_len(&pum_grid, rt, (int)STRLEN(rt), row, - col - size + 1, attr); + grid_col - size + 1, attr); xfree(rt_start); xfree(st); - col -= width; + grid_col -= width; } else { // use grid_puts_len() to truncate the text - grid_puts(&pum_grid, st, row, col, attr); + grid_puts(&pum_grid, st, row, grid_col, attr); xfree(st); - col += width; + grid_col += width; } if (*p != TAB) { @@ -554,12 +554,12 @@ void pum_redraw(void) // Display two spaces for a Tab. if (pum_rl) { - grid_puts_len(&pum_grid, (char_u *)" ", 2, row, col - 1, + grid_puts_len(&pum_grid, (char_u *)" ", 2, row, grid_col - 1, attr); - col -= 2; + grid_col -= 2; } else { - grid_puts_len(&pum_grid, (char_u *)" ", 2, row, col, attr); - col += 2; + grid_puts_len(&pum_grid, (char_u *)" ", 2, row, grid_col, attr); + grid_col += 2; } totwidth += 2; // start text at next char @@ -590,21 +590,21 @@ void pum_redraw(void) if (pum_rl) { grid_fill(&pum_grid, row, row + 1, col_off - pum_base_width - n + 1, - col + 1, ' ', ' ', attr); - col = col_off - pum_base_width - n + 1; + grid_col + 1, ' ', ' ', attr); + grid_col = col_off - pum_base_width - n + 1; } else { - grid_fill(&pum_grid, row, row + 1, col, + grid_fill(&pum_grid, row, row + 1, grid_col, col_off + pum_base_width + n, ' ', ' ', attr); - col = col_off + pum_base_width + n; + grid_col = col_off + pum_base_width + n; } totwidth = pum_base_width + n; } if (pum_rl) { - grid_fill(&pum_grid, row, row + 1, col_off - pum_width + 1, col + 1, + grid_fill(&pum_grid, row, row + 1, col_off - pum_width + 1, grid_col + 1, ' ', ' ', attr); } else { - grid_fill(&pum_grid, row, row + 1, col, col_off + pum_width, ' ', ' ', + grid_fill(&pum_grid, row, row + 1, grid_col, col_off + pum_width, ' ', ' ', attr); }