Use word boundary matching

[pfiller]

@harvesthq/chosen-developers

This is a new PR for @koenpunt's excellent #1465. In his words:

By making use of word boundary matching (http://www.regular-expressions.info/wordboundaries.html) the split word search is much cleaner (no more actual splitting of strings), and the search results are better.

I want to be very careful about merging this in, but I'm loving what I see so far. With default options set:

Search String	Expected to Match	Old way	New way
Democratic Republic of	Congo, The Democratic Republic of The	💩	👍
of America	United States (of America)	💩	👍
Holland	Netherlands/Holland	💩	👍

Here's what I think we should do before this gets a final

• Add unit tests for verifying that search results are correct. To do this, we might need to break the actual search match function out of the winnow method.
• Performance Test with 1000s of items (perhaps replacing test + match with a single match call)
• Determine if we need to make an API change related to enable_split_word_search or search_contains. I don't think you should be able to turn off enable_split_word_search if search_contains is true, but I want to do some experimenting there.

Fixes #75 and #1463

[pfiller]

If you want to make a change to this branch, please make a branch and target this with a PR just as you would a change to master.

[koenpunt]

Turning off enable_split_word_search when search_contains is true doesn't change the behavior of search_contains.

[pfiller]

Turning off enable_split_word_search when search_contains is true doesn't change the behavior of search_contains.

Right. And I think maybe this should be totally clear. Maybe a change isn't need.

[stof]

Another question is whether we should merge this in or switch to the fuzzy matching of #1037 instead

[koenpunt]

Depends on performance I guess, and with the PR's (#1486, #1487) open for this branch merged the code would definitely looks nicer than that of the fuzzy search PR.

[koenpunt]

This would also fix #424, as word boundary matching doesn't count a . as part of a word.

[koenpunt]

@pfiller I've just rebased this branch on current master and I like to finish this PR, so we probably have to make some decisions :)

[pavdro]

Hey!
lovin the plugin.
Any plans on updating it with this change?

Cheers

[stof]

@pfiller @koenpunt have you made a choice about the way to go ?

[jrochkind]

Am very interested in this feature! Any chance?

[travislward]

Word boundary doesn't work as one might expect in the following example...

This is an example - with hyphen This is an example -with hyphen This is an example-with hyphen This is an example/with forward slash This is an example/ with forward slash This is an example / with forward slash

different search results...

Search Text Results
example All options above
example - No results
example - w This is an example - with hyphen
example- This is an example-with hyphen
example - This is an example -with hyphen

[dereksmart]

@pfiller @koenpunt Has the spacing issue been resolved?

[joelbreton]

This fix would help me out lots. Any ETA for this fix?

[travislward]

Joel,

I'm using select2 instead of chosen now.

I added this to the select2 matcher...

matcher: function(term, text) {
var escaped = term.replace(/[-[]{}()*+?.,^$|#\s]/g, "$&");

    var regex = new RegExp('(^|\\s|/|-|\\$|\\()' + escaped + '.*',

'i');
return regex.test(text);
},

Travis

joelbreton notifications@github.com 8/28/2014 8:18 AM >>>

This fix would help me out lots. Any ETA for this fix?
—
Reply to this email directly or view it on GitHub (
#1483 (comment) ).

[joelbreton]

@pfiller @koenpunt Any updates on this one?

[dillonwelch]

This would be helpful for me as well.

[chrisvfritz]

I think after 2 years it's pretty clear that this relatively simple fix is not going to happen. I recommend using selectize instead. You not only won't deal with this particular issue, but in my experience, it's been better in every way.

[alxndrmlr]

Agreed. I went with select2

[koenpunt]

@pfiller should we add the target label to this one for our next hangout?

[tjschuck]

@koenpunt Is there anything actually blocking this (aside from needing to be rebased)? Are @pfiller's three todos on the main body of this "real" todos? Which ones are blockers vs. nice-to-haves?

[koenpunt]

Rebase is done, and no real blockers I think;

• testing could have some impact.. but since we're going with almost no tests currently, it's hard to say what we have to test in the first place.
• Second TODO about performance; I don't expect any issues there, especially since we're going from 2 regular expressions to just 1.
• Also I don't think the public API changes (and I believe we came to an understanding on that about 4 years ago already)

[koenpunt]

Also would like to quote @pfiller

Wow, this is great. I've been testing it and testing it and I think it works great. That being said, it's a pretty significant change to the way searches are performed in Chosen so I want to tread lightly.
@pfiller commented on 16 Aug 2013

Is 4 years light enough? 🤓

[tjschuck]

@koenpunt I agree.

• Lack of tests has never stopped us before 😏
• We already have purported very-long-list performance issues, and I think this will make the happy path much happier.
• I also don't see any public API change.

If you're giving a 👍 to the code, I say ship it!

(In a rare fit of proximity, I can physically see @pfiller right now. I'll try to force him to... do anything? Respond? ¯\_(ツ)_/¯ Anyway, don't wait for him if you feel good about it.)

[koenpunt]

It goes a bit against our (or mine) prior notice;

From this version on, until further notice, no new features will be added, so pull requests for features will not be accepted. However, pull requests for bugfixes are still very much appreciated.

But technically it's not a new feature anymore.

[koenpunt]

Just had a final thought that maybe it would be a downside that searching for ( or ) wouldn't work anymore, but apparently that didn't work anyway 😂

[tjschuck]

It's also not really a new feature — it's just fixing an existing feature to work way better. I might even call it a bugfix, based on all the linked issues!

@pfiller said @satchmorun might have some insight here, since he was allegedly poking around similar areas of Chosen recently. Regardless, I think we should merge this by Monday at the latest. If you want to wait and see if @pfiller or @satchmorun has anything to say, go for it, but if you don't, also go for it.

[koenpunt]

Done 🚀

[tjschuck]

@koenpunt ❤️ ❤️ 😻 ❤️