GNOME Bugzilla – Bug 562052
Remove deprecated GTK+ symbols in gedit
Last modified: 2010-04-12 18:51:00 UTC
I'm working on implementing http://live.gnome.org/GnomeGoals/RemoveDeprecatedSymbols/GTK%2B If it's not a good idea, or not currently on the agenda, please let me know. Otherwise I'll be working on this for now.
It's definately a good idea and help is welcome. There should not be too much deprecated stuff... one of the things that uses deprecated widgets is the encoding selector in the file chooser: unfortunately last time we checked we could not get the same behavior with undeprecated widgets.
Created attachment 123295 [details] [review] patch for this bug Use GtkComboBox in the encoding selector widgets && other
Thanks. However as I anticipated on comment 2 moving to combobox for the encoding selection we lose the current encoding indication we have in the current widget... I guess at some point we'll have to leave with that.... Paolo, Jesse, Steve: Opinions? In any case if we change that widget we should also refactor its name to GeditEncodingComboBox. In the mean time I committed the rest of the patch.
The current encoding indication can be solved with a different kind of hint (like bold face) or with toggle buttons in the combobox view. The problem is that this also means that the item drawn in the combobox when the drop down is not shown, also is displayed like this. There might be a trick to render it differently in the display entry, than in the drop down.
Created attachment 123388 [details] [review] corrected patch current encoding now has PANGO_STYLE_ITALIC style & change widget name to GeditEncodingComboBox
*** Bug 369150 has been marked as a duplicate of this bug. ***
Paolo, can this get a review?
the patch looks mostly good, but I have not made my mind yet wrt to the ui change involved. I decided to punt this for the next cycle.
Targetting to 2.28 to make sure that this won't be forgotten.
Gedit has branched now so this would be possible to get in.
Paolo: Any decisions about the UI change? Can this get into master?
Current to-do list: gtk_action_connect_proxy, gtk_box_pack_start_defaults, gtk_option_menu_get_history, gtk_option_menu_set_history, gtk_option_menu_set_menu, gtk_rc_style_unref, gtk_widget_get_action, gtk_widget_unref
Created attachment 151170 [details] [review] Remove deprecated GTK+ symbol: gtk_box_pack_start_defaults()
Review of attachment 151170 [details] [review]: Javier's patch is ok.
I assume that fixing gtk_rc_style_unref/gtk_widget_unref would be also accepted?
Comment on attachment 151170 [details] [review] Remove deprecated GTK+ symbol: gtk_box_pack_start_defaults() commit a2459132e00e716fd14ca0268c97c8cbf48c31ff
(In reply to comment #15) > I assume that fixing gtk_rc_style_unref/gtk_widget_unref would be also > accepted? I think this was already fixed?
Created attachment 152021 [details] [review] port to gtkcombobox This patch is based on the previous one.
Review of attachment 152021 [details] [review]: In the end as discussed on irc we do not like using italic much... let's just use a plain combo and see if anyone complains. Minor comments inline in the patch, apart from those let's get this committed ::: gedit/gedit-encodings-combo-box.c @@ +234,3 @@ + +static gboolean +RowSeparatorFunc (GtkTreeModel *model, GtkTreeIter *iter, gpointer data) Why camel case? let's keep the usual codestyle @@ +240,3 @@ + gtk_tree_model_get (model, iter, NAME_COLUMN, &str, -1); + + if (str != NULL && strlen (str) != 0) no need for strlen here: if (str != NULL && *str != '\0')
Comment on attachment 152021 [details] [review] port to gtkcombobox Pushed fixing what you said and also disposing the store, that I forgot doing before.
Also removed gtk_action_connect_proxy. I'm closing this bug now.
Reopening as GTK_WIDGET_TOPLEVEL and gtk_widget_get_action are used. ./gedit/gedit-panel.c: if (GTK_WIDGET_TOPLEVEL (toplevel) && GEDIT_IS_WINDOW (toplevel)) ./gedit/gedit-encodings-combo-box.c: if (!GTK_WIDGET_TOPLEVEL (toplevel)) ./gedit/gedit-window.c: action = gtk_widget_get_action (GTK_WIDGET (proxy));
Fixed gtk_widget_get_action as gtk_activatable_get_related_action requires only 2.16.
Fixed GTK_WIDGET_TOPLEVEL ifdeffing. Thanks for reporting.