Skip to content

Conversation

@LinqLover
Copy link
Contributor

@LinqLover LinqLover commented Jan 20, 2026

  • 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)

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.

* 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)
Copilot AI review requested due to automatic review settings January 20, 2026 23:48
Copy link

Copilot AI left a 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 clipboardWriteWithType from void to sqInt across 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 enhanced sqPasteboardClear()
  • 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.

Comment on lines +729 to +730
size_t capacity= 3;
SelectionOffer *offer;
Copy link

Copilot AI Jan 20, 2026

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.

Suggested change
size_t capacity= 3;
SelectionOffer *offer;
size_t capacity;
SelectionOffer *offer;
capacity= 3;

Copilot uses AI. Check for mistakes.
Comment on lines +731 to +734
for (offer= offerTable; offer; offer= offer->next)
capacity++;

Atom *targets= (Atom *)malloc(capacity * sizeof(Atom));
Copy link

Copilot AI Jan 20, 2026

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.

Suggested change
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));

Copilot uses AI. Check for mistakes.
/* 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;
Copy link

Copilot AI Jan 20, 2026

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant