[gtk/wip/matthiasc/shortcut: 3/102] bindings: Replace GtkBindingArg arguments with GVariant

[Matthias Clasen <matthiasc src gnome org>]

commit ab7d964b8682a3523f7ca5ca51bdc966a2f1a81c
Author: Benjamin Otte <otte redhat com>
Date:   Tue Jul 31 20:09:03 2018 +0200

    bindings: Replace GtkBindingArg arguments with GVariant
    
    This only replaces invocation, not yet parsing.

 gtk/gtkbindings.c | 173 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 112 insertions(+), 61 deletions(-)
---
diff --git a/gtk/gtkbindings.c b/gtk/gtkbindings.c
index 9f3d879d04..dd1de0abc2 100644
--- a/gtk/gtkbindings.c
+++ b/gtk/gtkbindings.c
@@ -167,8 +167,7 @@ struct _GtkBindingSignalSignal
 {
   GtkBindingSignal  parent;
   const gchar      *signal_name;
-  guint             n_args;
-  GtkBindingArg    *args;
+  GVariant         *args;
 };
 
 struct _GtkBindingSignalAction
@@ -199,16 +198,17 @@ static GQuark            key_id_class_binding_set = 0;
 
 static GtkBindingSignal*
 binding_signal_new_signal (const gchar *signal_name,
-                           guint        n_args)
+                           GVariant    *args)
 {
   GtkBindingSignalSignal *signal;
 
-  signal = (GtkBindingSignalSignal *) g_slice_alloc0 (sizeof (GtkBindingSignalSignal) + n_args * sizeof 
(GtkBindingArg));
+  signal = g_slice_new0 (GtkBindingSignalSignal);
   signal->parent.next = NULL;
   signal->parent.action_type = GTK_BINDING_SIGNAL;
   signal->signal_name = g_intern_string (signal_name);
-  signal->n_args = n_args;
-  signal->args = (GtkBindingArg *)(signal + 1);
+  signal->args = args;
+  if (args)
+    g_variant_ref_sink (args);
 
   return &signal->parent;
 }
@@ -254,19 +254,13 @@ binding_signal_new_callback (GtkBindingCallback  callback,
 static void
 binding_signal_free (GtkBindingSignal *signal)
 {
-  guint i;
-
   switch (signal->action_type)
     {
     case GTK_BINDING_SIGNAL:
       {
         GtkBindingSignalSignal *sig = (GtkBindingSignalSignal *) signal;
-        for (i = 0; i < sig->n_args; i++)
-          {
-            if (G_TYPE_FUNDAMENTAL (sig->args[i].arg_type) == G_TYPE_STRING)
-              g_free (sig->args[i].d.string_data);
-          }
-        g_slice_free1 (sizeof (GtkBindingSignalSignal) + sig->n_args * sizeof (GtkBindingArg), sig);
+        g_clear_pointer (&sig->args, g_variant_unref);
+        g_slice_free (GtkBindingSignalSignal, sig);
       }
       break;
 
@@ -513,9 +507,9 @@ binding_entry_destroy (GtkBindingEntry *entry)
 }
 
 static GtkBindingEntry*
-binding_ht_lookup_entry (GtkBindingSet  *set,
-                         guint           keyval,
-                         GdkModifierType modifiers)
+binding_ht_lookup_entry (GtkBindingSet   *set,
+                         guint            keyval,
+                         GdkModifierType  modifiers)
 {
   GtkBindingEntry lookup_entry = { 0 };
   GtkBindingEntry *entry;
@@ -535,10 +529,10 @@ binding_ht_lookup_entry (GtkBindingSet  *set,
 }
 
 static gboolean
-binding_compose_params (GObject         *object,
-                        GtkBindingArg   *args,
-                        GSignalQuery    *query,
-                        GValue         **params_p)
+binding_compose_params (GObject       *object,
+                        GVariantIter  *args,
+                        GSignalQuery  *query,
+                        GValue       **params_p)
 {
   GValue *params;
   const GType *types;
@@ -559,20 +553,22 @@ binding_compose_params (GObject         *object,
   for (i = 1; i < query->n_params + 1 && valid; i++)
     {
       GValue tmp_value = G_VALUE_INIT;
+      GVariant *tmp_variant;
 
       g_value_init (params, *types);
+      tmp_variant = g_variant_iter_next_value (args);
 
-      switch (G_TYPE_FUNDAMENTAL (args->arg_type))
+      switch ((guint) g_variant_classify (tmp_variant))
         {
-        case G_TYPE_DOUBLE:
+        case G_VARIANT_CLASS_DOUBLE:
           g_value_init (&tmp_value, G_TYPE_DOUBLE);
-          g_value_set_double (&tmp_value, args->d.double_data);
+          g_value_set_double (&tmp_value, g_variant_get_double (tmp_variant));
           break;
-        case G_TYPE_LONG:
+        case G_VARIANT_CLASS_INT64:
           g_value_init (&tmp_value, G_TYPE_LONG);
-          g_value_set_long (&tmp_value, args->d.long_data);
+          g_value_set_long (&tmp_value, g_variant_get_int64 (tmp_variant));
           break;
-        case G_TYPE_STRING:
+        case G_VARIANT_CLASS_STRING:
           /* gtk_rc_parse_flags/enum() has fancier parsing for this; we can't call
            * that since we don't have a GParamSpec, so just do something simple
            */
@@ -580,12 +576,13 @@ binding_compose_params (GObject         *object,
             {
               GEnumClass *class = G_ENUM_CLASS (g_type_class_ref (*types));
               GEnumValue *enum_value;
+              const char *s = g_variant_get_string (tmp_variant, NULL);
 
               valid = FALSE;
 
-              enum_value = g_enum_get_value_by_name (class, args->d.string_data);
+              enum_value = g_enum_get_value_by_name (class, s);
               if (!enum_value)
-                enum_value = g_enum_get_value_by_nick (class, args->d.string_data);
+                enum_value = g_enum_get_value_by_nick (class, s);
 
               if (enum_value)
                 {
@@ -604,12 +601,13 @@ binding_compose_params (GObject         *object,
             {
               GFlagsClass *class = G_FLAGS_CLASS (g_type_class_ref (*types));
               GFlagsValue *flags_value;
+              const char *s = g_variant_get_string (tmp_variant, NULL);
 
               valid = FALSE;
 
-              flags_value = g_flags_get_value_by_name (class, args->d.string_data);
+              flags_value = g_flags_get_value_by_name (class, s);
               if (!flags_value)
-                flags_value = g_flags_get_value_by_nick (class, args->d.string_data);
+                flags_value = g_flags_get_value_by_nick (class, s);
               if (flags_value)
                 {
                   g_value_init (&tmp_value, *types);
@@ -622,7 +620,7 @@ binding_compose_params (GObject         *object,
           else
             {
               g_value_init (&tmp_value, G_TYPE_STRING);
-              g_value_set_static_string (&tmp_value, args->d.string_data);
+              g_value_set_static_string (&tmp_value, g_variant_get_string (tmp_variant, NULL));
             }
           break;
         default:
@@ -638,9 +636,9 @@ binding_compose_params (GObject         *object,
           g_value_unset (&tmp_value);
         }
 
+      g_variant_unref (tmp_variant);
       types++;
       params++;
-      args++;
     }
 
   if (!valid)
@@ -659,41 +657,60 @@ binding_compose_params (GObject         *object,
 
 static gboolean
 binding_signal_activate_signal (GtkBindingSignalSignal *sig,
+                                const char             *set_name,
+                                guint                   keyval,
+                                GdkModifierType         modifiers,
                                 GObject                *object)
 {
   GSignalQuery query;
   guint signal_id;
   GValue *params = NULL;
   GValue return_val = G_VALUE_INIT;
+  GVariantIter args_iter;
+  gsize n_args;
   gboolean handled = FALSE;
 
   signal_id = g_signal_lookup (sig->signal_name, G_OBJECT_TYPE (object));
   if (!signal_id)
     {
-      g_warning ("gtk_binding_entry_activate(): "
+      char *accelerator = gtk_accelerator_name (keyval, modifiers);
+      g_warning ("gtk_binding_entry_activate(): binding \"%s::%s\": "
                  "could not find signal \"%s\" in the '%s' class ancestry",
+                 set_name, accelerator,
                  sig->signal_name,
                  g_type_name (G_OBJECT_TYPE (object)));
+      g_free (accelerator);
       return FALSE;
     }
 
   g_signal_query (signal_id, &query);
-  if (query.n_params != sig->n_args ||
+  if (sig->args)
+    n_args = g_variant_iter_init (&args_iter, sig->args);
+  else
+    n_args = 0;
+
+  if (query.n_params != n_args ||
       (query.return_type != G_TYPE_NONE && query.return_type != G_TYPE_BOOLEAN) ||
-      !binding_compose_params (object, sig->args, &query, &params))
+      !binding_compose_params (object, &args_iter, &query, &params))
     {
-      g_warning ("gtk_binding_entry_activate(): "
+      char *accelerator = gtk_accelerator_name (keyval, modifiers);
+      g_warning ("gtk_binding_entry_activate(): binding \"%s::%s\": "
                  "signature mismatch for signal \"%s\" in the '%s' class ancestry",
+                 set_name, accelerator,
                  sig->signal_name,
                  g_type_name (G_OBJECT_TYPE (object)));
+      g_free (accelerator);
       return FALSE;
     }
   else if (!(query.signal_flags & G_SIGNAL_ACTION))
     {
-      g_warning ("gtk_binding_entry_activate(): "
+      char *accelerator = gtk_accelerator_name (keyval, modifiers);
+      g_warning ("gtk_binding_entry_activate(): binding \"%s::%s\": "
                  "signal \"%s\" in the '%s' class ancestry cannot be used for action emissions",
+                 set_name, accelerator,
                  sig->signal_name,
                  g_type_name (G_OBJECT_TYPE (object)));
+      g_free (accelerator);
       return FALSE;
     }
 
@@ -726,22 +743,31 @@ binding_signal_activate_signal (GtkBindingSignalSignal *sig,
 
 static gboolean
 binding_signal_activate_action (GtkBindingSignalAction *sig,
+                                const char             *set_name,
+                                guint                   keyval,
+                                GdkModifierType         modifiers,
                                 GObject                *object)
 {
   if (!GTK_IS_WIDGET (object))
     {
-      g_warning ("gtk_binding_entry_activate(): "
+      char *accelerator = gtk_accelerator_name (keyval, modifiers);
+      g_warning ("gtk_binding_entry_activate(): binding \"%s::%s\": "
                  "actions must be emitted on GtkWidget subtypes, %s is not supported",
+                 set_name, accelerator,
                  G_OBJECT_TYPE_NAME (object));
+      g_free (accelerator);
       return FALSE;
     }
 
   if (!gtk_widget_activate_action_variant (GTK_WIDGET (object), sig->action_name, sig->variant))
     {
-      g_warning ("gtk_binding_entry_activate(): "
+      char *accelerator = gtk_accelerator_name (keyval, modifiers);
+      g_warning ("gtk_binding_entry_activate(): binding \"%s::%s\": "
                  "action \"%s\" does not exist on class \"%s\"",
+                 set_name, accelerator,
                  sig->action_name,
                  G_OBJECT_TYPE_NAME (object));
+      g_free (accelerator);
       return FALSE;
     }
 
@@ -750,13 +776,19 @@ binding_signal_activate_action (GtkBindingSignalAction *sig,
 
 static gboolean
 binding_signal_activate_callback (GtkBindingSignalCallback *sig,
+                                  const char               *set_name,
+                                  guint                     keyval,
+                                  GdkModifierType           modifiers,
                                   GObject                  *object)
 {
   if (!GTK_IS_WIDGET (object))
     {
-      g_warning ("gtk_binding_entry_activate(): "
+      char *accelerator = gtk_accelerator_name (keyval, modifiers);
+      g_warning ("gtk_binding_entry_activate(): \"%s::%s\": "
                  "callbacks must be run on GtkWidget subtypes, %s is not supported",
+                 set_name, accelerator,
                  G_OBJECT_TYPE_NAME (object));
+      g_free (accelerator);
       return FALSE;
     }
 
@@ -783,15 +815,24 @@ gtk_binding_entry_activate (GtkBindingEntry *entry,
       switch (sig->action_type)
         {
         case GTK_BINDING_SIGNAL:
-          handled = binding_signal_activate_signal ((GtkBindingSignalSignal *) sig, object);
+          handled = binding_signal_activate_signal ((GtkBindingSignalSignal *) sig,
+                                                    entry->binding_set->set_name,
+                                                    entry->keyval, entry->modifiers,
+                                                    object);
           break;
 
         case GTK_BINDING_ACTION:
-          handled = binding_signal_activate_action ((GtkBindingSignalAction *) sig, object);
+          handled = binding_signal_activate_action ((GtkBindingSignalAction *) sig,
+                                                    entry->binding_set->set_name,
+                                                    entry->keyval, entry->modifiers,
+                                                    object);
           break;
 
         case GTK_BINDING_CALLBACK:
-          handled = binding_signal_activate_callback ((GtkBindingSignalCallback *) sig, object);
+          handled = binding_signal_activate_callback ((GtkBindingSignalCallback *) sig,
+                                                      entry->binding_set->set_name,
+                                                      entry->keyval, entry->modifiers,
+                                                      object);
           break;
 
         default:
@@ -1059,14 +1100,18 @@ gtk_binding_entry_add_signall (GtkBindingSet  *binding_set,
   GtkBindingSignal *signal;
   GSList *slist;
   guint n = 0;
-  GtkBindingArg *arg;
+  GVariantBuilder builder;
+  GtkBindingEntry *entry;
+  GtkBindingSignal **signal_p;
 
   g_return_if_fail (binding_set != NULL);
   g_return_if_fail (signal_name != NULL);
 
-  signal = binding_signal_new_signal (signal_name, g_slist_length (binding_args));
+  keyval = gdk_keyval_to_lower (keyval);
+  modifiers = modifiers & BINDING_MOD_MASK ();
+
+  g_variant_builder_init (&builder, G_VARIANT_TYPE_TUPLE);
 
-  arg = ((GtkBindingSignalSignal *) signal)->args;
   for (slist = binding_args; slist; slist = slist->next)
     {
       GtkBindingArg *tmp_arg;
@@ -1075,40 +1120,46 @@ gtk_binding_entry_add_signall (GtkBindingSet  *binding_set,
       if (!tmp_arg)
         {
           g_warning ("gtk_binding_entry_add_signall(): arg[%u] is 'NULL'", n);
-          binding_signal_free (signal);
           return;
         }
+
       switch (G_TYPE_FUNDAMENTAL (tmp_arg->arg_type))
         {
         case  G_TYPE_LONG:
-          arg->arg_type = G_TYPE_LONG;
-          arg->d.long_data = tmp_arg->d.long_data;
+          g_variant_builder_add (&builder, "x", (gint64) tmp_arg->d.long_data);
           break;
         case  G_TYPE_DOUBLE:
-          arg->arg_type = G_TYPE_DOUBLE;
-          arg->d.double_data = tmp_arg->d.double_data;
+          g_variant_builder_add (&builder, "d", (double) tmp_arg->d.double_data);
           break;
         case  G_TYPE_STRING:
-          arg->arg_type = G_TYPE_STRING;
-          arg->d.string_data = g_strdup (tmp_arg->d.string_data);
-          if (!arg->d.string_data)
+          if (!tmp_arg->d.string_data)
             {
               g_warning ("gtk_binding_entry_add_signall(): value of 'string' arg[%u] is 'NULL'", n);
-              binding_signal_free (signal);
+              g_variant_builder_clear (&builder);
               return;
             }
+          g_variant_builder_add (&builder, "s", (gint64) tmp_arg->d.string_data);
           break;
         default:
           g_warning ("gtk_binding_entry_add_signall(): unsupported type '%s' for arg[%u]",
-                     g_type_name (arg->arg_type), n);
-          binding_signal_free (signal);
+                     g_type_name (tmp_arg->arg_type), n);
+          g_variant_builder_clear (&builder);
           return;
         }
-      arg++;
-      n++;
     }
 
-  gtk_binding_entry_add_binding_signal (binding_set, keyval, modifiers, signal);
+  signal = binding_signal_new_signal (signal_name, g_variant_builder_end (&builder));
+
+  entry = binding_ht_lookup_entry (binding_set, keyval, modifiers);
+  if (!entry)
+    {
+      gtk_binding_entry_clear_internal (binding_set, keyval, modifiers);
+      entry = binding_ht_lookup_entry (binding_set, keyval, modifiers);
+    }
+  signal_p = &entry->signals;
+  while (*signal_p)
+    signal_p = &(*signal_p)->next;
+  *signal_p = signal;
 }
 
 /**