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

[Eduardo Lima (Etrunko)]

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.

> 
>> ---
>>  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_selection),
>> +            },
>> +            &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, 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.

Regards, Eduardo.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com