fix(v3/linux): Fix menu update by clearing menu#4875
Conversation
WalkthroughAdds platform-specific Changes
Sequence Diagram(s)(No sequence diagrams generated — changes are focused and do not introduce a multi-component new control flow warranting a diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @v3/pkg/application/linux_purego.go:
- Around line 528-545: The GList returned by gtkContainerGetChildren in
menuClear is not freed (the `_ = originalList` no-op and comment are wrong); add
a g_list_free binding (e.g., gListFree) in init() alongside the other GTK
bindings and then call gListFree(originalList) at the end of menuClear after
removing children (remove the incorrect comment and the `_ = originalList`
line); reference the CGO implementation pattern used in linux_cgo.go for the
correct g_list_free usage and ensure the binding name matches the one you call
from menuClear.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v3/UNRELEASED_CHANGELOG.mdv3/pkg/application/linux_cgo.gov3/pkg/application/linux_purego.gov3/pkg/application/menu_linux.go
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
📚 Learning: 2025-01-24T22:41:18.566Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
Applied to files:
v3/pkg/application/linux_cgo.gov3/pkg/application/menu_linux.gov3/pkg/application/linux_purego.go
📚 Learning: 2026-01-04T08:01:00.038Z
Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.
Applied to files:
v3/pkg/application/linux_cgo.gov3/pkg/application/menu_linux.gov3/pkg/application/linux_purego.go
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.
Applied to files:
v3/pkg/application/linux_purego.go
🧬 Code graph analysis (2)
v3/pkg/application/linux_cgo.go (1)
v3/pkg/application/menu.go (1)
Menu(30-35)
v3/pkg/application/linux_purego.go (1)
v3/pkg/application/menu.go (1)
Menu(30-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: validate-changelog
🔇 Additional comments (4)
v3/UNRELEASED_CHANGELOG.md (1)
26-26: LGTM!The changelog entry accurately describes the fix and follows the project's formatting guidelines.
v3/pkg/application/linux_purego.go (1)
142-143: LGTM!The GTK container function declarations follow the established pattern in this file and correctly match the GTK3 API signatures.
v3/pkg/application/menu_linux.go (1)
32-35: LGTM!The logic correctly clears existing menu items before rebuilding when updating an existing menu. The comment clearly explains the intent.
v3/pkg/application/linux_cgo.go (1)
734-750: LGTM!The CGO implementation correctly handles memory management by freeing the GList after iterating through and removing children. This is the proper pattern for using
gtk_container_get_children.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @v3/pkg/application/linux_purego.go:
- Around line 527-543: The call to gListFree in menuClear is missing a native
function declaration and init registration, causing a compile error; add a var
declaration for gListFree (matching the pattern used for gFree) after the
existing gFree var and then register it in init() (after the gFree registration)
so gListFree is available to menuClear and other callers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/pkg/application/linux_purego.go
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.
Applied to files:
v3/pkg/application/linux_purego.go
📚 Learning: 2025-01-24T22:41:18.566Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
Applied to files:
v3/pkg/application/linux_purego.go
📚 Learning: 2026-01-04T08:01:00.038Z
Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.
Applied to files:
v3/pkg/application/linux_purego.go
🧬 Code graph analysis (1)
v3/pkg/application/linux_purego.go (1)
v3/pkg/application/menu.go (1)
Menu(30-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
🔇 Additional comments (2)
v3/pkg/application/linux_purego.go (2)
141-142: LGTM!The function pointer declarations correctly match the GTK3 API signatures for
gtk_container_get_childrenandgtk_container_remove.
298-299: LGTM!Function registrations follow the existing pattern and use correct GTK3 symbol names.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v3/pkg/application/linux_purego.go (1)
529-545: Implementation looks correct; consider adding a nil guard for robustness.The GList iteration pattern is well-implemented: saving the original pointer before iteration and freeing it afterward. The GTK semantics are correctly handled—
gtk_container_removemanages widget reference counts, andg_list_freefrees the list structure.However, there's no nil check on the
menuparameter or itsimplfield. If called with a nil menu or uninitialized impl, this will panic on the type assertion at line 530. While this follows the same pattern as other menu functions in this file, a defensive check could prevent runtime panics from propagating.🔧 Optional: Add defensive nil check
func menuClear(menu *Menu) { + if menu == nil || menu.impl == nil { + return + } menuShell := pointer((menu.impl).(*linuxMenu).native) children := gtkContainerGetChildren(menuShell)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/pkg/application/linux_purego.go
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.
Applied to files:
v3/pkg/application/linux_purego.go
📚 Learning: 2025-01-24T22:41:18.566Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
Applied to files:
v3/pkg/application/linux_purego.go
📚 Learning: 2026-01-04T08:01:00.038Z
Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.
Applied to files:
v3/pkg/application/linux_purego.go
🧬 Code graph analysis (1)
v3/pkg/application/linux_purego.go (1)
v3/pkg/application/menu.go (1)
Menu(30-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cleanup build artifacts
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
🔇 Additional comments (2)
v3/pkg/application/linux_purego.go (2)
108-108: LGTM!The GTK/GLib function declarations follow the correct signatures for
g_list_free,gtk_container_get_children, andgtk_container_remove.Also applies to: 142-143
266-266: LGTM!The function registrations follow the established pattern and use the correct GTK symbol names.
Also applies to: 300-301
* clear menu * update changelog * free list * register glistfree
Description
Added
menuClearthat gets called whenprocessMenuruns to clear menu children if a menu already exists.Fixes # (issue)
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.