Skip to content

Commit

Permalink
Merge pull request #2391 from DotNetSparky/fix-bom-imports
Browse files Browse the repository at this point in the history
Remove BOM in imports.
  • Loading branch information
lukeapage committed Jan 15, 2015
2 parents 577afc3 + d7f79be commit ed1abbb
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 29 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ test/browser/less.js
test/sourcemaps/**/*.map
test/sourcemaps/*.map
test/sourcemaps/*.css
test/less-bom

# grunt
.grunt
Expand Down
2 changes: 2 additions & 0 deletions bin/lessc
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,8 @@ var parseLessFile = function (e, data) {
return;
}

data = data.replace(/^\uFEFF/, '');

options.paths = [path.dirname(input)].concat(options.paths);
options.filename = input;

Expand Down
2 changes: 1 addition & 1 deletion lib/less/functions/data-uri.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ module.exports = function(environment) {

var fileSync = fileManager.loadFileSync(filePath, currentDirectory, this.context, environment);
if (!fileSync.contents) {
logger.warn("Skipped data-uri embedding because file not found");
logger.warn("Skipped data-uri embedding of " + filePath + " because file not found");
return fallback(this, filePathNode || mimetypeNode);
}
var buf = fileSync.contents;
Expand Down
2 changes: 1 addition & 1 deletion lib/less/import-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ module.exports = function(environment) {

var loadFileCallback = function(loadedFile) {
var resolvedFilename = loadedFile.filename,
contents = loadedFile.contents;
contents = loadedFile.contents.replace(/^\uFEFF/, '');

// Pass on an updated rootpath if path of imported file is relative and file
// is in a (sub|sup) directory
Expand Down
66 changes: 66 additions & 0 deletions test/copy-bom.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*jshint latedef: nofunc */

// This is used to copy a folder (the test/less/* files & sub-folders), adding a BOM to the start of each LESS and CSS file.
// This is a based on the copySync method from fs-extra (https://github.com/jprichardson/node-fs-extra/).

module.exports = function() {
var path = require('path'),
fs = require('fs');

var BUF_LENGTH = 64 * 1024;
var _buff = new Buffer(BUF_LENGTH);

function copyFolderWithBom(src, dest) {
var stats = fs.lstatSync(src);
var destFolder = path.dirname(dest);
var destFolderExists = fs.existsSync(destFolder);
var performCopy = false;

if (stats.isFile()) {
if (!destFolderExists)
fs.mkdirSync(destFolder);
if (src.match(/\.(css|less)$/))
copyFileAddingBomSync(src, dest);
else
copyFileSync(src, dest)
}
else if (stats.isDirectory()) {
if (!fs.existsSync(destFolder))
fs.mkdirSync(destFolder);
if (!fs.existsSync(dest))
fs.mkdirSync(dest);
fs.readdirSync(src).forEach(function(d) {
if (d !== 'bom')
copyFolderWithBom(path.join(src, d), path.join(dest, d));
});
}
}

function copyFileAddingBomSync(srcFile, destFile) {
var contents = fs.readFileSync(srcFile, { encoding: 'utf8' });
if (!contents.length || contents.charCodeAt(0) !== 0xFEFF)
contents = '\ufeff' + contents;
fs.writeFileSync(destFile, contents, { encoding: 'utf8' });
}

function copyFileSync(srcFile, destFile) {
var fdr = fs.openSync(srcFile, 'r')
var stat = fs.fstatSync(fdr)
var fdw = fs.openSync(destFile, 'w', stat.mode)
var bytesRead = 1
var pos = 0

while (bytesRead > 0) {
bytesRead = fs.readSync(fdr, _buff, 0, BUF_LENGTH, pos)
fs.writeSync(fdw, _buff, 0, bytesRead)
pos += bytesRead
}

fs.closeSync(fdr)
fs.closeSync(fdw)
}

return {
copyFolderWithBom: copyFolderWithBom
};
};
20 changes: 10 additions & 10 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ var lessTest = require("./less-test"),
stylize = require('../lib/less-node/lessc-helper').stylize;

function getErrorPathReplacementFunction(dir) {
return function(input) {
return input.replace(
/\{path\}/g, path.join(process.cwd(), "/test/less/" + dir + "/"))
.replace(/\{node\}/g, "")
.replace(/\{\/node\}/g, "")
.replace(/\{pathrel\}/g, path.join("test", "less", dir + "/"))
return function(input, baseDir) {
return input.replace(/\{path\}/g, path.join(process.cwd(), baseDir, dir + "/"))
.replace(/\{node\}/g, "")
.replace(/\{\/node\}/g, "")
.replace(/\{pathrel\}/g, path.join(baseDir, dir + "/"))
.replace(/\{pathhref\}/g, "")
.replace(/\{404status\}/g, "")
.replace(/\r\n/g, '\n');
};
}

console.log("\n" + stylize("Less", 'underline') + "\n");
lessTester.prepBomTest();
lessTester.runTestSet({strictMath: true, relativeUrls: true, silent: true});
lessTester.runTestSet({strictMath: true, strictUnits: true}, "errors/",
lessTester.testErrors, null, getErrorPathReplacementFunction("errors"));
Expand All @@ -34,16 +34,16 @@ lessTester.runTestSet({strictMath: true, strictUnits: true}, "strict-units/");
lessTester.runTestSet({}, "legacy/");
lessTester.runTestSet({strictMath: true, strictUnits: true, sourceMap: true, globalVars: true }, "sourcemaps/",
lessTester.testSourcemap, null, null,
function(filename, type) {
function(filename, type, baseFolder) {
if (type === "vars") {
return path.join('test/less/', filename) + '.json';
return path.join(baseFolder, filename) + '.json';
}
return path.join('test/sourcemaps', filename) + '.json';
});
lessTester.runTestSet({globalVars: true, banner: "/**\n * Test\n */\n"}, "globalVars/",
null, null, null, function(name) { return path.join('test/less/', name) + '.json'; });
null, null, null, function(name, type, baseFolder) { return path.join(baseFolder, name) + '.json'; });
lessTester.runTestSet({modifyVars: true}, "modifyVars/",
null, null, null, function(name) { return path.join('test/less/', name) + '.json'; });
null, null, null, function(name, type, baseFolder) { return path.join(baseFolder, name) + '.json'; });
lessTester.runTestSet({urlArgs: '424242'}, "url-args/");
lessTester.runTestSet({paths: ['test/data/','test/less/import/']}, "include-path/");
lessTester.testSyncronous({syncImport: true}, "import");
Expand Down
54 changes: 37 additions & 17 deletions test/less-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module.exports = function() {
var path = require('path'),
fs = require('fs');
copyBom = require('./copy-bom')();

var less = require('../lib/less-node');
var stylize = require('../lib/less-node/lessc-helper').stylize;
Expand All @@ -14,6 +15,9 @@ module.exports = function() {

var isVerbose = process.env.npm_config_loglevel === 'verbose';

var normalFolder = 'test/less';
var bomFolder = 'test/less-bom';

less.logger.addListener({
info: function(msg) {
if (isVerbose) {
Expand Down Expand Up @@ -67,9 +71,9 @@ module.exports = function() {
}
});

function testSourcemap(name, err, compiledLess, doReplacements, sourcemap) {
function testSourcemap(name, err, compiledLess, doReplacements, sourcemap, baseFolder) {
fs.readFile(path.join('test/', name) + '.json', 'utf8', function (e, expectedSourcemap) {
process.stdout.write("- " + name + ": ");
process.stdout.write("- " + path.join(baseFolder, name) + ": ");
if (sourcemap === expectedSourcemap) {
ok('OK');
} else if (err) {
Expand All @@ -84,10 +88,10 @@ module.exports = function() {
});
}

function testErrors(name, err, compiledLess, doReplacements) {
fs.readFile(path.join('test/less/', name) + '.txt', 'utf8', function (e, expectedErr) {
process.stdout.write("- " + name + ": ");
expectedErr = doReplacements(expectedErr, 'test/less/errors/');
function testErrors(name, err, compiledLess, doReplacements, sourcemap, baseFolder) {
fs.readFile(path.join(baseFolder, name) + '.txt', 'utf8', function (e, expectedErr) {
process.stdout.write("- " + path.join(baseFolder, name) + ": ");
expectedErr = doReplacements(expectedErr, baseFolder);
if (!err) {
if (compiledLess) {
fail("No Error", 'red');
Expand Down Expand Up @@ -131,7 +135,7 @@ module.exports = function() {
totalTests++;
queue(function() {
var isSync = true;
toCSS(options, path.join('test/less/', filenameNoExtension + ".less"), function (err, result) {
toCSS(options, path.join(normalFolder, filenameNoExtension + ".less"), function (err, result) {
process.stdout.write("- Test Sync " + filenameNoExtension + ": ");

if (isSync) {
Expand All @@ -145,7 +149,21 @@ module.exports = function() {
});
}

function prepBomTest() {
copyBom.copyFolderWithBom(normalFolder, bomFolder);
}

function runTestSet(options, foldername, verifyFunction, nameModifier, doReplacements, getFilename) {
var options2 = options ? JSON.parse(JSON.stringify(options)) : {};
runTestSetInternal(normalFolder, options, foldername, verifyFunction, nameModifier, doReplacements, getFilename);
runTestSetInternal(bomFolder, options2, foldername, verifyFunction, nameModifier, doReplacements, getFilename);
}

function runTestSetNormalOnly(options, foldername, verifyFunction, nameModifier, doReplacements, getFilename) {
runTestSetInternal(normalFolder, options, foldername, verifyFunction, nameModifier, doReplacements, getFilename);
}

function runTestSetInternal(baseFolder, options, foldername, verifyFunction, nameModifier, doReplacements, getFilename) {
foldername = foldername || "";

if(!doReplacements) {
Expand All @@ -156,7 +174,7 @@ module.exports = function() {
return foldername + path.basename(file, '.less');
}

fs.readdirSync(path.join('test/less/', foldername)).forEach(function (file) {
fs.readdirSync(path.join(baseFolder, foldername)).forEach(function (file) {
if (! /\.less/.test(file)) { return; }

var name = getBasename(file);
Expand All @@ -169,19 +187,19 @@ module.exports = function() {

if (options.sourceMap) {
options.sourceMapOutputFilename = name + ".css";
options.sourceMapBasepath = path.join(process.cwd(), "test/less");
options.sourceMapBasepath = path.join(process.cwd(), baseFolder);
options.sourceMapRootpath = "testweb/";
// TODO separate options?
options.sourceMap = options;
}

options.getVars = function(file) {
return JSON.parse(fs.readFileSync(getFilename(getBasename(file), 'vars'), 'utf8'));
return JSON.parse(fs.readFileSync(getFilename(getBasename(file), 'vars', baseFolder), 'utf8'));
};

var doubleCallCheck = false;
var doubleCallCheck = false;
queue(function() {
toCSS(options, path.join('test/less/', foldername + file), function (err, result) {
toCSS(options, path.join(baseFolder, foldername + file), function (err, result) {
if (doubleCallCheck) {
totalTests++;
fail("less is calling back twice");
Expand All @@ -192,7 +210,7 @@ module.exports = function() {
doubleCallCheck = (new Error()).stack;

if (verifyFunction) {
var verificationResult = verifyFunction(name, err, result && result.css, doReplacements, result && result.map);
var verificationResult = verifyFunction(name, err, result && result.css, doReplacements, result && result.map, baseFolder);
release();
return verificationResult;
}
Expand All @@ -208,9 +226,9 @@ module.exports = function() {
var css_name = name;
if(nameModifier) { css_name = nameModifier(name); }
fs.readFile(path.join('test/css', css_name) + '.css', 'utf8', function (e, css) {
process.stdout.write("- " + css_name + ": ");
process.stdout.write("- " + path.join(baseFolder, css_name) + ": ");

css = css && doReplacements(css, 'test/less/' + foldername);
css = css && doReplacements(css, path.join(baseFolder, foldername));
if (result.css === css) { ok('OK'); }
else {
difference("FAIL", css, result.css);
Expand All @@ -225,10 +243,10 @@ module.exports = function() {
function diff(left, right) {
require('diff').diffLines(left, right).forEach(function(item) {
if(item.added || item.removed) {
var text = item.value.replace("\n", String.fromCharCode(182) + "\n");
var text = item.value.replace("\n", String.fromCharCode(182) + "\n").replace('\ufeff', '[[BOM]]');
process.stdout.write(stylize(text, item.added ? 'green' : 'red'));
} else {
process.stdout.write(item.value);
process.stdout.write(item.value.replace('\ufeff', '[[BOM]]'));
}
});
process.stdout.write("\n");
Expand Down Expand Up @@ -327,10 +345,12 @@ module.exports = function() {

return {
runTestSet: runTestSet,
runTestSetNormalOnly: runTestSetNormalOnly,
testSyncronous: testSyncronous,
testErrors: testErrors,
testSourcemap: testSourcemap,
testNoOptions: testNoOptions,
prepBomTest: prepBomTest,
finished: finished
};
};

0 comments on commit ed1abbb

Please sign in to comment.