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

[WIP] Wayland Primary Selection support #1095

Closed
wants to merge 3 commits into from

Conversation

tarmack
Copy link
Contributor

@tarmack tarmack commented Oct 25, 2018

This pull request introduces the 'de-facto', and very likely to become an actual, standard "primary selection" Wayland protocol from GTK. The wayland protocol for primary selection developed by Gnome seems to be on its way to becoming the standard , since GTK uses it, Sway uses it, it is implemented by Firefox and probably more. it feels like a valid effort to include this in Kitty.

The changes do need a little touching up here and there, though. The pull request as it stands now is mainly intended as a way to get some feedback.

One thing that I didn't really manage to figure out is how to get the code from the protocol file build from the setup.py, could you give me some pointers here? For now I #include the .c file in wl_window.c but that is ugly as can be. I did notice some things around protocols but that seems specific to the standard, packaged protocols.

There is also some duplication going on that can be reduced in a few places, I'll be working on that.

@tarmack tarmack force-pushed the wl-primary-selection branch 3 times, most recently from a0ea103 to 870fdef Compare October 25, 2018 21:19
@martinetd
Copy link
Contributor

setup.py already runs some programs directly; you can run wayland-scanner there as well I think - it should be available if they have wayland libs

@kovidgoyal
Copy link
Owner

If you want to add a protocol to the list of wayland protocols to scan for, add it to glfw/source-info.json then run

./glfw/glfw.py

then the kitty build system should take care of scanning for it automatically. Although I have to admit I've never tried it with private protocols, so I have no idea if it will work (it uses wayland-scanner internally).

Also, I am somewhat reluctant to have kitty depend on a not yet stable protocol. I'm not saying I cannot be convinced otherwise, but it will take a bit of hand holding :) For instance, presumably if this protocol is eventually stabilised wont it be under a different name which would then require kitty to change its code as well?

Note that kitty has a copy_on_select feature which automatically sets the clipboard to the selection. While obviously not quite as good as actual primary selection, it is in my experience a reasonable workaround.

@kovidgoyal
Copy link
Owner

Oh and I should add, that I really dont want wayland protocol .c and .h files in the kitty source tree.

@tarmack
Copy link
Contributor Author

tarmack commented Oct 26, 2018

Obviously the source files where not supposed to go in the repo those files where just there until I figured out how to generate them from the definition. Which turned out to be way simpler then I anticipated.

I understand your anxiousness regarding adding unofficial protocols. As far as I understand the workings of Wayland protocols, there is no need for any support on the compositor or other clients. If the compositor doesn't know about this protocol is becomes a no-op. All the code outside of GLFW and Kitty that is required is generated from the protocol definition, so there is no dependencies added. In other words, I think it is as optional as optional gets. ;-)

I too worry about the naming changes that might occur, or even more serious changes to the protocol however, I have the feeling that this protocol with this naming will end up being widely supported even if it is only because of the widespread use of Gnome.

For security reasons, as outlined in the documentation for the copy_on_select option, it may be an idea to allow people to turn this feature off. although generally people are familiar with this from X and the protocol actually has some protection (input event serial check by the compositor) build in.

As said before I will try and make the code a bit more concise, this protocol does mirror the regular clipboard closely after all. Their implementations might as well share as much code as possible.

@kovidgoyal
Copy link
Owner

Hmm well ok, I am willing to add it in principle. As for an option to turn it off, I think the proper place for that would be in the compositor, not individual applications.

@kovidgoyal
Copy link
Owner

let me know when you feel this PR is ready for merging and I will review and merge.

@tarmack
Copy link
Contributor Author

tarmack commented Oct 30, 2018

Kovid, I was planing on doing more de-duplication on the code but I find my C-fu lacking to do it in an elegant way and doubt whether the extra complexity would end up being worth it anyway.

I think that if you can live with the code as it is now it can be merged if you don't I'll work on it some more.

@kovidgoyal
Copy link
Owner

I have merged.

@kovidgoyal kovidgoyal closed this Oct 31, 2018
@martinetd
Copy link
Contributor

FWIW just two weeks later, the actual standard "primary-selection" has been standardized in wayland-protocols as primary-selection-unstable-v1:
https://cgit.freedesktop.org/wayland/wayland-protocols/commit/?id=2c3b11d76fd498fc3856cdd36ac76c1193686bf1

It's included in the v1.17 release that was commited. . . three minutes later.
Not sure what kind of timing we'll want, I guess compositors will slowly add support for this and support both for a while before removing the gtk one, so we can wait a bit, but it's up to you - I did a diff with the gtk one and aside of name/indentation I don't think anything changes.

@kovidgoyal
Copy link
Owner

annoying. since no version of kitty has been released with support for the gtk one, and the kitty wayland backend is still marked experimental, I am fine with just replacing it with the standard one. I really dont want to carry around the gtk one in kitty's source code.

@tarmack
Copy link
Contributor Author

tarmack commented Nov 13, 2018

I moved the code over to the "standard" version of the protocol in Pull request 1138.

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.

None yet

3 participants