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 693936 - shell-app-system: Ensure that we correctly order subsearches
shell-app-system: Ensure that we correctly order subsearches
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-02-16 04:42 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-02-16 18:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
shell-app-system: Ensure that we correctly order subsearches (2.27 KB, patch)
2013-02-16 04:42 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
shell-app-system: Ensure that we correctly order subsearches (1.03 KB, patch)
2013-02-16 18:29 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-02-16 04:42:51 UTC
As a simple reproduction example, type "f", then type "i" and quickly
delete it. Since we have the search timeout, it turns out that we
actually make an initial search request for terms ["f"], and then
a subsearch request for ["f"]. The subsearch for equal terms is another
bug, but the ordering here is something that could happen in
normal usage as well, so we should fix it as well.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-02-16 04:42:53 UTC
Created attachment 236344 [details] [review]
shell-app-system: Ensure that we correctly order subsearches

As we use g_slist_prepend for efficiency when building the list
of results before ordering it, we need to make sure we traverse
the list of previous results backwards so that it's built in
the same order.
Comment 2 Cosimo Cecchi 2013-02-16 14:22:52 UTC
Review of attachment 236344 [details] [review]:

::: src/shell-app-system.c
@@ +781,3 @@
   GSList *normalized_terms = normalize_terms (terms);
 
+  for (iter = g_list_last (previous_results); iter; iter = iter->prev)

Can't you just g_slist_reverse(normalized_terms)?

::: src/shell-app-system.h
@@ +58,3 @@
                                                             GSList          *terms);
 GSList         *shell_app_system_subsearch                 (ShellAppSystem  *system,
+                                                            GList           *previous_results,

I don't understand why the change to GList...
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-02-16 17:14:13 UTC
Review of attachment 236344 [details] [review]:

::: src/shell-app-system.c
@@ +781,3 @@
   GSList *normalized_terms = normalize_terms (terms);
 
+  for (iter = g_list_last (previous_results); iter; iter = iter->prev)

Yeah, I can, but I figured it's slightly more efficient to traverse the list in reverse order.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-02-16 18:29:58 UTC
Created attachment 236393 [details] [review]
shell-app-system: Ensure that we correctly order subsearches

As we use g_slist_prepend for efficiency when building the list
of results before ordering it, we need to make sure we traverse
the list of previous results backwards so that it's built in
the same order.
Comment 5 Cosimo Cecchi 2013-02-16 18:31:30 UTC
Review of attachment 236393 [details] [review]:

++
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-02-16 18:34:00 UTC
Attachment 236393 [details] pushed as 754df5e - shell-app-system: Ensure that we correctly order subsearches