Skip to content
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

Add Wayland support based on wl-clipboard-rs #65

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Gregory-Meyer
Copy link

This pull request adds mod wayland_clipboard and implements WaylandClipboardContext within. wl-clipboard-rs does most of the heavy lifting here, but WaylandClipboardContext has the following characteristics:

  • When copying, copies to all seats. If the primary selection protocol is supported, copies there as well.
  • When pasting, pastes from an arbitrary seat depending on the order returned by the compositor. If the primary selection protocol is supported, prefers to paste from that.
  • Only works with text MIME types.

Implementing a CompositeClipboardContext should be fairly simple; first attempt to create a WaylandClipboardContext, then attempt to create an X11ClipboardContext if the Wayland implementation fails for some reason or another.

@YaLTeR
Copy link

YaLTeR commented Apr 4, 2019

Hey! Just noticed this PR. :) This way of checking for primary selection support isn't entirely correct (it would return an error if only data-control version 1 is present). I just released a new version of wl-clipboard-rs where I added a function (utils::is_primary_selection_supported()) specifically for this purpose, with detailed documentation on its possible outcomes.

Also (as pointed out in #34) I'd like to note that the data-control protocol used by wl-clipboard-rs is intended for console applications and other apps without Wayland surfaces. For proper GUI Wayland clipboard support the smithay-clipboard crate can be used, but it's a little more involved (a seat name needs to be passed through).

@Gregory-Meyer
Copy link
Author

I wrote this because I switched from Alacritty to Kitty for Wayland clipboard support; since Alacritty is a windowed application with a surface, it seems I need to add a WaylandClipboardContext::with_seat constructor.

smithay-clipboard also appears to only send messages with the text/plain MIME type and receive messages that support that MIME type. This behavior seems roughly equivalent to what your crate provides when using MimeType::Text as I do, but any differences shouldn't be too great, yes?

@YaLTeR
Copy link

YaLTeR commented Apr 4, 2019

Yeah, the differences are roughly:

  • wlr-data-control is currently only supported by wlroots-based compositors (although it is expected that it will eventually be part of wayland-protocols and adopted more widely);
  • wlr-data-control is meant for clipboard managers, so compositors may want to require user confirmation for its use, while smithay-clipboard uses the core Wayland clipboard protocol which is supported everywhere and will (presumably) never require any sort of confirmation.

@Gregory-Meyer
Copy link
Author

This latest commit updates the dependency on wl-clipboard-rs to 0.3 and adds support for clipboard management via the wl_data_device protocol via smithay-clipboard. However, using the wl_data_device protocol requires us to specify a seat to operate on; to use this protocol, use the WaylandClipboardContext::with_seat constructor. Otherwise, if you're writing a CLI app that targets wlroots (read: sway), the WaylandClipboardContext::new constructor is suitable.

In addition, copies to both the primary selection and regular clipboard are now atomic. Previously, a selection was first copied to the primary selection and then to the regular clipboard.

smithay-clipboard 0.1 requires smithay-client-toolkit 0.5, which requires
wayland-client 0.22. Travis CI build was failing because the toolchain was
grabbing a different version of wayland-client instead of making the sensible
decision of picking whatever already worked with smithay-clipboard.
@YaLTeR
Copy link

YaLTeR commented Apr 4, 2019

Great! Just three things:

  • if is_primary_selection_supported() returns the NoSeats error, it's best to conservatively set supports_primary_selection to false in this case rather than just erroring out (since a seat might be connected later).
  • I think the differences between new() and with_seat() should be documented (specifically that one is intended for console applications and the other for GUI applications).
  • it's probably better to accept the seat name during the copy/paste operation for smithay-clipboard, although that would require interface changes. Not a big deal for the vast majority of use-cases I suppose.

@showermat
Copy link

@Gregory-Meyer Hey, it looks like this is really close to being mergeable. Can we get it finished up? If you're not able to finish, I may be able to pick it up.

@LennyPenny
Copy link

Any progress on this?

@showermat
Copy link

I created #71 to address some of the feedback here, but it seems like this repo may have been abandoned, so I'm not sure what the best path forward is. I would love to see this make it back into crates.io.

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.

4 participants