GNOME Bugzilla – Bug 604909
Allow optional GTK+ dependency
Last modified: 2010-01-24 13:00:59 UTC
Created attachment 149991 [details] patch against master HEAD Hi! here's a patch to make the GTK+ dependency optional, it adds a TotemPlPlaylist object, which is semantically quite similar to treemodel, and a totem_pl_parser_save() function which takes such object, the old totem_pl_parser_write(_with_title) have been reimplemented on top of that one. If this approach is liked, further patches could make totem_pl_parser_parse() return a TotemPlPlaylist, and make it implement the GtkTreeModel interface if compiled with GTK+ support.
Created attachment 149992 [details] [review] Oops, forgot to actually add the new object
I'm not happy with this implementation. I don't want GTK+ to be a compile-time dependency, but a run-time dependency. If the GTK+ dependency is going to be removed, it needs to be removed altogether, and applications updated to use the new code.
(In reply to comment #2) > I'm not happy with this implementation. > > I don't want GTK+ to be a compile-time dependency, but a run-time dependency. Hmm, why? Right now there really are two places this decision matters, GNOME and Maemo. Doing the extra check while running just adds overhead to the code and for no gain as far as I can see? What scenario can you envisage needing this to be run-time based? > If the GTK+ dependency is going to be removed, it needs to be removed > altogether, and applications updated to use the new code. Well, ideally this is how I would do it long term.
(In reply to comment #3) > (In reply to comment #2) > > I'm not happy with this implementation. > > > > I don't want GTK+ to be a compile-time dependency, but a run-time dependency. > > Hmm, why? > > Right now there really are two places this decision matters, GNOME and Maemo. > Doing the extra check while running just adds overhead to the code and for no > gain as far as I can see? And I don't want to have crippled versions of totem-pl-parser be available on Maemo. If any other applications want to use the saving, then there would be 2 APIs available, one would work on GNOME, and the other one on Maemo. > What scenario can you envisage needing this to be run-time based? > > > If the GTK+ dependency is going to be removed, it needs to be removed > > altogether, and applications updated to use the new code. > > Well, ideally this is how I would do it long term. What's the problem with using this new API, and removing the old one (and updating the few applications that use totem-pl-parser's saving code)?
(In reply to comment #4) > And I don't want to have crippled versions of totem-pl-parser be available on > Maemo. If any other applications want to use the saving, then there would be 2 > APIs available, one would work on GNOME, and the other one on Maemo. Currently there already is a crippled version on Maemo because it can't link with GTK+. I would rather not need GTK+ for such a library in the first place, but I understand that it is likely there mostly as a convenience. > > What scenario can you envisage needing this to be run-time based? > > > > > If the GTK+ dependency is going to be removed, it needs to be removed > > > altogether, and applications updated to use the new code. > > > > Well, ideally this is how I would do it long term. > > What's the problem with using this new API, and removing the old one (and > updating the few applications that use totem-pl-parser's saving code)? Absolutely nothing, other than if you would be happy to do that, which it seems you are by the very suggestion ;) The only reason it wasn't suggested was to keep totem and others happy while facilitating the needs in Maemo. But if we can merge the two into one solution that's great :D Which applications would need fixing (other than Totem)?
(In reply to comment #5) > (In reply to comment #4) > > And I don't want to have crippled versions of totem-pl-parser be available on > > Maemo. If any other applications want to use the saving, then there would be 2 > > APIs available, one would work on GNOME, and the other one on Maemo. > > Currently there already is a crippled version on Maemo because it can't link > with GTK+. I would rather not need GTK+ for such a library in the first place, > but I understand that it is likely there mostly as a convenience. You have a crippled version of the library which doesn't link to e-d-s as well, if you're using an old version of totem-pl-parser. Build it against gmime, and I'll remove this horrible option from the sources. > > > What scenario can you envisage needing this to be run-time based? > > > > > > > If the GTK+ dependency is going to be removed, it needs to be removed > > > > altogether, and applications updated to use the new code. > > > > > > Well, ideally this is how I would do it long term. > > > > What's the problem with using this new API, and removing the old one (and > > updating the few applications that use totem-pl-parser's saving code)? > > Absolutely nothing, other than if you would be happy to do that, which it seems > you are by the very suggestion ;) The only reason it wasn't suggested was to > keep totem and others happy while facilitating the needs in Maemo. But if we > can merge the two into one solution that's great :D > > Which applications would need fixing (other than Totem)? Rhythmbox and possibly Banshee.
(In reply to comment #6) > You have a crippled version of the library which doesn't link to e-d-s as well, > if you're using an old version of totem-pl-parser. Build it against gmime, and > I'll remove this horrible option from the sources. Hmm, I am not sure how this is relevant here? > > Which applications would need fixing (other than Totem)? > > Rhythmbox and possibly Banshee. We can find time to do that I'm sure.
Created attachment 150300 [details] [review] Updated patch This patch removes older write functions and the whole GTK+ dependency, also adds API docs for the new functions/object. So far I've discovered totem, rhythmbox and brasero to use totem_pl_parser_write(_with_title), I've done patches for these, will file them in separate bugs depending on this one.
Thanks for the patch! commit 01ded1197b0553ecb0685ef180e35c053a504b8e Author: Carlos Garnacho <carlos@lanedo.com> Date: Fri Dec 18 14:25:57 2009 +0100 Remove GTK+ dependency. TotemPlPlaylist has been added to replace GtkTreeModel usage in the write API, internal playlist implementations now use this to get songs/titles in order to write the playlist. Older write API has been replaced by totem_pl_parser_save().
(In reply to comment #7) > > > > Which applications would need fixing (other than Totem)? > > > > Rhythmbox and possibly Banshee. > > We can find time to do that I'm sure. And Brasero, see https://bugzilla.gnome.org/show_bug.cgi?id=607657