General comment on the approach: it seems to me that it would be cleaner
to implement this using virtual functions. For example, in patch #3, you
add a "can-reconnect" property to VirtViewerApp. This means that it's
technically possible to construct a RemoteViewer object with
can-reconnect=TRUE, even though RemoteViewer cannot reconnect.
There are two basic ways to achieve this with virtual functions:
1) make virt_viewer_app_get_preferences() a virtual function. Each class
(RemoteViewer and VirtViewer) would implement this virtual function and
return the preferences that it supports. There may be a fair amount of
code duplication in this approach, but it might be possible to overcome
that.
2) make virt_viewer_app_can_reconnect() a virtual function. RemoteViewer
would implement this function by returning FALSE, and VirtViewer would
return TRUE (look at virt_viewer_session_can_share_folder() for
inspiration)
In addition, "preferences" to me indicates a persistent setting, but it
doesn't appear that these settings are stored anywhere. Should we hook
them up to the existing config infrastructure in VirtViewerApp so they
end up stored in e.g. ~/.local/virt-viewer/settings? Furthermore, should
we add the existing "ask-quit" setting to this preferences dialog?
Opinions?
Jonathon
On Wed, 2015-05-13 at 16:51 +0200, Lukas Venhoda wrote:
> Moved preferences from virt-viewer-app to virt-viewer-window.
> Makes more sense to have all of the GUI in virt-viewer-window.
> This also makes it possible to implement virt-viewer/remote-viewer
> specific settings in preferences.
> ---
> New commit in version 3
> Moving preferences into window is better then ifdefs all over virt-viewer-app
> ---
> src/virt-viewer-app.c | 89 ----------------------------------------------
> src/virt-viewer-app.h | 1 -
> src/virt-viewer-window.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 91 insertions(+), 91 deletions(-)
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index b22a876..2570b8c 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -112,8 +112,6 @@ struct _VirtViewerAppPrivate {
> GHashTable *displays;
> GHashTable *initial_display_map;
> gchar *clipboard;
> - GtkWidget *preferences;
> - GtkFileChooser *preferences_shared_folder;
> gboolean direct;
> gboolean verbose;
> gboolean enable_accel;
> @@ -1668,10 +1666,6 @@ virt_viewer_app_dispose (GObject *object)
> VirtViewerApp *self = VIRT_VIEWER_APP(object);
> VirtViewerAppPrivate *priv = self->priv;
>
> - if (priv->preferences)
> - gtk_widget_destroy(priv->preferences);
> - priv->preferences = NULL;
> -
> if (priv->windows) {
> GList *tmp = priv->windows;
> /* null-ify before unrefing, because we need
> @@ -2442,89 +2436,6 @@ virt_viewer_app_get_windows(VirtViewerApp *self)
> return self->priv->windows;
> }
>
> -static void
> -share_folder_changed(VirtViewerApp *self)
> -{
> - gchar *folder;
> -
> - folder = gtk_file_chooser_get_filename(self->priv->preferences_shared_folder);
> -
> - g_object_set(virt_viewer_app_get_session(self),
> - "shared-folder", folder, NULL);
> -
> - g_free(folder);
> -}
> -
> -static GtkWidget *
> -virt_viewer_app_get_preferences(VirtViewerApp *self)
> -{
> - VirtViewerSession *session = virt_viewer_app_get_session(self);
> - GtkBuilder *builder = virt_viewer_util_load_ui("virt-viewer-preferences.xml");
> - gboolean can_share_folder = virt_viewer_session_can_share_folder(session);
> - GtkWidget *preferences = self->priv->preferences;
> - gchar *path;
> -
> - if (preferences)
> - goto end;
> -
> - gtk_builder_connect_signals(builder, self);
> -
> - preferences = GTK_WIDGET(gtk_builder_get_object(builder, "preferences"));
> - self->priv->preferences = preferences;
> -
> - g_object_set (gtk_builder_get_object(builder, "cbsharefolder"),
> - "sensitive", can_share_folder, NULL);
> - g_object_set (gtk_builder_get_object(builder, "cbsharefolderro"),
> - "sensitive", can_share_folder, NULL);
> - g_object_set (gtk_builder_get_object(builder, "fcsharefolder"),
> - "sensitive", can_share_folder, NULL);
> -
> - if (!can_share_folder)
> - goto end;
> -
> - g_object_bind_property(virt_viewer_app_get_session(self),
> - "share-folder",
> - gtk_builder_get_object(builder, "cbsharefolder"),
> - "active",
> - G_BINDING_BIDIRECTIONAL|G_BINDING_SYNC_CREATE);
> -
> - g_object_bind_property(virt_viewer_app_get_session(self),
> - "share-folder-ro",
> - gtk_builder_get_object(builder, "cbsharefolderro"),
> - "active",
> - G_BINDING_BIDIRECTIONAL|G_BINDING_SYNC_CREATE);
> -
> - self->priv->preferences_shared_folder =
> - GTK_FILE_CHOOSER(gtk_builder_get_object(builder, "fcsharefolder"));
> -
> - g_object_get(virt_viewer_app_get_session(self),
> - "shared-folder", &path, NULL);
> -
> - gtk_file_chooser_set_filename(self->priv->preferences_shared_folder, path);
> - g_free(path);
> -
> - virt_viewer_signal_connect_object(self->priv->preferences_shared_folder,
> - "file-set",
> - G_CALLBACK(share_folder_changed), self,
> - G_CONNECT_SWAPPED);
> -
> -end:
> - g_object_unref(builder);
> -
> - return preferences;
> -}
> -
> -void
> -virt_viewer_app_show_preferences(VirtViewerApp *self, GtkWidget *parent)
> -{
> - GtkWidget *preferences = virt_viewer_app_get_preferences(self);
> -
> - 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 f53fa73..ab1d7a9 100644
> --- a/src/virt-viewer-app.h
> +++ b/src/virt-viewer-app.h
> @@ -100,7 +100,6 @@ void virt_viewer_app_clear_hotkeys(VirtViewerApp *app);
> GList* virt_viewer_app_get_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);
> void virt_viewer_app_set_menus_sensitive(VirtViewerApp *self, gboolean sensitive);
>
> G_END_DECLS
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index d67fbc1..f66bd50 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -71,6 +71,7 @@ static void virt_viewer_window_toolbar_setup(VirtViewerWindow *self);
> static GtkMenu* virt_viewer_window_get_keycombo_menu(VirtViewerWindow *self);
> static void virt_viewer_window_get_minimal_dimensions(VirtViewerWindow *self, guint *width, guint *height);
> static gint virt_viewer_window_get_minimal_zoom_level(VirtViewerWindow *self);
> +static void virt_viewer_window_show_preferences(VirtViewerWindow *self, GtkWidget *parent);
>
> G_DEFINE_TYPE (VirtViewerWindow, virt_viewer_window, G_TYPE_OBJECT)
>
> @@ -97,6 +98,8 @@ struct _VirtViewerWindowPrivate {
> GtkAccelGroup *accel_group;
> VirtViewerNotebook *notebook;
> VirtViewerDisplay *display;
> + GtkWidget *preferences;
> + GtkFileChooser *preferences_shared_folder;
>
> gboolean accel_enabled;
> GValue accel_setting;
> @@ -171,6 +174,10 @@ virt_viewer_window_dispose (GObject *object)
> VirtViewerWindowPrivate *priv = VIRT_VIEWER_WINDOW(object)->priv;
> GSList *it;
>
> + if (priv->preferences)
> + gtk_widget_destroy(priv->preferences);
> + priv->preferences = NULL;
> +
> if (priv->display) {
> g_object_unref(priv->display);
> priv->display = NULL;
> @@ -1030,7 +1037,7 @@ G_MODULE_EXPORT void
> virt_viewer_window_menu_preferences_cb(GtkWidget *menu G_GNUC_UNUSED,
> VirtViewerWindow *self)
> {
> - virt_viewer_app_show_preferences(self->priv->app, GTK_WIDGET(self->priv->window));
> + virt_viewer_window_show_preferences(self, GTK_WIDGET(self->priv->window));
> }
>
> G_MODULE_EXPORT void
> @@ -1582,6 +1589,89 @@ virt_viewer_window_get_minimal_zoom_level(VirtViewerWindow *self)
> return CLAMP(zoom * ZOOM_STEP, MIN_ZOOM_LEVEL, NORMAL_ZOOM_LEVEL);
> }
>
> +static void
> +share_folder_changed(VirtViewerWindow *self)
> +{
> + gchar *folder;
> +
> + folder = gtk_file_chooser_get_filename(self->priv->preferences_shared_folder);
> +
> + g_object_set(virt_viewer_app_get_session(self->priv->app),
> + "shared-folder", folder, NULL);
> +
> + g_free(folder);
> +}
> +
> +static GtkWidget *
> +virt_viewer_window_get_preferences(VirtViewerWindow *self)
> +{
> + VirtViewerSession *session = virt_viewer_app_get_session(self->priv->app);
> + GtkBuilder *builder = virt_viewer_util_load_ui("virt-viewer-preferences.xml");
> + gboolean can_share_folder = virt_viewer_session_can_share_folder(session);
> + GtkWidget *preferences = self->priv->preferences;
> + gchar *path;
> +
> + if (preferences)
> + goto end;
> +
> + gtk_builder_connect_signals(builder, self);
> +
> + preferences = GTK_WIDGET(gtk_builder_get_object(builder, "preferences"));
> + self->priv->preferences = preferences;
> +
> + g_object_set (gtk_builder_get_object(builder, "cbsharefolder"),
> + "sensitive", can_share_folder, NULL);
> + g_object_set (gtk_builder_get_object(builder, "cbsharefolderro"),
> + "sensitive", can_share_folder, NULL);
> + g_object_set (gtk_builder_get_object(builder, "fcsharefolder"),
> + "sensitive", can_share_folder, NULL);
> +
> + if (!can_share_folder)
> + goto end;
> +
> + g_object_bind_property(virt_viewer_app_get_session(self->priv->app),
> + "share-folder",
> + gtk_builder_get_object(builder, "cbsharefolder"),
> + "active",
> + G_BINDING_BIDIRECTIONAL|G_BINDING_SYNC_CREATE);
> +
> + g_object_bind_property(virt_viewer_app_get_session(self->priv->app),
> + "share-folder-ro",
> + gtk_builder_get_object(builder, "cbsharefolderro"),
> + "active",
> + G_BINDING_BIDIRECTIONAL|G_BINDING_SYNC_CREATE);
> +
> + self->priv->preferences_shared_folder =
> + GTK_FILE_CHOOSER(gtk_builder_get_object(builder, "fcsharefolder"));
> +
> + g_object_get(virt_viewer_app_get_session(self->priv->app),
> + "shared-folder", &path, NULL);
> +
> + gtk_file_chooser_set_filename(self->priv->preferences_shared_folder, path);
> + g_free(path);
> +
> + virt_viewer_signal_connect_object(self->priv->preferences_shared_folder,
> + "file-set",
> + G_CALLBACK(share_folder_changed), self,
> + G_CONNECT_SWAPPED);
> +
> +end:
> + g_object_unref(builder);
> +
> + return preferences;
> +}
> +
> +void
> +virt_viewer_window_show_preferences(VirtViewerWindow *self, GtkWidget *parent)
> +{
> + GtkWidget *preferences = virt_viewer_window_get_preferences(self);
> +
> + gtk_window_set_transient_for(GTK_WINDOW(preferences),
> + GTK_WINDOW(parent));
> +
> + gtk_window_present(GTK_WINDOW(preferences));
> +}
> +
> /*
> * Local variables:
> * c-indent-level: 4
> --
> 2.4.0
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/virt-tools-list
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
