Re: [Geany-devel] gtk_separator_tool_item_new() patch

2012-05-12 Thread Dimitar Zhekov
Hi,

I looked into plugin toolbar support code in more detail, and there
seem to be three simple ways to fix the item order:

1. Apply the current patch and tell the plugin authors to add/remove
their items at once. They are likely to do so in the first place,
because mixed adding/removing means one must keep track of the items,
to be able to remove them on plugin_cleanup. It's easier to add
everything at once, show/hide as needed, and remove at once. A not
shown / hidden item is just as good as not added / removed item.

2. Add an item_count to GeanyAutoSeparator. We already have the means
required to track items, but ref_count is for show/hide. Having item
count has the advantage of being able to destroy autosep-widget when
it drops to 0. ref_count should be renamed to show_count or something.

3. Add a hidden separator item after autosep and always insert before
it. Will work like the current positioning, except the 1st item will be
OK. May be easier/simpler to implement than #2, but I don't like it.

Related to #1, note that mixed removing of items, too, is not supported
well by Geany - if you destroy a never shown / hidden item, ref_count
will be decremented anyway and autosep may be hidden even if visible
items exist.

--

On Wed, 09 May 2012 00:19:15 +0200
Colomban Wendling lists@herbesfolles.org wrote:

 def get_insert_position(plugin):
 [...]

 def add_item(plugin, item):
 [...]

It will work, of course, but I think the above are simpler.

-- 
E-gards: Jimmy
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel


Re: [Geany-devel] gtk_separator_tool_item_new() patch

2012-05-11 Thread Dimitar Zhekov
On Wed, 09 May 2012 00:19:15 +0200
Colomban Wendling lists@herbesfolles.org wrote:

 Le 29/04/2012 20:26, Dimitar Zhekov a écrit :
  
  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?

Yes. But Geany does not really support mixed add order: the items are
put in toolbar.c's plugin_items exactly in their add_toolbar_item order,
so a recreation of the toolbar will mix them if not added at once.

BTW, I placed a git patch in tracker bug item 3522755.

 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

It creates Plugin_1_Item_2..N | Plugin_1_Item_1 for each plugin, so:

Plugin_1_Item_2 | Plugin_1_Item_1 | Plugin_2_Item_1 | Quit

And for 2 plugins and 2 items:

Plugin_1_Item_2 | Plugin_1_Item_1 Plugin_2_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).

With the patch, we have a proper order if the plugins add their 2+
items at once, and that order matches toolbar.c plugin_items.

With the current code, the order of 2+ items is always wrong. They are
kept together, allright, unless the toolbar is rebuilt, but the
separators are wrong too.

-- 
E-gards: Jimmy
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel


Re: [Geany-devel] gtk_separator_tool_item_new() patch

2012-05-08 Thread Colomban Wendling
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
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel


[Geany-devel] gtk_separator_tool_item_new() patch

2012-04-29 Thread Dimitar Zhekov
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.

-- 
E-gards: Jimmy
--- ./src/pluginutils.c.orig	2011-10-15 19:28:57.0 +0300
+++ ./src/pluginutils.c	2012-04-29 21:17:36.685006711 +0300
@@ -47,7 +47,7 @@
 void plugin_add_toolbar_item(GeanyPlugin *plugin, GtkToolItem *item)
 {
 	GtkToolbar *toolbar = GTK_TOOLBAR(main_widgets.toolbar);
-	gint pos;
+	gint pos = toolbar_get_insert_position();
 	GeanyAutoSeparator *autosep;
 
 	g_return_if_fail(plugin);
@@ -55,26 +55,15 @@
 
 	if (!autosep-widget)
 	{
-		GtkToolItem *sep;
+		GtkToolItem *sep = gtk_separator_tool_item_new();
 
-		pos = toolbar_get_insert_position();
-
-		sep = gtk_separator_tool_item_new();
-		gtk_toolbar_insert(toolbar, sep, pos);
+		gtk_toolbar_insert(toolbar, sep, pos++);
 		autosep-widget = GTK_WIDGET(sep);
-
-		gtk_toolbar_insert(toolbar, item, pos + 1);
-
 		toolbar_item_ref(sep);
-		toolbar_item_ref(item);
-	}
-	else
-	{
-		pos = gtk_toolbar_get_item_index(toolbar, GTK_TOOL_ITEM(autosep-widget));
-		g_return_if_fail(pos = 0);
-		gtk_toolbar_insert(toolbar, item, pos);
-		toolbar_item_ref(item);
 	}
+
+	gtk_toolbar_insert(toolbar, item, pos);
+	toolbar_item_ref(item);
 	/* hide the separator widget if there are no toolbar items showing for the plugin */
 	ui_auto_separator_add_ref(autosep, GTK_WIDGET(item));
 }
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel