Skip to content

Commit

Permalink
Fix file sorting for more than 11 elements
Browse files Browse the repository at this point in the history
V8's Array.prototype.sort uses combination of quick sort and insertion sort.

The compare function should return integer (-1, 0, 1), but we were returning a boolean, which is incorrect. Boolean true equals to 1, but boolean false equals to 0, not -1. The way the condition was written, it worked for small arrays (where only insertion sort, which is stable, was used), but not for larger arrays, when unstable quick sort screwed it up.
  • Loading branch information
vojtajina committed Jan 13, 2013
1 parent b72b965 commit d759e40
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/file-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ var GLOB_OPTS = {


var byPath = function(a, b) {
return a.path > b.path;
if (a.path > b.path) return 1;
if (a.path < b.path) return -1;
return 0;
};


Expand Down
11 changes: 11 additions & 0 deletions test/unit/file-list.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ describe 'file-list', ->
'*.txt': ['/c.txt', '/a.txt', '/b.txt']
'*.js': ['/folder', '/folder/x.js']
'/a.*': ['/a.txt']
# we need at least 11 elements to trigger V8's quick sort
'**': ['/a.txt', '/b.txt', '/c.txt', '/a.txt', '/c.txt', '/b.txt', '/a.txt', '/c.txt',
'/a.txt', '/a.txt', '/c.txt']

mockFs = mocks.fs.create
some:
Expand Down Expand Up @@ -177,6 +180,14 @@ describe 'file-list', ->
expect(pathsFrom files.served).to.deep.equal ['/a.txt', '/some/a.js', '/b.txt', '/c.txt']
done()

it 'should sort files within buckets (if more than 11 elements)', (done) ->
# regression for sorting many items
list = new m.List patterns('**'), [], null, preprocessMock

list.refresh().then (files) ->
expect(pathsFrom files.served).to.deep.equal ['/a.txt', '/b.txt', '/c.txt']
done()

it 'should return only served files', (done) ->
# /a.* => /a.txt [served TRUE]
# /some/*.js => /some/a.js, /some/b.js [served FALSE]
Expand Down

0 comments on commit d759e40

Please sign in to comment.