Rewrite popup menu positioning logic and fix bugs by girishji · Pull Request #18441 · vim/vim

girishji · 2025-09-29T18:52:01Z

Refactor

Simplified and rewrote horizontal positioning logic (was overly complex).
Split horizontal and vertical positioning into separate functions.

Bug fixes

Fixed missing truncation marker (e.g. >) when items were truncated and pummaxwidth was not set.
Fixed occasional extra space being added to menu items.

- Simplified and rewrote horizontal positioning logic (was overly complex). - Split horizontal and vertical positioning into separate functions. - Fixed missing truncation marker (e.g. >) when items were truncated and pummaxwidth was not set. - Fixed occasional extra space being added to menu items.

src/popupmenu.c

M src/popupmenu.c M src/testdir/test_popupwin.vim

M src/testdir/test_popupwin.vim

girishji · 2025-09-30T08:11:43Z

Test_popupwin_infopopup_6.dump and Test_popupwin_infopopup_7.dump are already incorrect:
they show the info window but not the popup menu.

This issue occurs only in the test; I cannot reproduce it otherwise.
In Test_popupmenu_info_border(), setting of pumheight=3 causes the problem.
Without pumheight, the popup menu displays correctly, but pumheight is needed for the test.

If anyone has suggestions, please let me know.
Otherwise I’ll work around it by adding :set completepopup=border:off, which avoids the issue.

`Test_popupwin_infopopup_6.dump` and `Test_popupwin_infopopup_7.dump` are already incorrect: they show the info window but not the popup menu. This issue occurs only in the test; I cannot reproduce it otherwise. In `Test_popupmenu_info_border()`, setting of `pumheight=3` causes the problem. Without `pumheight`, the popup menu displays correctly, but `pumheight` is needed for the test. Work around is to add `:set completepopup=border:off` to failing tests.

chrisbra · 2025-10-01T20:50:31Z

thanks

girishji · 2025-10-02T05:38:27Z

thanks.

zeertzjq · 2025-10-02T09:17:44Z

This regresses the following change from #17081:

diff --git a/src/testdir/dumps/Test_pum_maxwidth_07.dump b/src/testdir/dumps/Test_pum_maxwidth_07.dump
index ada8acb0d..112e1f588 100644
--- a/src/testdir/dumps/Test_pum_maxwidth_07.dump
+++ b/src/testdir/dumps/Test_pum_maxwidth_07.dump
@@ -1,7 +1,7 @@
 |1+0&#ffffff0|2|3|4|5|6|7|8|9|_|1|2|3|4|5|6|7|8|9|_|1|2|3|4|5|6|7|8|9|_> @44
 |1+0#0000001#e0e0e08|2|3|4|5|6|7|8|9|>| +0#4040ff13#ffffff0@64
 |一*0#0000001#ffd7ff255|二|三|四| +&|>| +0#4040ff13#ffffff0@64
-|a+0#0000001#ffd7ff255|b|c|d|e|f|g|h|i|>| +0#4040ff13#ffffff0@64
+|a+0#0000001#ffd7ff255|b|c|d|e|f|g|h|i|j| +0#4040ff13#ffffff0@64
 |上*0#0000001#ffd7ff255|下|左|右| +&@1| +0#4040ff13#ffffff0@64
 |~| @73
 |~| @73
diff --git a/src/testdir/dumps/Test_pum_maxwidth_08.dump b/src/testdir/dumps/Test_pum_maxwidth_08.dump
index aa41b76d1..9f92ae706 100644
--- a/src/testdir/dumps/Test_pum_maxwidth_08.dump
+++ b/src/testdir/dumps/Test_pum_maxwidth_08.dump
@@ -1,7 +1,7 @@
 | +0&#ffffff0@43> |_|9|8|7|6|5|4|3|2|1|_|9|8|7|6|5|4|3|2|1|_|9|8|7|6|5|4|3|2|1
 | +0#4040ff13&@64|<+0#0000001#e0e0e08|9|8|7|6|5|4|3|2|1
 | +0#4040ff13#ffffff0@64|<+0#0000001#ffd7ff255| |四*&|三|二|一
-| +0#4040ff13#ffffff0@64|<+0#0000001#ffd7ff255|i|h|g|f|e|d|c|b|a
+| +0#4040ff13#ffffff0@64|j+0#0000001#ffd7ff255|i|h|g|f|e|d|c|b|a
 | +0#4040ff13#ffffff0@64| +0#0000001#ffd7ff255@1|右*&|左|下|上
 | +0#4040ff13#ffffff0@73|~
 | @73|~

Now abcdefghij is truncated even though it fits within 'pummaxwidth'.

girishji · 2025-10-02T09:33:05Z

There’s currently an extra space (or truncation character) after each item. If j is included, that space won’t appear. I wasn’t entirely sure how to handle this, but since pummaxwidth is set to 10, I think it’s fine to skip the extra space. I’ll address this in a separate PR.

zeertzjq · 2025-10-02T09:43:49Z

The regression is more obvious in the right-click menu (seen in the two Test_mouse_popup_position dumps in this PR, where the last character of the longest menu item is truncated). The following script (where all menu items has the same length) makes it even more obvious:

menu PopUp.aaaaaaaaaafoo :echo 'foo'<CR>
menu PopUp.bbbbbbbbbbfoo :echo 'foo'<CR>
menu PopUp.ccccccccccfoo :echo 'foo'<CR>
set mousemodel=popup

Right click near the right edge of the window, and you'll see that all of them are truncated at the last character:

girishji · 2025-10-02T12:25:10Z

It's the same issue. I think removing - 1 will fix this.

    truncated = width_limit - totwidth - 1 < cells + pad;

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'* '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:XXXXXXXX" use eight characters given after "custom:", in 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 vim#18441 (comment) *(Add screenshot)*

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'* '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:XXXXXXXX" use eight characters given after "custom:", in 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 vim#18441 (comment)

Problem: not possible to style popup borders globally Solution: Add the 'pumborder' option (Girish Palya) This commit introduces a new global option, 'pumborder' ('pb'), that allows users to define borders and optional decorations for the completion popup menu. ``` 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"singleuse thin box-drawing characters "double"doubleuse double-line box-drawing characters "round"rounduse rounded corners "ascii"asciiuse ASCII characters (-, |, +) "custom:XXXXXXXX" use eight characters given after "custom:", in order: top, right, bottom, left, topleft, topright, botright, botleft Additional flags: "margin"marginadds one-cell spacing inside the left and right border "shadow"shadowdraws a shadow at the right and bottom edges Highlight groups: |hl-PmenuBorder|hl-PmenuBorderused for the border characters |hl-PmenuShadow|hl-PmenuShadowused 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|. ``` fixes: #18441 (comment) closes: #18486 closes: #17091 Signed-off-by: Girish Palya <[email protected]> Signed-off-by: Christian Brabandt <[email protected]>

Problem: not possible to style popup borders globally Solution: Add the 'pumborder' option (Girish Palya) This commit introduces a new global option, 'pumborder' ('pb'), that allows users to define borders and optional decorations for the completion popup menu. ``` 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"singleuse thin box-drawing characters "double"doubleuse double-line box-drawing characters "round"rounduse rounded corners "ascii"asciiuse ASCII characters (-, |, +) "custom:XXXXXXXX" use eight characters given after "custom:", in order: top, right, bottom, left, topleft, topright, botright, botleft Additional flags: "margin"marginadds one-cell spacing inside the left and right border "shadow"shadowdraws a shadow at the right and bottom edges Highlight groups: |hl-PmenuBorder|hl-PmenuBorderused for the border characters |hl-PmenuShadow|hl-PmenuShadowused 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|. ``` fixes: vim#18441 (comment) closes: vim#18486 closes: vim#17091 Signed-off-by: Girish Palya <[email protected]> Signed-off-by: Christian Brabandt <[email protected]>

Problem: popup: there are some position logic bugs Solution: Refactor position logic and fix a few bugs (Girish Palya). This change does the following: - Simplified and rewrote horizontal positioning logic (was overly complex). - Split horizontal and vertical positioning into separate functions. - Fixed missing truncation marker (e.g. `>`) when items were truncated and `pummaxwidth` was not set. - Fixed occasional extra space being added to menu items. - Update tests closes: vim/vim#18441 vim/vim@e3ed558 Cherry-pick pum_display_{rtl,ltr}_text() changes from patch 9.1.1835. Co-authored-by: Girish Palya <[email protected]>

girishji force-pushed the refactor branch from 8a388d0 to 6599570 Compare September 29, 2025 18:54

zeertzjq reviewed Sep 30, 2025

View reviewed changes

src/popupmenu.c Outdated Show resolved Hide resolved

girishji added 2 commits September 30, 2025 04:03

Improve comment and fix test failure

d656595

M src/popupmenu.c M src/testdir/test_popupwin.vim

Undo test change

1cbcb01

M src/testdir/test_popupwin.vim

girishji added 2 commits September 30, 2025 10:22

Merge branch 'vim:master' into refactor

96dc6c8

chrisbra closed this in e3ed558 Oct 1, 2025

girishji deleted the refactor branch October 2, 2025 05:38

girishji mentioned this pull request Oct 4, 2025

Feat: Add border for popup menu ('pumborder' option) #18486

Closed

zeertzjq mentioned this pull request Oct 8, 2025

vim-patch:9.1.1817: popup: there are some position logic bugs neovim/neovim#36075

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rewrite popup menu positioning logic and fix bugs#18441

Rewrite popup menu positioning logic and fix bugs#18441
girishji wants to merge 5 commits intovim:masterfrom
girishji:refactor

girishji commented Sep 29, 2025

Uh oh!

Uh oh!

girishji commented Sep 30, 2025

Uh oh!

chrisbra commented Oct 1, 2025

Uh oh!

girishji commented Oct 2, 2025

Uh oh!

zeertzjq commented Oct 2, 2025 •

edited

Loading

Uh oh!

girishji commented Oct 2, 2025 •

edited

Loading

Uh oh!

zeertzjq commented Oct 2, 2025 •

edited

Loading

Uh oh!

girishji commented Oct 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

girishji commented Sep 29, 2025

Refactor

Bug fixes

Uh oh!

Uh oh!

girishji commented Sep 30, 2025

Uh oh!

chrisbra commented Oct 1, 2025

Uh oh!

girishji commented Oct 2, 2025

Uh oh!

zeertzjq commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

girishji commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeertzjq commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

girishji commented Oct 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zeertzjq commented Oct 2, 2025 •

edited

Loading

girishji commented Oct 2, 2025 •

edited

Loading

zeertzjq commented Oct 2, 2025 •

edited

Loading