[vim/vim] GTK4 buffers menu button does not look right (Issue #20262)

[Foxe Chen]

64-bitman created an issue (vim/vim#20262)

Steps to reproduce

Open the "Buffers" menu button at the top of the GTK4 GUI.

image.png (view on web)

It seems that the actions for each buffer is just placed in the main menu popover

Expected behaviour

Possibly create another menu on hover when hovering over one of the buffers in the menu?

Version of Vim

9.2.502

Environment

Operating system: Arch Linux, Wayland

Logs and stack traces

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262@github.com>

[Foxe Chen]

64-bitman left a comment (vim/vim#20262)

Hm, I can't reproduce anymore

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4494877079@github.com>

[Foxe Chen]

64-bitman left a comment (vim/vim#20262)

Ah nevermind, the buttons are duplicated in the menu when you press "Refresh menu"

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4494882854@github.com>

[Christian Brabandt]

chrisbra left a comment (vim/vim#20262)

Hm, that does not happen for me on my current Windows build. I think the issue might be in "$VIMRUNTIME/menu.vim" s:BMShow() function. Or may be GTK doesn't allow to delete items already present in the menu?

Actually no, it seems a GTK4 issue, this is what Claude tells me:

Found it

The GTK4 implementation is built on GMenu, and the comment at line 3635 says it outright:

// GMenu items cannot be individually removed easily.

gui_mch_destroy_menu() for a normal menu item (the menu->id == (GtkWidget *)1 case — every Refresh/Delete/Alternate/Next/Previous entry, since they're GMenu actions, not real widgets) does nothing to the GMenu. It just nulls menu->id and frees the action-name label. The actual g_menu_append'd item stays in the parent GMenu model forever.

So the flow on GTK4 is exactly what the screenshot shows:

1. BMShow() calls unmenu &Buffers → Vim walks its vimmenu_T list and calls gui_mch_destroy_menu() on each item → on GTK4 these are no-ops against the GMenu model. The visible items survive.
2. BMShow() re-adds Refresh/Delete/etc. via g_menu_append → second copy appended to the same GMenu.
3. Result: every entry doubles, including Dummy (which is also a GMenu action, removed from Vim's list but never from the model).

That's why it duplicates. Note: GTK3 uses the other file (gui_gtk.c), whose gui_mch_destroy_menu does properly gtk_widget_destroy. So if you're genuinely seeing it on GTK3 too, either that build is also routing through the GMenu code, or the GTK3 repro is a different trigger — worth re-confirming the GTK3 case. The GTK4 bug is unambiguous here.

The fix

The fix has to make gui_mch_destroy_menu() actually remove the item from the parent GMenu. GMenu does support removal — g_menu_remove(GMenu *menu, gint position) — the comment is just wrong that it "cannot be done easily." The hard part is tracking each item's index in the parent model, since g_menu_remove is position-based and positions shift as items are removed.

Two viable approaches:

1. Position tracking — store each item's position in the parent GMenu (e.g. in an unused vimmenu_T field or a side table), and in destroy_menu call g_menu_remove(parent_menu, pos), then fix up siblings' stored positions. Fiddly but precise.
2. Rebuild the section — compute the item's position at destroy time by walking the parent's vimmenu_T children to find its index among same-parent siblings, and g_menu_remove at that index. Avoids storing state but assumes Vim's list order matches the GMenu order (it should, since both are built in the same sequence).

Approach 2 is closer to how the GTK3 code already derives toolbar position via get_menu_position(menu). You likely have an equivalent index computation available.

Also worth noting

The separator path (lines 3562–3569) appends a new section every time a separator is created and never removes it either — so -SEP- will also leak/duplicate on refresh, same root cause.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4501324066@github.com>

[Foxe Chen]

64-bitman left a comment (vim/vim#20262)

Yes I also traced the issue to the BMShow() function in plugin/menu.vim. It seems like :unmenu is not actually deleting the sub entries in the menu?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4502502053@github.com>

[mattn]

mattn left a comment (vim/vim#20262)

Tentative fix — gui_mch_destroy_menu on GTK4 currently does not remove the entry from the parent GMenu, so :unmenu becomes a no-op against the menubar model. After Refresh menu, BMShow re-appends Refresh/Delete/etc. into a still-populated GMenu and everything doubles. The patch below removes the entry (and the orphaned action / submenu reference) properly; would appreciate confirmation it fixes the duplication on your end.

$(git diff src/gui_gtk4.c)

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4505062977@github.com>

[Foxe Chen]

64-bitman left a comment (vim/vim#20262)

Fixes the duplication issue, but now there is no "Dummy" button in the menu (that was above the "Refresh Menu" button before)

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4512122172@github.com>

[mattn]

mattn left a comment (vim/vim#20262)

Thanks for testing. The "Dummy" entry actually shouldn't appear in the final menu — runtime/menu.vim BMShow() adds &Buffers.Dummy at priority .1 as a placeholder so a torn-off menu doesn't disappear mid-refresh, then explicitly does unmenu &Buffers.Dummy at the end of the rebuild. Until now that final unmenu was a silent no-op on GTK4, so Dummy stayed visible; with this patch it's removed as intended, matching the behaviour on other GUIs. So I think the menu is now in its expected state — could you confirm that the rest of the entries (Refresh / Delete / Alternate / Next / Previous / -SEP- / buffer list) all look right, and that re-running "Refresh menu" no longer duplicates them?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4528418457@github.com>

[mattn]

mattn left a comment (vim/vim#20262)

Opened #20314 with this patch.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20262/4528432504@github.com>