numbers / numbers.js

Advanced Mathematics Library for Node.js and JavaScript
Apache License 2.0
1.77k stars 167 forks source link

#112 Corrected the assert argument order. #114

Closed LarryBattle closed 9 years ago

LarryBattle commented 9 years ago

Fixed assert.equal() and assert.deepEqual() argument order. Reference: https://github.com/sjkaliski/numbers.js/issues/112

Dakkers commented 9 years ago

@LarryBattle looks good, except that each element of an array is on a separate line. is there a quick way to fix that? if not, no worries.

LarryBattle commented 9 years ago

Currently, it's an open issue on the jsfmt page, and I don't have the time to fix it right now. https://github.com/rdio/jsfmt/issues/116

I'm thinking about submitting a pull request to switch the build tool from Make to gulp. That way, all the javascript can be auto formatted with each commit/build.

sjkaliski commented 9 years ago

@LarryBattle +1 on switching to gulp.

Dakkers commented 9 years ago

@LarryBattle jsfmt has had this whitespace problem for a while. it's also been referenced here: https://github.com/rdio/jsfmt/issues/115 .

could you remove the files affected by this whitespace BS from the commit? affected files are: test/basic.test.js, test/generators.test.js, test/prime.test.js . lemme know if that's doable and then we can merge it if so.

LarryBattle commented 9 years ago

I fixed it with this simple script.

Fix.js

var fs = require("fs");
var correctArrayIndention = function(source) {
    var arraysOnNewLinesRegEX = /\[\n([^\n;]+\n)+\s*\]/g;
    var spacesOrNewLineRegEx = /(\n|\s{2,})(\S)/g;
    return source.toString().replace(arraysOnNewLinesRegEX, function(str) {
        return str.replace(spacesOrNewLineRegEx, " $2");
    });
};
var testFiles = fs.readdirSync("./test/").filter(function(filename) {
    return /\.js$/.test(filename) && !/matrix|stat/.test(filename);
}).map(function(filename) {
    return "./test/" + filename;
});

testFiles.forEach(function(filepath) {
    var file = fs.readFileSync(filepath);
    var newContent = correctArrayIndention(file.toString());
    fs.writeFileSync(filepath, newContent);
});
Dakkers commented 9 years ago

@LarryBattle fantastic. unfortunately now, since you fixed the random tests, random.test.js is going to have a conflict. otherwise, this is good to go.

LarryBattle commented 9 years ago

Should be good to go.

Dakkers commented 9 years ago

looks good.