[Geany-devel] gtkbuilder branch and ui_lookup_widget functions

[Nick Treleaven]

On 18/10/2011 00:09, Matthew Brush wrote:
> 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.

I understand that, but see below.

> 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).

Here I think is the problem. As I mentioned earlier, ui_[lh]ookup_widget 
can be used *without* Glade or GtkBuilder! This is mentioned in the 'GUI 
example' for Stash (scroll down):

http://www.geany.org/manual/reference/stash_8h.html#_details

There is no reason why the owner dialog can't be destroyed, and also no 
reason why plugin API users have to use globally unique names for widgets.

So this change breaks existing API behaviour, and in addition I think 
those features are things that are good to support anyway.

So I think we need to restore this behaviour for those functions. 
ui_[lh]ookup_object should either be updated similarly or renamed to 
something like ui_builder_lookup to be clear about the difference.

> I'm not 100% sure what you mean by caching lookups or widgets being
> destroyed.

Hopefully I've explained this above.

> 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

Thanks for explaining, but I think my suspicions were correct.

> 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.

Thanks for that command, I hadn't learnt how to do that yet ;-)