Created attachment 98693 [details] [review]
first pass at "binding" support

This patch implements the functionality.

It also adds a gtk_builder_add_object() call so that you can manually insert a named GSettings object that can then be used as the "settings" attribute by the <binding> tags in the markup.  This is important for schemas that might have instances in more than once place.

Unimplemented is the default (ie: non _full() variant) bind_settings() call.  Implementing this would cause GTK to depend on GSettings and I'm not ready for that.  A sane implementation of the callback looks something like this:

static void
bind_settings (GtkBuilder *builder,
               GObject    *object,
               const char *property,
               const char *settings,
               const char *key,
               int         flags,
               gpointer    user_data)
{
  GSettings *gsettings;

  gsettings = (gpointer) gtk_builder_get_object (builder, settings);

  if (!G_IS_SETTINGS (gsettings))
    g_error ("Not a settings object: '%s'", settings);

  g_settings_bind (gsettings, key, object, property, flags);
}

First some general comments:

Wouldn't this be cleaner as an extension of the existing <property> element
which already "binds" properties to constant values ? One could imagine
something like:

<property name="value" settings="settings" key="port"/>


The name "binding" is already taken in the gtk context to mean key bindings. 
It would probably be a good idea to avoid the term in this context.


On to details of the patch:

-  /* Do not free the signal items, which GtkBuilder takes ownership of */
+  /* Do not free the signal/bindings items,
+   * which GtkBuilder takes ownership of */

This doesn't compute. Either leave it as is, or align all the stars, like

   /*
    *
    */


+ * This is the signature of a function used to bind settings.  It is used
+ * by the gtk_builder_connect_signals_full() method to allow the programmer

copy-and-paste error here ? this should not talk about connect_signals_full,
I assume.


+      object = g_hash_table_lookup (builder->priv->objects,
+				    binding->object_name);
+      g_assert (object != NULL);

I don't like the assert here. The corresponding signal function emits a warning in this situation.


+ * This call sinks the object.  The object may be referenced by other
+ * objects created by the builder.

I don't see the object getting sunken in the function.

i propose instead

  <property name='text'>
    <setting settings='settings' key='username'/>
  </property>


the only problem with this being the insane overload in the occurance of the word "setting" :)

to solve the problem of not having a "default" value for the property (for the case where the settings aren't hooked up) i propose allowing multiple <property> tags like so:

  <property name='text'>content</property>
  <property name='text'>
    <setting settings='settings' key='username'/>
  </property>

so then it will have the "content" string until the settings are bound.

btw: any mix of text and <settings/> tags inside a <property> is illegal.  whitespace excluded, of course.

all the other comments are just bugs that i'll fix :)

Ryan: I think the proper solution to this is to implement a custom GtkBuildable interface on DConfSettings and add a custom tag called <bind>, you'd then be able to write for example:

  <object class='DConfSettings' id='settings'>
    <property name='schema'>account.desrt.ca</property>
    <binding for='button1' property='value' key='port'/>
  </object>

This wouldn't require any changes to GtkBuilder or Gtk, instead it could live completely in DConf.

I think that this is conceptually inside-out.  The fact that an object property is bound to a particular setting is a fact that is most interesting to the object.  I'd guess that glade (et al.) will want to have the interface for binding settings to an object property available while interacting with the object.

We don't, for example, instantiate GtkAdjustment and have a clause in it saying "this GtkAdjustment controls the 'adjustment' property of gtkspinbutton1".  We rather refer to the adjustment from the spinbutton.

To do otherwise seems like spooky action at a distance and/or unobvious behaviour in the sense that if you want to find out why something is happening to the spinbutton you'd probably first look at the spinbutton entry in the UI file.  You probably wouldn't grep for all of the other entities in the file that refer to the spinbutton from a distance.


The other point is the fact that this sort of setup doesn't allow you to handle the case that I added gtk_builder_add_object for.  In some cases of constructing a dialog that depends on a GSettings object, you won't be able to know the location of the settings object from just the .ui file -- because the settings object will have multiple instances.  Think about applets or "accounts" in an email program.  This is the case where you provide a named settings object (which points to the right place) to GtkBuilder and the widgets in the UI description depend on it.

A workaround to make this work would be quite ugly, I think, and would, by its nature, probably eliminate the benefit/ability to have the bindings in the xml.

I think I'll side with Ryan here. 

While the ability to keep new builder syntax implementations separate is very valuable for 3rd party widgets, and is the right thing to do there, the settings
object is not like a widget. And it adds a new capability to GtkBuilder (binding properties to settings) that is worth supporting directly.

Very interesting and exciting stuff,
  a.) Is that GApplication class stuff comming along ?
      (will it make sense that an application already comes
      with a settings object ?)
  b.) Also, I'm not sure I follow how this is going to work:

   <interface>
     <object class='DConfSettings' id='settings'>
       <property name='schema'>account.desrt.ca</property>
     </object>
     ...

   What is the "schema" property ? how can I introspect what
   settings are available to bind to in the project ?

   If we're going to rely on DConf so deeply in the platform
   already (which I think is really cool btw), I think it would
   make sence at the same time to include a custom <schema>
   GtkBuildable construct for the DConfSettings object to load
   the schema inline from the builder file (so that we can also 
   load/save/manipulate the application's settings from Glade :)

*** This bug has been marked as a duplicate of bug 654417 ***