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 665158 - Split search into words
Split search into words
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2011-11-29 21:40 UTC by Florian Müllner
Modified: 2011-12-01 19:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
manager: Add forEachItem helper method (1.58 KB, patch)
2011-11-29 21:40 UTC, Florian Müllner
committed Details | Review
searchbar: Split search into words (2.37 KB, patch)
2011-11-29 21:40 UTC, Florian Müllner
reviewed Details | Review
searchbar: Split search into words (2.63 KB, patch)
2011-12-01 19:13 UTC, Florian Müllner
committed Details | Review

Description Florian Müllner 2011-11-29 21:40:12 UTC
See patches. This is what we do in gnome-shell, and I think it makes sense to adopt in gnome-documents as well. For instance, it is currently only possible to search for either author xor title. With attached patch, author/title are matched against each term, and the results AND'ed.
Comment 1 Florian Müllner 2011-11-29 21:40:14 UTC
Created attachment 202404 [details] [review]
manager: Add forEachItem helper method

Subclasses of BaseManager may want to iterate over the managed
items - add a helper method which allows to do this without
accessing private properties of the parent.
Comment 2 Florian Müllner 2011-11-29 21:40:18 UTC
Created attachment 202405 [details] [review]
searchbar: Split search into words

Rather than using the search entry text as a single term when building
the query, split the text into multiple terms and concatenate the
results. That way, a search can be refined by adding more words to a
query, which is generally more useful (possibly with the exception of
full-text search).
Comment 3 Cosimo Cecchi 2011-12-01 18:43:30 UTC
Review of attachment 202404 [details] [review]:

Looks good.
Comment 4 Cosimo Cecchi 2011-12-01 19:04:55 UTC
Review of attachment 202405 [details] [review]:

So, if I understand correctly the behaviour change you're proposing here is:
- let's pretend I am searching for title and I have a document called "gnome rocks"
- without the patch, searching for the string "gnome gno" would not return the result, as it would match the string verbatim
- with the patch the document would be returned, since it still matches both strings

If so, I am fine with the change, especially if the Shell does the same; wondering if it impacts the query performance at all to have more filter conditions put in AND that way.

::: src/searchbar.js
@@ +216,3 @@
+
+    getFilter: function() {
+        let terms = Global.searchController.getString().replace(/ +/g, ' ').split(' ');

Can you make this a method on SearchController? (e.g. getTerms())
I believe it's clearer.
Comment 5 Florian Müllner 2011-12-01 19:13:53 UTC
Created attachment 202542 [details] [review]
searchbar: Split search into words

(In reply to comment #4)
> Review of attachment 202405 [details] [review]:
> 
> So, if I understand correctly the behaviour change you're proposing here is:
> - let's pretend I am searching for title and I have a document called "gnome
> rocks"
> - without the patch, searching for the string "gnome gno" would not return the
> result, as it would match the string verbatim
> - with the patch the document would be returned, since it still matches both
> strings

Yes. Also, "tale dickens" currently does not match a document with author=="charles dickens" and title="a tale of two cities", while it does with the patch.


> If so, I am fine with the change, especially if the Shell does the same;
> wondering if it impacts the query performance at all to have more filter
> conditions put in AND that way.

I honestly don't know. We still use a single tracker query, so hopefully the performance impact (if any) is negligible.


> ::: src/searchbar.js
> Can you make this a method on SearchController? (e.g. getTerms())
> I believe it's clearer.

Sure.
Comment 6 Cosimo Cecchi 2011-12-01 19:35:18 UTC
Review of attachment 202542 [details] [review]:

Thanks, looks good.
Comment 7 Florian Müllner 2011-12-01 19:37:44 UTC
Attachment 202404 [details] pushed as a0206d2 - manager: Add forEachItem helper method
Attachment 202542 [details] pushed as d2456cd - searchbar: Split search into words