fix(ui): 'pumborder' parsing, shadow #36127

Problem:
1. Setting `pumborder=+,+,+,+,+,+,+,+` failed to render the custom
   border characters correctly. The issue occurred in `parse_winborder()`
   where it incorrectly used `p_winborder` instead of the `border_opt`
   parameter when the option value didn't contain commas.
2. In `pum_redraw()`, calling `parse_border_style()` directly with the
   option string failed to parse comma-separated border characters.
3. Missing documentation for PmenuShadow and PmenuShadowThrough
   highlight groups used by the shadow border style.
4. Coverity reports CID 631420: passing WinConfig (480 bytes) by value
   in `grid_draw_border()`.
5. crash when using `shadow` value on pumborder.

Solution:
1. Fix `parse_winborder()` to use `border_opt` parameter consistently,
   ensuring the correct option value is parsed regardless of which
   option (winborder/pumborder) is being set.
2. Update `pum_redraw()` to call `parse_winborder()` instead of
   `parse_border_style()`, properly handling both predefined styles
   and custom comma-separated border characters.
3. Add documentation for PmenuShadow (blended shadow areas) and
   PmenuShadowThrough (see-through corners) highlight groups.
4. Change `grid_draw_border()` to accept WinConfig by pointer.
5. When the "shadow" style is used, no additional row and column offset
   is applied, and the border width is reduced.
This commit is contained in:
glepnir
2025-10-12 10:48:27 +08:00
committed by GitHub
parent c7fd0c17b1
commit 072f126453
12 changed files with 169 additions and 29 deletions

View File

@@ -219,6 +219,7 @@ HIGHLIGHTS
• |hl-DiffTextAdd| highlights added text within a changed line. • |hl-DiffTextAdd| highlights added text within a changed line.
• |hl-OkMsg| |hl-StderrMsg| |hl-StdoutMsg| • |hl-OkMsg| |hl-StderrMsg| |hl-StdoutMsg|
• |hl-SnippetTabstopActive| highlights the currently active tabstop. • |hl-SnippetTabstopActive| highlights the currently active tabstop.
• |hl-PmenuBorder |hl-PmenuShadow| |hl-PmenuShadowThrough| see 'pumborder'.
LSP LSP
@@ -301,6 +302,7 @@ OPTIONS
• 'winborder' "bold" style, custom border style. • 'winborder' "bold" style, custom border style.
• |g:clipboard| accepts a string name to force any builtin clipboard tool. • |g:clipboard| accepts a string name to force any builtin clipboard tool.
• 'busy' sets a buffer "busy" status. Indicated in the default statusline. • 'busy' sets a buffer "busy" status. Indicated in the default statusline.
• 'pumborder' add a border to the popup menu.
PERFORMANCE PERFORMANCE

View File

@@ -4912,7 +4912,8 @@ A jump table for the options with a short description can be found at |Q_op|.
'pumborder' string (default "") 'pumborder' string (default "")
global global
Defines the default border style of popupmenu windows. Same as Defines the default border style of popupmenu windows. Same as
'winborder'. 'winborder'. |hl-PmenuBorder| is used. When style is "shadow", the
|hl-PmenuShadow| and |hl-PmenuShadowThrough| are used.
*'pumheight'* *'ph'* *'pumheight'* *'ph'*
'pumheight' 'ph' number (default 0) 'pumheight' 'ph' number (default 0)

View File

@@ -5354,6 +5354,11 @@ Normal Normal text.
NormalFloat Normal text in floating windows. NormalFloat Normal text in floating windows.
*hl-FloatBorder* *hl-FloatBorder*
FloatBorder Border of floating windows. FloatBorder Border of floating windows.
*hl-FloatShadow*
FloatShadow Blended areas when border is shadow.
*hl-FLoatShadowThrough*
FloatShadownThrough
shadow corners when border is shadow.
*hl-FloatTitle* *hl-FloatTitle*
FloatTitle Title of floating windows. FloatTitle Title of floating windows.
*hl-FloatFooter* *hl-FloatFooter*
@@ -5382,6 +5387,13 @@ PmenuMatch Popup menu: Matched text in normal item. Combined with
*hl-PmenuMatchSel* *hl-PmenuMatchSel*
PmenuMatchSel Popup menu: Matched text in selected item. Combined with PmenuMatchSel Popup menu: Matched text in selected item. Combined with
|hl-PmenuMatch| and |hl-PmenuSel|. |hl-PmenuMatch| and |hl-PmenuSel|.
*hl-PmenuBorder*
PmenuBorder Popup menu: border of popup menu.
*hl-PmenuShadow*
PmenuShadow Popup menu: blended areas when 'pumborder' is shadow.
*hl-PmenuShadowThrough*
PmenuShadownThrough
Popup menu: shadow corners when 'pumborder' is shadow.
*hl-ComplMatchIns* *hl-ComplMatchIns*
ComplMatchIns Matched text of the currently inserted completion. ComplMatchIns Matched text of the currently inserted completion.
*hl-PreInsert* *hl-PreInsert*

View File

@@ -5130,7 +5130,8 @@ vim.go.pumblend = vim.o.pumblend
vim.go.pb = vim.go.pumblend vim.go.pb = vim.go.pumblend
--- Defines the default border style of popupmenu windows. Same as --- Defines the default border style of popupmenu windows. Same as
--- 'winborder'. --- 'winborder'. `hl-PmenuBorder` is used. When style is "shadow", the
--- `hl-PmenuShadow` and `hl-PmenuShadowThrough` are used.
--- ---
--- @type string --- @type string
vim.o.pumborder = "" vim.o.pumborder = ""

View File

@@ -1079,7 +1079,7 @@ static void generate_api_error(win_T *wp, const char *attribute, Error *err)
} }
/// Parses a border style name or custom (comma-separated) style. /// Parses a border style name or custom (comma-separated) style.
bool parse_winborder(WinConfig *fconfig, const char *border_opt, Error *err) bool parse_winborder(WinConfig *fconfig, char *border_opt, Error *err)
{ {
if (!fconfig) { if (!fconfig) {
return false; return false;
@@ -1088,7 +1088,7 @@ bool parse_winborder(WinConfig *fconfig, const char *border_opt, Error *err)
if (strchr(border_opt, ',')) { if (strchr(border_opt, ',')) {
Array border_chars = ARRAY_DICT_INIT; Array border_chars = ARRAY_DICT_INIT;
char *p = p_winborder; char *p = border_opt;
char part[MAX_SCHAR_SIZE] = { 0 }; char part[MAX_SCHAR_SIZE] = { 0 };
int count = 0; int count = 0;
@@ -1116,7 +1116,7 @@ bool parse_winborder(WinConfig *fconfig, const char *border_opt, Error *err)
style = ARRAY_OBJ(border_chars); style = ARRAY_OBJ(border_chars);
} else { } else {
style = CSTR_TO_OBJ(p_winborder); style = CSTR_TO_OBJ(border_opt);
} }
parse_border_style(style, fconfig, err); parse_border_style(style, fconfig, err);

View File

@@ -658,7 +658,7 @@ int update_screen(void)
win_grid_alloc(wp); win_grid_alloc(wp);
if (wp->w_redr_border || wp->w_redr_type >= UPD_NOT_VALID) { if (wp->w_redr_border || wp->w_redr_type >= UPD_NOT_VALID) {
grid_draw_border(&wp->w_grid_alloc, wp->w_config, wp->w_border_adj, (int)wp->w_p_winbl, grid_draw_border(&wp->w_grid_alloc, &wp->w_config, wp->w_border_adj, (int)wp->w_p_winbl,
wp->w_ns_hl_attr); wp->w_ns_hl_attr);
} }

View File

@@ -1115,9 +1115,9 @@ static int get_bordertext_col(int total_col, int text_width, AlignTextPos align)
} }
/// draw border on floating window grid /// draw border on floating window grid
void grid_draw_border(ScreenGrid *grid, WinConfig config, int *adj, int winbl, int *hl_attr) void grid_draw_border(ScreenGrid *grid, WinConfig *config, int *adj, int winbl, int *hl_attr)
{ {
int *attrs = config.border_attr; int *attrs = config->border_attr;
int default_adj[4] = { 1, 1, 1, 1 }; int default_adj[4] = { 1, 1, 1, 1 };
if (adj == NULL) { if (adj == NULL) {
adj = default_adj; adj = default_adj;
@@ -1128,7 +1128,7 @@ void grid_draw_border(ScreenGrid *grid, WinConfig config, int *adj, int winbl, i
} }
for (int i = 0; i < 8; i++) { for (int i = 0; i < 8; i++) {
chars[i] = schar_from_str(config.border_chars[i]); chars[i] = schar_from_str(config->border_chars[i]);
} }
int irow = grid->rows - adj[0] - adj[2]; int irow = grid->rows - adj[0] - adj[2];
@@ -1144,9 +1144,9 @@ void grid_draw_border(ScreenGrid *grid, WinConfig config, int *adj, int winbl, i
grid_line_put_schar(i + adj[3], chars[1], attrs[1]); grid_line_put_schar(i + adj[3], chars[1], attrs[1]);
} }
if (config.title) { if (config->title) {
int title_col = get_bordertext_col(icol, config.title_width, config.title_pos); int title_col = get_bordertext_col(icol, config->title_width, config->title_pos);
grid_draw_bordertext(config.title_chunks, title_col, winbl, hl_attr, kBorderTextTitle); grid_draw_bordertext(config->title_chunks, title_col, winbl, hl_attr, kBorderTextTitle);
} }
if (adj[1]) { if (adj[1]) {
grid_line_put_schar(icol + adj[3], chars[2], attrs[2]); grid_line_put_schar(icol + adj[3], chars[2], attrs[2]);
@@ -1179,9 +1179,9 @@ void grid_draw_border(ScreenGrid *grid, WinConfig config, int *adj, int winbl, i
grid_line_put_schar(i + adj[3], chars[ic], attrs[ic]); grid_line_put_schar(i + adj[3], chars[ic], attrs[ic]);
} }
if (config.footer) { if (config->footer) {
int footer_col = get_bordertext_col(icol, config.footer_width, config.footer_pos); int footer_col = get_bordertext_col(icol, config->footer_width, config->footer_pos);
grid_draw_bordertext(config.footer_chunks, footer_col, winbl, hl_attr, kBorderTextFooter); grid_draw_bordertext(config->footer_chunks, footer_col, winbl, hl_attr, kBorderTextFooter);
} }
if (adj[1]) { if (adj[1]) {
grid_line_put_schar(icol + adj[3], chars[4], attrs[4]); grid_line_put_schar(icol + adj[3], chars[4], attrs[4]);

View File

@@ -6727,7 +6727,8 @@ local options = {
values = { '', 'double', 'single', 'shadow', 'rounded', 'solid', 'bold', 'none' }, values = { '', 'double', 'single', 'shadow', 'rounded', 'solid', 'bold', 'none' },
desc = [=[ desc = [=[
Defines the default border style of popupmenu windows. Same as Defines the default border style of popupmenu windows. Same as
'winborder'. 'winborder'. |hl-PmenuBorder| is used. When style is "shadow", the
|hl-PmenuShadow| and |hl-PmenuShadowThrough| are used.
]=], ]=],
short_desc = N_('border of popupmenu'), short_desc = N_('border of popupmenu'),
type = 'string', type = 'string',

View File

@@ -2125,7 +2125,7 @@ const char *did_set_winbar(optset_T *args)
return did_set_statustabline_rulerformat(args, false, false); return did_set_statustabline_rulerformat(args, false, false);
} }
static bool parse_border_opt(const char *border_opt) static bool parse_border_opt(char *border_opt)
{ {
WinConfig fconfig = WIN_CONFIG_INIT; WinConfig fconfig = WIN_CONFIG_INIT;
Error err = ERROR_INIT; Error err = ERROR_INIT;

View File

@@ -298,6 +298,9 @@ void pum_display(pumitem_T *array, int size, int selected, bool array_changed, i
pum_rl = (State & MODE_CMDLINE) == 0 && curwin->w_p_rl; pum_rl = (State & MODE_CMDLINE) == 0 && curwin->w_p_rl;
int pum_border_size = *p_pumborder != NUL ? 2 : 0; int pum_border_size = *p_pumborder != NUL ? 2 : 0;
if (strequal(p_pumborder, opt_winborder_values[3])) {
pum_border_size -= 1;
}
do { do {
// Mark the pum as visible already here, // Mark the pum as visible already here,
@@ -605,19 +608,34 @@ void pum_redraw(void)
pum_border_width = 2; pum_border_width = 2;
fconfig.border = true; fconfig.border = true;
Error err = ERROR_INIT; Error err = ERROR_INIT;
parse_border_style(CSTR_AS_OBJ(p_pumborder), &fconfig, &err); if (!parse_winborder(&fconfig, p_pumborder, &err)) {
// shadow style uses different highlights for different positions if (ERROR_SET(&err)) {
if (strcmp(p_pumborder, opt_winborder_values[3]) == 0) { emsg(err.msg);
}
api_clear_error(&err);
return;
}
// 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 blend = SYN_GROUP_STATIC("PmenuShadow");
int through = SYN_GROUP_STATIC("PmenuShadowThrough"); int through = SYN_GROUP_STATIC("PmenuShadowThrough");
int attrs[] = { 0, 0, through, blend, blend, blend, through, 0 }; fconfig.border_hl_ids[2] = through;
memcpy(fconfig.border_attr, attrs, sizeof(attrs)); fconfig.border_hl_ids[3] = blend;
} else { fconfig.border_hl_ids[4] = blend;
// Non-shadow styles use PumBorder highlight for all border chars fconfig.border_hl_ids[5] = blend;
int attr = hl_attr_active[HLF_PBR]; fconfig.border_hl_ids[6] = through;
for (int i = 0; i < 8; i++) {
fconfig.border_attr[i] = attr;
} }
// convert border highlight IDs to attributes, use PmenuBorder as default
for (int i = 0; i < 8; i++) {
int attr = hl_attr_active[HLF_PBR];
if (fconfig.border_hl_ids[i]) {
attr = hl_get_ui_attr(-1, HLF_PBR, fconfig.border_hl_ids[i], false);
}
fconfig.border_attr[i] = attr;
} }
api_clear_error(&err); api_clear_error(&err);
} }
@@ -653,10 +671,12 @@ void pum_redraw(void)
// avoid set border for mouse menu // avoid set border for mouse menu
int mouse_menu = State != MODE_CMDLINE && pum_grid.zindex == kZIndexCmdlinePopupMenu; int mouse_menu = State != MODE_CMDLINE && pum_grid.zindex == kZIndexCmdlinePopupMenu;
if (!mouse_menu && fconfig.border) { if (!mouse_menu && fconfig.border) {
grid_draw_border(&pum_grid, fconfig, NULL, 0, NULL); grid_draw_border(&pum_grid, &fconfig, NULL, 0, NULL);
if (!fconfig.shadow) {
row++; row++;
col_off++; col_off++;
} }
}
// Never display more than we have // Never display more than we have
pum_first = MIN(pum_first, scroll_range); pum_first = MIN(pum_first, scroll_range);

View File

@@ -1061,6 +1061,8 @@ describe('builtin popupmenu', function()
[111] = { background = Screen.colors.Plum1, foreground = Screen.colors.DarkBlue }, [111] = { background = Screen.colors.Plum1, foreground = Screen.colors.DarkBlue },
[112] = { background = Screen.colors.Plum1, foreground = Screen.colors.DarkGreen }, [112] = { background = Screen.colors.Plum1, foreground = Screen.colors.DarkGreen },
[113] = { background = Screen.colors.Yellow, foreground = Screen.colors.Black }, [113] = { background = Screen.colors.Yellow, foreground = Screen.colors.Black },
[114] = { background = Screen.colors.Grey0, blend = 100 },
[115] = { background = Screen.colors.Grey0, blend = 80 },
-- popup non-selected item -- popup non-selected item
n = { background = Screen.colors.Plum1 }, n = { background = Screen.colors.Plum1 },
-- popup scrollbar knob -- popup scrollbar knob
@@ -9130,6 +9132,106 @@ describe('builtin popupmenu', function()
]]) ]])
end end
end) end)
it('custom pumborder characters', function()
command('set pumborder=+,+,=,+,+,-,+,+')
feed('S<C-x><C-o>')
if not multigrid then
screen:expect([[
one^ |
++++++++++++++++={n:1info}{1: }|
+{12:one }+{1: }|
+{n:two }+{1: }|
+{n:three }+{1: }|
+---------------+{1: }|
{1:~ }|*4
{5:-- }{6:match 1 of 3} |
]])
end
end)
it('pumborder with shadow', function()
command('set pumborder=shadow')
feed('S<C-x><C-o>')
if multigrid then
screen:expect({
grid = [[
## grid 1
[2:------------------------------]|*10
[3:------------------------------]|
## grid 2
one^ |
{1:~ }|*9
## grid 3
{5:-- }{6:match 1 of 3} |
## grid 4
{n:1info}|
## grid 5
{12:one }{114: }|
{n:two }{115: }|
{n:three }{115: }|
{114: }{115: }|
]],
win_pos = {
[2] = {
height = 10,
startcol = 0,
startrow = 0,
width = 30,
win = 1000,
},
},
float_pos = {
[5] = { -1, 'NW', 2, 1, 0, false, 100, 2, 1, 0 },
[4] = { 1001, 'NW', 1, 1, 17, false, 50, 1, 1, 17 },
},
win_viewport = {
[2] = {
win = 1000,
topline = 0,
botline = 2,
curline = 0,
curcol = 3,
linecount = 1,
sum_scroll_delta = 0,
},
[4] = {
win = 1001,
topline = 0,
botline = 1,
curline = 0,
curcol = 0,
linecount = 1,
sum_scroll_delta = 0,
},
},
win_viewport_margins = {
[2] = {
bottom = 0,
left = 0,
right = 0,
top = 0,
win = 1000,
},
[4] = {
bottom = 0,
left = 0,
right = 0,
top = 0,
win = 1001,
},
},
})
else
screen:expect([[
one^ |
{12:one }{114: }{1: }{n:1info}{1: }|
{n:two }{115: }{1: }|
{n:three }{115: }{1: }|
{114: }{115: }{1: }|
{1:~ }|*5
{5:-- }{6:match 1 of 3} |
]])
end
end)
end) end)
end end

View File

@@ -300,6 +300,7 @@ let test_values = {
\ 'alpha,hex,bin'], \ 'alpha,hex,bin'],
\ ['xxx']], \ ['xxx']],
\ 'patchmode': [['', 'xxx', '.x'], [&backupext, '*']], \ 'patchmode': [['', 'xxx', '.x'], [&backupext, '*']],
\ 'pumborder': [['rounded', 'none', 'single', 'solid'], ['xxx', 'none,solid']],
"\ 'previewpopup': [['', 'height:13', 'width:20', 'highlight:That', "\ 'previewpopup': [['', 'height:13', 'width:20', 'highlight:That',
"\ " 'align:item', 'align:menu', 'border:on', 'border:off', "\ " 'align:item', 'align:menu', 'border:on', 'border:off',
"\ " 'width:10,height:234,highlight:Mine'], "\ " 'width:10,height:234,highlight:Mine'],