[FR] Add X11 Clipboard support

gezihuzi commented 1 month ago

Hello! I'm interested in using Yaxi for my project and wondering about clipboard functionality.

Does Yaxi have plans to support X11 clipboard operations (like CLIPBOARD and PRIMARY selection)? This would include basic operations like:

Copy text/data to clipboard
Paste from clipboard
Monitor clipboard changes
Handle different selection types

If there's no immediate plan for this feature, I'd be willing to contribute to implementing it. Would you be open to accepting a PR for clipboard support?

Thanks for creating this promising X11 Rust library!

proxin187 commented 1 month ago

Thanks for the issue!

Clipboard support is definitely a feature needed, it shouldn't be a problem for me to implement it so ill try to get it done today!

Also, error reporting is a little wacky right now so ill probably make it better too.

gezihuzi commented 4 weeks ago

Thanks for the quick response! That's great to hear.

I'd be happy to help with testing and provide feedback once the implementation is ready. I can try different use cases and report any issues or edge cases I find.

Please let me know when there's something ready for testing.

proxin187 commented 4 weeks ago

Hello, the clipboard implementation should be working now.

Its gated behind the "clipboard" feature and there should be an example of how to use it inside the examples folder.

Currently it only supports UTF8_STRING but adding support for other types shouldn't be hard!

Please let me know if there is anything missing!

gezihuzi commented 3 weeks ago

Thanks for the implementation! I've noticed some unexpected behavior while testing the clipboard feature:

Observed behavior:

When I copy text from other applications -> run paste example -> successfully read the content
When I run copy example (writes predefined text) -> shows success -> run paste example -> fails to read the content

I've also added three unit tests in PR #2 to verify basic clipboard operations:

Read text from clipboard
Write text to clipboard
Write-then-read consistency check

However, these tests are not passing as expected.

Could you help take a look at what might be causing these issues? I can provide more details or add debug logging if needed.

proxin187 commented 3 weeks ago

Thanks so much for the feedback and PR.

The issue where you couldn't read the clipboard after running copy is most likely because you lose x11 selections after the selection owner exits, on my machine it works as long as the copy example is still running while the paste example is run.

Short summary of x11 selection

Example here: example gif

When it comes to your tests you made, ill try to debug them and get them working as soon as possible, (im at school right now so it might not be done before later today)

Thanks for letting me know!

proxin187 commented 3 weeks ago

Hello, Sorry for the inconveniences but the clipboard is finally in a semi-stable state, i had to refactor the concurrency model in yaxi so it took a little longer then i would have liked but it is done.

feel free to submit PR's for more clipboard formats if you need them.

gezihuzi commented 3 weeks ago

Thank you for improving the clipboard, The implementation is working much better now.

I plan to submit new PRs to enhance the clipboard support, including:

[ ] Support for additional clipboard formats (imges, files, custom mime)
[ ] Monitor clipboard changes
[ ] More comprehensive test coverage
[ ] Potential improvements to clipboard operations
[ ] CLIPBOARD_MANAGER, SAVE_TARGETS and INCR

I'll create separate PRs for each enhancement to keep changes focused and manageable. Looking forward to contributing more to this project.

gezihuzi commented 2 weeks ago

I've submitted a new PR #7 that primarily focuses on type extensions. While implementing the planned features, I encountered several challenges that I'd like to discuss:

After writing content to the clipboard, attempting to read it immediately may result in a deadlock. I noticed the related comments in the code - are there any known solutions for this issue?
Regarding ownership transfer: How should we handle data cleanup when the process exits after the clipboard ownership has been transferred to another process?
Integration with CLIPBOARD_MANAGER: What's the best approach for implementing support for the clipboard manager protocol, specifically ensuring that written content is properly saved to the CLIPBOARD_MANAGER when our process terminates?
The current design uses std::thread::spawn with ListenerHandle and Listener to poll and process X11 requests and events asynchronously. However, this implementation only allows one clipboard operation at a time. I believe this design could be optimized for better concurrency and performance - perhaps we could explore a more efficient event handling architecture?

These questions are particularly important for ensuring robust clipboard handling, efficient event processing, and graceful process termination. I'd appreciate any insights or guidance on addressing these concerns.

proxin187 commented 2 weeks ago

Thanks for your great PR, these features where much needed.

Answers

Could you please make an example that reproduces this behavior? i wasnt able to reproduce it myself, but if i get a example i will most likely be able to fix it quite fast.
If you meant if we should clean/empty the ClipboardData structure once we lose the selection ownership, then yes but it isn't extremely important. On the other hand if you meant when our process exits and the clipboard structure is dropped then i think we should destroy the window and delete the atoms that are only used by us eg. (storage property).
When it comes to the clipboard manager i havent done too much research yet, but when it comes to saving properly when the process terminates, you could most likely just fit some sort of a save mechanism inside the Drop implementation for Clipboard like this:
```
impl Drop for Clipboard {
fn drop(&mut self) {
    // SOME SORT OF A SAVE MECHANISM HERE

    self.listener.kill();
}
}
```

Ill most likely have to look a little more into the clipboard manager though!

There is definitely big room for improvement when it comes to the current concurrency model of our clipboard implementation, and im a 100% in favor of exploring different architectures and trying to find the best one.

Unrelated Sidenote

Also just as a kind of unrelated side note, the queue implementation has gotten a massive performance boost as i switched it to using Condvar when waiting for events instead of the old method which would simply busy loop until the queue wasnt empty.

gezihuzi commented 2 weeks ago

Debugging at a specific line will result in an error. 2 & 3. I think we can store the content in CLIPBOARD_MANAGER in the Drop implementation and then clean up the data.

proxin187 commented 5 days ago

I've implemented saving to the clipboard manager once the clipboard structure is dropped and also cleaning up the selection structure when we lose the selection, there are a few notable things with my implementation:

Once the Clipboard structure is dropped it will ask the clipboard manager to save content using the SAVE_TARGETS atom then it will wait for a request from the clipboard manager with a timeout of 100ms, the potential problem with this is that this will result in a 100ms wait on all systems that dont have a clipboard manager running, im not sure if this is a problem that can be solved as when i looked into other clipboard implementations they seem to do the same thing notably arboard's implementation.

i would like to hear your opinion on this and if you have any proposals for a better way to do this!

gezihuzi commented 3 days ago

@proxin187

Thank you for completing the CLIPBOARD_MANAGER implementation. I think waiting for 100ms at this stage is a feasible solution. In the future, maybe we can check if the current CLIPBOARD_MANAGER exists in advance so that we can skip it directly, which may be more efficient?

Share with you some of my recent progress. I am currently working on refactoring the implementation of the clipboard module in my temporary work branch. But it's not finished yet.

The purpose of refactoring is not only to enhance support for different selection (PRIMARY, SECONDARY, CLIPBOARD), but also to support the reading and writing of different types of clipboard data; advanced features (CLIPBOARD_MANAGER and INCR etc.); optimize event polling and shared data reading and writing processing.

Please feel free to share your thoughts and suggestions on the part of my work that I am currently working on.

proxin187 commented 3 days ago

Hi

Thanks, After skimming through your code i found that i really like your refactored version of the clipboard implementation, i think its a great step forward to have more support for different clipboard data types.

Since you are working on a refactored version ill keep away from updating the clipboard implementation until the refactor is done.

proxin187 / Yaxi

[FR] Add X11 Clipboard support #1

Answers

Unrelated Sidenote