gnome-libs property changes, property notification

[Tim Janik <timj gtk org>]

hi martin,

i read over recent gnome-libs and basically have to say the new property stuff
you did looks very good, i just cought a few minor things ;)

* instead of x = g_strdup (g_value_get_string (value)) you can use
  x = g_value_dup_string (value)
* code like
  - case ARG_FONTSET:
  + case PROP_FONTSET:
      if (text->font)
        gdk_font_unref (text->font);
  -   text->font = gdk_fontset_load (GTK_VALUE_STRING (*arg));
  +   text->font = gdk_fontset_load (g_value_get_string (value));
  should account for g_value_get_string (value) possibly being NULL (old bug,
  the same held true for GTK_VALUE_STRING (*arg))
* you introduced a few leaks:
  case PROP_TEXT:
    g_value_set_string (value, g_strdup (text->text));
    break;
  g_value_set_string() will already duplicate the string internally (you don't
  need to add a reference count to an obj in g_value_set_object (val, obj) either)
* the casting macros work only on real objects, not NULL objects, so code like
  - obj = GTK_VALUE_OBJECT (*arg);
  + obj = GTK_OBJECT (g_value_get_object (value));
    if (obj) { ...
  should cast only after you know g_value_get_object (value) is non-NULL (the
  cast in this specific case is superfluous btw)


another note on notification. in 2.0 with object properties, we're emitting
a notification signal for each property that has changed on an object, the
signal's detail holds the property name. that means, code like:

gtk_object_set (object,
                "label", "foo",
                "width", 25,
                NULL);

will cause emission of two emissions of the GObject::notify signal, one
time as "notify::label" and the other as "notify::width".
that allowes users to connect notifiers to monitor properties e.g. via
gtk_signal_connect_object (object, "notify::label", printf, "the label changed\n");

of course, we'd also want to catch modifications of "label" and "width" that
didn't go through the property system, so additional notification code needs
to be added to ordinary accessors, i.e.:

 void
 my_object_set_label (GtkObject *object, const gchar *label)
 {
   g_return_if_fail (MY_IS_OBJECT (object));
   
   g_free (object->label);
   object->label = g_strdup (label);
+  /* take care to carry on propagation */
+  g_object_notify (G_OBJECT (object), "label");
 }

for accessors that modify multiple properties, we need to queue up
notification, so emissions of ::notify doesn't occour prematurely,
i.e. while the object is still being notified. for that
g_object_freeze_notify() and g_object_thaw_notify() are in place.
lemme give a brief example here as well:

 void
 my_object_set_size (GtkObject *object, guint width, guint height)
 {
   g_return_if_fail (MY_IS_OBJECT (object));

+  /* stop notification for the moment untill we're done setting up */
+  g_object_freeze_notify (G_OBJECT (object));
   
   object->width = width;
+  g_object_notify (G_OBJECT (object), "width"); // notify::width

   object->height = height;
+  g_object_notify (G_OBJECT (object), "height"); // notify::height
   
+  /* we're done, now do actuall emission of notify::width and notify::height */
+  g_object_thaw_notify (G_OBJECT (object));
 }


---
ciaoTJ


_______________________________________________
gnome-hackers mailing list
gnome-hackers gnome org
http://mail.gnome.org/mailman/listinfo/gnome-hackers