Fixed off by one error. #3

Merged
merged 1 commit into from Jan 3, 2014

Conversation

Projects
None yet
3 participants
Owner

amasad commented Dec 31, 2013

Not sure how this worked before but there is an off by one error that is clearly demonstrated in this test:

It used to be:

history.prev().should.equal('baz');
history.prev().should.equal('bar');
history.prev().should.equal('foo');
history.next().should.equal('foo');
history.next().should.equal('bar');
history.next().should.equal('baz');

See the repeat 'foo' in the middle. While using it in a REPL (or chat) when the user cycles between next and prev in the middle it gets stuck on one item: up -> foo, down -> foo, up -> foo, down -> foo.

The following should be the correct behavior:

history.add('foo');
history.add('bar');
history.add('baz');
history.prev().should.equal('baz');
history.prev().should.equal('bar');
history.prev().should.equal('foo');
history.next().should.equal('bar');
history.next().should.equal('baz');

cscott commented Jan 1, 2014

Is this the same as issue #1?

Owner

amasad commented Jan 1, 2014

Looks like it

On Wednesday, January 1, 2014, C. Scott Ananian wrote:

Is this the same as issue #1 #1
?


Reply to this email directly or view it on GitHubhttps://github.com/component/history/pull/3#issuecomment-31422334
.

Owner

amasad commented Jan 3, 2014

Yeah #1 has been around for almost a year, pinging @visionmedia @jonathanong

Contributor

jonathanong commented Jan 3, 2014

i don't use this module. i'll add it to the collaborators team

Contributor

jonathanong commented Jan 3, 2014

oh wait. it already is.

Owner

amasad commented Jan 3, 2014

yeah I saw you made the latest commit that's why I pinged you

Contributor

jonathanong commented Jan 3, 2014

that was more "admin" stuff though :P i'll just add you to the dev team.

Owner

amasad commented Jan 3, 2014

sweet, thanks!

amasad added a commit that referenced this pull request Jan 3, 2014

Merge pull request #3 from amasad/master
Fixed off by one error.

@amasad amasad merged commit 575e6d5 into component:master Jan 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment