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@xxxxxxxxxx> 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.
>
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
