Le 20/03/2011 22:35, Matthew Brush a écrit :
Hi,
I have a few patches working towards making the sidebar and message window notebooks more friendly.
#0001 Makes original sidebar tabs and any new tabs added reorderable.
I think it'd be better to add another listener to tab-added signal and only set_reorderable() on this very tab and 0003 would add a set_detachable() in this handler (like you did in patch 0002). This would avoid setting the flags on just removed tabs, and re-set the flags on all tabs.
#0002 Makes original message window tabs and any new tabs added reorderable.
Nothing more to say, but some general notes, see below.
#0003 Enables drag and drop support between the sidebar notebook and message window notebook tabs. This will reduce code in plugins who all do this independently and make projects such as geany-sidebarterm pointless.
I think it'd be a nice addition, I didn't know GTK+ provided this directly :) I think however the GTK checks to set the group would better be provided with a wrapper since used in more than one place, something like:
void ui_notebook_set_group_name(GtkNotebook *nb, const gchar *name) { #if GTK_CHECK_VERSION(2, 24, 0) gtk_notebook_set_group_name(nb, name); #elif GTK_CHECK_VERSION(2, 12, 0) guint h = g_str_hash(name);
gtk_notebook_set_group(nb, GUINT_TO_POINTER(h)); #elif GTK_CHECK_VERSION(2, 10, 0) guint h = g_str_hash(name);
gtk_notebook_set_group_id(nb, ABS((gint)h)); #endif }
then simply use:
ui_notebook_set_group_name(GTK_NOTEBOOK(nb), "my-name");
I have not thoroughly tested the implications of this, like if somewhere Geany is using the tab order or location within a specific notebook to access the children. If this is the case, it will be an opportunity to fix the code that is doing that.
I think it need to be better tested before adding it, but I agree that fixing code depending on tab ordering would be good (it there is some).
I need some guidance on how to make the order and location of the tabs to be persistent so it it's saved/loaded with the session. Any hints would be appreciated.
This is the big problem I guess, and it probably need to be addressed before applying the patches, because it's likely to need some tuning.
I think the problem is that we need to be able to identify a tab in order to save its state. Maybe simply needing the child to have a name (set with gtk_widget_set_name()) would work, though it'd need plugins to be adapted to benefit of the change.
More difficult will be the tab restoration I guess. Not only you have to be able to restore tab ordering in a notebook, and this even if some tabs are missing (e.g. a simple insertion at the appropriate position isn't enough); but it'll need to set add the tab to the right notebook. Quite a peace of code I guess ^^
And finally, some notes on your patches:
1) Rule of the thumb: always double check your patches.
2) Don't send 2 patches if the second corrects the first; just send 1 that is directly OK. (you did this in Class Builder UI patches -- here it's only for a blank line) You can easily use "git commit --amend" or "git rebase" to fixup your patches if you already made a commit of them.
2) Style! There is some style issues in your patches: 2.1) You added some useless blank lines (e.g., second addition to 0001) 2.2) You left tabs on blank lines, we don't 2.3) Always surround assignation operator with spaces, even in for() loops 2.4) We don't place one argument per line on function defs/decls
3) Most important: GTK version checking. You seem to have though of it, but you did it wrong in some places: Runtime checking, like gtk_check_version() is done... at runtime. This means that the code need to be compilable even with older version, so you can't use any newer function (like gtk_notebook_get_tab_reorderable()). Generally these runtime checks are used together with setting some properties by name: the check makes sure the property exists, but the code itself is compatible with older versions. You'll need gtk_container_child_set() ;)
Cheers, Colomban