[Spice-devel] [PATCH 1/2] usb-device-widget: Improve "there are no free USB channnels" string

[Fabiano Fidêncio]

On Wed, Jan 20, 2016 at 9:53 AM, Victor Toso <lists at victortoso.com> wrote:
> Hi,
>
> On Tue, Jan 19, 2016 at 10:36:37PM +0100, Fabiano Fidêncio wrote:
>> As the message can be understand as an error message, let's replace it
>> for a more user-friendly message and display it in the infobar as an
>> info message instead of a warning message.
>>
>> Related: rhbz#1298772
>> ---
>>  src/spice-client.h       |  2 ++
>>  src/usb-device-manager.c |  6 ++++--
>>  src/usb-device-widget.c  | 10 ++++++++--
>>  3 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/spice-client.h b/src/spice-client.h
>> index 32b79ea..cccb325 100644
>> --- a/src/spice-client.h
>> +++ b/src/spice-client.h
>> @@ -70,6 +70,7 @@ G_BEGIN_DECLS
>>   * @SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME: username is required
>>   * @SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME: password and username are required
>>   * @SPICE_CLIENT_ERROR_USB_SERVICE: USB service error
>> + * @SPICE_CLIENT_ERROR_USB_NO_FREE_CHANNELS_AVAILABLE: there are no more free channels available for redirecting a device
>>   *
>>   * Error codes returned by spice-client API.
>>   */
>> @@ -82,6 +83,7 @@ typedef enum
>>      SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
>>      SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
>>      SPICE_CLIENT_ERROR_USB_SERVICE,
>> +    SPICE_CLIENT_ERROR_USB_NO_FREE_CHANNELS_AVAILABLE,
>>  } SpiceClientError;
>>
>>  #ifndef SPICE_DISABLE_DEPRECATED
>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>> index c62f56e..a22d926 100644
>> --- a/src/usb-device-manager.c
>> +++ b/src/usb-device-manager.c
>> @@ -1673,8 +1673,10 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>>              break;
>>      }
>>      if (i == priv->channels->len) {
>> -        g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> -                            _("There are no free USB channels"));
>> +        g_set_error_literal(err,
>> +                SPICE_CLIENT_ERROR,
>> +                SPICE_CLIENT_ERROR_USB_NO_FREE_CHANNELS_AVAILABLE,
>> +                _("All available USB channels are in use.\nNo additional devices can be redirected"));
>
> I keep thinking that if they are 'in use' they aren't availabe USB
> channels. Maybe the first sentence could be "All USB channels are in
> use" ?

I can take that as well. The sentence used came from Jonathon's
suggestion. If he also agress with your suggestion, I will change and
submit a v2.
Also, I don't link that much of the error name, suggestions are welcome.

>
> My english is definetly not good so maybe the former is much better then
> my suggestion.
>
> You changed the identation format but both are fine, patch looks good.

Yeah. If I didin't change the indentation, the content wouldn't fit in
the max-columns used in this file.

>
>>          return FALSE;
>>      }
>>
>> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>> index fe983c9..830bdce 100644
>> --- a/src/usb-device-widget.c
>> +++ b/src/usb-device-widget.c
>> @@ -74,6 +74,7 @@ struct _SpiceUsbDeviceWidgetPrivate {
>>      GtkWidget *info_bar;
>>      gchar *err_msg;
>>      gsize device_count;
>> +    gboolean is_info_message;
>>  };
>>
>>  static guint signals[LAST_SIGNAL] = { 0, };
>> @@ -381,6 +382,10 @@ static void check_can_redirect(GtkWidget *widget, gpointer user_data)
>>      /* If we cannot redirect this device, append the error message to
>>         err_msg, but only if it is *not* already there! */
>>      if (!can_redirect) {
>> +        priv->is_info_message = g_error_matches(err,
>> +                SPICE_CLIENT_ERROR,
>> +                SPICE_CLIENT_ERROR_USB_NO_FREE_CHANNELS_AVAILABLE);
>> +
>>          if (priv->err_msg) {
>>              if (!strstr(priv->err_msg, err->message)) {
>>                  gchar *old_err_msg = priv->err_msg;
>> @@ -407,8 +412,9 @@ static gboolean spice_usb_device_widget_update_status(gpointer user_data)
>>
>>      if (priv->err_msg) {
>>          spice_usb_device_widget_show_info_bar(self, priv->err_msg,
>> -                                              GTK_MESSAGE_INFO,
>> -                                              GTK_STOCK_DIALOG_WARNING);
>> +                GTK_MESSAGE_INFO,
>> +                priv->is_info_message ?
>> +                GTK_STOCK_DIALOG_INFO : GTK_STOCK_DIALOG_WARNING);
>>          g_free(priv->err_msg);
>>          priv->err_msg = NULL;
>>      } else {
>> --
>> 2.5.0
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel