[Spice-devel] [PATCH 5/9] Add empty CD entry to usb-device-widget, create shared CD when toggled

[Alexander Nezhinsky]

On Fri, Dec 6, 2019 at 12:18 PM Frediano Ziglio <fziglio at redhat.com> wrote:

> >
> > +
> > +    if (gtk_toggle_button_get_active(toggle))
> > +        gtk_toggle_button_set_active(toggle, FALSE);
>
> style: always use brackets, here and below
>
> OK

> > +    else
> > +        return;
>
> I would handle the return in the if to reduce indentation
>
OK


> > +
> > +    dialog = gtk_file_chooser_dialog_new(_("Select ISO file or device"),
> > +
> > GTK_WINDOW(gtk_widget_get_toplevel(GTK_WIDGET(self))),
> > +                                         GTK_FILE_CHOOSER_ACTION_OPEN,
> > +                                         _("_Cancel"),
> GTK_RESPONSE_CANCEL,
> > +                                         _("_Open"),
> GTK_RESPONSE_ACCEPT,
> > +                                         NULL);
> > +
> > +    dialog_rc = gtk_dialog_run(GTK_DIALOG(dialog));
> > +    if (dialog_rc == GTK_RESPONSE_ACCEPT) {
> > +        gchar *filename =
> > gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));
> > +        GError *err = NULL;
> > +        gboolean rc;
> > +
> > +        rc =
> spice_usb_device_manager_create_shared_cd_device(priv->manager,
> > filename, &err);
> > +        if (!rc && err) {
> > +            gchar *err_msg = g_strdup_printf("shared CD %s, %s",
> > +
>  g_path_get_basename(filename),
> > err->message);
>
> I would add _( to string for i18n
>
OK

>
> > +
> > +            SPICE_DEBUG("Failed to create %s", err_msg);
> > +            spice_usb_device_widget_add_err_msg(self, err_msg);
> > +            spice_usb_device_widget_update_status(user_data);
> > +        }
> > +        g_clear_error(&err);
>
 Similar to the previous patch

> +    }
>
> +    empty_cd = gtk_check_button_new_with_label("SPICE CD (empty)");
>
> i18n also here

OK

>  static void spice_usb_device_widget_constructed(GObject *gobject)
> >  {
> > . . .
> > +    priv->empty_cd = NULL;
>

> This is not necessary, we don't support environment where NULL != 0 and all
> structure is initialized to 0 as default.
> And standard C decided now that NULL == 0 so no issue for the future.
>
OK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20191206/ac990895/attachment.html>