GtkBuilder: improved parsing error report for invalid properties and signals.

Added GTK_BUILDER_ERROR_INVALID_PROPERTY and GTK_BUILDER_ERROR_INVALID_SIGNAL
error codes

ObjectInfo: Use a GType instead of a char * for the class name.
PropertyInfo: Use a GParamSpec instead of a char * for the property name.
SignalInfo: Use signal id and detail quark instead of a detailed signal name string.

This not only save us a few malloc in each case but lets us simplify the code
and report unknown properties and signals as a parsing error instead of just
printing a warning.
This commit is contained in:
Juan Pablo Ugarte 2014-04-29 16:22:32 -03:00
parent c4afca906c
commit 49fa04212b
5 changed files with 147 additions and 122 deletions

View File

@ -436,7 +436,7 @@ gtk_builder_real_get_type_from_name (GtkBuilder *builder,
typedef struct
{
gchar *object;
gchar *name;
GParamSpec *pspec;
gchar *value;
} DelayedProperty;
@ -450,13 +450,8 @@ gtk_builder_get_parameters (GtkBuilder *builder,
GArray **filtered_parameters)
{
GSList *l;
GParamSpec *pspec;
GObjectClass *oclass;
DelayedProperty *property;
GError *error = NULL;
oclass = g_type_class_ref (object_type);
g_assert (oclass != NULL);
if (parameters)
*parameters = g_array_new (FALSE, FALSE, sizeof (GParameter));
@ -468,19 +463,10 @@ gtk_builder_get_parameters (GtkBuilder *builder,
PropertyInfo *prop = (PropertyInfo*)l->data;
GParameter parameter = { NULL };
pspec = g_object_class_find_property (G_OBJECT_CLASS (oclass),
prop->name);
if (!pspec)
{
g_warning ("Unknown property: %s.%s",
g_type_name (object_type), prop->name);
continue;
}
parameter.name = prop->pspec->name;
parameter.name = prop->name;
if (G_IS_PARAM_SPEC_OBJECT (pspec) &&
(G_PARAM_SPEC_VALUE_TYPE (pspec) != GDK_TYPE_PIXBUF))
if (G_IS_PARAM_SPEC_OBJECT (prop->pspec) &&
(G_PARAM_SPEC_VALUE_TYPE (prop->pspec) != GDK_TYPE_PIXBUF))
{
GObject *object = gtk_builder_get_object (builder, prop->data);
@ -491,17 +477,17 @@ gtk_builder_get_parameters (GtkBuilder *builder,
}
else
{
if (pspec->flags & G_PARAM_CONSTRUCT_ONLY)
if (prop->pspec->flags & G_PARAM_CONSTRUCT_ONLY)
{
g_warning ("Failed to get construct only property "
"%s of %s with value `%s'",
prop->name, object_name, prop->data);
prop->pspec->name, object_name, prop->data);
continue;
}
/* Delay setting property */
property = g_slice_new (DelayedProperty);
property->pspec = prop->pspec;
property->object = g_strdup (object_name);
property->name = g_strdup (prop->name);
property->value = g_strdup (prop->data);
builder->priv->delayed_properties =
g_slist_prepend (builder->priv->delayed_properties, property);
@ -515,18 +501,18 @@ gtk_builder_get_parameters (GtkBuilder *builder,
*/
continue;
}
else if (!gtk_builder_value_from_string (builder, pspec,
else if (!gtk_builder_value_from_string (builder, prop->pspec,
prop->data, &parameter.value, &error))
{
g_warning ("Failed to set property %s.%s to %s: %s",
g_type_name (object_type), prop->name, prop->data,
g_type_name (object_type), prop->pspec->name, prop->data,
error->message);
g_error_free (error);
error = NULL;
continue;
}
if (pspec->flags & filter_flags)
if (prop->pspec->flags & filter_flags)
{
if (filtered_parameters)
g_array_append_val (*filtered_parameters, parameter);
@ -537,8 +523,6 @@ gtk_builder_get_parameters (GtkBuilder *builder,
g_array_append_val (*parameters, parameter);
}
}
g_type_class_unref (oclass);
}
static GObject *
@ -628,7 +612,6 @@ _gtk_builder_construct (GtkBuilder *builder,
GError **error)
{
GArray *parameters, *construct_parameters;
GType object_type;
GObject *obj;
int i;
GtkBuildableIface *iface;
@ -636,26 +619,17 @@ _gtk_builder_construct (GtkBuilder *builder,
GtkBuildable *buildable;
GParamFlags param_filter_flags;
g_assert (info->class_name != NULL);
object_type = gtk_builder_get_type_from_name (builder, info->class_name);
if (object_type == G_TYPE_INVALID)
{
g_set_error (error,
GTK_BUILDER_ERROR,
GTK_BUILDER_ERROR_INVALID_VALUE,
"Invalid object type `%s'",
info->class_name);
return NULL;
}
else if (builder->priv->template_type != 0 &&
g_type_is_a (object_type, builder->priv->template_type))
g_assert (info->type != G_TYPE_INVALID);
if (builder->priv->template_type != 0 &&
g_type_is_a (info->type, builder->priv->template_type))
{
g_set_error (error,
GTK_BUILDER_ERROR,
GTK_BUILDER_ERROR_OBJECT_TYPE_REFUSED,
"Refused to build object of type `%s' because it "
"conforms to the template type `%s', avoiding infinite recursion.",
info->class_name, g_type_name (builder->priv->template_type));
g_type_name (info->type), g_type_name (builder->priv->template_type));
return NULL;
}
@ -676,7 +650,7 @@ _gtk_builder_construct (GtkBuilder *builder,
else
param_filter_flags = G_PARAM_CONSTRUCT | G_PARAM_CONSTRUCT_ONLY;
gtk_builder_get_parameters (builder, object_type,
gtk_builder_get_parameters (builder, info->type,
info->id,
info->properties,
param_filter_flags,
@ -723,7 +697,7 @@ _gtk_builder_construct (GtkBuilder *builder,
}
else
{
obj = g_object_newv (object_type,
obj = g_object_newv (info->type,
construct_parameters->len,
(GParameter *)construct_parameters->data);
@ -740,7 +714,7 @@ _gtk_builder_construct (GtkBuilder *builder,
g_object_ref_sink (obj);
GTK_NOTE (BUILDER,
g_print ("created %s of type %s\n", info->id, info->class_name));
g_print ("created %s of type %s\n", info->id, g_type_name (info->type)));
for (i = 0; i < construct_parameters->len; i++)
{
@ -800,18 +774,16 @@ _gtk_builder_apply_properties (GtkBuilder *builder,
GError **error)
{
GArray *parameters;
GType object_type;
GtkBuildableIface *iface;
GtkBuildable *buildable;
gboolean custom_set_property;
gint i;
g_assert (info->object != NULL);
g_assert (info->class_name != NULL);
object_type = gtk_builder_get_type_from_name (builder, info->class_name);
g_assert (info->type != G_TYPE_INVALID);
/* Fetch all properties that are not construct-only */
gtk_builder_get_parameters (builder, object_type,
gtk_builder_get_parameters (builder, info->type,
info->id,
info->properties,
G_PARAM_CONSTRUCT_ONLY,
@ -903,11 +875,6 @@ static void
gtk_builder_apply_delayed_properties (GtkBuilder *builder)
{
GSList *l, *props;
DelayedProperty *property;
GObject *object;
GType object_type;
GObjectClass *oclass;
GParamSpec *pspec;
/* take the list over from the builder->priv.
*
@ -919,36 +886,22 @@ gtk_builder_apply_delayed_properties (GtkBuilder *builder)
for (l = props; l; l = l->next)
{
property = (DelayedProperty*)l->data;
DelayedProperty *property = l->data;
GObject *object, *obj;
object = g_hash_table_lookup (builder->priv->objects, property->object);
g_assert (object != NULL);
object_type = G_OBJECT_TYPE (object);
g_assert (object_type != G_TYPE_INVALID);
oclass = g_type_class_ref (object_type);
g_assert (oclass != NULL);
pspec = g_object_class_find_property (G_OBJECT_CLASS (oclass),
property->name);
if (!pspec)
g_warning ("Unknown property: %s.%s", g_type_name (object_type),
property->name);
obj = g_hash_table_lookup (builder->priv->objects, property->value);
if (obj)
g_object_set (object, property->pspec->name, obj, NULL);
else
{
GObject *obj;
g_warning ("No object called: %s", property->value);
obj = g_hash_table_lookup (builder->priv->objects, property->value);
if (!obj)
g_warning ("No object called: %s", property->value);
else
g_object_set (object, property->name, obj, NULL);
}
g_free (property->value);
g_free (property->object);
g_free (property->name);
g_slice_free (DelayedProperty, property);
g_type_class_unref (oclass);
}
g_slist_free (props);
}
@ -957,7 +910,7 @@ static inline void
free_binding_info (gpointer data, gpointer user)
{
BindingInfo *info = data;
g_free (info->target_property);
g_free (info->source);
g_free (info->source_property);
g_slice_free (BindingInfo, data);
@ -975,7 +928,7 @@ gtk_builder_create_bindings (GtkBuilder *builder)
if ((source = gtk_builder_get_object (builder, info->source)))
g_object_bind_property (source, info->source_property,
info->target, info->target_property,
info->target, info->target_pspec->name,
info->flags);
else
g_warning ("Could not find source object '%s' to bind property '%s'",
@ -1724,6 +1677,7 @@ gtk_builder_connect_signals_full (GtkBuilder *builder,
GSList *l;
GObject *object;
GObject *connect_object;
GString *detailed_id = NULL;
g_return_if_fail (GTK_IS_BUILDER (builder));
g_return_if_fail (func != NULL);
@ -1735,9 +1689,12 @@ gtk_builder_connect_signals_full (GtkBuilder *builder,
for (l = builder->priv->signals; l; l = l->next)
{
SignalInfo *signal = (SignalInfo*)l->data;
const gchar *signal_name;
g_assert (signal != NULL);
g_assert (signal->name != NULL);
g_assert (signal->id != 0);
signal_name = g_signal_name (signal->id);
object = g_hash_table_lookup (builder->priv->objects,
signal->object_name);
@ -1751,17 +1708,30 @@ gtk_builder_connect_signals_full (GtkBuilder *builder,
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);
signal->connect_object_name, signal_name,
signal->object_name);
}
func (builder, object, signal->name, signal->handler,
connect_object, signal->flags, user_data);
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;
}
func (builder, object, signal_name, signal->handler,
connect_object, signal->flags, user_data);
}
g_slist_foreach (builder->priv->signals, (GFunc)_free_signal_info, NULL);
g_slist_free (builder->priv->signals);
builder->priv->signals = NULL;
if (detailed_id)
g_string_free (detailed_id, TRUE);
}
/**

View File

@ -62,6 +62,8 @@ typedef struct _GtkBuilderPrivate GtkBuilderPrivate;
* @GTK_BUILDER_ERROR_OBJECT_TYPE_REFUSED: A specified object type is of the same type or
* derived from the type of the composite class being extended with builder XML.
* @GTK_BUILDER_ERROR_TEMPLATE_MISMATCH: The wrong type was specified in a composite classs template XML
* @GTK_BUILDER_ERROR_INVALID_PROPERTY: The specified property is unknown for the object class.
* @GTK_BUILDER_ERROR_INVALID_SIGNAL: The specified signal is unknown for the object class.
*
* Error codes that identify various errors that can occur while using
* #GtkBuilder.
@ -78,7 +80,9 @@ typedef enum
GTK_BUILDER_ERROR_VERSION_MISMATCH,
GTK_BUILDER_ERROR_DUPLICATE_ID,
GTK_BUILDER_ERROR_OBJECT_TYPE_REFUSED,
GTK_BUILDER_ERROR_TEMPLATE_MISMATCH
GTK_BUILDER_ERROR_TEMPLATE_MISMATCH,
GTK_BUILDER_ERROR_INVALID_PROPERTY,
GTK_BUILDER_ERROR_INVALID_SIGNAL
} GtkBuilderError;
GDK_AVAILABLE_IN_ALL

View File

@ -216,24 +216,19 @@ builder_construct (ParserData *data,
return object;
}
static gchar *
static GType
_get_type_by_symbol (const gchar* symbol)
{
static GModule *module = NULL;
GTypeGetFunc func;
GType type;
if (!module)
module = g_module_open (NULL, 0);
if (!g_module_symbol (module, symbol, (gpointer)&func))
return NULL;
return G_TYPE_INVALID;
type = func ();
if (type == G_TYPE_INVALID)
return NULL;
return g_strdup (g_type_name (type));
return func ();
}
static void
@ -318,7 +313,7 @@ parse_object (GMarkupParseContext *context,
{
ObjectInfo *object_info;
ChildInfo* child_info;
const gchar *object_class = NULL;
GType object_type = G_TYPE_INVALID;
const gchar *object_id = NULL;
const gchar *constructor = NULL;
gchar *internal_id = NULL;
@ -334,19 +329,30 @@ parse_object (GMarkupParseContext *context,
for (i = 0; names[i] != NULL; i++)
{
if (strcmp (names[i], "class") == 0)
object_class = values[i];
{
object_type = gtk_builder_get_type_from_name (data->builder, values[i]);
if (object_type == G_TYPE_INVALID)
{
g_markup_parse_context_get_position (context, &line, NULL);
g_set_error (error, GTK_BUILDER_ERROR,
GTK_BUILDER_ERROR_INVALID_VALUE,
_("Invalid object type `%s' on line %d"),
values[i], line);
return;
}
}
else if (strcmp (names[i], "id") == 0)
object_id = values[i];
else if (strcmp (names[i], "constructor") == 0)
constructor = values[i];
else if (strcmp (names[i], "type-func") == 0)
{
/* Call the GType function, and return the name of the GType,
* it's guaranteed afterwards that g_type_from_name on the name
* will return our GType
*/
object_class = _get_type_by_symbol (values[i]);
if (!object_class)
/* Call the GType function, and return the GType, it's guaranteed afterwards
* that g_type_from_name on the name will return our GType
*/
object_type = _get_type_by_symbol (values[i]);
if (object_type == G_TYPE_INVALID)
{
g_markup_parse_context_get_position (context, &line, NULL);
g_set_error (error, GTK_BUILDER_ERROR,
@ -363,7 +369,7 @@ parse_object (GMarkupParseContext *context,
}
}
if (!object_class)
if (object_type == G_TYPE_INVALID)
{
error_missing_attribute (data, element_name, "class", error);
return;
@ -398,7 +404,7 @@ parse_object (GMarkupParseContext *context,
}
object_info = g_slice_new0 (ObjectInfo);
object_info->class_name = g_strdup (object_class);
object_info->type = object_type;
object_info->id = (internal_id) ? internal_id : g_strdup (object_id);
object_info->constructor = g_strdup (constructor);
state_push (data, object_info);
@ -486,7 +492,7 @@ parse_template (GMarkupParseContext *context,
++data->cur_object_level;
object_info = g_slice_new0 (ObjectInfo);
object_info->class_name = g_strdup (object_class);
object_info->type = parsed_type;
object_info->id = g_strdup (object_class);
object_info->object = gtk_builder_get_object (data->builder, object_class);
state_push (data, object_info);
@ -516,7 +522,6 @@ free_object_info (ObjectInfo *info)
(GFunc)free_property_info, NULL);
g_slist_free (info->properties);
g_free (info->constructor);
g_free (info->class_name);
g_free (info->id);
g_slice_free (ObjectInfo, info);
}
@ -584,14 +589,14 @@ parse_property (ParserData *data,
GError **error)
{
PropertyInfo *info;
const gchar *name = NULL;
const gchar *context = NULL;
const gchar *bind_source = NULL;
const gchar *bind_property = NULL;
GBindingFlags bind_flags = G_BINDING_DEFAULT;
gboolean translatable = FALSE;
ObjectInfo *object_info;
int i;
GParamSpec *pspec = NULL;
gint i;
object_info = state_peek_info (data, ObjectInfo);
if (!object_info ||
@ -605,7 +610,26 @@ parse_property (ParserData *data,
for (i = 0; names[i] != NULL; i++)
{
if (strcmp (names[i], "name") == 0)
name = values[i];
{
GObjectClass *oclass = g_type_class_ref (object_info->type);
gchar *name = g_strdelimit (g_strdup (values[i]), "_", '-');
g_assert (oclass != NULL);
pspec = g_object_class_find_property (oclass, name);
g_type_class_unref (oclass);
g_free (name);
if (!pspec)
{
gint line;
g_markup_parse_context_get_position (data->ctx, &line, NULL);
g_set_error (error, GTK_BUILDER_ERROR,
GTK_BUILDER_ERROR_INVALID_PROPERTY,
_("Invalid property: %s.%s on line %d"),
g_type_name (object_info->type), values[i], line);
return;
}
}
else if (strcmp (names[i], "translatable") == 0)
{
if (!_gtk_builder_boolean_from_string (values[i], &translatable,
@ -641,7 +665,7 @@ parse_property (ParserData *data,
}
}
if (!name)
if (!pspec)
{
error_missing_attribute (data, element_name, "name", error);
return;
@ -651,7 +675,7 @@ parse_property (ParserData *data,
{
BindingInfo *binfo = g_slice_new0 (BindingInfo);
binfo->target_property = g_strdup (name);
binfo->target_pspec = pspec;
binfo->source = g_strdup (bind_source);
binfo->source_property = g_strdup (bind_property);
binfo->flags = bind_flags;
@ -667,9 +691,9 @@ parse_property (ParserData *data,
}
info = g_slice_new0 (PropertyInfo);
info->name = g_strdelimit (g_strdup (name), "_", '-');
info->pspec = pspec;
info->translatable = translatable;
info->bound = (bind_source != NULL && bind_property != NULL);
info->bound = (bind_source && bind_property);
info->context = g_strdup (context);
info->text = g_string_new ("");
state_push (data, info);
@ -681,7 +705,6 @@ static void
free_property_info (PropertyInfo *info)
{
g_free (info->data);
g_free (info->name);
g_free (info->context);
/* info->text is already freed */
g_slice_free (PropertyInfo, info);
@ -695,14 +718,15 @@ parse_signal (ParserData *data,
GError **error)
{
SignalInfo *info;
const gchar *name = NULL;
const gchar *handler = NULL;
const gchar *object = NULL;
gboolean after = FALSE;
gboolean swapped = FALSE;
gboolean swapped_set = FALSE;
ObjectInfo *object_info;
int i;
guint id = 0;
GQuark detail = 0;
gint i;
object_info = state_peek_info (data, ObjectInfo);
if (!object_info ||
@ -716,7 +740,19 @@ parse_signal (ParserData *data,
for (i = 0; names[i] != NULL; i++)
{
if (strcmp (names[i], "name") == 0)
name = values[i];
{
if (!g_signal_parse_name (values[i], object_info->type,
&id, &detail, FALSE))
{
gint line;
g_markup_parse_context_get_position (data->ctx, &line, NULL);
g_set_error (error, GTK_BUILDER_ERROR,
GTK_BUILDER_ERROR_INVALID_SIGNAL,
_("Invalid signal `%s' for type `%s' on line %d"),
values[i], g_type_name (object_info->type), line);
return;
}
}
else if (strcmp (names[i], "handler") == 0)
handler = values[i];
else if (strcmp (names[i], "after") == 0)
@ -742,7 +778,7 @@ parse_signal (ParserData *data,
}
}
if (!name)
if (!id)
{
error_missing_attribute (data, element_name, "name", error);
return;
@ -758,7 +794,8 @@ parse_signal (ParserData *data,
swapped = TRUE;
info = g_slice_new0 (SignalInfo);
info->name = g_strdup (name);
info->id = id;
info->detail = detail;
info->handler = g_strdup (handler);
if (after)
info->flags |= G_CONNECT_AFTER;
@ -775,7 +812,6 @@ void
_free_signal_info (SignalInfo *info,
gpointer user_data)
{
g_free (info->name);
g_free (info->handler);
g_free (info->connect_object_name);
g_free (info->object_name);

View File

@ -31,7 +31,7 @@ typedef struct {
typedef struct {
TagInfo tag;
gchar *class_name;
GType type;
gchar *id;
gchar *constructor;
GSList *properties;
@ -60,7 +60,7 @@ typedef struct {
typedef struct {
TagInfo tag;
gchar *name;
GParamSpec *pspec;
GString *text;
gchar *data;
gboolean translatable:1;
@ -71,7 +71,8 @@ typedef struct {
typedef struct {
TagInfo tag;
gchar *object_name;
gchar *name;
guint id;
GQuark detail;
gchar *handler;
GConnectFlags flags;
gchar *connect_object_name;
@ -80,7 +81,7 @@ typedef struct {
typedef struct
{
GObject *target;
gchar *target_property;
GParamSpec *target_pspec;
gchar *source;
gchar *source_property;
GBindingFlags flags;

View File

@ -131,6 +131,20 @@ test_parser (void)
GTK_BUILDER_ERROR_DUPLICATE_ID));
g_error_free (error);
error = NULL;
gtk_builder_add_from_string (builder, "<interface><object class=\"GtkButton\" id=\"a\"><property name=\"deafbeef\"></property></object></interface>", -1, &error);
g_assert (g_error_matches (error,
GTK_BUILDER_ERROR,
GTK_BUILDER_ERROR_INVALID_PROPERTY));
g_error_free (error);
error = NULL;
gtk_builder_add_from_string (builder, "<interface><object class=\"GtkButton\" id=\"a\"><signal name=\"deafbeef\" handler=\"gtk_true\"/></object></interface>", -1, &error);
g_assert (g_error_matches (error,
GTK_BUILDER_ERROR,
GTK_BUILDER_ERROR_INVALID_SIGNAL));
g_error_free (error);
g_object_unref (builder);
}