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 680137 - Collect results from both search providers
Collect results from both search providers
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
: 671606 677929 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-07-17 23:01 UTC by William Jon McCann
Modified: 2012-08-03 21:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Collect results from both search providers (28.34 KB, patch)
2012-07-17 23:01 UTC, William Jon McCann
needs-work Details | Review
Collect results from both search providers (38.62 KB, patch)
2012-07-18 11:51 UTC, William Jon McCann
needs-work Details | Review
Collect results from both search providers (38.86 KB, patch)
2012-07-18 14:27 UTC, William Jon McCann
none Details | Review
Collect results from both search providers (38.89 KB, patch)
2012-07-18 14:39 UTC, William Jon McCann
committed Details | Review

Description William Jon McCann 2012-07-17 23:01:28 UTC
Currently we only use the compiled in search provider. This is unfortunate
because this means if tracker is enabled we get no results for any locations
it isn't configured to index.
Comment 1 William Jon McCann 2012-07-17 23:01:30 UTC
Created attachment 219076 [details] [review]
Collect results from both search providers

Previously we would only use the compiled in search provider. Now
it has been changed to run both providers at the same time and
merge the results by URI.
Comment 2 Cosimo Cecchi 2012-07-18 00:25:58 UTC
Review of attachment 219076 [details] [review]:

Looks mostly good, but I think you forgot to git add nautilus-search-provider.[ch]

::: libnautilus-private/nautilus-search-engine.c
@@ +91,3 @@
+		if (count == 0)
+			added = g_list_prepend (added, uri);
+		char *uri = l->data;

You should g_strdup() the uri

@@ +116,3 @@
+			g_hash_table_remove (engine->details->uris, uri);
+		} else {
+		count = GPOINTER_TO_INT (g_hash_table_lookup (engine->details->uris, uri));

Same here

@@ +132,2 @@
+{
+	nautilus_search_provider_error (NAUTILUS_SEARCH_PROVIDER (engine), error_message);

I think we should either also stop the other provider when an error happens, or avoid propagating the error unless both engines signal an error (if we do this, we should also run search_provider_finished() after the first error)...we don't want to end up in a situation where an error is reported to the view, triggering a dialog, but the directory keeps refreshing below because the other engine is still running.
Comment 3 William Jon McCann 2012-07-18 11:51:46 UTC
Created attachment 219091 [details] [review]
Collect results from both search providers

Previously we would only use the compiled in search provider. Now
it has been changed to run both providers at the same time and
merge the results by URI.
Comment 4 Cosimo Cecchi 2012-07-18 13:57:21 UTC
Review of attachment 219091 [details] [review]:

::: libnautilus-private/nautilus-search-engine.c
@@ +137,3 @@
+{
+	DEBUG ("Search provider error: %s", error_message);
+	engine->details->providers_error++;

I think the logic here is still slightly wrong:
- There should be no need to call _finished() after _error()
- Let's say the first engine finishes, and later the second one errors out (or viceversa). With this code, no "error" or "finished" signals would be emitted, since both providers_error and providers_finished would be 1.

::: libnautilus-private/nautilus-search-provider.c
@@ +64,3 @@
+	}
+
+	g_signal_new ("hits-added",

Should be consistent with other similar code here and use signals[SIGNAL_NAME] = g_signal_new()...

@@ +133,3 @@
+	g_return_if_fail (NAUTILUS_IS_SEARCH_PROVIDER (provider));
+
+	g_signal_emit_by_name (provider, "hits-added", hits);

...and then use g_signal_emit(provider, signals[SIGNAL_NAME]) here.
Comment 5 William Jon McCann 2012-07-18 14:27:37 UTC
Created attachment 219127 [details] [review]
Collect results from both search providers

Previously we would only use the compiled in search provider. Now
it has been changed to run both providers at the same time and
merge the results by URI.
Comment 6 William Jon McCann 2012-07-18 14:39:02 UTC
Created attachment 219129 [details] [review]
Collect results from both search providers

Previously we would only use the compiled in search provider. Now
it has been changed to run both providers at the same time and
merge the results by URI.
Comment 7 Cosimo Cecchi 2012-07-18 14:42:49 UTC
Review of attachment 219129 [details] [review]:

Looks good now, thanks!
Comment 8 William Jon McCann 2012-07-18 14:59:34 UTC
Attachment 219129 [details] pushed as 79869fd - Collect results from both search providers
Comment 9 William Jon McCann 2012-07-20 13:22:18 UTC
*** Bug 671606 has been marked as a duplicate of this bug. ***
Comment 10 William Jon McCann 2012-07-20 13:25:26 UTC
*** Bug 677929 has been marked as a duplicate of this bug. ***