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 535971 - Bring back ability to specify a "root" object
Bring back ability to specify a "root" object
Status: RESOLVED DUPLICATE of bug 447998
Product: gtk+
Classification: Platform
Component: Class: GtkBuilder
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GtkBuilder maintainers
GtkBuilder maintainers
Depends on:
Blocks:
 
 
Reported: 2008-05-31 20:36 UTC by Paolo Borelli
Modified: 2008-06-01 16:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (10.38 KB, patch)
2008-05-31 20:37 UTC, Paolo Borelli
none Details | Review
updated patch (13.49 KB, patch)
2008-06-01 11:56 UTC, Paolo Borelli
needs-work Details | Review
patch with set_root() method (15.43 KB, patch)
2008-06-01 16:54 UTC, Paolo Borelli
rejected Details | Review

Description Paolo Borelli 2008-05-31 20:36:24 UTC
In libglade it was possible to specify a "root" object. This is very useful when you want to use glade/builder to create the contents of a composed widget, in particular dialogs.

One could argue that since it's not mandatory that builder files start with a toplevel, the solution is to use a builder file where for instance the "contents_vbox" is the real root. However in real life this doesn't cut it because gui builders need a toplevel to edit the file properly.

I took a quick look and it seems it's not so hard to bring such a functionality back, proof of concept patch follows.
Comment 1 Paolo Borelli 2008-05-31 20:37:28 UTC
Created attachment 111869 [details] [review]
patch
Comment 2 Johan (not receiving bugmail) Dahlin 2008-05-31 20:46:12 UTC
Comment on attachment 111869 [details] [review]
patch

Nice patch!
This looks a lot better than previous attempts, a couple of comments though:
* Why not specifying the root object using a separate method? (gtk_builder_set_parse_root)
* Unit tests missing
* Convert printfs to debug statements.

This is a duplicate of 447998, or vice versa.
Comment 3 Paolo Borelli 2008-06-01 11:56:32 UTC
Created attachment 111884 [details] [review]
updated patch

Here is the patch updated to contain some unit tests and remove the debugging printfs (it also fixes a couple of buglets).

About set_root() yesterday on irc I said I was ok with that, but thinking better about it I do not think it is a good idea:
 - the semantics of set_root() would make sense only if it's called before the real parsing, which may be non obvious to someone who doesn't know the implementation
 - the root is specific to each file, so when using more than one file one either need to set_root multiple times or use the same root name, which would be a very artificial limitation (the root in a file could be main_vbox, while in the other foo_frame).
 - the api for the bindings would need to use set_root too, while this way it can simply override add_from_file with on more argument
Comment 4 Johan (not receiving bugmail) Dahlin 2008-06-01 12:37:07 UTC
I still think set_root is a good idea:
- If you don't add it you'll need to add another new method for adding a buffer with a root. We want to keep the API to a minimum
- Setting the root is similar to setting the translation domain, it needs to be done before parsing (otherwise the items won't be translated) and might be different for each file.
- IMHO bindings should try to behave as similar to the C api as possible, it makes sense to clone the C api to make it easier for developers to switch between different languages.

I'm not so sure about the naming though, set_root, set_parser_root, set_start_parsing_at, etc. Any other good suggestions.

Moving it to a property would make sense, then you would be able to do:

  builder = g_object_new (GTK_TYPE_BUILDER, "domain", ..., "root", ..., NULL);
  gtk_builder_add_from_file (GTK_BUILDER (builder), ...);

Isn't that enough convenience?
Comment 5 Paolo Borelli 2008-06-01 12:59:55 UTC
Well, I didn't add a method fo adding a buffer with a root since I think that's a pretty remote use case: if you create the string programmatically you surely can create it with the proper root, while the main use case for the root_object argument when adding from a file is that you want to still edit the file in a GUI builder with the proper toplevel etc.

As for the translation domain, I think that it would be extremely infrequent to add different files to the same builder with different translation domains, while when using a root_object it would be extremely frequent to have different roots for each files.

That said, since adding more than one file is not that common, as long as the functionality is there one way or the other I am willing to rework the patch to add the set_root() method.
Comment 6 Johan (not receiving bugmail) Dahlin 2008-06-01 13:43:57 UTC
Marking as duplicate, bug 447998 contains useful discussion, we should continue there.

*** This bug has been marked as a duplicate of 447998 ***
Comment 7 Paolo Borelli 2008-06-01 16:54:49 UTC
Created attachment 111904 [details] [review]
patch with set_root() method

as discussed in the duplicate bug, after all having a set_root() method is not what we want. Nonetheless I am attaching the patch here before reverting it from my tree, just in case we change our mind again.