[gtk/wip/otte/builder: 10/10] builder: Improve signal connecting

[Benjamin Otte <otte src gnome org>]

commit 052a0230efc056423cfbdaa7cf3e4f483f599acf
Author: Benjamin Otte <otte redhat com>
Date:   Fri Nov 22 08:06:35 2019 +0100

    builder: Improve signal connecting
    
    - Propagate the error back to the parser, so we get a proper GError
      instead of a g_warning().
    - Connect closures by id, don't construct a name from the ids so that
      glib can take it apart again.

 gtk/gtkbuilder.c        | 62 +++++++++++++++++++++----------------------------
 gtk/gtkbuilderparser.c  |  4 +++-
 gtk/gtkbuilderprivate.h |  3 ++-
 3 files changed, 32 insertions(+), 37 deletions(-)
---
diff --git a/gtk/gtkbuilder.c b/gtk/gtkbuilder.c
index be1630b139..a70e7340ba 100644
--- a/gtk/gtkbuilder.c
+++ b/gtk/gtkbuilder.c
@@ -1700,33 +1700,28 @@ gtk_builder_expose_object (GtkBuilder    *builder,
  *   be created and @error was set.
  */
 
-static void
-gtk_builder_connect_signals (GtkBuilder *builder)
+static gboolean
+gtk_builder_connect_signals (GtkBuilder  *builder,
+                             GError     **error)
 {
   GtkBuilderPrivate *priv = gtk_builder_get_instance_private (builder);
   GSList *l;
   GObject *object;
   GObject *connect_object;
-  GString *detailed_id = NULL;
-
-  g_return_if_fail (GTK_IS_BUILDER (builder));
+  gboolean result = FALSE;
 
   if (!priv->signals)
-    return;
+    return TRUE;
 
   priv->signals = g_slist_reverse (priv->signals);
   for (l = priv->signals; l; l = l->next)
     {
       SignalInfo *signal = (SignalInfo*)l->data;
-      const gchar *signal_name;
       GClosure *closure;
-      GError *error = NULL;
 
       g_assert (signal != NULL);
       g_assert (signal->id != 0);
 
-      signal_name = g_signal_name (signal->id);
-
       object = g_hash_table_lookup (priv->objects, signal->object_name);
       g_assert (object != NULL);
 
@@ -1737,50 +1732,47 @@ gtk_builder_connect_signals (GtkBuilder *builder)
           connect_object = g_hash_table_lookup (priv->objects,
                                                 signal->connect_object_name);
           if (!connect_object)
-              g_warning ("Could not lookup object %s on signal %s of object %s",
-                         signal->connect_object_name, signal_name,
-                         signal->object_name);
-        }
-
-      if (signal->detail)
-        {
-          if (detailed_id == NULL)
-            detailed_id = g_string_new ("");
-
-          g_string_printf (detailed_id, "%s::%s", signal_name,
-                           g_quark_to_string (signal->detail));
-          signal_name = detailed_id->str;
+            {
+              g_set_error (error,
+                           GTK_BUILDER_ERROR, GTK_BUILDER_ERROR_INVALID_ID,
+                           "Could not lookup object %s on signal %s of object %s",
+                           signal->connect_object_name, g_signal_name (signal->id),
+                           signal->object_name);
+              break;
+            }
         }
 
       closure = gtk_builder_create_closure (builder,
                                             signal->handler,
                                             signal->flags & G_CONNECT_SWAPPED ? TRUE : FALSE,
                                             connect_object,
-                                            &error);
+                                            error);
 
       if (closure == NULL)
-        {
-          g_warning ("%s", error->message);
-          g_error_free (error);
-          continue;
-        }
+        break;
 
-      g_signal_connect_closure (object, signal_name, closure, signal->flags & G_CONNECT_AFTER ? TRUE : 
FALSE);
+      g_signal_connect_closure_by_id (object,
+                                      signal->id,
+                                      signal->detail,
+                                      closure,
+                                      signal->flags & G_CONNECT_AFTER ? TRUE : FALSE);
     }
+  if (l == NULL)
+    result = TRUE;
 
   g_slist_free_full (priv->signals, (GDestroyNotify)_free_signal_info);
   priv->signals = NULL;
 
-  if (detailed_id)
-    g_string_free (detailed_id, TRUE);
+  return result;
 }
 
-void
-_gtk_builder_finish (GtkBuilder *builder)
+gboolean
+_gtk_builder_finish (GtkBuilder  *builder,
+                     GError     **error)
 {
   gtk_builder_apply_delayed_properties (builder);
   gtk_builder_create_bindings (builder);
-  gtk_builder_connect_signals (builder);
+  return gtk_builder_connect_signals (builder, error);
 }
 
 /**
diff --git a/gtk/gtkbuilderparser.c b/gtk/gtkbuilderparser.c
index 954a06b772..a853151806 100644
--- a/gtk/gtkbuilderparser.c
+++ b/gtk/gtkbuilderparser.c
@@ -1594,7 +1594,9 @@ _gtk_builder_parser_parse_buffer (GtkBuilder   *builder,
   if (!gtk_buildable_parse_context_parse (&data.ctx, buffer, length, error))
     goto out;
 
-  _gtk_builder_finish (builder);
+  if (!_gtk_builder_finish (builder, error))
+    goto out;
+
   if (_gtk_builder_lookup_failed (builder, error))
     goto out;
 
diff --git a/gtk/gtkbuilderprivate.h b/gtk/gtkbuilderprivate.h
index e0352cd5cf..0e72efe273 100644
--- a/gtk/gtkbuilderprivate.h
+++ b/gtk/gtkbuilderprivate.h
@@ -175,7 +175,8 @@ void      _gtk_builder_add (GtkBuilder *builder,
                             ChildInfo *child_info);
 void      _gtk_builder_add_signals (GtkBuilder *builder,
                                    GSList     *signals);
-void      _gtk_builder_finish (GtkBuilder *builder);
+gboolean  _gtk_builder_finish (GtkBuilder  *builder,
+                               GError     **error);
 void _free_signal_info (SignalInfo *info,
                         gpointer user_data);