[vim/vim] Feat: Add border for popup menu ('pumborder' option) (PR #18486)

28 views
Skip to first unread message

girish

unread,
Oct 4, 2025, 4:28:31 AM (3 days ago) Oct 4
to vim/vim, Subscribed

This PR introduces a new global option, 'pumborder' ('pb'), that allows
users to define borders and optional decorations for the completion popup
menu.

'pumborder' 'pb'	number	(default 0)
			global
	Defines a border and optional decorations for the popup menu in
	completion.  The value is a comma-separated list of keywords.

	Border styles (at most one):
	"single"	use thin box-drawing characters
	"double"	use double-line box-drawing characters
	"round"		use rounded corners
	"ascii"		use ASCII characters (-, |, +)
	"custom:X;X;X;X;X;X;X;X"
			use eight characters separated by semicolons, in the
			order: top, right, bottom, left,
			topleft, topright, botright, botleft

	Additional flags:
	"margin"	adds one-cell spacing inside the left and right border
	"shadow"	draws a shadow at the right and bottom edges

	Highlight groups:
	|hl-PmenuBorder|	used for the border characters
	|hl-PmenuShadow|	used for the shadow

	Examples: >
		:set pumborder=single
		:set pumborder=double,margin,shadow
		:set pumborder=custom:─;│;─;│;┌;┐;┘;└,shadow
<
	Border styles using box-drawing characters ("single", "double",
	"round") are only available when |'encoding'| is "utf-8" and
	|'ambiwidth'| is "single".  "margin" requires a border style.
	See also: |ins-completion-menu|.

Also, fixes #18441 (comment)

Screenshot.2025-09-24.at.10.55.21.AM.png (view on web)

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/18486

Commit Summary

  • 62371e0 Feat: Add border for popup menu (`'pumborder'` option)
  • 62fcf13 Removed unused var
  • 1b68c4e Merge branch 'vim:master' into border2
  • 7461f07 Add shadow to info window
  • 940d68e Require semicolon between box characters
  • 7d3220d Undo shadow for info window
  • f5c0b24 Add test

File Changes

(72 files)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486@github.com>

girish

unread,
Oct 4, 2025, 4:52:36 AM (3 days ago) Oct 4
to vim/vim, Push

@girishji pushed 1 commit.

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/before/e96c681039a4dba22cc5f4d39463274f4bfedcfe/after/6b59642cdf7c6f4a355de91c0207bb61e8c95c22@github.com>

girish

unread,
Oct 4, 2025, 4:57:49 AM (3 days ago) Oct 4
to vim/vim, Push

@girishji pushed 1 commit.

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/before/6b59642cdf7c6f4a355de91c0207bb61e8c95c22/after/3cbe43f0b551959b1926f1a584b4f53f04decc7e@github.com>

girish

unread,
Oct 4, 2025, 4:14:08 PM (3 days ago) Oct 4
to vim/vim, Push

@girishji pushed 1 commit.

  • 4c58b9c Merge branch 'vim:master' into border2

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/before/3cbe43f0b551959b1926f1a584b4f53f04decc7e/after/4c58b9c951cecdcabe0ffe5a6b6f82b82399c30a@github.com>

Maxim Kim

unread,
Oct 4, 2025, 8:26:13 PM (3 days ago) Oct 4
to vim/vim, Subscribed
habamax left a comment (vim/vim#18486)

huh, looks good

image.png (view on web) image.png (view on web) image.png (view on web)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/c3368627807@github.com>

Christian Brabandt

unread,
Oct 5, 2025, 9:21:28 AM (2 days ago) Oct 5
to vim/vim, Subscribed
chrisbra left a comment (vim/vim#18486)

So this is essentially the same as #17091 ?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/c3369054726@github.com>

Shane-XB-Qian

unread,
Oct 5, 2025, 9:31:22 AM (2 days ago) Oct 5
to vim/vim, Subscribed
Shane-XB-Qian left a comment (vim/vim#18486)
yes and no, if you checked that I referred that lsp ticket, that had been lived OK without this, but if you merged this PR, someone may need to redo that prev code for lsp.
to vim core itself, it can config pum via set_option, seems not have to have (this PR).

--
shane.xb.qian

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/c3369060554@github.com>

girish

unread,
Oct 5, 2025, 12:13:13 PM (2 days ago) Oct 5
to vim/vim, Subscribed
girishji left a comment (vim/vim#18486)

@chrisbra This PR builds on #18441, which simplifies the positioning logic, making it easier to add borders and shadows without introducing bugs.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/c3369157325@github.com>

Copilot

unread,
Oct 6, 2025, 2:58:46 PM (20 hours ago) Oct 6
to vim/vim, Subscribed

@Copilot commented on this pull request.

Pull Request Overview

This PR introduces a new global option 'pumborder' ('pb') that allows users to define borders and optional decorations for the completion popup menu. This enhancement provides visual improvements to the popup menu by adding customizable borders, margins, and shadows.

  • Adds the 'pumborder' option with support for different border styles (single, double, round, ascii, custom)
  • Implements margin and shadow features for the popup menu
  • Adds new highlight groups PmenuBorder and PmenuShadow for customization

Reviewed Changes

Copilot reviewed 73 out of 73 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/vim.h Adds new highlight group constants for popup menu border and shadow
src/popupmenu.c Core implementation of border drawing, shadow effects, and popup positioning logic
src/optionstr.c Handles parsing and validation of the pumborder option
src/optiondefs.h Defines the new pumborder option in the options table
src/option.h Declares the global variable for the pumborder option
src/highlight.c Sets up default highlight groups and color schemes for border and shadow
runtime/ files Updates documentation, syntax highlighting, and option window support
src/testdir/ files Adds comprehensive tests and screen dumps for the new functionality
Comments suppressed due to low confidence (1)

src/popupmenu.c:211

  • The condition changed from width > p_pmw to width >= p_pmw. This means when width equals p_pmw, it will now be truncated and end_padding disabled. This could be a subtle behavior change that affects popup menu rendering. Verify this change is intentional and doesn't break existing functionality.
    if (p_pmw > 0 && width >= p_pmw)
    {
	width = p_pmw;
	end_padding = FALSE;
    }

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

In src/testdir/test_options.vim:

> @@ -637,7 +637,7 @@ func Test_set_completion_string_values()
   set keyprotocol&
 
   " previewpopup / completepopup

This change appears to be updating test expectations due to reordering of completion options. While the change itself is correct, consider adding a comment explaining why 'align:' is now expected first instead of 'height:' to help future maintainers understand this ordering dependency.

⬇️ Suggested change 
-  " previewpopup / completepopup
+  " previewpopup / completepopup
+  " The completion options for 'set previewpopup=' are order-dependent..
+  " 'align:' is currently the first option due to internal ordering.
+  " If the order changes in the future, update this test and document the reason.

In src/testdir/test_highlight.vim:

> @@ -820,7 +820,7 @@ endfunc
 " Test for 'highlight' option
 func Test_highlight_opt()
   let save_hl = &highlight

The change from 'j:b' to 'K:b' in this test suggests that 'j' is now a valid highlight group identifier (likely for PmenuBorder). Consider adding a comment explaining that 'j' was previously invalid but is now used for PmenuBorder highlighting.

⬇️ Suggested change 
-  let save_hl = &highlight
+  let save_hl = &highlight
+  " Previously, 'j' was an invalid highlight group identifier and was used in this test.
+  " Now, 'j' is valid (used for PmenuBorder), so we use 'K' as an invalid identifier.

In src/popupmenu.c:

> @@ -568,7 +609,7 @@ pum_display_rtl_text(
 
     char_u *rt_start = rt;
     cells = mb_string2cells(rt, -1);

The removal of '- 1' from the truncation calculation appears in multiple places. While this might be correct for the new border logic, consider adding a comment explaining why this adjustment was needed to help future maintainers understand the calculation.

⬇️ Suggested change 
-    cells = mb_string2cells(rt, -1);
+    cells = mb_string2cells(rt, -1);
+    // The '- 1' was removed from the truncation calculation below to match the new border logic.
+    // This ensures that the calculation correctly accounts for the available space, including borders and padding.
+    // If you change the border or padding logic, revisit this calculation.

In src/popupmenu.c:

> @@ -209,34 +255,29 @@ pum_compute_horizontal_placement(int cursor_col)
 	return;
     }
 
-    // Truncated pum is no longer aligned with "cursor_col"
-#ifdef FEAT_RIGHTLEFT
-    if (pum_rl)
-	available_width = Columns - pum_scrollbar;
-    else
-#endif
-	available_width += cursor_col;
+    // Case 2: Truncated pum is no longer aligned with "cursor_col"
+    available_width = Columns - trailing_overhead - col_offset;
 
     if (available_width > p_pw)
     {

The removal of '+ 1' from p_pw + 1 changes the popup width calculation. This appears to be related to border adjustments, but consider adding a more detailed comment explaining why this change was necessary and how it relates to the new border feature.

⬇️ Suggested change 
-    {
+    {
+	// Previously, pum_width was set to p_pw + 1 to account for the border.
+	// With the new border feature, the border is now handled separately,
+	// so the '+ 1' is no longer needed here. This ensures the popup width
+	// calculation is accurate and avoids off-by-one errors with borders.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/review/3306662538@github.com>

Christian Brabandt

unread,
Oct 6, 2025, 3:26:07 PM (19 hours ago) Oct 6
to vim/vim, Subscribed
chrisbra left a comment (vim/vim#18486)

Does this also apply to popups or do you plan to make use of it for popups?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/c3373546259@github.com>

girish

unread,
2:47 AM (8 hours ago) 2:47 AM
to vim/vim, Subscribed

@girishji commented on this pull request.

In src/popupmenu.c:

> @@ -568,7 +609,7 @@ pum_display_rtl_text(
 
     char_u *rt_start = rt;
     cells = mb_string2cells(rt, -1);

not needed.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/review/3308650351@github.com>

girish

unread,
2:48 AM (8 hours ago) 2:48 AM
to vim/vim, Subscribed

@girishji commented on this pull request.

In src/testdir/test_highlight.vim:

> @@ -820,7 +820,7 @@ endfunc
 " Test for 'highlight' option
 func Test_highlight_opt()
   let save_hl = &highlight

not needed.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/review/3308651897@github.com>

girish

unread,
2:48 AM (8 hours ago) 2:48 AM
to vim/vim, Subscribed

@girishji commented on this pull request.

In src/testdir/test_options.vim:

> @@ -637,7 +637,7 @@ func Test_set_completion_string_values()
   set keyprotocol&
 
   " previewpopup / completepopup

not needed.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/review/3308652767@github.com>

girish

unread,
2:49 AM (8 hours ago) 2:49 AM
to vim/vim, Subscribed
girishji left a comment (vim/vim#18486)

Does this also apply to popups or do you plan to make use of it for popups?

popupwin.c already has border. Only popupmenu.c didn't have border, and now it does.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/18486/c3375449417@github.com>

Reply all
Reply to author
Forward
0 new messages