Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search doesn't work for items in parantheses: searching for "foo" doesn't find "(Foo)" #121

Closed
archon810 opened this issue Jul 31, 2011 · 15 comments

Comments

@archon810
Copy link

commented Jul 31, 2011

Pretty much see topic - parentheses shouldn't be considered word parts and should be ignored during search, just like most other punctuation.

@brennanmceachran

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2011

I believe this has to do with the search regex. I''ve never gone near coffee script, but all you need to do is add an "i" at the end of the regex (i think)

regex = new RegExp('^' + searchText.replace(/[-[]{}()+?.,^$|#\s]/g, "$&"), 'i')
zregex = new RegExp(searchText.replace(/[-[]{}()
+?.,^$|#\s]/g, "$&"), 'i')

becomes

regex = new RegExp('^' + searchText.replace(/[-[]{}()+?.,^$|#\s]/gi, "$&"), 'i')
zregex = new RegExp(searchText.replace(/[-[]{}()
+?.,^$|#\s]/gi, "$&"), 'i')

Can anyone confirm this or merge it with the master?

@CHgsd

This comment has been minimized.

Copy link

commented Aug 3, 2011

In regular expressions, the "i" flag deals with case (in)sensitivity. In the listed examples where it was recommended to put an "i" is a regular expression used to sanitize the search string so that it can be fed to another regex.

The "problem" which Chosen has, which I also ran into, is that it only considers word matches starting from the beginning of a word.

For example, "and" will match the following: (bold denotes where a match occurs)

  • Anderson
  • Cats and dogs
  • Robots vs. Androids

But will specifically NOT match: (bold denotes where a match might be expected, but does not occur)

  • Land department
  • Illegal landfill
  • Panda bear

The solution is quite easy and is to remove the '^'+ before searchText resulting in a line which looks like this:
regex = new RegExp(searchText.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, "\\$&"), 'i');

In essence making it the same as zregex. The ^ is used to denote the beginning of the string and thus restricts matches to such.
I am not certain of why the developers decided to perform the search this way, but by looking at the code I would say that it is "by design" and not an accident.

Cheers,

CHgsd

@robpodosek

This comment has been minimized.

Copy link

commented Aug 3, 2011

Yep, I dug into the source a few days ago and found the offending line, removed the four characters and it worked perfectly.

@sokai

This comment has been minimized.

Copy link

commented Nov 29, 2011

Thanks CHgsd for your fix! - It works like a charm... :)

Maybe you (or someone else) can give me an answer/hint for the following three things (that is related to the search aspect in this issue):
1st: What for is the mentioned "zregex"?
2nd: With the above fix I can search within whole words. - But how is it possible to highlight all occurrences of the search string? (i.e. if you search for "a" only "America" is highlighted, not "America".)
3rd: How I can define that highlighting should start until I typed in (i.e.) three letters?

Thanks a lot & best regards! :)

@CHgsd

This comment has been minimized.

Copy link

commented Nov 30, 2011

@sokai:
I cannot guarantee these answers as I am merely a user of Chosen, however they are my best guesses/deductions:

A1: zregex appears to only serve for the highlighting portion of the code. This is what leads me to believe that someone wanted to only match on the beginning of words, otherwise the same regex could be used for both matching and highlighting, no need to separate them. If I were to rewrite this portion of the code I would replace zregex with regex, however removing the carrot is a simpler solution in the short term.

A2: I am not in a position where I can test this right now, but if you want to give this a go, it maybe will work:

Replace this:

              if (searchText.length) {
                startpos = option.html.search(zregex);
                text = option.html.substr(0, startpos + searchText.length) + '</em>' + option.html.substr(startpos + searchText.length);
                text = text.substr(0, startpos) + '<em>' + text.substr(startpos);
              } else {
                text = option.html;
              }

With this:

              if (searchText.length) {
                text = option.html.replace( zregex, function(matched) {
                  return "<em>"+matched+"</em>";
                });
              } else {
                text = option.html;
              }

A3: This is again completely guessing, but perhaps this:

Replace this:

              if (searchText.length) {

With this:

              if ((searchText.length) && (searchText.length >= 3)) {

I hope this helps, please post back if it works so that others can know.

@sokai

This comment has been minimized.

Copy link

commented Dec 6, 2011

Hey CHgsd,

thanks a lot for your examples!
Sadly I have to confess that I don't use Chosen any longer... - So I can't (ATM) test your hints... :-/

Thank you all the same and best regards!

@sontek

This comment has been minimized.

Copy link

commented Feb 9, 2012

I have text like "US/Central" and its not finding central, I think this is a similar issue.

@CHgsd

This comment has been minimized.

Copy link

commented Feb 14, 2012

@sontek You could be running into a couple of different issues. The code examples I provided in this issue should resolve all of them. If they do not please let me know and I will have a deeper look.

@wachunga

This comment has been minimized.

Copy link

commented Feb 22, 2012

Had same problem. Fixed by applying #212.

@mcanlas

This comment has been minimized.

Copy link

commented Jun 7, 2013

The options should be documented somewhere.

@Aymkdn

This comment has been minimized.

Copy link

commented Jul 8, 2016

For people that are looking at this thread, there is now the option search_contains:true that's doing the trick, but it's not documented.

@stof

This comment has been minimized.

Copy link
Collaborator

commented Jul 8, 2016

@Aymkdn it is documented, together with all options: https://harvesthq.github.io/chosen/options.html

@Aymkdn

This comment has been minimized.

Copy link

commented Jul 8, 2016

@stof thank you! I never noticed this page.... !

@koenpunt

This comment has been minimized.

Copy link
Member

commented Jul 9, 2016

So this issue can be closed now?

@Aymkdn

This comment has been minimized.

Copy link

commented Jul 9, 2016

@tjschuck tjschuck closed this Jul 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.