GNOME Bugzilla – Bug 497967
Tracker integration for desktop search
Last modified: 2007-11-23 02:11:07 UTC
Some plugin that integrates totem with tracker, that allows to find videos and manage the video metadata
Created attachment 99300 [details] [review] sample tracker plugin, hope somebody could review it Here's a sample of a very simple plugin
From a preliminary glance: there are quite a few code style issues, translation support needs to be added, and there are a couple of other issues, but it generally looks good. I'll try compiling with it tomorrow morning, and give it a test run. :-) +static void +impl_deactivate (TotemPlugin *plugin, Indentation typo. +#define TRACKER_SERVICE "org.freedesktop.Tracker" Another indentation typo; it should be tabs. +enum { + FILE_COLUMN, + NAME_COLUMN, + N_COLUMNS +}; And again. A single tab should be used here. +static void _populate_result (TotemTrackerWidget *widget, gchar *result) It would probably be better to drop the leading underscore from all the static function names here, as no other part of the Totem code uses such a convention. + // clear the list store + gtk_list_store_clear (GTK_LIST_STORE(widget->result_store)); + + gtk_widget_set_sensitive (GTK_WIDGET(widget->previous_button), FALSE); + gtk_widget_set_sensitive (GTK_WIDGET(widget->next_button), FALSE); Need a space before the macro call, as with function calls (this happens in a few other places, too). + if (widget->current_result_page < widget->total_result_count / TOTEM_TRACKER_MAX_RESULTS_SIZE) { + widget->current_result_page ++; + } No need for the braces. + widget->label = gtk_label_new ("Tracker search: "); Needs intltool support, so the string should be in a call to the "_" macro. It would probably be best if you moved a lot of the fields in TotemTrackerWidget to an internal private struct (TotemTrackerWidgetPrivate) to enforce layering. src/plugins/tracker/tracker.totem-plugin is a generated file, and shouldn't be in the patch. ;-) Only src/plugins/tracker/tracker.totem-plugin.in should be in the patch.
I tried to get it running this morning... +# override to _not_ install the test plugins +install-pluginLTLIBRARIES: If you want the plugin installed, these two lines need removing, and the following line... noinst_DATA = $(plugin_in_files:.totem-plugin.in=.totem-plugin) Needs changing to... plugin_DATA = $(plugin_in_files:.totem-plugin.in=.totem-plugin) You should also change configure.in to only build the Tracker plugin if the Tracker client library is available, and use autoconf to get the compiler flags for it. (See how the Galago plugin does it.) Now that I've got the plugin running, it seems to work. Some more changes need to be made, though: The scrollbars for the list view need to be hidden unless the list view actually needs scrolling. When playing an MRL, it needs to be added to the playlist (instead of just calling totem_action_set_mrl_and_play). See around line 245 of totem-video-list.c. While on the subject of TotemVideoList, you might want to consider using that as the list widget so that you get thumbnail support. Finally, it would be a good idea to activate the "Find" button when the enter key is pressed in the search field. That's it for now. It's a good start, and once you've dealt with these issues I'll be able to take a closer look at the plugin and its code.
Created attachment 99503 [details] [review] Totem tracker plugin Here's the patch with the modifications from your comments. Thanks for your useful help!.
Did some minimal testing on it, and committed it. 2007-11-23 Bastien Nocera <hadess@hadess.net> * configure.in: * src/plugins/tracker/*: Add Tracker plugin from Javier Goday <jgoday@gmail.com> (Closes: #497967) FYI, here's what I did before committing: - fix g_strdup_printf memleak - add translations as necessary in the code, and in po/ - remove useless title label - fix creating of the video list for trunk (you don't need to use GValues to set GObject properties btw, just use g_object_set) - Remove use of C++ comments - Add configure.in test Any changes you'd want to make, please file a separate bug. Thanks for the patch! PS: You should take a look at bug 393094, it's about implementing a tagging sidebar that could use tracker as the backend.