[Spice-devel] [PATCH spice-gtk] usb-device-widget: Migrate to GtkContainer api

[Frediano Ziglio]

> 
> Hi,
> 
> On Fri, 2017-06-23 at 11:35 -0400, Frediano Ziglio wrote:
> > > 
> > > GtkVBox is deprecated since Gtk 3.2, GtkBox is going to be
> > > deprecated. Switch to GtkContainer api if possible.
> > > ---
> > >  src/usb-device-widget.c | 15 ++++++++-------
> > >  1 file changed, 8 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
> > > index b394499..e87bdff 100644
> > > --- a/src/usb-device-widget.c
> > > +++ b/src/usb-device-widget.c
> > > @@ -40,14 +40,14 @@
> > >  
> > >  struct _SpiceUsbDeviceWidget
> > >  {
> > > -    GtkVBox parent;
> > > +    GtkBox parent;
> > >  
> > >      SpiceUsbDeviceWidgetPrivate *priv;
> > >  };
> > >  
> > >  struct _SpiceUsbDeviceWidgetClass
> > >  {
> > > -    GtkVBoxClass parent_class;
> > > +    GtkBoxClass parent_class;
> > >  
> > >      /* signals */
> > >      void (*connect_failed) (SpiceUsbDeviceWidget *widget,
> > > @@ -168,15 +168,16 @@
> > > spice_usb_device_widget_show_info_bar(SpiceUsbDeviceWidget *self,
> > >  
> > >      widget = gtk_image_new_from_icon_name(stock_icon_id,
> > >                                            GTK_ICON_SIZE_SMALL_TOO
> > > LBAR);
> > > -    gtk_box_pack_start(GTK_BOX(hbox), widget, FALSE, FALSE, 0);
> > > +    gtk_container_add(GTK_CONTAINER(hbox), widget);
> > >  
> > >      widget = gtk_label_new(message);
> > > -    gtk_box_pack_start(GTK_BOX(hbox), widget, TRUE, TRUE, 0);
> > > +    g_object_set(G_OBJECT(widget), "expand", TRUE, NULL);
> > > +    gtk_container_add(GTK_CONTAINER(hbox), widget);
> > >  
> > >      priv->info_bar = gtk_alignment_new(0.0, 0.0, 1.0, 0.0);
> > >      gtk_alignment_set_padding(GTK_ALIGNMENT(priv->info_bar), 0,
> > > 0, 12, 0);
> > >      gtk_container_add(GTK_CONTAINER(priv->info_bar), info_bar);
> > > -    gtk_box_pack_start(GTK_BOX(self), priv->info_bar, FALSE,
> > > FALSE, 0);
> > > +    gtk_container_add(GTK_CONTAINER(self), priv->info_bar);
> > >      gtk_widget_show_all(priv->info_bar);
> > >  }
> > >  
> > > @@ -208,7 +209,7 @@ static GObject
> > > *spice_usb_device_widget_constructor(
> > >      gtk_label_set_markup(GTK_LABEL (priv->label), str);
> > >      g_free(str);
> > >      gtk_misc_set_alignment(GTK_MISC(priv->label), 0.0, 0.5);
> > > -    gtk_box_pack_start(GTK_BOX(self), priv->label, FALSE, FALSE,
> > > 0);
> > > +    gtk_container_add(GTK_CONTAINER(self), priv->label);
> > >  
> > >      priv->manager = spice_usb_device_manager_get(priv->session,
> > > &err);
> > >      if (err) {
> > > @@ -572,7 +573,7 @@ static void
> > > device_added_cb(SpiceUsbDeviceManager
> > > *manager,
> > >      align = gtk_alignment_new(0, 0, 0, 0);
> > >      gtk_alignment_set_padding(GTK_ALIGNMENT(align), 0, 0, 12, 0);
> > >      gtk_container_add(GTK_CONTAINER(align), check);
> > > -    gtk_box_pack_end(GTK_BOX(self), align, FALSE, FALSE, 0);
> > > +    gtk_container_add(GTK_CONTAINER(self), align);
> > >      spice_usb_device_widget_update_status(self);
> > >      gtk_widget_show_all(align);
> > >  }
> > 
> > This patch looks like a partial attempt to remove GtkVBox (full) and
> > GtkBox (partial, still SpiceUsbDeviceWidget base).
> 
> in fact it is a fix of the parent type - the widget is defined as
> GTK_TYPE_BOX (not as GTK_TYPE_VBOX). There are no issues, because the
> structure is the *same*. I'll split it.
> 

What I was trying to say is that you replaced multiple calls to
gtk_box_pack_start to gtk_container_add but the object is still
based on GtkBox while on the comment you wrote "GtkBox is going
to be deprecated. Switch to GtkContainer api if possible" maybe
I'm confused by the "if possible" that seems to indicate that
is not possible to move away from GtkBox.

OT: it seems Spicy is using GtkBox too.

> > 
> > Would not be better to have a patch that fully remove GtkVBox only
> > and
> > then another that fully remove GtkBox (using GtkContainer or
> > GtkGrid,
> > don't know if GtkGrid is going to be deprecated too) ?
> 
> I checked that GtkBox is still in gtk+ master (gtk4). Changing the
> G_DEFINE_TYPE is an abi change (It can be done for gtk4 version of
> spice-gtk).
> 
> Pavel
> 

Frediano