search

brianreavis / sifter.js

A library for textually searching arrays and hashes of objects by property (or multiple properties). Designed specifically for autocomplete.
1.09k stars 125 forks source link

Nested properties #27

Closed noirbizarre closed 8 years ago

noirbizarre commented 9 years ago

This pull request add support for searching and sorting on nested properties using dot-notation when declaring fields (ie. nested.property).

I tried updating the documentation but I'm not sure on where to put this.

This pull request fix #26.

noirbizarre commented 9 years ago

Hi !

Is there any chance this can be reviewed and if it fit, merged ?

If not, what can I do to make it mergeable ?

noirbizarre commented 8 years ago

Hi ! I just saw that you merged some pull-request but not this one and not any comment.

Is there any reason not to merge this one ? If there is, what can I do/change to make it mergeable ?

brianreavis commented 8 years ago

This may make no visible change to the API, but it does come with a performance hit. If this is to be merged, it needs to either (1) be a completely different code path that's enabled via an option like nesting:true or (2) compile the accessor.

noirbizarre commented 8 years ago

Thanks for the feedback. I will adjust the PR.

I fear that the completely different path introduce too much complexity and maintainability drop.

I'm not sure about the second option meaning (compile the accessor), what do you mean by compile ? Make the getattr a native function ?

I just noted the benchmark task. I will try to find a solution with less performance hit. What value do you find acceptable ?

noirbizarre commented 8 years ago

OK, I just:

holic commented 8 years ago

Looks great. Thanks for the PR!