Hey,
Sorry for taking some time to review your patches!
On Sun, Jan 21, 2018 at 09:03:12PM +0100, Jakub Janků wrote:
> Hi,
> these two patches replace the current clipboard handling code,
> which uses Xlib, with code utilizing GTK+. The code was moved
> from x11.c to clipboard.c
>
> Although GTK+ can work on Wayland natively, this code currently
> works only under X. gdk_set_allowed_backends("x11") in
> vdagent.c is called to force GTK+ to always use X11 backend.
>
> The implementation is partially based on the code in spice-gtk
> (spice-gtk-session.c).
>
> Cheers,
Awesome work. Tested and seems to work as expected on F26 guest.
I'll be reviewing this inline but I'd like to suggest a different
approach to the patches.
For the clipboard, GTK+ is going to replace x11 fairly well and I
don't expect major problems but we might have some, we never
know. I would like to do one last vdagent release with x11
clipboard to explicit say that we will replace x11 with gtk+
after that. I think we can do a release as soon as this gets in.
So, removing x11 calls would be a problem.
Another issue is that, it is hard to review/compare when one
patch removes the feature and the other add it back besides the
fact that bisecting would be broken to that feature (although its
not a big problem in this case, I think).
My suggestion is to have a ./configure --with-gtk or --enable-gtk
to wrap new code around #ifdef GTK_ENABLED for the new code
otherwise it calls the x11 code as before.
I think the clipboard function functions exported by x11.h should
be moved to clipboard.[ch] and together with the proposed
VDAgentClipboards (interface) changes.
What do you think?
Cheers,
toso
>
> Makefile.am | 2 +
> src/vdagent/clipboard.c | 401 ++++++++++++++++++
> src/vdagent/clipboard.h | 42 ++
> src/vdagent/vdagent.c | 38 +-
> src/vdagent/x11-priv.h | 91 ----
> src/vdagent/x11.c | 1074 +----------------------------------------------
> src/vdagent/x11.h | 10 -
> 7 files changed, 472 insertions(+), 1186 deletions(-)
> create mode 100644 src/vdagent/clipboard.c
> create mode 100644 src/vdagent/clipboard.h
>
> --
> 2.14.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel