Multiple word searching with List.search - closes #680 #682
Multiple word searching with List.search - closes #680 #682sheffieldnikki wants to merge 38 commits into
Conversation
Changed license date
Now LICENSE is up to date.
Update GitHub pages
Update contribution guidelines
Update devDependencies
Documentation correction
LICENSE and README.md files are not up to date
Fix typos in error message
Add jsDelivr hits badge
Codecov Report
@@ Coverage Diff @@
## master #682 +/- ##
=======================================
Coverage 94.78% 94.78%
=======================================
Files 19 19
Lines 806 806
Branches 190 190
=======================================
Hits 764 764
Misses 29 29
Partials 13 13 Continue to review full report at Codecov.
|
|
If this multi-word search is accepted into the library, replacing the default search, is it worth having fuzzy search built-in as well? Fuzzy search is 10% of the List.min.js size, and it could easily be built as a separate file |
1cfea52 to
ec685e7
Compare
Codecov Report
@@ Coverage Diff @@
## master #682 +/- ##
=========================================
Coverage ? 94.78%
=========================================
Files ? 19
Lines ? 806
Branches ? 190
=========================================
Hits ? 764
Misses ? 29
Partials ? 13 Continue to review full report at Codecov.
|
| for (var k = 0, kl = list.items.length; k < kl; k++) { | ||
| search.item(list.items[k]); | ||
| } | ||
| }, | ||
| item: function(item) { | ||
| item.found = false; | ||
| for (var j = 0, jl = columns.length; j < jl; j++) { | ||
| if (search.values(item.values(), columns[j])) { | ||
| item.found = true; | ||
| return; | ||
| } | ||
| } | ||
| }, | ||
| values: function(values, column) { | ||
| if (values.hasOwnProperty(column)) { | ||
| text = list.utils.toString(values[column]).toLowerCase(); | ||
| if ((searchString !== "") && (text.search(searchString) > -1)) { | ||
| return true; | ||
| var item = list.items[k]; |
There was a problem hiding this comment.
for (var item of list.items) { ?
(same with following loops)
There was a problem hiding this comment.
No. The array iteration used in the code was tested as being the fastest across multiple browsers, and is also compatible with all browsers. for ... of ... isn't compatible with older browsers.
There was a problem hiding this comment.
Then its certainly better the way it is 👍
| for (var i = 0, il = words.length; i < il; i++) { | ||
| var word_found = false; | ||
| for (var j = 0, jl = columns.length; j < jl; j++) { | ||
| var values = item.values(), column = columns[j]; |
There was a problem hiding this comment.
Lift var values = item.values()out of the two inner loops?
There was a problem hiding this comment.
Yes, definite improvement :) Thanks
|
Merged into #696 |
|
Great update! Looks like it unexpectedly (well, at least to me 🙂) changed the behavior of searches that contain URLs though: previously in version 1.5.0 I could search for "example.com" just fine, and now it seems to split it into two words and it stopped working. Any way to search for an exact match (quoted |
Can you submit a new issue with this bug please? and example code showing it failing. This code should only split words on whitespace (space, tab, newline) characters, not on a period, so 'example.com' should work without needing exact-match quoting. |
Default search function now supports fast multi-word searching as well as exact matching of quoted phrases. See #680
Added documentation to List API and test units to
__test__/search.test.js(also fixes a few typos in
__test__/search.test.js)