search

olivernn / lunr.js

A bit like Solr, but much smaller and not as bright
http://lunrjs.com
MIT License
8.96k stars 548 forks source link

fix pipeline related issue #147

Closed weixsong closed 9 years ago

weixsong commented 9 years ago

Issue link: https://github.com/olivernn/lunr.js/issues/146

olivernn commented 9 years ago

Thanks for taking a look at this, as I've mentioned in a code comment I think that if you ask to put something before or after a specific item in the pipeline, and that item does not exist, then this should fail with some kind of error.

I think in the case of asking to remove something that doesn't exist, there should be no warning and no error, the user asked for a thing to be removed, and once the method is done that thing is not in the pipeline, so there is no harm done.

Also, please provide some tests for these changes.

weixsong commented 9 years ago

@olivernn , yes, I think you are right. Thanks for the comments.

weixsong commented 9 years ago

@olivernn , I've made a change and added some test cases, pls help to review the code.

olivernn commented 9 years ago

Looks good, perhaps its worth expanding on the tests to make sure that, as well as throwing an error, that the functions are not added to the pipeline. Once that is done I think its ready to go, just squash all your commits down into one commit. Thanks again for your work!

weixsong commented 9 years ago

@olivernn , sorry for the inconvenience, I'm not familiar with git rebase command, I've some trouble with rebase, so I refork your repo and commit the change patch. :)

new commit: #149