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

[Christophe Fergeau]

On Fri, Sep 28, 2012 at 08:08:19PM +0200, Hans de Goede wrote:
> 
> On 09/28/2012 12:56 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 b1bf090..660ea03 100644
> >--- a/gtk/usb-device-widget.c
> >+++ b/gtk/usb-device-widget.c
> >@@ -220,6 +220,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)
> >          goto end;
> 
> This of course no longer is correct ...

Oh really? I've updated this part of the log after making the changes, but
maybe something went wrong during the updating ;)

> 
> >---
> >
> >Changes in v2:
> >- the 'no usb device' logic has been moved to spice_usb_device_widget_update_status
> >   as suggested by Hans
> >- removed debugging code
> >- instead of adding a device_count member to SpiceUsbDeviceWidget, I've
> >   done this through a local variable
> 
> Sorry to be a PITA, but I'm not really happy with making it a local variable, that
> makes the code more complicated (IMHO), for saving just 4 bytes of ram. Likely the
> extra code-size of the new code will amply compensate that ...

Oh, my goal was not to save memory. What I don't like with putting the
count in the global state is that the count will be invalid most of the
time, which can be confusing when hacking on the code. When I have an
object instance, I expect its members to be meaningful during the object
lifecycle, not just in some specific places I need to know about.

> Anyways, please do one of:
> 1) Split the patch in 2 patches, with the first one removing the use of global
> state between check_can_redirect and spice_usb_device_widget_update_status
> (err_msg), and the second one adding the actual message on no devices, or
> 2) Just make count part of the global state

I'll go with 2) since this has your preference

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20120928/4cd61913/attachment.pgp>