[Geany-devel] gtk_separator_tool_item_new() patch

Colomban Wendling lists.ban at xxxxx
Tue May 8 22:19:15 UTC 2012


Hi,

Le 29/04/2012 20:26, Dimitar Zhekov a écrit :
> Hi again, and excuse me for stuffing the list.
> 
> Actually there is 1/2 error. The plugin toolbar items are inserted
> improperly, but added to plugin_items list in the right order. So
> using "Customize Toolbar" and adding/removing items or otherwise
> changing them fixes the order.
> 
> Patch attached. Not in git format, sorry.

If I read the thing correctly, the patch is wrong because it would
possibly mixup tool items from different plugins if they aren't added at
the same time, wouldn't it?

But you're right that there is a problem.  Currently, it creates:

| Plugin_1_Item_2 Plugin_1_Item_1 | Plugin_2_Item_1 | Quit

and should create

| Plugin_1_Item_1 Plugin_1_Item_2 | Plugin_2_Item_1 | Quit

However with your patch, if plugins are added in the order
Plugin_1_Item_1, Plugin_2_Item_1, Plugin_1_Item_2, it would give:

| Plugin_1_Item_1 | Plugin_2_Item_1 Plugin_1_Item_2 | Quit

Which is also wrong (more wrong if I could say).


Getting that right seems a bit harder and would need to be able to know
what's the last item added by a given plugin.  Maybe tagging the widget
with the plugin it belongs to, and then search for the first
non-matching one would do the trick:

def get_insert_position(plugin):
	pos = toolbar.get_default_insert_pos()

	if plugin.autosep:
		# find the last item belonging to @plugin
		while pos < toolbar.get_n_items():
			item = toolbar.get_item(pos)
			if item.get_data("plugin") != plugin:
				break

	return pos

def add_item(plugin, item):
	pos = get_insert_pos(plugin)

	if not plugin.autosep:
		plugin.autosep = create_sep()
		toolbar.insert(plugin.autosep, pos)
		pos += 1

	item.set_data("plugin", plugin)
	toolbar.insert(item, pos)

Maintaining an index don't seem really a good idea since it would be one
another thing to keep, and I don't think that adding a tool item is a
performance-critical thing so the possible overhead finding the position
(if there is already an item) should not be a problem.

Thoughts?

Cheers,
Colomban



More information about the Devel mailing list