[Geany-devel] gtkbuilder branch and ui_lookup_widget functions

[Matthew Brush]

On 11-10-18 09:05 AM, Nick Treleaven wrote:
> On 18/10/2011 00:09, Matthew Brush wrote:

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

ui_[hl]ookup_object() are not used with GtkBuilder or Glade[1], they 
access an internal hash table.  GtkBuilder is completely out of the 
picture by the end of the ui_init_builder()[2] function. 
Glade3/GtkBuilder does require unique names (as did glade 2 IIUC, since 
all the names were unique), but the only reason those two functions 
currently do also is because the names are used as the key in a hash 
table, and so must be unique.  If you look back in the history you 
should see where I started out with using a GList but eventually moved 
to a GHashTable since it's a better data structure for the purpose.

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

I still don't understand what destroying the owner dialog has to do with 
anything.  TBH I don't even know what use the owner widget parameter had 
in those functions, was it just to provide a "namespace" so that widget 
names didn't clash between plugins?  As for the globally unique names, I 
guess this could be a problem if some new code was using the pointless 
ui_[hl]ookup_object() functions to associate a completely arbitrary 
"name" with a GObject (not *it's* name like gtk_widget_set_name(), just 
*a* name - used to lookup the object in the GHashTable).

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

It might yes, especially if different plugins were calling 
ui_hookup_widget() with clashing names (which they don't).

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

It used to be called ui_add_object() but then I renamed it :) 
ui_builder_lookup() is misleading since it's not looking up anything in 
the builder, which has long since been destroyed.  It's looking up in 
the GHashTable which is only there to provide backwards compatibility 
with old code.

>> I'm not 100% sure what you mean by caching lookups or widgets being
>> destroyed.
>
> Hopefully I've explained this above.
>

Not really (I'm kinda stupid remember :)  I think I'm confused about how 
it used to work and you're confused about how it works now :)

I'm not too familiar with the stash library, but aside from it, what 
other purpose do the ui_[lh]ookup_widget() functions serve that couldn't 
be accomplished with the way plugins typically use widgets[3]?  I know 
glade generated code used to have lookup/hookup functions for some 
reason, is it to do with that?

I looked in the Geany-Plugin code, and there's no use of 
ui_hookup_widget() and most uses of ui_lookup_widget() are using a 
geany->main_widgets->foo widget as the owner, which is fine since the 
widgets that came from Geany's Glade file are guaranteed to have unique 
names.

P.S. Are you ever on IRC or IM?  It's hard to have this type of 
discussion in 1 message per day email communications :)

Cheers,
Matthew Brush

[1] here's the lifecycle of the GtkBuilder instance:
	- enter the ui_init_builder() function
	- initialize a gtkbuilder instance
	- add the xml file to parse to the gtkbuilder
	- extract all the newly parsed/initialized gobjects
	- add those objects to the ghashtable
	- destroy the gtkbuilder instance
	- leave ui_init_builder()
[2] https://github.com/geany/geany/blob/gtkbuilder/src/ui_utils.c#L2116
[3] that is, holding a pointer and destroying the widget when the plugin 
is unloaded.