GNOME Bugzilla – Bug 589700
GNOME Goal: Use GtkBuilder instead of libglade
Last modified: 2021-06-04 20:55:31 UTC
GNOME 3 (2010) will completely remove libglade. See http://live.gnome.org/GnomeGoals/RemoveLibGladeUseGtkBuilder . See http://www.gnome.org/~fpeters/299.html for automated statistics.
Indeed, this needs to be done. Thanks for the report.
I'm working on it now!
Created attachment 144721 [details] [review] Dropped libglade dependencies This patch drops the libglade dependencies from planner, also introduces one "problem". The eds-plugin (evolution plugin) will not run because evolution depends on libglade. (I think the po files also need to be refactorized to translate the .ui files instead the .glade files)
Evolution-data-server does not depend on libglade, Evolution still does. Adding dependency to bug 589153.
I readed the code of "planner-eds-plugin.c", and found some things that I can't change because I don't understand where are stored the code of some structs: for example, there are PlannerPlugin *plugin variable and later appears that in the code "plugin->priv->glade", priv is PlannerPluginPriv type, but the PlannerPluginPriv type doesn't have the glade field... :s I supose there is a redifinition on the eds .h files included in the file planner-eds-plugin.c , there isn't any redefinition in the planner files that matches with this usage (priv->glade). I spent a lot of time searching U_U .
Can the patch here please get a review/commit?
Created attachment 154653 [details] [review] Dropped libglade dependencies This is an update of the previous patch, aligned to the current HEAD. Compiles also with --enable-eds --enable-eds-backend. Please review and commit this ASAP, due overlap with #575129 .
Created attachment 154654 [details] [review] GtkBuilder .ui files Oops... I forgot to add Andreu's files in the previous patch... This is to be applied just after previous one. Sorry :-\
Review of attachment 154653 [details] [review]: Hello, could you take a look to the comments below? ::: src/planner-sql-plugin.c @@ +972,3 @@ user_entry = gnome_entry_gtk_entry ( + password_entry = GTK_WIDGET (gtk_builder_get_object (gui, "password_entry"));; + GNOME_ENTRY (GTK_WIDGET (gtk_builder_get_object (gui, "user_entry")))); I'm pretty sure that this doesn't work. You need to port GnomeEntry to GtkComboBoxEntry.
Created attachment 156976 [details] [review] Dropped libglade dependencies Yet another update against HEAD. GnomeEntry has been substitute by normal GtkEntry, which has less functionality but seems the most similar widget in accepted libraries for Gnome 3.0. No indications has been supplied for correct migration from GnomeEntry, and reproduce the whole functionality in Planner seems a waste. I personally suggest to temporary leave GtkEntry and talk with other ex-GnomeEntry users and GTK+ developers to provide a shared implementation.
Hello Robert, How about GtkComboBoxEntry?
(In reply to comment #11) > How about GtkComboBoxEntry? > Problem is not the graphic widget, but the history management, which is the real difference between GnomeEntry and everything else. After little discussion on #gnome-love it seems no one is going to port the functionality in a commonly shared library (see: GTK+), I will try to provide a replacement in Planner codebase but also will push for GTK+ integration.
Created attachment 158311 [details] [review] Removed GnomeEntry I've included code from GnomeEntry in a new PlannerEntry widget, and dropped dependency from libgnomeui. This patch depends on the previous one.
Alexandre, can the patches here get a review and commit?
ping
Review of attachment 156976 [details] [review]: The idea is definitely there, and the glade files looks like they have been split the right way, but a few things should be fixed. ::: src/planner-calendar-dialog.c @@ +596,1 @@ + GError* error = NULL; Please define the GError variable at the top of the function, where other variables are defined. This is important for coding style. @@ +597,3 @@ + builder = gtk_builder_new (); + if (!gtk_builder_add_from_file (builder, filename, &error)) + { Coding style: the '{' should be a the end of the 'if' line. This should be checked everywhere through the patch. @@ +599,3 @@ + { + g_warning ("Couldn't load builder file: %s", error->message); + g_error_free (error); You should: - free the filename variable - unref the builder - return NULL I'm pretty sure something will go wrong later if you continue with a buggy builder. Same goes for every instance of the same copy-pasted code in every touched C files. ::: src/planner-calendar-selector.c @@ +110,2 @@ g_object_set_data_full (G_OBJECT (selector), "data", data, The builder object is not unreffed in this function, so it is leaked. ::: src/planner-day-type-dialog.c @@ +184,2 @@ g_signal_connect_object (data->project, "day_added", builder object seems leeked here too. ::: src/planner-default-week-dialog.c @@ +214,2 @@ default_week_dialog_setup_day_option_menu (GTK_OPTION_MENU (data->day_option_menu), data->project, And here. I think every place where a builder object is created should be verified. Looks like a lot of Glade objects were leaked :p
Created attachment 210568 [details] [review] Drop libglade dependencies Right, I was a little too precipitous :-P Better this one?
Review of attachment 210568 [details] [review]: The patch looks alright, just a small nitpick highlighted below. Also, your patch is missing two parts: - removing the libglade dependency from the configure.ac and Makefile.am files - removing the old, now-unused glade files and the glade/ directory containing them. ::: src/planner-calendar-dialog.c @@ +597,1 @@ + error = NULL; I think you should use GError *error = NULL at the top instead of leaving it undefined and assigning it just before use. I think so because: - this is the usual way of using error pointers within gnome so people are expecting it - if the error pointer is not null, you had an error before and should error out anyway.
Review of attachment 158311 [details] [review]: We decided with the maintainer to remove GnomeEntry instead of keeping its dead code around. There's a minimal loss of functionnality for now, but a similar feature could be reimplemented if required. Thanks for your effort.
Removing libgnomeui dependency is in bug #589045, let's keep this one only for libglade.
A port away from libglade exists in https://gitlab.gnome.org/World/planner/-/merge_requests/22 and will hopefully be merged soon. Closing the report here in preparations of bugzilla shutdown. Please track that merge request instead for port away from libglade (and initial gtk3 port)