GNOME Bugzilla – Bug 703505
Boxes --help should include spice-gtk option group
Last modified: 2017-01-09 14:01:21 UTC
[hans@shalem ~]$ gnome-boxes --help Usage: gnome-boxes [OPTION...] - A simple application to access remote or virtual machines Application Options: --version Display version number -f, --full-screen Open in full screen --checks Check virtualization capabilities --open-uuid Open box with UUID --search Search term Given that boxes uses clutter-gtk and spice-gtk I would expect there option groups to be present ie something like: Usage: gnome-boxes [OPTION...] - A simple application to access remote or virtual machines Help Options: -h, --help Show help options --help-all Show all help options --help-gtk Show GTK+ Options --help-cogl Show Cogl options --help-clutter Show Clutter Options --help-spice Show Spice Options Application Options: --version Display version number -f, --full-screen Open in full screen --checks Check virtualization capabilities --open-uuid Open box with UUID --search Search term Note the GoptionGroup for clutter-gtk are somewhat tricky to use, see the clutter-gtk docs. The resulting code should look something like this: GOptionContext *context; gboolean res; /* We cannot simply call gtk_clutter_init_with_args() here, since that * will result in the commandline being parsed without spice support. */ context = g_option_context_new (parameter_string); g_option_context_add_main_entries (context, boxes_main_entries, translation_domain); g_option_context_set_translation_domain (context, translation_domain); g_option_context_add_group (context, gtk_get_option_group (TRUE)); g_option_context_add_group (context, cogl_get_option_group ()); g_option_context_add_group (context, clutter_get_option_group_without_init ()); g_option_context_add_group (context, gtk_clutter_get_option_group ()); g_option_context_add_group (context, spice_set_session_option()); res = g_option_context_parse (context, argc, argv, error); And you can then drop any direct calls to clutter_gtk_init (and others), as calling the _get_option_group () function typically also initializes the lib (hence the need to call clutter_get_option_group_without_init () so that gtk_clutter_get_option_group () can do the clutter init).
Erm in the example code: g_option_context_add_group (context, spice_set_session_option()); Should be: g_option_context_add_group (context, spice_get_option_group()); Of course, copy and paste fail.
We don't use clutter-gtk anymore so its only about spice-gtk now.
Actually we don't even show gtk+ options anymore in --help output. :(
Created attachment 327661 [details] [review] app: Include various help option groups Add option groups for spice-gtk, gtk-vnc and gtk+.
Review of attachment 327661 [details] [review]: ::: src/app.vala @@ +197,3 @@ var opt_context = new OptionContext (parameter_string); opt_context.add_main_entries (options, null); + opt_context.add_group (Spice.get_option_group ()); Bindings for spice_get_option_group() will be available in spice-gtk v0.32 (Boxes require spice-gtk v0.27). You should use it conditionally or increase the dependency of spice-gtk
Review of attachment 327661 [details] [review]: ::: src/app.vala @@ +197,3 @@ var opt_context = new OptionContext (parameter_string); opt_context.add_main_entries (options, null); + opt_context.add_group (Spice.get_option_group ()); Yes, you need to bump requirement of spice-gtk in configure.ac, preferably in a separate patch that comes before this one (See commit cd8252f573db0c326a18ddc291270f95f6eeb80a for example).
Review of attachment 327661 [details] [review]: Well, actually the patch itself doesn't need any change.
Hi Zeeshan, Since I no longer work on spice, I hope that you can take care of bumping the spice-gtk requirement in configure.ac ? Regards, Hans
(In reply to Hans de Goede from comment #8) > Hi Zeeshan, > > Since I no longer work on spice, I hope that you can take care of bumping > the spice-gtk requirement in configure.ac ? This was a review of the patch so I was expecting patch contributor to act on that, not the reporter. :)
Created attachment 328217 [details] [review] Bump spice-gtk dependency Boxes is going to use a method to display the spice-gtk option group for which a Vala binding was reintroduced in 0.32 release.
Review of attachment 328217 [details] [review]: Shortlog could be more specific, bump to what? My fault really, I pointed you to a not the best example. :) See 8949047fdc0c41fdf5d896e58078011d4f826d65 instead.
Created attachment 328227 [details] [review] configure: Bump spice-gtk dependency to 0.32 We need this in order to use the reintroduced Vala binding for the spice-gtk option group.
Review of attachment 328227 [details] [review]: ACK, let's not forget this once spice-gtk is released.
Arghh forgot about it, sorry. spice-gtk 0.32 was released, so I think we can push these patches.
Review of attachment 328227 [details] [review]: I failed to push this because of bad email address. I'll fix of course but please make sure you have proper email address on other patches in bz and in future. :)
Attachment 327661 [details] pushed as 931b07c - app: Include various help option groups Attachment 328227 [details] pushed as 4709154 - configure: Bump spice-gtk dependency to 0.32