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.

Sign up for GitHub

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

Single string test #1486

Merged
merged 1 commit into from
Jul 26, 2014
Merged

Single string test #1486

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions coffee/lib/abstract-chosen.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ class AbstractChosen

option.search_match = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change this to:

option.search_match = null

And then change our search match tests to:

if option.search_match?

results_group = null
search_match = null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make the above change, we can eliminate this variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd think of that, but that would mean that the value of option.search_match can be of 3 different types, null when exec fails, an array with index property if exec succeeds and true if option.group_array_index? .... And that I want to avoid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfiller Any further words/thoughts on this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfiller ping?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfiller ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - wow, almost a year old.

Yeah, I'm fine with the way you've done it. I think it's a little confusing that we use the same variable names for two different things, but it's not the end of the world.


if this.include_option_in_results(option)

Expand All @@ -152,14 +153,14 @@ class AbstractChosen
unless option.group and not @group_search

option.search_text = if option.group then option.label else option.text
option.search_match = regex.test(option.search_text)
search_match = this.search_string_match(option.search_text, regex)
option.search_match = search_match?

results += 1 if option.search_match and not option.group

if option.search_match
if searchText.length
match = regex.exec(option.search_text)
startpos = match.index
startpos = search_match.index
text = option.search_text.substr(0, startpos + searchText.length) + '</em>' + option.search_text.substr(startpos + searchText.length)
option.search_text = text.substr(0, startpos) + '<em>' + text.substr(startpos)

Expand All @@ -182,6 +183,9 @@ class AbstractChosen
regex_string = "^#{regex_string}" unless @enable_split_word_search or @search_contains
new RegExp(regex_string, 'i')

search_string_match: (search_string, regex) ->
regex.exec(search_string)

choices_count: ->
return @selected_option_count if @selected_option_count?

Expand Down