[Geany-devel] gtkbuilder branch and ui_lookup_widget functions

[Matthew Brush]

On 11-10-17 05:22 AM, Nick Treleaven wrote:
> Hi Matthew,
> I'm a bit concerned about the changed ui_lookup_widget (and hookup)
> functions - these are in the plugin API and can be used independently
> from Glade. (plugin) API function behaviour should not normally change,
> but it seems the owner parameter is now ignored. If we are caching
> lookups this is probably wrong as the widget may get destroyed.
>
> I haven't looked at the new implementation but perhaps you could help
> explain the changes.

Long story short: compatibility is the reason.

Typical short story long:

I didn't want to break all the existing code in core and plugins that 
were using ui_lookup_widget/ui_hookup_widget() functions, so I dropped 
the (now) pointless first parameter.  There's no need to associate 
things with a parent/owner widget since all objects are required to have 
unique names in GtkBuilder (and the GHashTable).

I'm not 100% sure what you mean by caching lookups or widgets being 
destroyed.  I'll try and explain how it works and hopefully we can get 
some more experienced developer's eyeballs on the code to review it 
(that's you!).

So the first function called is ui_init_builder() and what it does is to 
initialize the GtkBuilder so that it creates all of the GObjects.  Then 
it iterates over all of the objects in the GtkBuilder and stores their 
pointers into a GHashTable (also ref'ing them) using their names as the 
key.  This is basically a "drop-in" replacement for the old Glade 2 
generated code.  Replacing the hookup and lookup functions are the new 
ui_lookup_object() and ui_hookup_object() functions.  The former just 
does a g_hash_table_lookup() to find the GObject with the associated 
key(name).  The latter just inserts the object into the hash table using 
the name as the key (and ref'ing it).

It's my understanding that all objects that were hooked up were eternal 
until program close, though if this is not the case, we'll be leaking 
exactly 1 GObject per object that the user/plugin code destroys/unrefs 
since the GHashTable is holding a ref on it.  It would be quite trivial 
to add a function to remove objects from the hash table, but existing 
code wouldn't be using this obviously.  We could also not ref user-added 
object and only ref those from GtkBuilder if we wanted.

In a perfect world, we could drop the GHashTable and use GtkBuilder 
directly to track the objects, but it seems GtkBuilder can only add 
objects from an XML string. Actually, in a perfect world, we wouldn't be 
creating widgets outside of the GUI file at all and so we could use 
GtkBuilder to track all the objects.

So the basic plan I had thought up is to do these steps:

1. Convert the Glade 2 to Glade 3 (done)
2. Get rid of the old generated code with some compat code (done)
3. Separate out all the hard-coded GUI stuff and move it into the proper 
place, the GtkBuilder file (not even remotely done)
4. Stop adding new code that mixes UI and business logic (todo)

Only the first 2 are essential to do now to actually switch to 
GtkBuilder, the next two can be done gradually over time.

I hope this clears up somewhat how the GtkBuilder stuff is currently 
working.  If you want to review the implementation, it's quite trivial 
really, you can get a pretty clear view of the meat of it by using:

	$ git diff master..gtkbuilder src/ui_utils.c

That should show pretty much only the implementation and a few other 
minor changes that have happened since.

Do let me know if you have any more questions, and feel free to hack 
away on the code as usual.  I don't think any more experienced 
developers have really actually reviewed the code in-depth, so I would 
be grateful if you get the time.  It's something that will need to 
happen before we merge into master.

In the meanwhile, I'll keep plugging away on the odd little bugs I 
notice resulting from the changes.

Cheers,
Matthew Brush