[glib: 1/2] gapplication: Expose zero-valued numbers in handle-local-options

[Philip Withnall <pwithnall src gnome org>]

commit 1ed67a9c44508dc70b4bbdb02ed91037643fdfd0
Author: Philip Withnall <pwithnall endlessos org>
Date:   Mon Feb 15 22:07:59 2021 +0000

    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

 gio/gapplication.c       | 13 +++++++------
 gio/tests/gapplication.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 6 deletions(-)
---
diff --git a/gio/gapplication.c b/gio/gapplication.c
index cbc467a92..96c565019 100644
--- a/gio/gapplication.c
+++ b/gio/gapplication.c
@@ -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`
diff --git a/gio/tests/gapplication.c b/gio/tests/gapplication.c
index 6f1a27e0f..f9a95cd31 100644
--- a/gio/tests/gapplication.c
+++ b/gio/tests/gapplication.c
@@ -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);