Skip to content

Add methods to handle sending the image and badge over a DO connection via GSDockManager protocol - #928

Open
gcasa wants to merge 12 commits into
masterfrom
issue_927_GSIconManager_protocol_change
Open

Add methods to handle sending the image and badge over a DO connection via GSDockManager protocol#928
gcasa wants to merge 12 commits into
masterfrom
issue_927_GSIconManager_protocol_change

Conversation

@gcasa

@gcasa gcasa commented Aug 28, 2026

Copy link
Copy Markdown
Member

This addresses issue#927. This change adds methods to send the image data and badge information over the GSIconManager protocol. These methods are optional so that this doesn't break the existing contract between GNUstep and older users of this protocol. I am using NSData to keep the utilization overhead low.

I am using the app in https://github.com/gcasa/apps-dock to test this change. It is currently working. The main driver for this is to make integration between GNUstep and external dock applications easier. This does not change how the external app works, only how the dock manages the app. I've used my work on the apps-dock DockWM to shape the changes I've made to libs-gui. I have made them as minimal as possible.

Disclosure: I am using AI to help debug and test this. The apps-dock application uses AI pretty extensively; this change uses it in a limited way to suggest fixes when needed and to optimize the libs-gui changes.

@gcasa
gcasa requested a review from fredkiefer as a code owner August 28, 2026 18:13
@gcasa

gcasa commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

I found an additional issue with AClock: it does not reflect it's changes because it grabs the view and updates it. So there is another way this needs to be updated. I also found an error where the proxy isn't released properly.

@fredkiefer

Copy link
Copy Markdown
Member

Is this still work in progress or ready for review? If the former is the case please convert it to draft.

@gcasa
gcasa marked this pull request as draft August 28, 2026 22:01
@gcasa

gcasa commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Is this still work in progress or ready for review? If the former is the case please convert it to draft.

My apologies, still a WIP.

@gcasa
gcasa force-pushed the issue_927_GSIconManager_protocol_change branch from 20bf34e to 9d7b74b Compare August 30, 2026 00:07
@gcasa
gcasa marked this pull request as ready for review August 30, 2026 05:52
@gcasa

gcasa commented Aug 30, 2026

Copy link
Copy Markdown
Member Author

@fredkiefer This PR is ready for review.

@gcasa
gcasa force-pushed the issue_927_GSIconManager_protocol_change branch from 9d7b74b to 0ffda06 Compare August 30, 2026 14:38
@gcasa
gcasa marked this pull request as draft September 1, 2026 19:00
@gcasa

gcasa commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

@fredkiefer I have converted this back to a draft. As I am testing both with the new dock app and legacy apps I am discovering issues as I go. Give me a couple of days to finish this up and I will convert it back. I just didn't want to have you start reviewing and then change things out from under you. When I do convert it back it will be ready.

@gcasa
gcasa marked this pull request as ready for review September 3, 2026 00:53
@gcasa

gcasa commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

@fredkiefer This should be safe to review now in its current form.

@fredkiefer
fredkiefer removed their request for review September 6, 2026 15:37
@gcasa
gcasa requested a lite review from Copilot September 7, 2026 06:01
@gcasa
gcasa requested a review from fredkiefer September 7, 2026 06:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are API/behavioral correctness issues in the updated dock tile rendering path and the -requestUserAttention: return semantics that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds support for pushing application icon image data and badge text over a Distributed Objects (DO) connection to an external dock/icon manager, enabling better integration with external dock applications while keeping the protocol methods optional for backward compatibility.

Changes:

  • Render the dock tile into an offscreen NSImage and send its image data + badge label to GSIconManager (NSDockTile / NSApplication).
  • Extend GSIconManager protocol/client logic to send icon data, badge text, and user-attention requests over DO with reconnection/loss handling.
  • Align NSDockTile ownership test with the new default contentView behavior.
File summaries
File Description
Tests/gui/NSDockTile/ownership.m Updates the ownership test to explicitly set a content view before asserting retain/release behavior.
Source/NSDockTile.m Switches dock tile rendering to offscreen drawing and forwards the rendered image + badge to GSIconManager.
Source/NSApplication.m Sends app icon + badge updates to GSIconManager and forwards user-attention requests.
Source/GSIconManager.m Expands the DO client/protocol to send icon image data, badge text, and attention requests, with connection monitoring.
Source/GSIconManager.h Exposes new GSUpdateIconManager / GSRequestUserAttention APIs.
Headers/AppKit/NSDockTile.h Updates ivars/forward declarations consistent with the new dock tile implementation.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Source/NSApplication.m
Comment thread Source/NSDockTile.m Outdated
@gcasa
gcasa removed the request for review from fredkiefer September 7, 2026 06:29
Co-authored-by: gcasa <27943+gcasa@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants