[virt-tools-list] [PATCH 3/4] Port to GtkApplication API's
Jonathon Jongsma
jjongsma at redhat.com
Mon Nov 30 21:29:37 UTC 2015
On Fri, 2015-11-27 at 17:24 -0200, Eduardo Lima (Etrunko) wrote:
> Most of this patch consists in code being shuffled around to fit the
> expected flow while using the new APIs. I tried my best to make this
> patch the less intrusive as possible. Main changes are:
>
> - VirtViewerApp is now a subclass of GtkApplication
>
> Also, some mainloop calls were replaced, as follows:
> * gtk_main() -> g_application_run()
> * gtk_quit() -> g_application_quit()
>
> - Unified command line option handling:
> The logic has moved from the main functions and split in three, the
> common options, and specific ones for each application. With this, the
> main functions were highly simplified, and now basically responsible
> for instantiating the App object and running the main loop.
>
> - All Window objects must be associated with the Application, and with
> this, there is no need to emit our own 'window-added' signal, it will
> be done by GtkApplication by the time gtk_application_add_window() is
> called. The 'window-removed' signal has also been removed, as it was
> not being used anyway.
The 'window-removed' signal was not being used, but I suspect that the main
reason it was removed was that it conflicts with a signal of the same name from
GtkApplication. Perhaps useful to note that here.
>
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
> ---
> src/remote-viewer-main.c | 122 +-------------------------
> src/remote-viewer.c | 137 ++++++++++++++++++++++-------
> src/remote-viewer.h | 3 +-
> src/virt-viewer-app.c | 221 ++++++++++++++++++++++++++++++----------------
> -
> src/virt-viewer-app.h | 12 ++-
> src/virt-viewer-main.c | 102 +---------------------
> src/virt-viewer.c | 105 +++++++++++++++++-----
> src/virt-viewer.h | 8 +-
> 8 files changed, 346 insertions(+), 364 deletions(-)
>
> diff --git a/src/remote-viewer-main.c b/src/remote-viewer-main.c
> index 81cf736..0329d16 100644
> --- a/src/remote-viewer-main.c
> +++ b/src/remote-viewer-main.c
> @@ -30,32 +30,11 @@
> #include <io.h>
> #endif
>
> -#ifdef HAVE_GTK_VNC
> -#include <vncdisplay.h>
> -#endif
> -#ifdef HAVE_SPICE_GTK
> -#include <spice-option.h>
> -#endif
> -#ifdef HAVE_OVIRT
> -#include <govirt/ovirt-options.h>
> -#endif
> -
> #include "remote-viewer.h"
> #include "virt-viewer-app.h"
> #include "virt-viewer-session.h"
>
> static void
> -remote_viewer_version(void)
> -{
> - g_print(_("remote-viewer version %s"), VERSION BUILDID);
> -#ifdef REMOTE_VIEWER_OS_ID
> - g_print(" (OS ID: %s)", REMOTE_VIEWER_OS_ID);
> -#endif
> - g_print("\n");
> - exit(EXIT_SUCCESS);
> -}
> -
> -static void
> recent_add(gchar *uri, const gchar *mime_type)
> {
> GtkRecentManager *recent;
> @@ -87,118 +66,23 @@ static void connected(VirtViewerSession *session,
> int
> main(int argc, char **argv)
> {
> - GOptionContext *context;
> - GError *error = NULL;
> int ret = 1;
> - gchar **args = NULL;
> - gchar *uri = NULL;
> - char *title = NULL;
> RemoteViewer *viewer = NULL;
> -#ifdef HAVE_SPICE_GTK
> - gboolean controller = FALSE;
> -#endif
> VirtViewerApp *app;
> - const GOptionEntry options [] = {
> - { "version", 'V', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK,
> - remote_viewer_version, N_("Display version information"), NULL },
> - { "title", 't', 0, G_OPTION_ARG_STRING, &title,
> - N_("Set window title"), NULL },
> -#ifdef HAVE_SPICE_GTK
> - { "spice-controller", '\0', 0, G_OPTION_ARG_NONE, &controller,
> - N_("Open connection using Spice controller communication"), NULL },
> -#endif
> - { G_OPTION_REMAINING, '\0', 0, G_OPTION_ARG_STRING_ARRAY, &args,
> - NULL, "URI|VV-FILE" },
> - { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL }
> - };
> - GOptionGroup *app_options = NULL;
>
> virt_viewer_util_init(_("Remote Viewer"));
>
> - /* Setup command line options */
> - context = g_option_context_new (NULL);
> - g_option_context_set_summary(context, _("Remote viewer client"));
> - app_options = virt_viewer_app_get_option_group();
> - g_option_group_add_entries (app_options, options);
> - g_option_context_set_main_group (context, app_options);
> - g_option_context_add_group (context, gtk_get_option_group (TRUE));
> -#ifdef HAVE_GTK_VNC
> - g_option_context_add_group (context, vnc_display_get_option_group ());
> -#endif
> -#ifdef HAVE_SPICE_GTK
> - g_option_context_add_group (context, spice_get_option_group ());
> -#endif
> -#ifdef HAVE_OVIRT
> - g_option_context_add_group (context, ovirt_get_option_group ());
> -#endif
> - g_option_context_parse (context, &argc, &argv, &error);
> - if (error) {
> - char *base_name;
> - base_name = g_path_get_basename(argv[0]);
> - g_printerr(_("%s\nRun '%s --help' to see a full list of available
> command line options\n"),
> - error->message, base_name);
> - g_free(base_name);
> - goto cleanup;
> - }
> -
> - g_option_context_free(context);
> -
> -#ifdef HAVE_SPICE_GTK
> - if (controller) {
> - if (args) {
> - g_printerr(_("Error: extra arguments given while using Spice
> controller\n"));
> - goto cleanup;
> - }
> - } else
> -#endif
> - if (args) {
> - if (g_strv_length(args) > 1) {
> - g_printerr(_("Error: can't handle multiple URIs\n"));
> - goto cleanup;
> - } else if (g_strv_length(args) == 1) {
> - uri = g_strdup(args[0]);
> - }
> - }
> -
> -#ifdef HAVE_SPICE_GTK
> - if (controller) {
> - viewer = remote_viewer_new_with_controller();
> - g_object_set(viewer, "guest-name", "defined by Spice controller",
> NULL);
> - } else {
> -#endif
> - viewer = remote_viewer_new(uri);
> - if (title)
> - g_object_set(viewer, "title", title, NULL);
> -#ifdef HAVE_SPICE_GTK
> - }
> -#endif
> + viewer = remote_viewer_new();
> if (viewer == NULL)
> goto cleanup;
>
> app = VIRT_VIEWER_APP(viewer);
> -
> - if (!virt_viewer_app_start(app, &error)) {
> - if (g_error_matches(error, VIRT_VIEWER_ERROR,
> VIRT_VIEWER_ERROR_CANCELLED))
> - ret = 0;
> - else if (error) {
> - virt_viewer_app_simple_message_dialog(app, error->message);
> - }
> - goto cleanup;
> - }
> -
> g_signal_connect(virt_viewer_app_get_session(app), "session-connected",
> G_CALLBACK(connected), app);
> -
> - gtk_main();
> -
> - ret = 0;
> + ret = g_application_run(G_APPLICATION(viewer), argc, argv);
>
> cleanup:
> - g_free(uri);
> - if (viewer)
> - g_object_unref(viewer);
> - g_strfreev(args);
> - g_clear_error(&error);
> + g_object_unref(viewer);
>
> return ret;
> }
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 3f530a3..7a1e870 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -36,11 +36,9 @@
>
> #ifdef HAVE_SPICE_GTK
> #include <spice-controller.h>
> -#endif
> -
> -#ifdef HAVE_SPICE_GTK
> #include "virt-viewer-session-spice.h"
> #endif
> +
This hunk is fine, but not strictly necessary for porting to GtkApplication.
Consider moving it to a separate cleanup patch?
> #include "virt-viewer-app.h"
> #include "virt-viewer-auth.h"
> #include "virt-viewer-file.h"
> @@ -84,6 +82,12 @@ static OvirtVm * choose_vm(GtkWindow *main_window,
> #endif
>
> static gboolean remote_viewer_start(VirtViewerApp *self, GError **error);
> +
> +/* VirtViewerApp overrides */
> +static void remote_viewer_add_main_options(VirtViewerApp *self);
> +static gint remote_viewer_handle_options(VirtViewerApp *self, GVariantDict
> *options);
> +static const gchar *remote_viewer_version_string(void);
> +
> #ifdef HAVE_SPICE_GTK
> static gboolean remote_viewer_activate(VirtViewerApp *self, GError **error);
> static void remote_viewer_window_added(VirtViewerApp *self, VirtViewerWindow
> *win);
> @@ -195,10 +199,14 @@ remote_viewer_class_init (RemoteViewerClass *klass)
>
> object_class->get_property = remote_viewer_get_property;
> object_class->set_property = remote_viewer_set_property;
> + object_class->dispose = remote_viewer_dispose;
>
> app_class->start = remote_viewer_start;
> app_class->deactivated = remote_viewer_deactivated;
> - object_class->dispose = remote_viewer_dispose;
This move could also possibly be moved to a cleanup patch, I suppose. Not sure
it's worth it though...
> + app_class->add_main_options = remote_viewer_add_main_options;
> + app_class->handle_options = remote_viewer_handle_options;
> + app_class->version_string = remote_viewer_version_string;
> +
> #ifdef HAVE_SPICE_GTK
> app_class->activate = remote_viewer_activate;
> app_class->window_added = remote_viewer_window_added;
> @@ -210,7 +218,6 @@ remote_viewer_class_init (RemoteViewerClass *klass)
> "Spice controller",
>
> SPICE_CTRL_TYPE_CONTROLLER,
> G_PARAM_READWRITE |
> -
> G_PARAM_CONSTRUCT_ONLY |
>
> G_PARAM_STATIC_STRINGS));
> g_object_class_install_property(object_class,
> PROP_CTRL_FOREIGN_MENU,
> @@ -219,7 +226,6 @@ remote_viewer_class_init (RemoteViewerClass *klass)
> "Spice foreign menu",
>
> SPICE_CTRL_TYPE_FOREIGN_MENU,
> G_PARAM_READWRITE |
> -
> G_PARAM_CONSTRUCT_ONLY |
>
> G_PARAM_STATIC_STRINGS));
> #endif
> g_object_class_install_property(object_class,
> @@ -229,7 +235,6 @@ remote_viewer_class_init (RemoteViewerClass *klass)
> "Open recent
> dialog",
> FALSE,
> G_PARAM_READWRITE |
> -
> G_PARAM_CONSTRUCT_ONLY |
>
> G_PARAM_STATIC_STRINGS));
> }
I understand why these properties are no longer construct-only, however I wonder
if we want to add a warning if we try to set these properties after starting the
application. We can now technically change these properties after construction,
but we only use these values within remote_viewer_start(). So any property
changes that are made after calling remote_viewer_start() will not have any
effect. Do we care?
>
> @@ -240,11 +245,11 @@ remote_viewer_init(RemoteViewer *self)
> }
>
> RemoteViewer *
> -remote_viewer_new(const gchar *uri)
> +remote_viewer_new(void)
> {
> return g_object_new(REMOTE_VIEWER_TYPE,
> - "guri", uri,
> - "open-recent-dialog", uri == NULL,
> + "application-id", "org.fedorahosted.remote-viewer",
should we use "org.spice-space.remote-viewer" instead? or "org.virt
-manager.remote-viewer" (since virt-manager.org is where the releases are
hosted)?
> + "flags", G_APPLICATION_NON_UNIQUE,
> NULL);
> }
>
> @@ -267,26 +272,6 @@ foreign_menu_title_changed(SpiceCtrlForeignMenu *menu
> G_GNUC_UNUSED,
> spice_foreign_menu_updated(self);
> }
>
> -RemoteViewer *
> -remote_viewer_new_with_controller(void)
> -{
> - RemoteViewer *self;
> - SpiceCtrlController *ctrl = spice_ctrl_controller_new();
> - SpiceCtrlForeignMenu *menu = spice_ctrl_foreign_menu_new();
> -
> - self = g_object_new(REMOTE_VIEWER_TYPE,
> - "controller", ctrl,
> - "foreign-menu", menu,
> - NULL);
> - g_signal_connect(menu, "notify::title",
> - G_CALLBACK(foreign_menu_title_changed),
> - self);
> - g_object_unref(ctrl);
> - g_object_unref(menu);
> -
> - return self;
> -}
> -
> static void
> spice_ctrl_do_connect(SpiceCtrlController *ctrl G_GNUC_UNUSED,
> VirtViewerApp *self)
> @@ -1187,6 +1172,98 @@ cleanup:
> return ret;
> }
>
> +static const gchar*
> +remote_viewer_version_string(void)
> +{
> +#ifdef REMOTE_VIEWER_OS_ID
> + return " (OS ID: " REMOTE_VIEWER_OS_ID ")";
> +#endif
> + return "";
> +}
> +
This function seems a little strange. It doesn't really match the name of the
function. It just returns an OSID rather than a version string.
> +static const char OPT_TITLE[] = "title";
> +static const char OPT_CONTROLLER[] = "spice-controller";
I'd use
static const char *VARNAME
rather than
static const char VARNAME[]
However, see my comments about handle_local_options() below. Depending on what
we do there, we may not need to define constant variables for these option
names.
> +
> +static void
> +remote_viewer_add_main_options(VirtViewerApp *self)
> +{
> + GApplication *app = G_APPLICATION (self);
> + const GOptionEntry options [] = {
I know this is just moved code, but perhaps it's worthwhile to fix the coding
style to remove the spaces before the parentheses and brackets:
G_APPLICATION(self)
options[]
> + { OPT_TITLE, 't', 0, G_OPTION_ARG_STRING, NULL,
> + N_("Set window title"), NULL },
> +#ifdef HAVE_SPICE_GTK
> + { OPT_CONTROLLER, '\0', 0, G_OPTION_ARG_NONE, NULL,
> + N_("Open connection using Spice controller communication"), NULL },
> +#endif
> + { G_OPTION_REMAINING, '\0', 0, G_OPTION_ARG_STRING_ARRAY, NULL,
> + NULL, "URI|VV-FILE" },
> + { NULL },
> + };
> +
> + g_application_add_main_option_entries(app, options);
> +
> +#ifdef HAVE_OVIRT
> + g_application_add_option_group(app, ovirt_get_option_group ());
> +#endif
> +}
> +
> +static gint
> +remote_viewer_handle_options(VirtViewerApp *app, GVariantDict *options)
> +{
> + gint ret = -1;
> + gchar *title = NULL;
> + gchar **args = NULL;
> + gboolean controller = FALSE;
> +
> + if (g_variant_dict_lookup(options, G_OPTION_REMAINING, "^as", &args)) {
> + if (args && (g_strv_length(args) != 1)) {
> + g_printerr(_("Error: can't handle multiple URIs\n"));
> + ret = 0;
> + goto end;
> + }
> +
> + g_object_set(app, "guri", args[0], NULL);
> + }
> +
> + g_variant_dict_lookup(options, OPT_TITLE, "s", &title);
> +
> +#ifdef HAVE_SPICE_GTK
> + if (g_variant_dict_lookup(options, OPT_CONTROLLER, "b", &controller)) {
> + if (args) {
> + g_printerr(_("Error: extra arguments given while using Spice
> controller\n\n"));
> + ret = 0;
> + goto end;
> + } else {
> + RemoteViewer *self = REMOTE_VIEWER(app);
> + SpiceCtrlController *ctrl = spice_ctrl_controller_new();
> + SpiceCtrlForeignMenu *menu = spice_ctrl_foreign_menu_new();
> +
> + g_object_set(self, "guest-name", "defined by Spice controller",
> + "controller", ctrl,
> + "foreign-menu", menu,
> + NULL);
> +
> + g_signal_connect(menu, "notify::title",
> + G_CALLBACK(foreign_menu_title_changed),
> + self);
> +
> + g_object_unref(ctrl);
> + g_object_unref(menu);
> + }
> + }
> +#endif
> +
> + if (title && !controller) {
> + g_printerr("***** setting title to '%s'\n", title);
> + g_object_set(app, "title", title, NULL);
> + }
> +
> +end:
> + g_strfreev(args);
> + g_free(title);
> + return ret;
> +}
Right now you have a virtual function the base class (handle_local_options())
that does some work and then calls a different virtual function
(handle_options()) that is implemented in each subclass (but not in the parent
class). I think it might be cleaner to just get rid of the separate
handle_options() vfunc and implement handle_local_options() in each of the
subclasses. Then it could chain up and call the parent vfunc to do the common
work in the parent class.
But a bigger issue is that I think that handle_local_options() is not really the
function we're generally expected to use here. handle_local_options() is passed
a GVariantDict of options, which means that you have to inspect and handle them
all manually rather than letting the GOptionContext stuff do the work for you.
That means that if we want to add or remove options, we update the list of
GOptionEntry objects and also update the handle_local_options function to handle
that new option. This adds maintenance burden and makes it more likely they'll
get out of sync. If we instead handled the command_line() vfunc, we could take
advantage of GOptionContext parsing instead of manually inspecting the
GVariantDict. See
https://git.gnome.org/browse/glib/tree/gio/tests/gapplication-example-cmdline3.c
for an example of using g_option_context_parse() in a command-line handler.
I admit that GApplication is designed to enable a slightly more complicated
approach than we want here (a single-instance unique application), and I don't
have extensive experience with using this class, but I get the impression that
handle-local-options is not intended to be a signal that you normally need to
handle. Is there a reason you chose this signal/vfunc over 'command-line'?
> +
> /*
> * Local variables:
> * c-indent-level: 4
> diff --git a/src/remote-viewer.h b/src/remote-viewer.h
> index 6d445ca..e200fc1 100644
> --- a/src/remote-viewer.h
> +++ b/src/remote-viewer.h
> @@ -48,8 +48,7 @@ typedef struct {
>
> GType remote_viewer_get_type (void);
>
> -RemoteViewer* remote_viewer_new(const gchar *uri);
> -RemoteViewer* remote_viewer_new_with_controller(void);
> +RemoteViewer* remote_viewer_new (void);
>
> G_END_DECLS
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 653b30c..adabb27 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -32,6 +32,7 @@
> #include <string.h>
> #include <unistd.h>
> #include <locale.h>
> +#include <gio/gio.h>
> #include <glib/gprintf.h>
> #include <glib/gi18n.h>
>
> @@ -60,9 +61,11 @@
> #endif
> #ifdef HAVE_SPICE_GTK
> #include "virt-viewer-session-spice.h"
> +#include <spice-client.h>
This change is probably not directly related to the GtkApplication stuff so
could possibly be in a separate commit?
> #endif
>
> gboolean doDebug = FALSE;
> +static int opt_zoom = NORMAL_ZOOM_LEVEL;
>
> /* Signal handlers for about dialog */
> void virt_viewer_app_about_close(GtkWidget *dialog, VirtViewerApp *self);
> @@ -103,7 +106,13 @@ static void
> virt_viewer_app_update_pretty_address(VirtViewerApp *self);
> static void virt_viewer_app_set_fullscreen(VirtViewerApp *self, gboolean
> fullscreen);
> static void virt_viewer_app_update_menu_displays(VirtViewerApp *self);
> static void virt_viewer_update_smartcard_accels(VirtViewerApp *self);
> +static void virt_viewer_app_add_main_options(VirtViewerApp *self);
>
> +/* From GApplication */
> +static gint virt_viewer_app_handle_local_options(GApplication *self,
> + GVariantDict *options);
> +static void virt_viewer_app_startup_cb(GApplication *app, gpointer data);
> +static void virt_viewer_app_activate_cb(GApplication *app, gpointer data);
>
> struct _VirtViewerAppPrivate {
> VirtViewerWindow *main_window;
> @@ -155,7 +164,7 @@ struct _VirtViewerAppPrivate {
> };
>
>
> -G_DEFINE_ABSTRACT_TYPE(VirtViewerApp, virt_viewer_app, G_TYPE_OBJECT)
> +G_DEFINE_ABSTRACT_TYPE(VirtViewerApp, virt_viewer_app, GTK_TYPE_APPLICATION)
> #define GET_PRIVATE(o)
> \
> (G_TYPE_INSTANCE_GET_PRIVATE ((o), VIRT_VIEWER_TYPE_APP,
> VirtViewerAppPrivate))
>
> @@ -174,14 +183,6 @@ enum {
> PROP_UUID,
> };
>
> -enum {
> - SIGNAL_WINDOW_ADDED,
> - SIGNAL_WINDOW_REMOVED,
> - SIGNAL_LAST,
> -};
> -
> -static guint signals[SIGNAL_LAST];
> -
> void
> virt_viewer_app_set_debug(gboolean debug)
> {
> @@ -298,7 +299,7 @@ virt_viewer_app_quit(VirtViewerApp *self)
> }
> }
>
> - gtk_main_quit();
> + g_application_quit(G_APPLICATION(self));
> }
>
> static gint
> @@ -926,12 +927,11 @@ virt_viewer_app_window_new(VirtViewerApp *self, gint
> nth)
> virt_viewer_session_get_has_usbredir(self->priv
> ->session));
> }
>
> - g_signal_emit(self, signals[SIGNAL_WINDOW_ADDED], 0, window);
> -
> if (self->priv->fullscreen)
> app_window_try_fullscreen(self, window, nth);
>
> w = virt_viewer_window_get_window(window);
> + gtk_application_add_window(GTK_APPLICATION(self), w);
This is a slight change in behavior. It will now emit the window-added signal
after the call to app_window_try_fullscreen() rather than before. Not sure
whether this is important, just noting it. Perhaps move this where the original
signal was emitted?
> g_signal_connect(w, "hide", G_CALLBACK(viewer_window_visible_cb), self);
> g_signal_connect(w, "show", G_CALLBACK(viewer_window_visible_cb), self);
> g_signal_connect(w, "focus-in-event",
> G_CALLBACK(viewer_window_focus_in_cb), self);
> @@ -1046,8 +1046,6 @@ static void
> virt_viewer_app_remove_nth_window(VirtViewerApp *self,
> g_debug("Remove window %d %p", nth, win);
> self->priv->windows = g_list_remove(self->priv->windows, win);
>
> - g_signal_emit(self, signals[SIGNAL_WINDOW_REMOVED], 0, win);
> -
> g_object_unref(win);
> }
>
> @@ -1401,7 +1399,7 @@ virt_viewer_app_default_deactivated(VirtViewerApp *self,
> gboolean connect_error)
> }
>
> if (self->priv->quit_on_disconnect)
> - gtk_main_quit();
> + g_application_quit(G_APPLICATION(self));
> }
>
> static void
> @@ -1479,7 +1477,7 @@ virt_viewer_app_disconnected(VirtViewerSession *session
> G_GNUC_UNUSED, const gch
> virt_viewer_app_hide_all_windows(self);
>
> if (priv->quitting)
> - gtk_main_quit();
> + g_application_quit(G_APPLICATION(self));
I'd need to test to make sure, but I think it's possible that the call to
virt_viewer_app_hide_all_windows() a few lines above might result in the
application quitting because the application no longer has any associated
windows. In other words, even if priv->quitting is false, the application might
quit here. I could be wrong though. Have you tested this path?
>
> if (connect_error) {
> GtkWidget *dialog = virt_viewer_app_make_message_dialog(self,
> @@ -1744,13 +1742,6 @@ gboolean virt_viewer_app_start(VirtViewerApp *self,
> GError **error)
> return self->priv->started;
> }
>
> -static int opt_zoom = NORMAL_ZOOM_LEVEL;
> -static gchar *opt_hotkeys = NULL;
> -static gboolean opt_verbose = FALSE;
> -static gboolean opt_debug = FALSE;
> -static gboolean opt_fullscreen = FALSE;
> -static gboolean opt_kiosk = FALSE;
> -static gboolean opt_kiosk_quit = FALSE;
>
> static void
> title_maybe_changed(VirtViewerApp *self, GParamSpec* pspec G_GNUC_UNUSED,
> gpointer user_data G_GNUC_UNUSED)
> @@ -1765,8 +1756,6 @@ virt_viewer_app_init(VirtViewerApp *self)
> self->priv = GET_PRIVATE(self);
>
> gtk_window_set_default_icon_name("virt-viewer");
> - virt_viewer_app_set_debug(opt_debug);
> - virt_viewer_app_set_fullscreen(self, opt_fullscreen);
>
> self->priv->displays = g_hash_table_new_full(g_direct_hash,
> g_direct_equal, NULL, g_object_unref);
> self->priv->config = g_key_file_new();
> @@ -1782,17 +1771,14 @@ virt_viewer_app_init(VirtViewerApp *self)
>
> g_clear_error(&error);
>
> - if (opt_zoom < MIN_ZOOM_LEVEL || opt_zoom > MAX_ZOOM_LEVEL) {
> - g_printerr(_("Zoom level must be within %d-%d\n"), MIN_ZOOM_LEVEL,
> MAX_ZOOM_LEVEL);
> - opt_zoom = NORMAL_ZOOM_LEVEL;
> - }
> -
> self->priv->initial_display_map =
> virt_viewer_app_get_monitor_mapping_for_section(self, "fallback");
> - self->priv->verbose = opt_verbose;
> - self->priv->quit_on_disconnect = opt_kiosk ? opt_kiosk_quit : TRUE;
> g_signal_connect(self, "notify::guest-name",
> G_CALLBACK(title_maybe_changed), NULL);
> g_signal_connect(self, "notify::title", G_CALLBACK(title_maybe_changed),
> NULL);
> g_signal_connect(self, "notify::guri", G_CALLBACK(title_maybe_changed),
> NULL);
> +
> + /* From GApplication */
> + g_signal_connect(self, "startup", G_CALLBACK(virt_viewer_app_startup_cb),
> NULL);
> + g_signal_connect(self, "activate",
> G_CALLBACK(virt_viewer_app_activate_cb), NULL);
> }
>
> static void
> @@ -1851,13 +1837,17 @@ static void
> virt_viewer_app_constructed(GObject *object)
> {
> VirtViewerApp *self = VIRT_VIEWER_APP(object);
> + virt_viewer_app_add_main_options(self);
> +}
> +
> +static void
> +virt_viewer_app_startup_cb(GApplication *app, G_GNUC_UNUSED gpointer data)
> +{
> + VirtViewerApp *self = VIRT_VIEWER_APP(app);
>
> self->priv->main_window = virt_viewer_app_window_new(self,
>
> virt_viewer_app_get_first_monitor(self));
> self->priv->main_notebook =
> GTK_WIDGET(virt_viewer_window_get_notebook(self->priv->main_window));
> -
> - virt_viewer_app_set_kiosk(self, opt_kiosk);
> - virt_viewer_app_set_hotkeys(self, opt_hotkeys);
> virt_viewer_window_set_zoom_level(self->priv->main_window, opt_zoom);
>
> virt_viewer_set_insert_smartcard_accel(self, GDK_F8, GDK_SHIFT_MASK);
> @@ -1871,9 +1861,26 @@ virt_viewer_app_constructed(GObject *object)
> }
Do we really need a separate 'startup' handler? Or can we move stuff from here
into either the "handle-local-options" or "activate" handlers? (or to "command
-line" if we choose that route)
>
> static void
> +virt_viewer_app_activate_cb(GApplication *app, G_GNUC_UNUSED gpointer data)
> +{
> + GError *error = NULL;
> + VirtViewerApp *self = VIRT_VIEWER_APP(app);
> +
> + if (!virt_viewer_app_start(VIRT_VIEWER_APP(self), &error)) {
> + if (error && !g_error_matches(error, VIRT_VIEWER_ERROR,
> VIRT_VIEWER_ERROR_CANCELLED)) {
> + virt_viewer_app_simple_message_dialog(self, error->message);
> + }
> +
> + g_clear_error(&error);
> + g_application_quit(app);
> + }
> +}
> +
> +static void
> virt_viewer_app_class_init (VirtViewerAppClass *klass)
> {
> GObjectClass *object_class = G_OBJECT_CLASS (klass);
> + GApplicationClass *app_class = G_APPLICATION_CLASS (klass);
>
> g_type_class_add_private (klass, sizeof (VirtViewerAppPrivate));
>
> @@ -1882,6 +1889,8 @@ virt_viewer_app_class_init (VirtViewerAppClass *klass)
> object_class->set_property = virt_viewer_app_set_property;
> object_class->dispose = virt_viewer_app_dispose;
>
> + app_class->handle_local_options = virt_viewer_app_handle_local_options;
> +
> klass->start = virt_viewer_app_default_start;
> klass->initial_connect = virt_viewer_app_default_initial_connect;
> klass->activate = virt_viewer_app_default_activate;
> @@ -1992,28 +2001,6 @@ virt_viewer_app_class_init (VirtViewerAppClass *klass)
> G_PARAM_READABLE |
> G_PARAM_WRITABLE |
>
> G_PARAM_STATIC_STRINGS));
> -
> - signals[SIGNAL_WINDOW_ADDED] =
> - g_signal_new("window-added",
> - G_OBJECT_CLASS_TYPE(object_class),
> - G_SIGNAL_RUN_LAST,
> - G_STRUCT_OFFSET(VirtViewerAppClass, window_added),
> - NULL, NULL,
> - g_cclosure_marshal_VOID__OBJECT,
> - G_TYPE_NONE,
> - 1,
> - G_TYPE_OBJECT);
> -
> - signals[SIGNAL_WINDOW_REMOVED] =
> - g_signal_new("window-removed",
> - G_OBJECT_CLASS_TYPE(object_class),
> - G_SIGNAL_RUN_LAST,
> - G_STRUCT_OFFSET(VirtViewerAppClass, window_removed),
> - NULL, NULL,
> - g_cclosure_marshal_VOID__OBJECT,
> - G_TYPE_NONE,
> - 1,
> - G_TYPE_OBJECT);
> }
>
> void
> @@ -2536,48 +2523,122 @@ virt_viewer_app_show_preferences(VirtViewerApp *self,
> GtkWidget *parent)
> }
>
> static gboolean
> -option_kiosk_quit(G_GNUC_UNUSED const gchar *option_name,
> - const gchar *value,
> - G_GNUC_UNUSED gpointer data, GError **error)
> +virt_viewer_app_option_kiosk_quit(VirtViewerApp *self, const gchar *value)
> {
> - if (g_str_equal(value, "never")) {
> - opt_kiosk_quit = FALSE;
> + if (!g_strcmp0(value, "never")) {
I'd prefer an explicit ' == 0' here and in the following test
> + self->priv->quit_on_disconnect = FALSE;
> return TRUE;
> }
> - if (g_str_equal(value, "on-disconnect")) {
> - opt_kiosk_quit = TRUE;
> + if (!g_strcmp0(value, "on-disconnect")) {
> + self->priv->quit_on_disconnect = TRUE;
> return TRUE;
> }
>
> - g_set_error(error, G_OPTION_ERROR, G_OPTION_ERROR_FAILED, _("Invalid
> kiosk-quit argument: %s"), value);
> + g_printerr(_("Invalid kiosk-quit argument: %s\n\n"), value);
> +
> return FALSE;
> }
>
> -GOptionGroup*
> -virt_viewer_app_get_option_group(void)
> +static const char OPT_VERSION[] = "version";
> +static const char OPT_ZOOM[] = "zoom";
> +static const char OPT_FULLSCREEN[] = "full-screen";
> +static const char OPT_HOTKEYS[] = "hotkeys";
> +static const char OPT_KIOSK[] = "kiosk";
> +static const char OPT_KIOSK_QUIT[] = "kiosk-quit";
> +static const char OPT_VERBOSE[] = "verbose";
> +static const char OPT_DEBUG[] = "debug";
Again, I'd prefer string declarations (char*) rather than array declarations
here if we need these variables.
> +
> +static void
> +virt_viewer_app_add_main_options(VirtViewerApp *self)
> {
> - static const GOptionEntry options [] = {
> - { "zoom", 'z', 0, G_OPTION_ARG_INT, &opt_zoom,
> + GApplication *app = G_APPLICATION (self);
> + VirtViewerAppClass *app_class = VIRT_VIEWER_APP_GET_CLASS(self);
> +
> + static const GOptionEntry virt_viewer_app_options [] = {
> + { OPT_VERSION, 'V', 0, G_OPTION_ARG_NONE, NULL,
> + N_("Display version information"), NULL },
> + { OPT_ZOOM, 'z', 0, G_OPTION_ARG_INT, NULL,
> N_("Zoom level of window, in percentage"), "ZOOM" },
> - { "full-screen", 'f', 0, G_OPTION_ARG_NONE, &opt_fullscreen,
> + { OPT_FULLSCREEN, 'f', 0, G_OPTION_ARG_NONE, NULL,
> N_("Open in full screen mode (adjusts guest resolution to fit the
> client)"), NULL },
> - { "hotkeys", 'H', 0, G_OPTION_ARG_STRING, &opt_hotkeys,
> + { OPT_HOTKEYS, 'H', 0, G_OPTION_ARG_STRING, NULL,
> N_("Customise hotkeys"), NULL },
> - { "kiosk", 'k', 0, G_OPTION_ARG_NONE, &opt_kiosk,
> + { OPT_KIOSK, 'k', 0, G_OPTION_ARG_NONE, NULL,
> N_("Enable kiosk mode"), NULL },
> - { "kiosk-quit", '\0', 0, G_OPTION_ARG_CALLBACK, option_kiosk_quit,
> + { OPT_KIOSK_QUIT, '\0', 0, G_OPTION_ARG_STRING, NULL,
> N_("Quit on given condition in kiosk mode"), N_("<never|on
> -disconnect>") },
> - { "verbose", 'v', 0, G_OPTION_ARG_NONE, &opt_verbose,
> + { OPT_VERBOSE, 'v', 0, G_OPTION_ARG_NONE, NULL,
> N_("Display verbose information"), NULL },
> - { "debug", '\0', 0, G_OPTION_ARG_NONE, &opt_debug,
> + { OPT_DEBUG, '\0', 0, G_OPTION_ARG_NONE, NULL,
> N_("Display debugging information"), NULL },
> - { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL }
> + { NULL },
> };
> - GOptionGroup *group;
> - group = g_option_group_new("virt-viewer", NULL, NULL, NULL, NULL);
> - g_option_group_add_entries(group, options);
>
> - return group;
> + g_application_add_main_option_entries(app, virt_viewer_app_options);
> + app_class->add_main_options(self);
> +
> +#ifdef HAVE_GTK_VNC
> + g_application_add_option_group(app, vnc_display_get_option_group());
> +#endif
> +#ifdef HAVE_SPICE_GTK
> + g_application_add_option_group(app, spice_get_option_group());
> +#endif
> +}
> +
> +static gint
> +virt_viewer_app_handle_local_options(GApplication *app,
> + GVariantDict *options)
> +{
> + VirtViewerApp *self = VIRT_VIEWER_APP(app);
> + VirtViewerAppClass *app_class = VIRT_VIEWER_APP_GET_CLASS(self);
> + gchar *opt_hotkeys = NULL;
> + gint ret = -1;
> +
> + /* Finish program execution if version is supplied. */
> + if (g_variant_dict_contains(options, OPT_VERSION)) {
> + gchar *version_str = g_strdup_printf(_("%s version %s%s\n"),
> + g_get_prgname(),
> + VERSION BUILDID,
> + app_class->version_string ?
> app_class->version_string() : "");
> + g_print(version_str);
> + g_free(version_str);
> + return 0;
> + }
> +
> + virt_viewer_app_set_fullscreen(self, g_variant_dict_contains(options,
> OPT_FULLSCREEN));
> + virt_viewer_app_set_debug(g_variant_dict_contains(options, OPT_DEBUG));
> + self->priv->verbose = g_variant_dict_contains(options, OPT_VERBOSE);
> +
> + if (g_variant_dict_contains(options, OPT_KIOSK)) {
> + gchar *opt_kiosk_quit = NULL;
> +
> + virt_viewer_app_set_kiosk(self, TRUE);
> + self->priv->quit_on_disconnect = TRUE;
> +
> + if (g_variant_dict_lookup(options, OPT_KIOSK_QUIT, "s",
> &opt_kiosk_quit) &&
> + !virt_viewer_app_option_kiosk_quit(self, opt_kiosk_quit)) {
> + ret = 0;
> + goto end;
> + }
> + }
> +
> + if (g_variant_dict_lookup(options, OPT_ZOOM, "i", &opt_zoom)) {
> + if (opt_zoom < MIN_ZOOM_LEVEL || opt_zoom > MAX_ZOOM_LEVEL) {
> + g_printerr(_("Zoom level must be within %d-%d\n"),
> MIN_ZOOM_LEVEL, MAX_ZOOM_LEVEL);
> + opt_zoom = NORMAL_ZOOM_LEVEL;
> + }
> + }
> +
> + if (g_variant_dict_lookup(options, OPT_HOTKEYS, "s", &opt_hotkeys))
> + virt_viewer_app_set_hotkeys(self, opt_hotkeys);
> +
> + ret = app_class->handle_options(self, options);
> +
> +end:
> + if (!ret)
> + g_printerr(_("Run with --help to see a full list of available command
> line options\n\n"));
> +
> + return ret;
> }
Same comment here about handle-local-options vs. command-line
>
> gboolean virt_viewer_app_get_session_cancelled(VirtViewerApp *self)
> diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
> index bbbc9b4..3e47d08 100644
> --- a/src/virt-viewer-app.h
> +++ b/src/virt-viewer-app.h
> @@ -24,6 +24,7 @@
> #define VIRT_VIEWER_APP_H
>
> #include <glib-object.h>
> +#include <gtk/gtk.h>
> #include "virt-viewer-util.h"
> #include "virt-viewer-window.h"
>
> @@ -39,16 +40,15 @@ G_BEGIN_DECLS
> typedef struct _VirtViewerAppPrivate VirtViewerAppPrivate;
>
> typedef struct {
> - GObject parent;
> + GtkApplication parent;
> VirtViewerAppPrivate *priv;
> } VirtViewerApp;
>
> typedef struct {
> - GObjectClass parent_class;
> + GtkApplicationClass parent_class;
>
> /* signals */
> void (*window_added) (VirtViewerApp *self, VirtViewerWindow *window);
> - void (*window_removed) (VirtViewerApp *self, VirtViewerWindow *window);
I think we need to remove the 'window_added' vfunc here as well, since we
removed the signal that was associated with this vfunc.
>
> /*< private >*/
> gboolean (*start) (VirtViewerApp *self, GError **error);
> @@ -56,6 +56,11 @@ typedef struct {
> gboolean (*activate) (VirtViewerApp *self, GError **error);
> void (*deactivated) (VirtViewerApp *self, gboolean connect_error);
> gboolean (*open_connection)(VirtViewerApp *self, int *fd);
> +
> + void (*add_main_options) (VirtViewerApp *self);
> + int (*handle_options) (VirtViewerApp *self, GVariantDict *options);
> +
> + const gchar* (*version_string) (void);
> } VirtViewerAppClass;
>
> GType virt_viewer_app_get_type (void);
> @@ -95,7 +100,6 @@ GList* virt_viewer_app_get_windows(VirtViewerApp *self);
> gboolean virt_viewer_app_get_enable_accel(VirtViewerApp *self);
> VirtViewerSession* virt_viewer_app_get_session(VirtViewerApp *self);
> gboolean virt_viewer_app_get_fullscreen(VirtViewerApp *app);
> -GOptionGroup* virt_viewer_app_get_option_group(void);
> 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);
> diff --git a/src/virt-viewer-main.c b/src/virt-viewer-main.c
> index 505b472..dd51dcb 100644
> --- a/src/virt-viewer-main.c
> +++ b/src/virt-viewer-main.c
> @@ -25,118 +25,24 @@
> #include <gtk/gtk.h>
> #include <glib/gi18n.h>
> #include <stdlib.h>
> -#ifdef HAVE_GTK_VNC
> -#include <vncdisplay.h>
> -#endif
> -#ifdef HAVE_SPICE_GTK
> -#include <spice-option.h>
> -#endif
> -#include "virt-viewer.h"
> -
> -static void virt_viewer_version(void)
> -{
> - g_print(_("%s version %s\n"), PACKAGE, VERSION BUILDID);
> -
> - exit(EXIT_SUCCESS);
> -}
>
> +#include "virt-viewer.h"
>
> int main(int argc, char **argv)
> {
> - GOptionContext *context;
> - GError *error = NULL;
> int ret = 1;
> - char *uri = NULL;
> - gchar **args = NULL;
> - gboolean direct = FALSE;
> - gboolean attach = FALSE;
> - gboolean waitvm = FALSE;
> - gboolean reconnect = FALSE;
> VirtViewer *viewer = NULL;
> - char *base_name;
> - char *help_msg = NULL;
> - const GOptionEntry options [] = {
> - { "version", 'V', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK,
> - virt_viewer_version, N_("Display version information"), NULL },
> - { "direct", 'd', 0, G_OPTION_ARG_NONE, &direct,
> - N_("Direct connection with no automatic tunnels"), NULL },
> - { "attach", 'a', 0, G_OPTION_ARG_NONE, &attach,
> - N_("Attach to the local display using libvirt"), NULL },
> - { "connect", 'c', 0, G_OPTION_ARG_STRING, &uri,
> - N_("Connect to hypervisor"), "URI"},
> - { "wait", 'w', 0, G_OPTION_ARG_NONE, &waitvm,
> - N_("Wait for domain to start"), NULL },
> - { "reconnect", 'r', 0, G_OPTION_ARG_NONE, &reconnect,
> - N_("Reconnect to domain upon restart"), NULL },
> - { G_OPTION_REMAINING, '\0', 0, G_OPTION_ARG_STRING_ARRAY, &args,
> - NULL, "-- DOMAIN-NAME|ID|UUID" },
> - { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL }
> - };
> - GOptionGroup* app_options = NULL;
>
> virt_viewer_util_init(_("Virt Viewer"));
>
> - base_name = g_path_get_basename(argv[0]);
> - help_msg = g_strdup_printf(_("Run '%s --help' to see a full list of
> available command line options"),
> - base_name);
> - g_free(base_name);
> -
> - /* Setup command line options */
> - context = g_option_context_new (NULL);
> - g_option_context_set_summary (context, _("Virtual machine graphical
> console"));
> - app_options = virt_viewer_app_get_option_group();
> - g_option_group_add_entries (app_options, options);
> - g_option_context_set_main_group (context, app_options);
> - g_option_context_add_group (context, gtk_get_option_group (TRUE));
> -#ifdef HAVE_GTK_VNC
> - g_option_context_add_group (context, vnc_display_get_option_group ());
> -#endif
> -#ifdef HAVE_SPICE_GTK
> - g_option_context_add_group (context, spice_get_option_group ());
> -#endif
> - g_option_context_parse (context, &argc, &argv, &error);
> - if (error) {
> - g_printerr("%s\n%s\n",
> - error->message, help_msg);
> - goto cleanup;
> - }
> -
> - g_option_context_free(context);
> -
> - if (args && (g_strv_length(args) != 1)) {
> - g_printerr(_("\nUsage: %s [OPTIONS] [DOMAIN
> -NAME|ID|UUID]\n\n%s\n\n"), argv[0], help_msg);
> - goto cleanup;
> - }
> -
> - if (args == NULL && waitvm) {
> - g_printerr(_("\nNo DOMAIN-NAME|ID|UUID was specified for '-
> -wait'\n\n"));
> - goto cleanup;
> - }
> -
> - viewer = virt_viewer_new(uri, (args) ? args[0] : NULL, direct, attach,
> waitvm, reconnect);
> + viewer = virt_viewer_new();
> if (viewer == NULL)
> goto cleanup;
>
> - if (!virt_viewer_app_start(VIRT_VIEWER_APP(viewer), &error)) {
> - if (g_error_matches(error, VIRT_VIEWER_ERROR,
> VIRT_VIEWER_ERROR_CANCELLED))
> - ret = 0;
> - else if (error) {
> - virt_viewer_app_simple_message_dialog(VIRT_VIEWER_APP(viewer),
> error->message);
> - }
> - goto cleanup;
> - }
> -
> - gtk_main();
> -
> - ret = 0;
> + ret = g_application_run(G_APPLICATION(viewer), argc, argv);
>
> cleanup:
> - if (viewer)
> - g_object_unref(viewer);
> - g_free(uri);
> - g_strfreev(args);
> - g_free(help_msg);
> - g_clear_error(&error);
> + g_object_unref(viewer);
>
> return ret;
> }
> diff --git a/src/virt-viewer.c b/src/virt-viewer.c
> index 10f624d..4fab44a 100644
> --- a/src/virt-viewer.c
> +++ b/src/virt-viewer.c
> @@ -73,6 +73,10 @@ static gboolean virt_viewer_start(VirtViewerApp *self,
> GError **error);
> static void virt_viewer_dispose (GObject *object);
> static int virt_viewer_connect(VirtViewerApp *app, GError **error);
>
> +/* VirtViewerApp overrides */
> +static void virt_viewer_add_main_options(VirtViewerApp *self);
> +static gint virt_viewer_handle_options(VirtViewerApp *self, GVariantDict
> *options);
> +
> static void
> virt_viewer_class_init (VirtViewerClass *klass)
> {
> @@ -87,6 +91,9 @@ virt_viewer_class_init (VirtViewerClass *klass)
> app_class->deactivated = virt_viewer_deactivated;
> app_class->open_connection = virt_viewer_open_connection;
> app_class->start = virt_viewer_start;
> +
> + app_class->add_main_options = virt_viewer_add_main_options;
> + app_class->handle_options = virt_viewer_handle_options;
> }
>
> static void
> @@ -106,7 +113,7 @@ virt_viewer_connect_timer(void *opaque)
>
> if (!virt_viewer_app_is_active(app) &&
> !virt_viewer_app_initial_connect(app, NULL))
> - gtk_main_quit();
> + g_application_quit(G_APPLICATION(self));
>
> if (virt_viewer_app_is_active(app)) {
> self->priv->reconnect_poll = 0;
> @@ -975,34 +982,84 @@ virt_viewer_start(VirtViewerApp *app, GError **error)
> return VIRT_VIEWER_APP_CLASS(virt_viewer_parent_class)->start(app,
> error);
> }
>
> -VirtViewer *
> -virt_viewer_new(const char *uri,
> - const char *name,
> - gboolean direct,
> - gboolean attach,
> - gboolean waitvm,
> - gboolean reconnect)
> +static const char OPT_DIRECT[] = "direct";
> +static const char OPT_ATTACH[] = "attach";
> +static const char OPT_CONNECT[] = "connect";
> +static const char OPT_WAIT[] = "wait";
> +static const char OPT_RECONNECT[] = "reconnect";
string vs. array again.
> +
> +static void
> +virt_viewer_add_main_options(VirtViewerApp *self)
> {
> - VirtViewer *self;
> - VirtViewerApp *app;
> - VirtViewerPrivate *priv;
> + GApplication *app = G_APPLICATION (self);
> + const GOptionEntry options [] = {
> + { OPT_DIRECT, 'd', 0, G_OPTION_ARG_NONE, NULL,
> + N_("Direct connection with no automatic tunnels"), NULL },
> + { OPT_ATTACH, 'a', 0, G_OPTION_ARG_NONE, NULL,
> + N_("Attach to the local display using libvirt"), NULL },
> + { OPT_CONNECT, 'c', 0, G_OPTION_ARG_STRING, NULL,
> + N_("Connect to hypervisor"), "URI"},
> + { OPT_WAIT, 'w', 0, G_OPTION_ARG_NONE, NULL,
> + N_("Wait for domain to start"), NULL },
> + { OPT_RECONNECT, 'r', 0, G_OPTION_ARG_NONE, NULL,
> + N_("Reconnect to domain upon restart"), NULL },
> + { G_OPTION_REMAINING, '\0', 0, G_OPTION_ARG_STRING_ARRAY, NULL,
> + NULL, "-- DOMAIN-NAME|ID|UUID" },
> + { NULL },
> + };
>
> - self = g_object_new(VIRT_VIEWER_TYPE,
> - "guest-name", name,
> - NULL);
> - app = VIRT_VIEWER_APP(self);
> - priv = self->priv;
> + g_application_add_main_option_entries(app, options);
> +}
>
> - virt_viewer_app_set_direct(app, direct);
> - virt_viewer_app_set_attach(app, attach);
> +static gint
> +virt_viewer_handle_options(VirtViewerApp *app, GVariantDict *options)
> +{
> + VirtViewer *self = VIRT_VIEWER(app);
> + gint ret = -1;
> + gchar *uri = NULL;
> + gchar **args = NULL;
> +
> + if (g_variant_dict_lookup(options, G_OPTION_REMAINING, "^as", &args)) {
> + /* FIXME: with gapplication we should be able to accept more than one
> domain */
> + if (args && (g_strv_length(args) != 1)) {
> + g_printerr(_("\nUsage: %s [OPTIONS] [DOMAIN-NAME|ID|UUID]\n\n"),
> PACKAGE);
> + ret = 0;
> + goto end;
> + }
>
> - /* should probably be properties instead */
> - priv->uri = g_strdup(uri);
> - priv->domkey = g_strdup(name);
> - priv->waitvm = waitvm;
> - priv->reconnect = reconnect;
> + self->priv->domkey = g_strdup(args[0]);
> + }
>
> - return self;
> + if (g_variant_dict_contains(options, OPT_WAIT)) {
> + g_printerr("self->priv->domkey = %s\n", self->priv->domkey);
> + if (!self->priv->domkey) {
> + g_printerr(_("\nNo DOMAIN-NAME|ID|UUID was specified for '-
> -wait'\n\n"));
> + ret = 0;
> + goto end;
> + }
> +
> + self->priv->waitvm = TRUE;
> + }
> +
> + virt_viewer_app_set_direct(app, g_variant_dict_contains(options,
> OPT_DIRECT));
> + virt_viewer_app_set_attach(app, g_variant_dict_contains(options,
> OPT_ATTACH));
> + self->priv->reconnect = g_variant_dict_contains(options, OPT_RECONNECT);
> +
> + if (g_variant_dict_lookup(options, OPT_CONNECT, "s", &uri))
> + self->priv->uri = g_strdup(uri);
> +
> +end:
> + g_strfreev(args);
> + return ret;
> +}
> +
> +VirtViewer *
> +virt_viewer_new(void)
> +{
> + return g_object_new(VIRT_VIEWER_TYPE,
> + "application-id", "org.fedorahosted.virt-viewer",
> + "flags", G_APPLICATION_NON_UNIQUE,
> + NULL);
> }
>
> /*
> diff --git a/src/virt-viewer.h b/src/virt-viewer.h
> index c962615..5aeacb0 100644
> --- a/src/virt-viewer.h
> +++ b/src/virt-viewer.h
> @@ -48,13 +48,7 @@ typedef struct {
>
> GType virt_viewer_get_type (void);
>
> -VirtViewer *
> -virt_viewer_new(const char *uri,
> - const char *name,
> - gboolean direct,
> - gboolean attach,
> - gboolean waitvm,
> - gboolean reconnect);
> +VirtViewer *virt_viewer_new (void);
>
> G_END_DECLS
>
Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
More information about the virt-tools-list
mailing list