Add macOS menu bar status item (NSStatusItem) with system tray icon#712
Merged
Conversation
Reviewer's GuideAdds macOS-specific support for the application tray indicator using a new Objective‑C++ implementation, updates the Meson build to enable ObjC++ on macOS, and wires the new status item implementation into the iptux target as a Cocoa-based alternative to libappindicator. Sequence diagram for macOS status item menu activationsequenceDiagram
actor User
participant NSStatusItem_button as NSStatusItemButton
participant IptuxStatusItemHelper as StatusItemHelper
participant GActionGroup as ActionGroup
participant IptuxApp as IptuxApplication
User ->> NSStatusItem_button: Click menu item
NSStatusItem_button ->> IptuxStatusItemHelper: send openMainWindow
activate IptuxStatusItemHelper
IptuxStatusItemHelper ->> ActionGroup: g_action_group_activate_action("open_main_window", NULL)
activate ActionGroup
ActionGroup ->> IptuxApp: Dispatch action open_main_window
deactivate ActionGroup
deactivate IptuxStatusItemHelper
User ->> NSStatusItem_button: Click Preferences
NSStatusItem_button ->> IptuxStatusItemHelper: send openPreferences
IptuxStatusItemHelper ->> ActionGroup: g_action_group_activate_action("preferences", NULL)
ActionGroup ->> IptuxApp: Dispatch action preferences
User ->> NSStatusItem_button: Click Quit
NSStatusItem_button ->> IptuxStatusItemHelper: send quit
IptuxStatusItemHelper ->> ActionGroup: g_action_group_activate_action("quit", NULL)
ActionGroup ->> IptuxApp: Dispatch action quit
Class diagram for the new macOS tray indicator implementationclassDiagram
class IptuxStatusItemHelper {
- GActionGroup* actionGroup_
- iptux::IptuxAppIndicator* owner_
+ IptuxStatusItemHelper initWithActionGroup_actionGroup_owner_owner(GActionGroup* actionGroup, iptux::IptuxAppIndicator* owner)
+ void openMainWindow(id sender)
+ void openPreferences(id sender)
+ void quit(id sender)
+ void statusItemClicked(id sender)
}
class IptuxAppIndicatorPrivate {
+ IptuxAppIndicatorPrivate()
+ ~IptuxAppIndicatorPrivate()
+ NSStatusItem* statusItem
+ IptuxStatusItemHelper* helper
+ NSImage* normalIcon
+ NSImage* attentionIcon
}
class IptuxAppIndicator {
- std::shared_ptr~IptuxAppIndicatorPrivate~ priv
+ IptuxAppIndicator(GActionGroup* action_group)
+ void SetUnreadCount(int count)
}
class NSStatusItem {
}
class NSImage {
}
class NSMenu {
}
class GActionGroup {
+ void g_action_group_activate_action(GActionGroup* action_group, const char* action_name, GVariant* parameter)
}
IptuxAppIndicator o-- IptuxAppIndicatorPrivate : owns
IptuxAppIndicatorPrivate *-- NSStatusItem : uses
IptuxAppIndicatorPrivate *-- IptuxStatusItemHelper : uses
IptuxAppIndicatorPrivate *-- NSImage : uses
IptuxAppIndicator ..> NSMenu : uses
IptuxStatusItemHelper ..> GActionGroup : triggers_actions
IptuxStatusItemHelper ..> IptuxAppIndicator : calls_owner_signals
Flow diagram for platform-specific app indicator implementation selectionflowchart TD
A[Build on host_machine] --> B{host_machine.system == darwin}
B -->|true| C[Enable ObjC++ language<br>add_languages objcpp]
C --> D[Add GLib min version define<br>-DGLIB_VERSION_MIN_REQUIRED]
D --> E{get_option werror}
E -->|true| F[Add -U_LIBCPP_ENABLE_ASSERTIONS for cpp]
F --> G[Add -U_LIBCPP_ENABLE_ASSERTIONS for objcpp]
E -->|false| H[No extra warning flags]
B -->|false| I[Skip ObjC++ setup]
subgraph iptux_target_sources
J[Base C++ sources]
K{host_machine.system == darwin}
K -->|true| L[Add AppIndicatorMac.mm]
L --> M[Add dependency appleframeworks Cocoa]
K -->|false| N{appindicator_dep.found}
N -->|true| O[Add AppIndicator.cpp<br>Add dependency appindicator_dep]
N -->|false| P[Build without tray indicator]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/iptux/AppIndicatorMac.mm:144-145` </location>
<code_context>
+ priv->attentionIcon = loadIcon("iptux-attention", 64);
+
+ // Create status item
+ priv->statusItem =
+ [[[NSStatusBar systemStatusBar] statusItemWithLength:NSVariableStatusItemLength] retain];
+
+ if (priv->normalIcon) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential over-retain of NSStatusItem leads to a leak.
`-[NSStatusBar statusItemWithLength:]` already returns a retained `NSStatusItem`, so the extra `retain` here over-retains it. The destructor only does one `[statusItem release]`, leaving the status item leaked.
Remove the explicit `retain`:
```objc
priv->statusItem = [[NSStatusBar systemStatusBar]
statusItemWithLength:NSVariableStatusItemLength];
```
The existing release in the private dtor will then correctly balance the retain.
</issue_to_address>
### Comment 2
<location> `src/iptux/AppIndicatorMac.mm:90-96` </location>
<code_context>
+ return [image autorelease];
+}
+
+static NSImage* loadIcon(const char* iconName, int size) {
+ auto theme = gtk_icon_theme_get_default();
+ GError* error = nullptr;
+ auto pixbuf = gtk_icon_theme_load_icon(theme, iconName, size,
+ GtkIconLookupFlags(0), &error);
+ if (!pixbuf) {
+ g_warning("Couldn't load icon %s: %s", iconName, error->message);
+ g_error_free(error);
+ return nil;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Error handling in loadIcon assumes error is always set when pixbuf is null.
`gtk_icon_theme_load_icon` is documented to set `error` on failure, but for robustness we shouldn't assume it’s always non-null before using `error->message`. A defensive pattern would be:
```cpp
if (!pixbuf) {
const char *msg = (error && error->message) ? error->message : "unknown error";
g_warning("Couldn't load icon %s: %s", iconName, msg);
if (error) g_error_free(error);
return nil;
}
```
```suggestion
auto pixbuf = gtk_icon_theme_load_icon(theme, iconName, size,
GtkIconLookupFlags(0), &error);
if (!pixbuf) {
const char* msg = (error && error->message) ? error->message : "unknown error";
g_warning("Couldn't load icon %s: %s", iconName, msg);
if (error) {
g_error_free(error);
}
return nil;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a native macOS menu bar status item implementation and updates the Meson build to support Objective-C++ and Cocoa framework linking on macOS.
Changes:
- Add
AppIndicatorMac.mmimplementing a macOSNSStatusItem-based app indicator with menu actions mapped to existingGActionGroupcommands. - Update Meson build files to compile Objective-C++ on macOS and link against the Cocoa framework.
- Extend macOS build flags to apply the GLib version requirement and libc++ assertion behavior consistently across C++ and Objective-C++.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
meson.build |
Enables Objective-C++ on macOS and aligns build flags for objcpp sources. |
src/iptux/meson.build |
Adds macOS-specific app indicator source and links Cocoa framework when building on darwin. |
src/iptux/AppIndicatorMac.mm |
Implements the macOS menu bar status item and menu actions for app control. |
Comment on lines
+50
to
+53
| } | ||
|
|
||
| - (void)statusItemClicked:(id)sender { | ||
| (void)sender; |
There was a problem hiding this comment.
statusItemClicked: is currently dead code: it isn’t wired to the NSStatusItem button (no target/action assignment), so this callback (and the owner_ field it depends on) will never be invoked. Either hook this up via the status item button’s target/action if you want click-to-open behavior, or remove the unused method/field to avoid confusion.
Comment on lines
+144
to
+146
|
|
||
| // Create status item | ||
| priv->statusItem = |
There was a problem hiding this comment.
This Objective-C++ file doesn’t appear to follow the project’s clang-format style (e.g., very long lines here). Please run clang-format on AppIndicatorMac.mm so it matches the existing formatting seen in src/iptux/AppIndicator.cpp:36-56 and the rest of the codebase.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #712 +/- ##
=======================================
Coverage 51.99% 51.99%
=======================================
Files 64 64
Lines 8601 8601
=======================================
Hits 4472 4472
Misses 4129 4129 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lidaobing
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add native macOS menu bar status item implementation and integrate Objective-C++ support into the build.
New Features:
Enhancements: