[virt-tools-list] [PATCH virt-viewer] window: Factor out common code for toolbar items

[Pavel Grunt]

On Tue, 2016-06-28 at 16:30 -0300, Eduardo Lima (Etrunko) wrote:
> On 06/28/2016 12:16 PM, Fabiano Fidêncio wrote:
> > 
> > On Tue, Jun 28, 2016 at 5:10 PM, Pavel Grunt <pgrunt at redhat.com> wrote:
> > > 
> > > Create toolbar widget in the loop
> > 
> > NACK from my side.
> > There is any gain on re-factoring a code that will be removed as soon
> > as we do the release.
> > Actually, it just makes my life harder in order to rebase Sagar's
> > patches on top of this change.
> 
> Agreed, only a small comment below.

I didn't want to make life of anyone harder by sending the patch, I just noticed
that part of the code because of the crash...
> 
> > 
> > 
> > > 
> > > ---
> > >  src/virt-viewer-window.c | 121 ++++++++++++++++++++++++++++++++--------
> > > -------
> > >  1 file changed, 83 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> > > index 1ebb423..b276ae8 100644
> > > --- a/src/virt-viewer-window.c
> > > +++ b/src/virt-viewer-window.c
> > > @@ -1062,56 +1062,101 @@ virt_viewer_window_menu_help_about(GtkWidget
> > > *menu G_GNUC_UNUSED,
> > >      g_object_unref(G_OBJECT(about));
> > >  }
> > > 
> > > +typedef struct {
> > > +    GtkWidget *icon;
> > > +    const gchar *icon_name;
> > > +    const gchar *label;
> > > +    const gchar *tooltip;
> > > +    const gboolean sensitive;
> > > +    const gboolean show_label;
> > > +    const GCallback callback;
> > > +} VirtViewerToolbarButton;
> > > +
> > > +static void
> > > +virt_viewer_window_toolbar_add_button(VirtViewerWindow *self,
> > > +                                      const VirtViewerToolbarButton
> > > *tb_button,
> > > +                                      GtkWidget **dest_widget)
> > > +{
> > > +    VirtViewerWindowPrivate *priv = self->priv;
> > > +    GtkToolItem *button = gtk_tool_button_new(tb_button->icon, tb_button-
> > > >label);
> > > +
> > > +    gtk_tool_button_set_icon_name(GTK_TOOL_BUTTON(button), tb_button-
> > > >icon_name);
> > > +    gtk_tool_item_set_tooltip_text(button, tb_button->tooltip);
> > > +    gtk_tool_item_set_is_important(button, tb_button->show_label);
> > > +    gtk_widget_set_sensitive(GTK_WIDGET(button), tb_button->sensitive);
> > > +    gtk_widget_show_all(GTK_WIDGET(button));
> > > +    gtk_toolbar_insert(GTK_TOOLBAR(priv->toolbar), button, 0);
> > > +    g_signal_connect(button, "clicked", tb_button->callback, self);
> > > +
> > > +    if (dest_widget != NULL)
> > > +        *dest_widget = GTK_WIDGET(button);
> > > +}
> > > 
> > >  static void
> > >  virt_viewer_window_toolbar_setup(VirtViewerWindow *self)
> > >  {
> > > -    GtkWidget *button;
> > >      GtkWidget *overlay;
> > >      VirtViewerWindowPrivate *priv = self->priv;
> > > +    guint i;
> > > +
> > > +    const struct {
> > > +        const VirtViewerToolbarButton tb_button;
> > > +        GtkWidget **dest_widget;
> > > +    } toolbar_buttons[] = {
> > > +        {   /* Close connection */
> > > +            {
> > > +                NULL,
> > > +                "window-close",
> > > +                NULL,
> > > +                _("Disconnect"),
> > > +                TRUE,
> > > +                FALSE,
> > > +                G_CALLBACK(virt_viewer_window_menu_file_quit),
> > > +            },
> > > +            NULL,
> > > +        },{ /* USB Device selection */
> > > +            {
> > > +                gtk_image_new_from_resource(VIRT_VIEWER_RESOURCE_PREFIX"/
> > > icons/24x24/virt-viewer-usb.png"),
> > > +                NULL,
> > > +                _("USB device selection"),
> > > +                _("USB device selection"),
> > > +                TRUE,
> > > +                FALSE,
> > > +                G_CALLBACK(virt_viewer_window_menu_file_usb_device_select
> > > ion),
> > > +            },
> > > +            &priv->toolbar_usb_device_selection,
> > > +        },{ /* Send key */
> > > +            {
> > > +                NULL,
> > > +                "preferences-desktop-keyboard-shortcuts",
> > > +                NULL,
> > > +                _("Send key combination"),
> > > +                FALSE,
> > > +                FALSE,
> > > +                G_CALLBACK(virt_viewer_window_toolbar_send_key),
> > > +            },
> > > +            &priv->toolbar_send_key,
> > > +        },{ /* Leave fullscreen */
> > > +            {
> > > +                NULL,
> > > +                "view-restore",
> > > +                _("Leave fullscreen"),
> > > +                _("Leave fullscreen"),
> > > +                TRUE,
> > > +                TRUE,
> > > +                G_CALLBACK(virt_viewer_window_toolbar_leave_fullscreen),
> > > +            },
> > > +            NULL,
> > > +        },
> > > +    };
> 
> In the case we did not have patches in the queue that makes this
> obsolete,
Last time I saw the "redesign patches" they were using the current toolbar.
Probably there is something new...
>  it would be a good idea to have explicit field initializers
> for the items in the list. Besides being easier to read, you could save
> some lines of code for the NULL and FALSE ones.

I agree, it is a nice feature of C99

Thanks,
Pavel
> 
> Regards, Eduardo.
>