gapplication: Expose zero-valued numbers in handle-local-options

This is a bit of a compromise. Since the option parsing in
`GApplication` is built on `GOptionContext`, there’s no way to
reliably indicate that a given option was passed by the user, other than
by its value changing. If the default value is zero, but the user
explicitly passed zero, nothing changes, so it’s not obvious that the
option was explicitly provided.

When just `GOptionContext` is being used, this is fine, as that’s
obvious what will happen from the way the API is built. With
`GApplication::handle-local-options`, though, the `GVariantDict`
provided by GLib to the callback claims to only contain the values of
the options provided by the user, and no defaults.

It’s not actually possible for GLib to do that reliably.

Previously, GLib was dropping all numeric values which were zero valued
(i.e. the defaults), as they *could* have been the defaults. It seems
like a slightly better behaviour to instead *not* drop those numeric
values, and err on the side of reporting some defaults as user-provided
(even if they weren’t) rather than dropping some user-provided values
which happen to be the defaults.

This adds a test for the case of parsing a double; the cases for
integers are analogous.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #2329
This commit is contained in:
Philip Withnall 2021-02-15 22:07:59 +00:00
parent 7b6ccc8bdb
commit 1ed67a9c44
2 changed files with 55 additions and 6 deletions

View File

@ -434,8 +434,7 @@ g_application_pack_option_entries (GApplication *application,
break;
case G_OPTION_ARG_INT:
if (*(gint32 *) entry->arg_data)
value = g_variant_new_int32 (*(gint32 *) entry->arg_data);
value = g_variant_new_int32 (*(gint32 *) entry->arg_data);
break;
case G_OPTION_ARG_FILENAME:
@ -454,13 +453,11 @@ g_application_pack_option_entries (GApplication *application,
break;
case G_OPTION_ARG_DOUBLE:
if (*(gdouble *) entry->arg_data)
value = g_variant_new_double (*(gdouble *) entry->arg_data);
value = g_variant_new_double (*(gdouble *) entry->arg_data);
break;
case G_OPTION_ARG_INT64:
if (*(gint64 *) entry->arg_data)
value = g_variant_new_int64 (*(gint64 *) entry->arg_data);
value = g_variant_new_int64 (*(gint64 *) entry->arg_data);
break;
default:
@ -699,6 +696,10 @@ add_packed_option (GApplication *application,
* consumed, they will no longer be visible to the default handling
* (which treats them as filenames to be opened).
*
* The dict includes options that have been explicitly specified on the parsed
* commandline, as well as zero values for numeric options that were not
* necessarily specified.
*
* It is important to use the proper GVariant format when retrieving
* the options with g_variant_dict_lookup():
* - for %G_OPTION_ARG_NONE, use `b`

View File

@ -1020,6 +1020,53 @@ test_handle_local_options_passthrough (void)
g_test_trap_assert_passed ();
}
static gint
test_local_options_double (GApplication *app,
GVariantDict *options,
gpointer data)
{
gboolean *called = data;
gdouble number = 1.0;
*called = TRUE;
g_assert_true (g_variant_dict_lookup (options, "number", "d", &number));
g_assert_cmpfloat (number, ==, 0.0);
return 0;
}
static void
test_handle_local_options_double (void)
{
g_test_summary ("Test that a double of value 0.0 can be parsed using the options handling");
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2329");
if (g_test_subprocess ())
{
char *binpath = g_test_build_filename (G_TEST_BUILT, "unimportant", NULL);
gchar *argv[] = { binpath, "--number", "0.0", NULL };
GApplication *app;
gboolean called = FALSE;
int status;
app = g_application_new ("org.gtk.TestApplication", 0);
g_application_add_main_option (app, "number", 0, G_OPTION_FLAG_NONE, G_OPTION_ARG_DOUBLE, "", "");
g_signal_connect (app, "handle-local-options", G_CALLBACK (test_local_options_double), &called);
status = g_application_run (app, G_N_ELEMENTS (argv) -1, argv);
g_assert_true (called);
g_assert_cmpint (status, ==, 0);
g_object_unref (app);
g_free (binpath);
return;
}
g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_INHERIT_STDOUT | G_TEST_SUBPROCESS_INHERIT_STDERR);
g_test_trap_assert_passed ();
}
static void
test_api (void)
{
@ -1222,6 +1269,7 @@ main (int argc, char **argv)
g_test_add_func ("/gapplication/test-handle-local-options1", test_handle_local_options_success);
g_test_add_func ("/gapplication/test-handle-local-options2", test_handle_local_options_failure);
g_test_add_func ("/gapplication/test-handle-local-options3", test_handle_local_options_passthrough);
g_test_add_func ("/gapplication/test-handle-local-options4", test_handle_local_options_double);
g_test_add_func ("/gapplication/api", test_api);
g_test_add_data_func ("/gapplication/replace", GINT_TO_POINTER (TRUE), test_replace);
g_test_add_data_func ("/gapplication/no-replace", GINT_TO_POINTER (FALSE), test_replace);