GNOME Bugzilla – Bug 729027
Prioritize names over descriptions in search
Last modified: 2014-05-16 10:03:14 UTC
Created attachment 275220 [details] Bad hit Searching for "Displays" brings up "Color". That's wrong. Prioritize names over description.
Created attachment 275237 [details] [review] shell: Keep the filter terms cached This avoids calling g_strsplit() for every model item when filtering.
Created attachment 275238 [details] [review] shell: Make search results sorting smarter Instead of just sorting by the name the sort order will now be: 1. Panels whose name match a search term 2. Panels whose keywords match the most search terms 3. Panels whose description match the most search terms 4. The remaining panels by name
If I search for "disp" - will it list "Displays" as the first hit? Currently I get "Color" when i search for "disp". I mean - will this work for partial names?
(In reply to comment #1) > Created an attachment (id=275237) [details] [review] > shell: Keep the filter terms cached > > This avoids calling g_strsplit() for every model item when filtering. Might be better to use g_str_match_string / g_str_tokenize_and_fold now?
Review of attachment 275237 [details] [review]: Is this really needed? What impact does it have in terms of performance?
Review of attachment 275238 [details] [review]: That looks fine. ::: shell/cc-shell-model.c @@ +110,3 @@ + for (j = 0; keywords[j]; ++j) + if (strstr (keywords[j], terms[i])) + c += 1; I prefer c++;
(In reply to comment #5) > Review of attachment 275237 [details] [review]: > > Is this really needed? What impact does it have in terms of performance? Didn't measure but I'd be surprised if has any impact given the number of panels. I just did this because we need the split terms on search_entry_changed_cb to feed into the shell model for sorting in the following patch. And since we now have the terms at that point it seemed wasteful to repeat that work in the filter func.
Created attachment 276619 [details] [review] display: Add "Monitor" to keywords It's a likely word that might be used when looking for this panel and, with the new search results ordering, the power panel is coming up ahead due to it preferring keyword matches when they exist.
Created attachment 276620 [details] [review] search-provider: Use panel IDs as results instead of tree model paths Since we now re-sort the model according to the search terms, we can't use tree model paths as results since they're not stable. Instead we'll use panel IDs and keep a map of IDs to GtkTreeIters to retrieve the result metas from the model. -- I was testing this a bit more and noticed that something wasn't right. We need something like this to make results work properly now.
Review of attachment 276619 [details] [review]: Looks good
Review of attachment 276620 [details] [review]: GtkTreeIters aren't "stable" either. You're thinking of GtkTreeRowReferences. Is there any reason not to search the treeview for it instead? The performance hit would be small, as the number of items is limited.
(In reply to comment #11) > Review of attachment 276620 [details] [review]: > > GtkTreeIters aren't "stable" either. You're thinking of GtkTreeRowReferences. They aren't generally. But in the specific case of a GtkListStore the documentation says: "The GtkListStore sets the GTK_TREE_MODEL_ITERS_PERSIST flag, which means that GtkTreeIters can be cached while the row exists. Thus, if access to a particular row is needed often and your code is expected to run on older versions of GTK+, it is worth keeping the iter around." > Is there any reason not to search the treeview for it instead? The performance > hit would be small, as the number of items is limited. True. But given the above and the fact that the code has been written already I see little value in doing that.
Then please mention it in the commit log and in the code, because it's certainly not clear when one's used to GtkTreeView.
Yeah, I did go to the docs in the first place to be sure about this so a comment sounds good indeed. Attachment 275237 [details] pushed as 98a2ab2 - shell: Keep the filter terms cached Attachment 275238 [details] pushed as 13abc75 - shell: Make search results sorting smarter Attachment 276619 [details] pushed as 7d9da10 - display: Add "Monitor" to keywords Attachment 276620 [details] pushed as 70dbd36 - search-provider: Use panel IDs as results instead of tree model paths