[virt-tools-list] [PATCHv2 virt-viewer 07/10] Show preferences dialog

[Marc-André Lureau]

----- Original Message -----
> On Wed, Mar 4, 2015 at 7:01 PM, Marc-André Lureau
> <marcandre.lureau at gmail.com> wrote:
> > Add a menu item Preferences under File and show the preferences dialog
> > ---
> >  src/virt-viewer-app.c    | 26 +++++++++++++++++++++++++-
> >  src/virt-viewer-app.h    |  1 +
> >  src/virt-viewer-window.c |  7 +++++++
> >  src/virt-viewer.xml      |  9 +++++++++
> >  4 files changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > index 4800beb..ed47a19 100644
> > --- a/src/virt-viewer-app.c
> > +++ b/src/virt-viewer-app.c
> > @@ -112,7 +112,7 @@ struct _VirtViewerAppPrivate {
> >      GHashTable *displays;
> >      GHashTable *initial_display_map;
> >      gchar *clipboard;
> > -
> > +    GtkWidget *preferences;
> >      gboolean direct;
> >      gboolean verbose;
> >      gboolean enable_accel;
> > @@ -1629,6 +1629,9 @@ virt_viewer_app_dispose (GObject *object)
> >      VirtViewerApp *self = VIRT_VIEWER_APP(object);
> >      VirtViewerAppPrivate *priv = self->priv;
> >
> > +    gtk_widget_destroy(priv->preferences);
> > +    priv->preferences = NULL;
> > +
> 
> Please, before call gtk_widget_destroy() check if preferences is not
> NULL, otherwise you can end up with this critical:
> (remote-viewer:9218): Gtk-CRITICAL **: gtk_widget_destroy: assertion
> 'GTK_IS_WIDGET (widget)' failed
> 

ok

> >      if (priv->windows) {
> >          GList *tmp = priv->windows;
> >          /* null-ify before unrefing, because we need
> > @@ -2401,6 +2404,27 @@ virt_viewer_app_get_windows(VirtViewerApp *self)
> >      return self->priv->windows;
> >  }
> >
> > +void
> > +virt_viewer_app_show_preferences(VirtViewerApp *self, GtkWidget *parent)
> > +{
> > +    GtkWidget *preferences = self->priv->preferences;
> > +
> > +    if (preferences)
> > +        goto present;
> 
> Instead of the goto here, I'd prefer to have something like ...
> if (!preferences) {
>     /* code */
> }

ok

> 
> > +
> > +    GtkBuilder *builder =
> > virt_viewer_util_load_ui("virt-viewer-preferences.xml");
> > +    gtk_builder_connect_signals(builder, self);
> > +
> > +    preferences = GTK_WIDGET(gtk_builder_get_object(builder,
> > "preferences"));
> > +    self->priv->preferences = preferences;
> 
> And you're leaking builder here ...

ok

> 
> > +
> > +present:
> > +    gtk_window_set_transient_for(GTK_WINDOW(preferences),
> > +                                 GTK_WINDOW(parent));
> > +
> > +    gtk_window_present(GTK_WINDOW(preferences));
> > +}
> > +
> >  static gboolean
> >  option_kiosk_quit(G_GNUC_UNUSED const gchar *option_name,
> >                    const gchar *value,
> > diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
> > index f127b32..d214279 100644
> > --- a/src/virt-viewer-app.h
> > +++ b/src/virt-viewer-app.h
> > @@ -100,6 +100,7 @@ void virt_viewer_app_clear_hotkeys(VirtViewerApp *app);
> >  gint virt_viewer_app_get_n_initial_displays(VirtViewerApp* self);
> >  gint virt_viewer_app_get_initial_monitor_for_display(VirtViewerApp* self,
> >  gint display);
> >  void virt_viewer_app_set_enable_accel(VirtViewerApp *app, gboolean
> >  enable);
> > +void virt_viewer_app_show_preferences(VirtViewerApp *app, GtkWidget
> > *parent);
> >
> >  G_END_DECLS
> >
> > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> > index c5f7c75..6a900ff 100644
> > --- a/src/virt-viewer-window.c
> > +++ b/src/virt-viewer-window.c
> > @@ -1018,6 +1018,13 @@
> > virt_viewer_window_menu_file_smartcard_remove(GtkWidget *menu
> > G_GNUC_UNUSED,
> >  }
> >
> >  G_MODULE_EXPORT void
> > +virt_viewer_menu_preferences_cb(GtkWidget *menu G_GNUC_UNUSED,
> > +                                VirtViewerWindow *self)
> > +{
> > +    virt_viewer_app_show_preferences(self->priv->app,
> > GTK_WIDGET(self->priv->window));
> > +}
> 
> ../../src/virt-viewer-window.c:1021:1: warning: no previous prototype
> for 'virt_viewer_menu_preferences_cb' [-Wmissing-prototypes]
>  virt_viewer_menu_preferences_cb(GtkWidget *menu G_GNUC_UNUSED,
> 

Ah, a useless warning imho, I'll add the declaration

Weird I missed that, it must be because I usually compile with my own debug CFLAGS...

> > +
> > +G_MODULE_EXPORT void
> >  virt_viewer_window_menu_view_release_cursor(GtkWidget *menu G_GNUC_UNUSED,
> >                                              VirtViewerWindow *self)
> >  {
> > diff --git a/src/virt-viewer.xml b/src/virt-viewer.xml
> > index 849fc7d..0fa5c26 100644
> > --- a/src/virt-viewer.xml
> > +++ b/src/virt-viewer.xml
> > @@ -69,6 +69,15 @@
> >                        </object>
> >                      </child>
> >                      <child>
> > +                      <object class="GtkMenuItem" id="menu-preferences">
> > +                        <property name="visible">True</property>
> > +                        <property name="can_focus">False</property>
> > +                        <property name="label"
> > translatable="yes">Preferences</property>
> > +                        <property name="use_underline">True</property>
> > +                        <signal name="activate"
> > handler="virt_viewer_menu_preferences_cb" swapped="no"/>
> > +                      </object>
> > +                    </child>
> > +                    <child>
> >                        <object class="GtkSeparatorMenuItem"
> >                        id="separatormenuitem1">
> >                          <property name="visible">True</property>
> >                          <property name="can_focus">False</property>
> > --
> > 2.1.0
> >
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> 
> 
> --
> Fabiano Fidêncio
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list