After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 654417 - [GSoC] Add <binding> element to GtkBuilder syntax
[GSoC] Add <binding> element to GtkBuilder syntax
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Class: GtkBuilder
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: GtkBuilder maintainers
GtkBuilder maintainers
: 494329 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-07-11 20:02 UTC by Denis Washington
Modified: 2014-04-19 03:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Diff between "gtkbuilder-gbinding" branch and master (12.54 KB, patch)
2012-02-14 20:38 UTC, Denis Washington
reviewed Details | Review
New "gtkbuilder-gbinding" / master diff (15.35 KB, patch)
2012-02-15 22:03 UTC, Denis Washington
none Details | Review
Adds <binding> new tag to GtkBuilder (15.67 KB, patch)
2014-04-10 20:37 UTC, Juan Pablo Ugarte
reviewed Details | Review
Adds binding ssupport in <property> tag (13.72 KB, patch)
2014-04-11 22:33 UTC, Juan Pablo Ugarte
reviewed Details | Review
Binding support in <property> element (13.76 KB, patch)
2014-04-14 22:49 UTC, Juan Pablo Ugarte
needs-work Details | Review
Binding support in <property> element (13.74 KB, patch)
2014-04-15 07:00 UTC, Juan Pablo Ugarte
none Details | Review
Binding support in <property> element (updated) (12.87 KB, patch)
2014-04-18 18:34 UTC, Juan Pablo Ugarte
none Details | Review

Description Denis Washington 2011-07-11 20:02:01 UTC
Part of my Google Summer of Code project "GObject property binding support in GtkBuilder and Glade" [1] (  ) is to add support for a new <binding> element to GtkBuilder to describe bindings between properties. It can appear inside of an <object> element and has the following form:

  <binding to="target-property" from="source-property" source="source-object"/>

This declares that the property "target-property" of the defined object should always have the same value as the property "source-property" of "source-object". The accompanying Glade branch [2] uses this to read and save property bindings.

The code can be found in the "gtkbuilder-gbinding" branch on git.gnome.org (the GtkBuilder doc comment has also been updated):

  http://git.gnome.org/browse/gtk+/log/?h=gtkbuilder-gbinding

This bug is a request for review and feedback, and for tracking the readiness of the branch for inclusion to GTK+ master.

[1] http://www.google-melange.com/gsoc/project/google/gsoc2011/denisw/12001
[2] http://git.gnome.org/browse/glade/log/?h=gbinding
Comment 1 Denis Washington 2011-07-23 12:15:43 UTC
I recently added support for GBinding transformation functions to the branch (commit 173bd1618dbeff02805535791336d16d243812f0 in the gtkbuilder-gbinding branch). For this purpose, I introduced the following definitions to GtkBuilder's public API:

  gtk_builder_create_bindings (GtkBuilder            *builder,
                               gpointer               user_data);

  gtk_builder_create_bindings (GtkBuilder            *builder,
                               GtkBuilderBindingFunc  func,
                               gointer                user_data);

  typedef void (*GtkBuilderBindingFunc) (GtkBuilder    *builder,
                                         GObject       *source,
                                         const gchar   *source_property,
                                         GObject       *target,
                                         const gchar   *target_property,
                                         const gchar   *transform_func,
                                         GBindingFlags  flags,
                                         gpointer       user_data);

The gtk_builder_create_bindings*() functions create physical GBindings from the definitions read from the interface definition. They work exactly like gtk_builder_connect_signals*() wrt locating transformation functions from their names.

With this commit, the branch is essentially feature complete and doesn't need any substantial additions. Documentation and two test cases (one with and one without transformation functions) are also included. It should thus be "safe" to review. I would be very gladful about any reviews or other kind of feedback!
Comment 2 Tristan Van Berkom 2011-07-23 18:55:38 UTC
It's unfortunate that we cannot harness the default GSignal codepaths
to use transform functions as callbacks instead of an assigned function
pointer.

Currently, I have some doubts about how worthwhile it is to support
the transform function. Some things to consider:

  a.) So far, before adding the transform function support, the 
      bindings branch extends the builder functionality by allowing
      implicit bindings to be declared, without requiring any extra
      work on behalf of the user.

  b.) If I understand correctly, now that we have the transform-function,
      the user must explicitly call gtk_builder_create_bindings(), which
      is a kindof annoying extra step.

  c.) Also, now that we have the transform-function, language bindings
      become more complex (this is why it would be best if the only way
      to load a function from a builder file would always be via a GSignal),
      now language bindings need to add custom code in order to be able
      to even use gtk_builder_create_bindings().

  d.) Does the transform function bring that much added value ?

      Can't we achieve something close already by just using the generic
      g_value_register_transform_func()[0] ?

      Perhaps the GBinding code does not fallback on g_value_transform(),
      which might then be a bug, or perhaps the transform-function is only
      there to provide specific custom transforms for those properties.

From what I can see it seems that the transform-function is indeed only
useful for custom transforms for that binding in particular, it is indeed
a nice feature to have, is there some way we can get this without the
added apis and leverage GSignal for this instead ?

Pie-in-the-sky-vague-idea:
   a.) Deprecate the transform-func in favor of a proper GSignal
       on the GBinding object proper
   b.) Use similar syntax that you currently use, except setup
       all the bindings unconditionally at parse time
   c.) While parsing bindings, add the actual resulting GBinding objects
       to the GtkBuilder id hashmap with some specialized identifiers
   d.) Collect the transform-functions as if they were gsignals to be
       connected to the GBinding objects you have in the hash
   e.) When and if the user ever calls gtk_builder_connect_signals(),
       then the "transform" callbacks GSignals will be connected as
       a result.

This would mean bindings would work but any transform functions would
not be connected until gtk_builder_connect_signals() is called, it would
also come with the weird side-effect of having ids like:
  "generated-binding-togglebutton1:active" 
in the GtkBuilder objects hash table... but it would allow us to achieve
the whole transform-function thing without demanding any extra work from 
the api user or language binding implementor.

Perhaps even something more elegant can be done, perhaps something
can be done using g_object_bind_property_with_closures() ?

Maybe GtkBuilder should break down signal connection logic into 2 steps:
  1.) Create/Collect the GClosures needed to run abstract client code
      (this step requires cooperation from language bindings)
  2.) Connecting the closures to the said signals

Having a logical split at that level would allow us to use a GClosure
created by the GtkBuilder in the same way as it would be created for a 
signal... however that's all very complex and heavy... I dont know, we
should ask ourselves though, 
  - do we want a cleaner solution for loading client code from GtkBuilder ?
  - do we want to just ditch the 'transform-function' features from
    GBinding in GtkBuilder for now until we do address GtkBuilder apis
    more thoroughly ?
  - can we live with adding more api to GtkBuilder that essentially
    does the same thing as loading signals, and push that api on our
    users (program authors and binding authors alike)... and be stuck
    maintaining this new api.


[0]http://developer.gnome.org/gobject/unstable/gobject-Generic-values.html#g-value-register-transform-func
Comment 3 Denis Washington 2011-08-01 12:06:17 UTC
(In reply to comment #2)
> From what I can see it seems that the transform-function is indeed only
> useful for custom transforms for that binding in particular, it is indeed
> a nice feature to have, is there some way we can get this without the
> added apis and leverage GSignal for this instead ?

Transformation functions are for a particular binding and are indeed pretty useful. Omitting them would seriously limit the utility of the whole feature.

I chose my solution so that it requires a minimal amount of changes to the code and especially the public API. The reason that I didn't put it in gtk_builder_connect_signals() is that GtkBuilderConnectFunc doesn't provide all information needed for property bindings to be created (e.g., the binding source object and binding flags).

If GBinding would be changed to use a signal as callback, it might be possible to ditch the new functions, but is it worth the required changes? What do others think?
Comment 4 Denis Washington 2011-08-21 20:06:06 UTC
On request of my mentor and Tristan, transformation function support has been removed for now in the "gtkbuilder-gbinding" branch because my implementation would have required special case logic in language bindings to make the automatic locating of transformation functions work (like is needed for gtk_builder_connect_signals() currently), and that other implementations would require changes to GBinding itself.

For reference, the old code including transformation function support can still be found in a newly created "gtkbuilder-gbinding-transform" branch.
Comment 5 Denis Washington 2012-02-14 20:36:27 UTC
Juan Pablo Ugarto, now (co-?)maintainer of Glade, wrote me today and expressed his wish to merge my GSoC branch into Glade master, preferably before GNOME 3.4 feature freeze (if this is still possible). This depends on the "gtkbuilder-gbinding" branch landing in GTK+, so I would be really glad if someone could review this now. As a convenience, I'll attach the diff to this bug (it's not big).
Comment 6 Denis Washington 2012-02-14 20:38:14 UTC
Created attachment 207588 [details] [review]
Diff between "gtkbuilder-gbinding" branch and master
Comment 7 Matthias Clasen 2012-02-15 04:16:08 UTC
Review of attachment 207588 [details] [review]:

::: gtk/gtkbuilder.c
@@ +138,3 @@
+ * property "from" of object "source" is set to, even if that property
+ * is later set to another value. For more information about the property
+ * binding mechanism, see #GBinding (which GTK+ uses internally).

In addition to this, the relax ng schema should be updated to reflect this new syntax.

@@ +890,3 @@
+          g_object_bind_property (source, binding->from,
+                                  target, binding->to,
+                                  G_BINDING_SYNC_CREATE);

Do we need to allow other binding flags to be specified in the xml ?
I think we may need to.

::: gtk/gtkbuilderparser.c
@@ +1145,3 @@
+                           "Duplicate binding for property: `%s'",
+                           b->to);
+            }

What about having both <binding to="foo"... and <property name="foo"... ?
Your code doesn't seem to check for that.
Comment 8 Juan Pablo Ugarte 2012-02-15 21:02:18 UTC
Hmm its already Wednesday... so I think it will be better to leave this for next release. Ok so my idea is to make a stable release right after UI freeze (3.12) so then we can merge gbindings
Comment 9 Denis Washington 2012-02-15 22:03:14 UTC
Created attachment 207707 [details] [review]
New "gtkbuilder-gbinding" / master diff

The branch now includes the needed changes to the GtkBuilder schemas and checks for conflicting <binding> and <property> elements.

As to binding flags, I haven't added this feature as the Glade branch currently doesn't deal use this. However, I think that the patch in its current form should make it possible to add it quite easily, possibly at a later time, and at least G_BINDING_INVERT_BOOLEAN would probably be good to have. In any case, G_SYNC_CREATE should be hardcoded as not setting that flag would leave the UI in an inconsistent state at start (the bound property might stay at its default value until the source property is changed).
Comment 10 Denis Washington 2012-03-07 06:55:36 UTC
As Glade 3.12 has been released now and the Glade gbinding branch could be theoretically merged now, I would be happy if the improved patch could be reviewed somewhere in the near future.
Comment 11 Matthias Clasen 2013-02-11 05:59:48 UTC
*** Bug 494329 has been marked as a duplicate of this bug. ***
Comment 12 Juan Pablo Ugarte 2014-04-10 20:37:23 UTC
Created attachment 274039 [details] [review]
Adds <binding> new tag to GtkBuilder

Hi guys I cleaned up and updated this patch to master.

I assume we still want to have this new functionality.

I know its been a long time but I cant remember if there any reason why we dont want this to be added to <property> tag?

<property name="sensitive" bind-to="object.property" bind-flags="G_BINDING_SYNC_CREATE"/>
Comment 13 Juan Pablo Ugarte 2014-04-11 22:33:41 UTC
Created attachment 274140 [details] [review]
Adds binding ssupport in <property> tag

This patch implements a binding by adding a few extra parameters to <property>
"bind-source" to specify the source object
"bind-property" to specify the source property
"bind-flags" to specify the binding flags
Comment 14 Matthias Clasen 2014-04-13 21:32:11 UTC
Review of attachment 274140 [details] [review]:

::: gtk/gtkbuilderprivate.h
@@ +65,3 @@
   gchar *data;
+  gboolean translatable:1;
+  gboolean is_binded:1;

that should probably by named 'bound' instead of 'is_binded'
Comment 15 Matthias Clasen 2014-04-13 21:36:07 UTC
Review of attachment 274039 [details] [review]:

::: gtk/gtkbuilder.c
@@ +136,3 @@
  *
+ * It is also possible to define the value of a property by binding it to
+ * another property with a &lt;binding&gt; element. This causes the value

We don't use xml escaping anymore - just say <binding>
Comment 16 Matthias Clasen 2014-04-13 21:38:47 UTC
I like the <property> extension slightly better, I think. If we ever make bindings more flexible (eg multiple sources being combined in various ways), then the <binding> syntax might be a better fit.
Comment 17 Juan Pablo Ugarte 2014-04-14 18:44:32 UTC
That is exactly how I feel, if we are going to keep this thing simple, and I think we all agreed on this, then I also like the <property> extension better.
So in the current patch it will look like

<property name="sensitive" bind-source="object" bind-property="property"
bind-flags="G_BINDING_SYNC_CREATE"/>

But I think specifying both the object and property in one parameter would be better

<property name="sensitive" bind-source="object::property" bind-flags="G_BINDING_SYNC_CREATE"/>

BTW any reason not to add "transform-to" and "transform-from" signals to GBinding?

That is the only feature I can think of that can not be used creating a GBinding object directly in GtkBuilder
Comment 18 Matthias Clasen 2014-04-14 22:01:24 UTC
(In reply to comment #17)
> That is exactly how I feel, if we are going to keep this thing simple, and I
> think we all agreed on this, then I also like the <property> extension better.
> So in the current patch it will look like
> 
> <property name="sensitive" bind-source="object" bind-property="property"
> bind-flags="G_BINDING_SYNC_CREATE"/>
> 
> But I think specifying both the object and property in one parameter would be
> better
> 
> <property name="sensitive" bind-source="object::property"
> bind-flags="G_BINDING_SYNC_CREATE"/>

Thats only going to require you to add a pointless parser to gtkbuilder. 
Does it have any real advantage ?
I'm also afraid we haven't declared "::" in ids illegal 

> BTW any reason not to add "transform-to" and "transform-from" signals to
> GBinding?

What would this do ? For the custom transform support ?
Comment 19 Juan Pablo Ugarte 2014-04-14 22:15:07 UTC
(In reply to comment #18)
> Thats only going to require you to add a pointless parser to gtkbuilder. 
> Does it have any real advantage ?

Not really, I meant it would look better more compact

> I'm also afraid we haven't declared "::" in ids illegal 

fair enough, no problem then

> > BTW any reason not to add "transform-to" and "transform-from" signals to
> > GBinding?
> 
> What would this do ? For the custom transform support ?

Right, that would be the easiest way to define custom transform functions in builder if you want to create an gbinding object directly (without using this <property> extension)

Anyways I am getting offtopic :)
Comment 20 Juan Pablo Ugarte 2014-04-14 22:49:37 UTC
Created attachment 274317 [details] [review]
Binding support in <property> element

Same patch, relax ng schema files fixed
Comment 21 Matthias Clasen 2014-04-15 05:58:20 UTC
Review of attachment 274317 [details] [review]:

::: gtk/gtkbuilderprivate.h
@@ +65,3 @@
   gchar *data;
+  gboolean translatable:1;
+  gboolean is_binded:1;

Still: 'bound'
Comment 22 Juan Pablo Ugarte 2014-04-15 07:00:09 UTC
Created attachment 274330 [details] [review]
Binding support in <property> element

ahh, forgot to amend :)
Comment 23 Juan Pablo Ugarte 2014-04-15 07:03:55 UTC
As you can see I am not fond of irregular verbs :D
Comment 24 Emmanuele Bassi (:ebassi) 2014-04-18 17:52:34 UTC
as discussed on IRC, storing the GBinding instance inside GtkBuilder should go away.
Comment 25 Juan Pablo Ugarte 2014-04-18 18:34:45 UTC
Created attachment 274705 [details] [review]
Binding support in <property> element (updated)

This patch does not expose gbindings objects in GtkBuilder
Comment 26 Juan Pablo Ugarte 2014-04-19 03:18:13 UTC
Pushed to master!