[Spice-devel] [spice-gtk] usb: Add info message when USB dialog is empty

[Hans de Goede]

Hi,

On 09/27/2012 06:01 PM, Christophe Fergeau wrote:
>  From rh bug #804187:
> « The redirection dialog can feel a bit strange when there is no device to
> redirect.
>
> It could be useful to provide a help message indicating that there is no
> device to redirect yet, and that the user can insert a USB device to
> redirect, and some related guidance. »
>
> This commit adds a "No USB devices detected" infobar in the USB
> dialog below the 'Select USB devices to redirect" label.
> Content could probably be improved, but this is a step in the right
> direction ;)
>
> This can be tested with
> diff --git a/gtk/usb-device-widget.c b/gtk/usb-device-widget.c
> index 2261fd6..089b040 100644
> --- a/gtk/usb-device-widget.c
> +++ b/gtk/usb-device-widget.c
> @@ -219,6 +219,11 @@ static GObject *spice_usb_device_widget_constructor(
>                        G_CALLBACK(device_error_cb), self);
>
>       devices = spice_usb_device_manager_get_devices(priv->manager);
> +    if (devices) {
> +        g_ptr_array_unref(devices);
> +        devices = NULL;
> +    }
> +
>       if (!devices) {
>           spice_usb_device_widget_show_info_bar(self, _("No USB devices detected"),
>                                                 GTK_MESSAGE_INFO,
> ---
>   gtk/usb-device-widget.c | 33 +++++++++++++++++++++++++++++++--
>   1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/gtk/usb-device-widget.c b/gtk/usb-device-widget.c
> index 3ed81e4..089b040 100644
> --- a/gtk/usb-device-widget.c
> +++ b/gtk/usb-device-widget.c
> @@ -219,8 +219,17 @@ static GObject *spice_usb_device_widget_constructor(
>                        G_CALLBACK(device_error_cb), self);
>
>       devices = spice_usb_device_manager_get_devices(priv->manager);
> -    if (!devices)
> +    if (devices) {
> +        g_ptr_array_unref(devices);
> +        devices = NULL;
> +    }

Looks like you left the test code in the commit!

> +    if (!devices) {
> +        spice_usb_device_widget_show_info_bar(self, _("No USB devices detected"),
> +                                              GTK_MESSAGE_INFO,
> +                                              GTK_STOCK_DIALOG_INFO);
>           goto end;
> +    }
>

And this is not going to work, if there are no devices spice_usb_device_manager_get_devices
will return a 0 sized array, not NULL. It will only return NULL when compiled
without USE_USBREDIR defined, but then spice_usb_device_manager_get will already have
failed with an error of: _("USB redirection support not compiled in"). So I must admit the
devices check already present is not really necessary, and I can understand this pointed
you in the wrong direction.


>       for (i = 0; i < devices->len; i++)
>           device_added_cb(NULL, g_ptr_array_index(devices, i), self);
> @@ -508,6 +517,23 @@ static void destroy_widget_by_usb_device(GtkWidget *widget, gpointer user_data)
>           gtk_widget_destroy(widget);
>   }
>
> +static gsize has_usb_devices(SpiceUsbDeviceWidget *self)
> +{
> +    GList *children;
> +    GList *it;
> +
> +    children = gtk_container_get_children(GTK_CONTAINER(self));
> +
> +    for (it = children; it != NULL; it = it->next) {
> +        if (get_usb_device(it->data) != NULL)
> +            break;
> +    }
> +    if (it == NULL)

I think this if needs to be removed, as you always want to free the list,
then again I would prefer for this entire function to go away, see below.

> +    g_list_free(children);
> +
> +    return (it != NULL);
> +}
> +
>   static void device_removed_cb(SpiceUsbDeviceManager *manager,
>       SpiceUsbDevice *device, gpointer user_data)
>   {
> @@ -515,8 +541,11 @@ static void device_removed_cb(SpiceUsbDeviceManager *manager,
>
>       gtk_container_foreach(GTK_CONTAINER(self),
>                             destroy_widget_by_usb_device, device);
> -
>       spice_usb_device_widget_update_status(self);
> +    if (!has_usb_devices(self))
> +        spice_usb_device_widget_show_info_bar(self, _("No USB devices detected"),
> +                                              GTK_MESSAGE_INFO,
> +                                              GTK_STOCK_DIALOG_INFO);
>   }


spice_usb_device_widget_update_status already loops over all the devices, how
about:
1) adding a device count to _SpiceUsbDeviceWidgetPrivate
2) setting this count to 0 at the start of spice_usb_device_widget_update_status
3) increment it in check_can_redirect when get_usb_device() does not return 0 there
4) Make spice_usb_device_widget_update_status() set the _("No USB devices detected")
    message itself when the count is 0 after the gtk_container_foreach call
5) call spice_usb_device_widget_update_status() from
    spice_usb_device_widget_constructor instead of having it check for no devices
    itself.

Regards,

Hans