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

pipeline insert by "after", "before", pipeline remove issues #146

Closed weixsong closed 9 years ago

weixsong commented 9 years ago

There are some issues with pipeline module.

  1. lunr.Pipeline.prototype.after = function (existingFn, newFn) if existingFn is not found in this._stack, then variable pos will be -1, and pos + 1 will be 0, then this._stack.splice(pos, 0, newFn) will add the newFn into the front of pipeline queue, This will make confusing effective, for example, when user add some functions to pipeline, but then call this after function, but give an existingFn that do not exist in _stack, the newFn will be put in front of the queue, and newFn will be execute first, this will bring some unexpected results.
  2. lunr.Pipeline.prototype.before = function (existingFn, newFn) if existingFn is not in this._stack, then pos will be -1, this._stack.splice(pos, 0, newFn) then will insert the newFn by that 1 elements from the end. This will confuse users. for example:

a = [1, 2, 3, 4, 5] a.splice(-1, 0, 100)

a [1, 2, 3, 4, 100, 5]

  1. lunr.Pipeline.prototype.remove = function (fn) if fn is not in this._stack, then pos will -1, this._stack.splice(pos, 1) will remove the last element, this doesn't make sense.
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: https://github.com/olivernn/lunr.js/pull/149