GNOME Bugzilla – Bug 704798
wacom: Call the OSD window for assigning the tablets' buttons
Last modified: 2013-07-26 18:08:36 UTC
See https://bugzilla.gnome.org/show_bug.cgi?id=704797 for some issues that occur with these changes.
Created attachment 250021 [details] [review] wacom: Call the OSD window for assigning the tablets' buttons It falls back to the listbox view in case the OSD cannot be shown.
Review of attachment 250021 [details] [review]: Remove the libgd changes from the patch... ::: panels/wacom/cc-wacom-page.c @@ +389,2 @@ g_assert (priv->mapping_builder == NULL); + Whitespace change. @@ +431,3 @@ + result = g_dbus_proxy_call_finish (G_DBUS_PROXY (source_object), res, &error); + + if (page->priv->cancellable != NULL) { That's not how you check for cancelled calls, as explained in the other bug. @@ +459,3 @@ + if (priv->cancellable != NULL) { + g_cancellable_cancel (priv->cancellable); + g_object_unref (priv->cancellable); No need to cancel it, and you can just run g_clear_object() instead. @@ +464,3 @@ + + if (proxy == NULL) { + g_message ("Couldn't get the GSD Wacom DBus proxy. Launching the button mapping dialog."); I don't want that in the code. We don't support mix'n'match environment. @@ +493,3 @@ + g_object_get (priv->pad, "gdk-device", &gdk_device, NULL); + + if (gdk_device != NULL) g_return_if_fail (gdk_device != NULL); ::: panels/wacom/cc-wacom-panel.c @@ +223,3 @@ + { + g_cancellable_cancel (priv->cancellable); + g_object_unref (priv->cancellable); g_clear_object(); @@ +440,3 @@ + priv->proxy = g_dbus_proxy_new_for_bus_finish (res, &error); + + if (priv->cancellable != NULL) Again, wrongly used cancellables. ::: panels/wacom/test-wacom.c @@ +20,3 @@ +cc_wacom_panel_get_gsd_wacom_bus_proxy (CcWacomPanel *self) +{ + g_message ("Should get the g-s-d wacom dbus proxy here"); This should be fixed.
Created attachment 250209 [details] [review] wacom: Call the OSD window for assigning the tablets' buttons New version. I still don't know how you want me to use the cancellable, I used clear_object but this way I think the cancellable is pretty much useless. Please let me know what's the correct approach.
Created attachment 250221 [details] [review] wacom: Call the OSD window for assigning the tablets' buttons It falls back to the listbox view in case the OSD cannot be shown.
Attachment 250221 [details] pushed as d83e0ff - wacom: Call the OSD window for assigning the tablets' buttons