GNOME Bugzilla – Bug 763859
gdkdevice-wayland.c cleanups
Last modified: 2016-03-21 16:26:36 UTC
When GdkSeat was introduced, the older GdkWaylandDeviceData (that mostly represented a seat) was renamed to GdkWaylandSeat, but a typedef to GdkWaylandDeviceData was kept around, as the changes were too many, too unrelated, and would create many conflicts with patches/branches that were pending merging. Now that the situation is more or less clear, I'm attaching some patches doing this cleanup. Further future refactoring might split this into gdkseat-wayland.[ch] and gdkdevice-wayland.[ch] so we don't manage all of GdkSeat+GdkDeviceManager+GdkDevice (+ parts of GdkDragContext) in a single C file, I'll refrain from this till wip/tablet-support is merged though. The patches are basically a manual search and replace, sanity checks/review appreciated though.
Created attachment 324247 [details] [review] wayland: Remove GdkWaylandDeviceData pointer in GdkWaylandDevice It's the same than gdk_device_get_seat() nowadays. Also, rename the usages of GdkWaylandDeviceData to GdkWaylandSeat in the functions affected by the removal.
Created attachment 324248 [details] [review] wayland: Remove GdkWaylandDeviceData typedef And let GdkWaylandSeat be the only one. All the remaining places using GdkWaylandDeviceData have been updated, and so are its variable names.
Created attachment 324249 [details] [review] wayland: Rename internal functions with misleading naming Now that GdkWaylandDeviceData is gone, the functions prefixed "gdk_wayland_device_" and taking a GdkWaylandSeat as first parameter feel out of place. Renaming those makes it more obvious that it's seat functions.
Review of attachment 324247 [details] [review]: Looks good to me. Do you want this on 3.20.x too, in addition to master ? Might make cherry picking easier
Review of attachment 324248 [details] [review]: Does the gdkdevice-wayland.c belong into the previous patch ? Or maybe a separate one. In any case, it might be nice to have "Remove unused typedef" be a separate patch that does just that.
Review of attachment 324249 [details] [review]: Sure, looks fine to me
(In reply to Matthias Clasen from comment #4) > Review of attachment 324247 [details] [review] [review]: > > Looks good to me. Do you want this on 3.20.x too, in addition to master ? > Might make cherry picking easier Yeah, I was thinking the same, might not help as much in the end if we go full blown into separating gdkseat-wayland.c after tablet support... but still better to have code match anyway. (In reply to Matthias Clasen from comment #5) > Review of attachment 324248 [details] [review] [review]: > > Does the gdkdevice-wayland.c belong into the previous patch ? Or maybe a > separate one. In any case, it might be nice to have "Remove unused typedef" > be a separate patch that does just that. Right, will at least separate the typedef.
Created attachment 324268 [details] [review] wayland: Replace all remaining uses of GdkWaylandDeviceData And use GdkWaylandSeat in all of those. The variable names have also been updated.
Created attachment 324269 [details] [review] wayland: Remove GdkWaylandDataDevice typedef It's no longer used.
Review of attachment 324268 [details] [review]: .
Review of attachment 324269 [details] [review]: .
Attachment 324247 [details] pushed as c9f9163 - wayland: Remove GdkWaylandDeviceData pointer in GdkWaylandDevice Attachment 324249 [details] pushed as 219eedd - wayland: Rename internal functions with misleading naming Attachment 324268 [details] pushed as 81f0d23 - wayland: Replace all remaining uses of GdkWaylandDeviceData Attachment 324269 [details] pushed as 1597f31 - wayland: Remove GdkWaylandDataDevice typedef