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
Attachment:
pgpbvL6qcpghc.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel