Add support for publishing multiple clipboard format on unix/x11#756
Add support for publishing multiple clipboard format on unix/x11#756LinqLover wants to merge 9 commits intoOpenSmalltalk:Cogfrom
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.
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.
| size_t capacity= 3; | ||
| SelectionOffer *offer; |
There was a problem hiding this comment.
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)); |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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 <175728472+Copilot@users.noreply.github.com>
…r/opensmalltalk-vm into unix-extended-clipboard
- write back and free atom list for xaMultiple - do not absorb events in waitNotify() - properly read selection properties on 32-bit systems
…oahter offset slip when getting seleciton properties
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.