-
Notifications
You must be signed in to change notification settings - Fork 124
Add support for publishing multiple clipboard format on unix/x11 #756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: Cog
Are you sure you want to change the base?
Conversation
* Implement clearing of clipboard * Different string formats are only offered when calling display_clipboardWriteWithType() with empty type - legacy interface used by old clipboard primitive. Users of ExtendedClipboardPlugin will have to provide different formats on the image side. * Implement clipboard size/type requests (not sure why legacy clipboard even worked before)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for publishing multiple clipboard formats on Unix/X11 systems by implementing a multi-format clipboard offer table and enhancing the clipboard API to support typed data. The implementation includes clipboard clearing functionality and proper size/type request handling.
Changes:
- Changed return type of
clipboardWriteWithTypefromvoidtosqIntacross all implementations to enable error reporting - Implemented multi-format clipboard offer table using a linked list data structure to store multiple clipboard formats simultaneously
- Added clipboard clearing functionality via
relinquishClipboard()and enhancedsqPasteboardClear() - Refactored
sendSelection()to serve clipboard data from the offer table instead of hardcoded format conversions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 25 comments.
Show a summary per file
| File | Description |
|---|---|
| platforms/unix/vm/sqUnixMain.c | Updated wrapper to return sqInt instead of void for clipboardWriteWithType |
| platforms/unix/vm/SqDisplay.h | Changed function pointer signature for clipboardWriteWithType to return sqInt |
| platforms/unix/vm-display-null/sqUnixDisplayNull.c | Updated stub implementation to return 0 instead of void |
| platforms/unix/vm-display-fbdev/sqUnixFBDev.c | Updated stub implementation to return 0 instead of void |
| platforms/unix/vm-display-X11/sqUnixX11.c | Core implementation: added SelectionOffer struct, offer table management functions, refactored clipboard write/read operations, and implemented clipboard clearing |
| platforms/unix/plugins/ClipboardExtendedPlugin/sqUnixExtendedClipboard.c | Implemented clipboard clearing and added formatCounter to track multi-format writes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
platforms/unix/plugins/ClipboardExtendedPlugin/sqUnixExtendedClipboard.c
Show resolved
Hide resolved
| size_t capacity= 3; | ||
| SelectionOffer *offer; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable declarations after statements. In C89/C90, all variable declarations must appear at the beginning of a block before any statements. The declarations 'size_t capacity' and 'SelectionOffer *offer' appear inside an else if block after the capacity assignment statement. This may cause compilation errors with strict C89/C90 compilers. Move these variable declarations to the beginning of the function.
| size_t capacity= 3; | |
| SelectionOffer *offer; | |
| size_t capacity; | |
| SelectionOffer *offer; | |
| capacity= 3; |
| for (offer= offerTable; offer; offer= offer->next) | ||
| capacity++; | ||
|
|
||
| Atom *targets= (Atom *)malloc(capacity * sizeof(Atom)); |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable declarations after statements. In C89/C90, all variable declarations must appear at the beginning of a block before any statements. The declaration 'Atom *targets' appears after the for loop that increments capacity. This may cause compilation errors with strict C89/C90 compilers. Move the variable declaration to the beginning of the block.
| for (offer= offerTable; offer; offer= offer->next) | |
| capacity++; | |
| Atom *targets= (Atom *)malloc(capacity * sizeof(Atom)); | |
| Atom *targets; | |
| for (offer= offerTable; offer; offer= offer->next) | |
| capacity++; | |
| targets= (Atom *)malloc(capacity * sizeof(Atom)); |
| /* Add all offers from the table, deduplicating against meta-formats */ | ||
| for (offer= offerTable; offer; offer= offer->next) { | ||
| /* Skip if already in the list (handles user-provided meta-formats) */ | ||
| int i, duplicate= 0; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable declarations after statements. In C89/C90, all variable declarations must appear at the beginning of a block before any statements. The declarations 'int i' and 'duplicate' appear inside the for loop after assignments to the targets array. This may cause compilation errors with strict C89/C90 compilers. Move these variable declarations to the beginning of the enclosing block.
Co-authored-by: Copilot <[email protected]>
…r/opensmalltalk-vm into unix-extended-clipboard
Please review carefully before merging. I have little experience with C, and 80% of this patch came out of GitHub Copilot, even though I of course did my best to review and challenge it. Seems to work fine in practice on Ubuntu 24/GNOME though. Use System-ct.1490 (inbox) to test this in Squeak.