From c80bb5b63e0a470b4f458f639b9429db6a49f758 Mon Sep 17 00:00:00 2001 From: glepnir Date: Tue, 21 Oct 2025 06:26:23 +0800 Subject: [PATCH] fix(pumborder): wrong layout with pumborder=none #36208 Problem: Border width calculations were scattered with repeated `*p_pumborder != NUL ? 2 : 0` patterns. The "none" value was not consistently checked, causing borders to appear when pumborder="none". When "shadow" the info floating window have an extra cell of spacing. Solution: Add `pum_border_width()` helper that returns 0 when pumborder is unset or "none" (opt_winborder_values[7]), returns 1 when pumborder is shadow, otherwise return 2. --- src/nvim/popupmenu.c | 37 ++++++------ test/functional/ui/popupmenu_spec.lua | 81 ++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/src/nvim/popupmenu.c b/src/nvim/popupmenu.c index e172c6f312..e68b38baf5 100644 --- a/src/nvim/popupmenu.c +++ b/src/nvim/popupmenu.c @@ -270,6 +270,15 @@ static void pum_compute_horizontal_placement(win_T *target_win, int cursor_col) pum_width = max_col - pum_scrollbar; } +static inline int pum_border_width(void) +{ + if (*p_pumborder == NUL || strequal(p_pumborder, opt_winborder_values[7])) { + return 0; // No border + } + // Shadow (1) only has right+bottom, others (2) have full border + return strequal(p_pumborder, opt_winborder_values[3]) ? 1 : 2; +} + /// Show the popup menu with items "array[size]". /// "array" must remain valid until pum_undisplay() is called! /// When possible the leftmost character is aligned with cursor column. @@ -297,11 +306,7 @@ void pum_display(pumitem_T *array, int size, int selected, bool array_changed, i pum_rl = (State & MODE_CMDLINE) == 0 && curwin->w_p_rl; - int pum_border_size = *p_pumborder != NUL ? 2 : 0; - if (strequal(p_pumborder, opt_winborder_values[3])) { - pum_border_size -= 1; - } - + int border_width = pum_border_width(); do { // Mark the pum as visible already here, // to avoid that must_redraw is set when 'cursorcolumn' is on. @@ -390,10 +395,10 @@ void pum_display(pumitem_T *array, int size, int selected, bool array_changed, i // Figure out the vertical size and position of the pum. pum_compute_vertical_placement(size, target_win, pum_win_row, above_row, below_row, - pum_border_size); + border_width); // don't display when we only have room for one line - if (pum_border_size == 0 && (pum_height < 1 || (pum_height == 1 && size > 1))) { + if (border_width == 0 && (pum_height < 1 || (pum_height == 1 && size > 1))) { return; } @@ -413,8 +418,8 @@ void pum_display(pumitem_T *array, int size, int selected, bool array_changed, i // Figure out the horizontal size and position of the pum. pum_compute_horizontal_placement(target_win, cursor_col); - if (pum_col + pum_border_size + pum_width > Columns) { - pum_col -= pum_border_size; + if (pum_col + border_width + pum_width > Columns) { + pum_col -= border_width; } // Set selected item and redraw. If the window size changed need to redo @@ -602,10 +607,9 @@ void pum_redraw(void) } WinConfig fconfig = WIN_CONFIG_INIT; - int pum_border_width = 0; + int border_width = pum_border_width(); // setup popup menu border if 'pumborder' option is set - if (*p_pumborder != NUL) { - pum_border_width = 2; + if (border_width > 0) { fconfig.border = true; Error err = ERROR_INIT; if (!parse_winborder(&fconfig, p_pumborder, &err)) { @@ -619,7 +623,6 @@ void pum_redraw(void) // Shadow style: only adds border on right and bottom edges if (strequal(p_pumborder, opt_winborder_values[3])) { fconfig.shadow = true; - pum_border_width = 1; int blend = SYN_GROUP_STATIC("PmenuShadow"); int through = SYN_GROUP_STATIC("PmenuShadowThrough"); fconfig.border_hl_ids[2] = through; @@ -644,14 +647,14 @@ void pum_redraw(void) pum_left_col = pum_col - col_off; pum_right_col = pum_left_col + grid_width; bool moved = ui_comp_put_grid(&pum_grid, pum_row, pum_left_col, - pum_height + pum_border_width, grid_width + pum_border_width, false, + pum_height + border_width, grid_width + border_width, false, true); bool invalid_grid = moved || pum_invalid; pum_invalid = false; must_redraw_pum = false; if (!pum_grid.chars || pum_grid.rows != pum_height || pum_grid.cols != grid_width) { - grid_alloc(&pum_grid, pum_height + pum_border_width, grid_width + pum_border_width, + grid_alloc(&pum_grid, pum_height + border_width, grid_width + border_width, !invalid_grid, false); ui_call_grid_resize(pum_grid.handle, pum_grid.cols, pum_grid.rows); } else if (invalid_grid) { @@ -950,8 +953,8 @@ static void pum_preview_set_text(buf_T *buf, char *info, linenr_T *lnum, int *ma /// adjust floating info preview window position static void pum_adjust_info_position(win_T *wp, int width) { - int extra_width = *p_pumborder != NUL ? 2 : 0; - int col = pum_col + pum_width + pum_scrollbar + 1 + extra_width; + int border_width = pum_border_width(); + int col = pum_col + pum_width + pum_scrollbar + 1 + border_width; // TODO(glepnir): support config align border by using completepopup // align menu int right_extra = Columns - col; diff --git a/test/functional/ui/popupmenu_spec.lua b/test/functional/ui/popupmenu_spec.lua index a3baf622ed..0fece8049e 100644 --- a/test/functional/ui/popupmenu_spec.lua +++ b/test/functional/ui/popupmenu_spec.lua @@ -9149,7 +9149,7 @@ describe('builtin popupmenu', function() ]]) end end) - it('pumborder with shadow', function() + it("'pumborder' with shadow", function() command('set pumborder=shadow') feed('S') if multigrid then @@ -9182,7 +9182,7 @@ describe('builtin popupmenu', function() }, float_pos = { [5] = { -1, 'NW', 2, 1, 0, false, 100, 2, 1, 0 }, - [4] = { 1001, 'NW', 1, 1, 17, false, 50, 1, 1, 17 }, + [4] = { 1001, 'NW', 1, 1, 16, false, 50, 1, 1, 16 }, }, win_viewport = { [2] = { @@ -9224,7 +9224,7 @@ describe('builtin popupmenu', function() else screen:expect([[ one^ | - {12:one }{114: }{1: }{n:1info}{1: }| + {12:one }{114: }{n:1info}{1: }| {n:two }{115: }{1: }| {n:three }{115: }{1: }| {114: }{115: }{1: }| @@ -9233,6 +9233,81 @@ describe('builtin popupmenu', function() ]]) end end) + it("'pumborder' with none #36207", function() + command('set wildoptions=pum pumborder=none') + feed(':') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + [2:------------------------------]|*10 + [3:------------------------------]| + ## grid 2 + | + {1:~ }|*9 + ## grid 3 + :!^ | + ## grid 4 + {12: ! }{c: }| + {n: # }{12: }| + {n: & }{12: }| + {n: < }{12: }| + {n: = }{12: }| + {n: > }{12: }| + {n: @ }{12: }| + {n: Next }{12: }| + {n: abbreviate }{12: }| + {n: abclear }{12: }| + ]], + win_pos = { + [2] = { + height = 10, + startcol = 0, + startrow = 0, + width = 30, + win = 1000, + }, + }, + float_pos = { + [4] = { -1, 'SW', 1, 10, 0, false, 250, 2, 0, 0 }, + }, + win_viewport = { + [2] = { + win = 1000, + topline = 0, + botline = 2, + curline = 0, + curcol = 0, + linecount = 1, + sum_scroll_delta = 0, + }, + }, + win_viewport_margins = { + [2] = { + bottom = 0, + left = 0, + right = 0, + top = 0, + win = 1000, + }, + }, + }) + else + screen:expect([[ + {12: ! }{c: } | + {n: # }{12: }{1: }| + {n: & }{12: }{1: }| + {n: < }{12: }{1: }| + {n: = }{12: }{1: }| + {n: > }{12: }{1: }| + {n: @ }{12: }{1: }| + {n: Next }{12: }{1: }| + {n: abbreviate }{12: }{1: }| + {n: abclear }{12: }{1: }| + :!^ | + ]]) + end + end) end) end