GNOME Bugzilla – Bug 581100
drop libglade dependency
Last modified: 2009-07-30 23:25:59 UTC
As you probably know libglade is going to be officially declared deprecated with 2.27.1 in favor of GtkBuilder. So, to get the conversion started in control-center I converted the display and window preferences capplets to GtkBuilder.
Created attachment 133806 [details] [review] [PATCH 1/2] Drop libgnome dep from display-properties capplet --- capplets/display/Makefile.am | 6 +- capplets/display/build.sh | 2 +- capplets/display/display-capplet.ui | 531 +++++++++++++++++++++++++++++++++++ capplets/display/xrandr-capplet.c | 62 +++-- 4 files changed, 573 insertions(+), 28 deletions(-)
Created attachment 133807 [details] [review] [2/2] Strip libglade dep from gnome-window-properties Glade file converted using Glade-3 this time because of GtkComboBox. --- capplets/windows/Makefile.am | 8 +- capplets/windows/gnome-window-properties.c | 60 +++-- capplets/windows/gnome-window-properties.ui | 381 +++++++++++++++++++++++++++ 3 files changed, 419 insertions(+), 30 deletions(-)
Please don't forget to update also po/POTFILES.in with a similar line: [type: gettext/glade]<path>.ui
First off, sorry for being so slow to respond. g-c-c has only just branched, but I hope we can get back up to speed now. (In reply to comment #2) > [2/2] Strip libglade dep from gnome-window-properties In general, indentation is broken (tabs vs. spaces). -static GtkWidget *focus_mode_checkbutton; -static GtkWidget *autoraise_checkbutton; -static GtkWidget *autoraise_delay_slider; +static GObject *focus_mode_checkbutton; +static GObject *autoraise_checkbutton; +static GObject *autoraise_delay_slider; It seems to me like casting once on _get_object is better than casting any time one of these is actually used. + if (gtk_builder_add_from_file (builder, UIDIR "/gnome-window-properties.ui", &error) == 0) { + g_warning ("Could not parse UI file: %s", error->message); + g_error_free (error); If we don't look at the error, anyway, don't pass it.
Created attachment 137813 [details] [review] remove libglade from gnome-window-properties second try (In reply to comment #4) > First off, sorry for being so slow to respond. g-c-c has only just branched, > but I hope we can get back up to speed now. > No problem. :-) > (In reply to comment #2) > > [2/2] Strip libglade dep from gnome-window-properties > > In general, indentation is broken (tabs vs. spaces). > Well, this seems to be pretty inconsistent already in the original code. Spaces seem to be the desired way (at least in the gnome-window-properties capplet), so I changed my additions to indent using spaces. Didn't fix the other already present "broken" lines as that belongs into a separate patch imho. > -static GtkWidget *focus_mode_checkbutton; > -static GtkWidget *autoraise_checkbutton; > -static GtkWidget *autoraise_delay_slider; > +static GObject *focus_mode_checkbutton; > +static GObject *autoraise_checkbutton; > +static GObject *autoraise_delay_slider; > > It seems to me like casting once on _get_object is better than casting any > time one of these is actually used. These are rarely used as GtkWidget rather than as their specific widget type (GtkRange,GtkToolButton,...). I could change the definition to use this as well, but I'm not sure if you'd like that as it looks more uncommon and makes the patch larger. > + if (gtk_builder_add_from_file (builder, UIDIR > "/gnome-window-properties.ui", &error) == 0) { > + g_warning ("Could not parse UI file: %s", error->message); > + g_error_free (error); > > If we don't look at the error, anyway, don't pass it. > Actually, the error is used in the warning that's logged to the console/log. Or do you mean an error dialog?
(In reply to comment #5) > Created an attachment (id=137813) [edit] > remove libglade from gnome-window-properties second try +++ b/po/POTFILES.skip @@ -14,6 +14,7 @@ capplets/mouse/gnome-settings-mouse.desktop.in capplets/network/gnome-network-properties.desktop.in capplets/sound/gnome-settings-sound.desktop.in capplets/windows/window-properties.desktop.in +capplets/windows/gnome-window-properties.glade The glade file is removed from the repo, is it not? Why add it to the skip list then? > Well, this seems to be pretty inconsistent already in the original code. > Spaces seem to be the desired way (at least in the gnome-window-properties > capplet), so I changed my additions to indent using spaces. Didn't fix the > other already present "broken" lines as that belongs into a separate patch > imho. Yes, it's a mess. All we can do is try to not make it any messier... > These are rarely used as GtkWidget rather than as their specific widget type > (GtkRange,GtkToolButton,...). I could change the definition to use this as > well, but I'm not sure if you'd like that as it looks more uncommon and makes > the patch larger. Fair enough. > Actually, the error is used in the warning that's logged to the console/log. Right, missed that.
(In reply to comment #6) > (In reply to comment #5) > > Created an attachment (id=137813) [edit] > > remove libglade from gnome-window-properties second try > > +++ b/po/POTFILES.skip > @@ -14,6 +14,7 @@ capplets/mouse/gnome-settings-mouse.desktop.in > capplets/network/gnome-network-properties.desktop.in > capplets/sound/gnome-settings-sound.desktop.in > capplets/windows/window-properties.desktop.in > +capplets/windows/gnome-window-properties.glade > > The glade file is removed from the repo, is it not? Why add it to the skip list > then? > That's because when I first did the patches, I was planning to keep the glade file as backup. But yes, I think we can remove it now, as glade3 seems to work pretty well now. Should be no problem to strip this chunk before applying. (In reply to comment #1) > Created an attachment (id=133806) [edit] > [PATCH 1 [edit]/2] Drop libgnome dep from display-properties capplet I'll need to enhance this patch a bit (re-convert UI file and one glade_xml -> gtk_builder change) to incorporate the changes Federico did in the meantime.
Created attachment 137987 [details] [review] remove libglade from gnome-display-properties (In reply to comment #7) > (In reply to comment #1) > > Created an attachment (id=133806) [edit] > > [PATCH 1 [edit] [edit]/2] Drop libgnome dep from display-properties capplet > > I'll need to enhance this patch a bit (re-convert UI file and one glade_xml -> > gtk_builder change) to incorporate the changes Federico did in the meantime. > Did this now.
Created attachment 137988 [details] [review] clean gnome-window-properties Identical to attachment 137813 [details] [review] except that the glade file is removed now.
The windows properties patch has broken indentation in main. Please commit after fixing that. Thanks!
Patches committed, with broken indentation fixed
I reopen the bug because gnome-control-center still uses libglade library. See http://www.gnome.org/~fpeters/299.html for a complete list
Created attachment 138184 [details] [review] Remove libglade depency from at-properties capplet This ones also does some optimization: It creates and assigns the GtkImage instances (using stock-images) used by a few of the buttons through GtkBuilder.
Created attachment 138188 [details] [review] Remove libglade dep from default-applications capplet
Created attachment 138195 [details] [review] Remove libglade dependency from keybindings capplet
Created attachment 138184 [details] [review] Remove libglade depency from at-properties capplet + g_warning ("Could not load UI: %s\n", error->message); g_warning and friends don't need a trailing newline. Otherwise good to go. Created an attachment (id=138195) Remove libglade dependency from keybindings capplet Same here.
(In reply to comment #16) > Created an attachment (id=138184) [edit] > Remove libglade depency from at-properties capplet > > + g_warning ("Could not load UI: %s\n", error->message); > > g_warning and friends don't need a trailing newline. Otherwise good to go. > Oops, old habit of mine, appending \n to printf-style strings. Deleted them before committing.
Status: find . -name *.glade ./capplets/keyboard/gnome-keyboard-properties.glade ./capplets/about-me/gnome-about-me.glade ./capplets/about-me/gnome-about-me-fingerprint.glade ./capplets/network/gnome-network-properties.glade ./capplets/mouse/gnome-mouse-properties.glade ./capplets/localization/localization.glade grep -rn '#include <glade' * capplets/keyboard/gnome-keyboard-properties.c:32:#include <glade/glade.h> capplets/keyboard/gnome-keyboard-properties-xkb.c:31:#include <glade/glade.h> capplets/keyboard/gnome-keyboard-properties-xkbpv.c:28:#include <glade/glade.h> capplets/keyboard/gnome-keyboard-properties-xkbltadd.c:30:#include <glade/glade.h> capplets/keyboard/gnome-keyboard-properties-xkbmc.c:30:#include <glade/glade.h> capplets/keyboard/gnome-keyboard-properties-xkbot.c:32:#include <glade/glade.h> capplets/keyboard/gnome-keyboard-properties-a11y.h:29:#include <glade/glade.h> capplets/keyboard/gnome-keyboard-properties-xkblt.c:29:#include <glade/glade.h> capplets/about-me/gnome-about-me-fingerprint.h:20:#include <glade/glade.h> capplets/about-me/gnome-about-me-password.c:34:#include <glade/glade.h> capplets/about-me/gnome-about-me.c:30:#include <glade/glade.h> capplets/about-me/gnome-about-me-fingerprint.c:22:#include <glade/glade.h> capplets/network/gnome-network-properties.c:29:#include <glade/glade.h> capplets/mouse/gnome-mouse-accessibility.c:19:#include <glade/glade.h> capplets/mouse/gnome-mouse-accessibility.h:21:#include <glade/glade.h> capplets/mouse/gnome-mouse-properties.c:31:#include <glade/glade.h> capplets/localization/main.c:23:#include <glade/glade-xml.h>
Created attachment 138472 [details] [review] Remove libglade dependency from gnome-network-properties (In reply to comment #18) > Status: > ./capplets/localization/localization.glade > ... > capplets/localization/main.c:23:#include <glade/glade-xml.h> > So, what about this one. It doesn't seem to be built. Deprecated? Removable?
localization capplet is almost empty, so just forget about it for now, unless you want to change it. But it's not used at all, since it was started but never did anything useful. We might want to have it for future releases, so I guess, if you have time, it would be good to just change it to GtkBuilder and have someone work on it for next cycle
Created attachment 139513 [details] [review] Remove libglade dependency from keyboard capplet Conservative patch, splits the existing .glade file into multiple .ui files and overides the WID() macro
Created attachment 139514 [details] [review] Remove libglade dependency from keyboard capplet Previous patch generated incorrectly
Created attachment 139517 [details] [review] Remove libglade dependency from keyboard capplet Had wrong name for a11y notifications .ui file
Created attachment 139518 [details] [review] Remove libglade dependency from keyboard capplet I hate git so much...
Created attachment 139522 [details] [review] Remove libglade dependency from about-me capplet
Created attachment 139523 [details] [review] Remove libglade dependency from mouse capplet
Created attachment 139526 [details] [review] Remove libglade dependency from about-me capplet
Created attachment 139527 [details] [review] Screwed up the patch again
Created attachment 139528 [details] [review] Remove libglade dependency from keyboard capplet
Created attachment 139529 [details] [review] Remove libglade dependency from mouse capplet
Created attachment 139530 [details] [review] Remove Glade from the build system Assumes the localization code is to be removed. Also fixes the previous patches not putting the MIME prefix on po/POTFILES.in
Sorry for all the noise - I have an ongoing battle with git... Anyway the patches should remove all libglade from GNOME Control centre.
Please commit them all!
All committed, I also changed localization to GtkBuilder. No more Glade!