After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 497967 - Tracker integration for desktop search
Tracker integration for desktop search
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: Plugins
2.21.x
Other All
: Normal enhancement
: ---
Assigned To: General Totem maintainer(s)
General Totem maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2007-11-18 18:25 UTC by jgoday
Modified: 2007-11-23 02:11 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
sample tracker plugin, hope somebody could review it (19.60 KB, patch)
2007-11-18 18:26 UTC, jgoday
needs-work Details | Review
Totem tracker plugin (21.54 KB, patch)
2007-11-22 22:54 UTC, jgoday
none Details | Review

Description jgoday 2007-11-18 18:25:12 UTC
Some plugin that integrates totem with tracker, that allows to find videos and manage the video metadata
Comment 1 jgoday 2007-11-18 18:26:32 UTC
Created attachment 99300 [details] [review]
sample tracker plugin, hope somebody could review it

Here's a sample of a very simple plugin
Comment 2 Philip Withnall 2007-11-18 23:29:36 UTC
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.
Comment 3 Philip Withnall 2007-11-19 07:31:31 UTC
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.
Comment 4 jgoday 2007-11-22 22:54:08 UTC
Created attachment 99503 [details] [review]
Totem tracker plugin

Here's the patch with the modifications from your comments.
Thanks for your useful help!.
Comment 5 Bastien Nocera 2007-11-23 02:11:07 UTC
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.