From b23ffe1e4bd76dee852d14f58ff0b2bf1a845e89 Mon Sep 17 00:00:00 2001 From: Kirill Rogovoy Date: Wed, 12 Oct 2016 11:15:31 +0300 Subject: [PATCH 1/2] Move error formatting logic to LessError --- bin/lessc | 8 +++- lib/less-node/index.js | 56 ---------------------------- lib/less/less-error.js | 83 ++++++++++++++++++++++++++++++++++++++++-- test/less-test.js | 12 +++--- 4 files changed, 92 insertions(+), 67 deletions(-) diff --git a/bin/lessc b/bin/lessc index e109ce08..60b8ebc6 100755 --- a/bin/lessc +++ b/bin/lessc @@ -37,7 +37,7 @@ var less = require('../lib/less-node'), modifyVars: null, urlArgs: '', plugins: plugins - }; + }; if (less.options) { for (var i = 0, keys = Object.keys(options); i < keys.length; i++) { @@ -310,7 +310,11 @@ function render() { } }, function(err) { - less.writeError(err, options); + if (!options.silent) { + console.error(err.toString({ + stylize: options.color && less.lesscHelper.stylize + })); + } process.exitCode = 1; }); }; diff --git a/lib/less-node/index.js b/lib/less-node/index.js index ab278336..4e379845 100644 --- a/lib/less-node/index.js +++ b/lib/less-node/index.js @@ -14,62 +14,6 @@ less.FileManager = FileManager; less.UrlFileManager = UrlFileManager; less.options = less.options || {}; -less.formatError = function(ctx, options) { - options = options || {}; - - var message = ""; - var extract = ctx.extract; - var error = []; - var stylize = options.color ? lesscHelper.stylize : function (str) { return str; }; - - // only output a stack if it isn't a less error - if (ctx.stack && !ctx.type) { return stylize(ctx.stack, 'red'); } - - if (!ctx.hasOwnProperty('index') || !extract) { - return ctx.stack || ctx.message; - } - - if (typeof extract[0] === 'string') { - error.push(stylize((ctx.line - 1) + ' ' + extract[0], 'grey')); - } - - if (typeof extract[1] === 'string') { - var errorTxt = ctx.line + ' '; - if (extract[1]) { - errorTxt += extract[1].slice(0, ctx.column) + - stylize(stylize(stylize(extract[1].substr(ctx.column, 1), 'bold') + - extract[1].slice(ctx.column + 1), 'red'), 'inverse'); - } - error.push(errorTxt); - } - - if (typeof extract[2] === 'string') { - error.push(stylize((ctx.line + 1) + ' ' + extract[2], 'grey')); - } - error = error.join('\n') + stylize('', 'reset') + '\n'; - - message += stylize(ctx.type + 'Error: ' + ctx.message, 'red'); - if (ctx.filename) { - message += stylize(' in ', 'red') + ctx.filename + - stylize(' on line ' + ctx.line + ', column ' + (ctx.column + 1) + ':', 'grey'); - } - - message += '\n' + error; - - if (ctx.callLine) { - message += stylize('from ', 'red') + (ctx.filename || '') + '/n'; - message += stylize(ctx.callLine, 'grey') + ' ' + ctx.callExtract + '/n'; - } - - return message; -}; - -less.writeError = function (ctx, options) { - options = options || {}; - if (options.silent) { return; } - console.error(less.formatError(ctx, options)); -}; - // provide image-size functionality require('./image-size')(less.environment); diff --git a/lib/less/less-error.js b/lib/less/less-error.js index acaac22b..d4a2cca6 100644 --- a/lib/less/less-error.js +++ b/lib/less/less-error.js @@ -1,7 +1,27 @@ -var utils = require("./utils"); - +var utils = require('./utils'); +/** + * This is a centralized class of any error that could be thrown internally (mostly by the parser). + * Besides standard .message it keeps some additional data like a path to the file where the error + * occurred along with line and column numbers. + * + * @class + * @extends Error + * @type {module.LessError} + * + * @prop {string} type + * @prop {string} filename + * @prop {number} index + * @prop {number} line + * @prop {number} column + * @prop {number} callLine + * @prop {number} callExtract + * @prop {string[]} extract + * + * @param {Object} e - An error object to wrap around or just a descriptive object + * @param {Object} importManager - An instance of ImportManager (see import-manager.js) + * @param {string} [currentFilename] + */ var LessError = module.exports = function LessError(e, importManager, currentFilename) { - Error.call(this); var filename = e.filename || currentFilename; @@ -40,3 +60,60 @@ if (typeof Object.create === 'undefined') { } LessError.prototype.constructor = LessError; + +/** + * An overridden version of the default Object.prototype.toString + * which uses additional information to create a helpful message. + * + * @param {Object} options + * @returns {string} + */ +LessError.prototype.toString = function(options) { + options = options || {}; + + var message = ''; + var extract = this.extract; + var error = []; + var stylize = function (str) { return str; }; + if (options.stylize) { + var type = typeof options.stylize; + if (type !== 'function') { + throw Error('options.stylize should be a function, got a ' + type + '!'); + } + stylize = options.stylize; + } + + if (typeof extract[0] === 'string') { + error.push(stylize((this.line - 1) + ' ' + extract[0], 'grey')); + } + + if (typeof extract[1] === 'string') { + var errorTxt = this.line + ' '; + if (extract[1]) { + errorTxt += extract[1].slice(0, this.column) + + stylize(stylize(stylize(extract[1].substr(this.column, 1), 'bold') + + extract[1].slice(this.column + 1), 'red'), 'inverse'); + } + error.push(errorTxt); + } + + if (typeof extract[2] === 'string') { + error.push(stylize((this.line + 1) + ' ' + extract[2], 'grey')); + } + error = error.join('\n') + stylize('', 'reset') + '\n'; + + message += stylize(this.type + 'Error: ' + this.message, 'red'); + if (this.filename) { + message += stylize(' in ', 'red') + this.filename + + stylize(' on line ' + this.line + ', column ' + (this.column + 1) + ':', 'grey'); + } + + message += '\n' + error; + + if (this.callLine) { + message += stylize('from ', 'red') + (this.filename || '') + '/n'; + message += stylize(this.callLine, 'grey') + ' ' + this.callExtract + '/n'; + } + + return message; +}; diff --git a/test/less-test.js b/test/less-test.js index 7a2a3e50..5b2be471 100644 --- a/test/less-test.js +++ b/test/less-test.js @@ -136,20 +136,20 @@ module.exports = function() { 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) + ": "); + process.stdout.write('- ' + path.join(baseFolder, name) + ": "); expectedErr = doReplacements(expectedErr, baseFolder); if (!err) { if (compiledLess) { - fail("No Error", 'red'); + fail('No Error', 'red'); } else { - fail("No Error, No Output"); + fail('No Error, No Output'); } } else { - var errMessage = less.formatError(err); + var errMessage = err.toString(); if (errMessage === expectedErr) { ok('OK'); } else { - difference("FAIL", expectedErr, errMessage); + difference('FAIL', expectedErr, errMessage); } } }); @@ -254,7 +254,7 @@ module.exports = function() { var doubleCallCheck = false; queue(function() { toCSS(options, path.join(baseFolder, foldername + file), function (err, result) { - + if (doubleCallCheck) { totalTests++; fail("less is calling back twice"); From 068d6546037ba371f5b493c6ea0dba9f578325a0 Mon Sep 17 00:00:00 2001 From: Kirill Rogovoy Date: Wed, 12 Oct 2016 11:15:56 +0300 Subject: [PATCH 2/2] Refactor test/index.js removing code duplicates --- test/index.js | 79 ++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/test/index.js b/test/index.js index c239e2fe..e2027f96 100644 --- a/test/index.js +++ b/test/index.js @@ -18,42 +18,49 @@ function getErrorPathReplacementFunction(dir) { console.log("\n" + stylize("Less", 'underline') + "\n"); lessTester.prepBomTest(); -lessTester.runTestSet({strictMath: true, relativeUrls: true, silent: true, javascriptEnabled: true}); -lessTester.runTestSet({strictMath: true, strictUnits: true, javascriptEnabled: true}, "errors/", - lessTester.testErrors, null, getErrorPathReplacementFunction("errors")); -lessTester.runTestSet({strictMath: true, strictUnits: true, javascriptEnabled: false}, "no-js-errors/", - lessTester.testErrors, null, getErrorPathReplacementFunction("no-js-errors")); -lessTester.runTestSet({strictMath: true, dumpLineNumbers: 'comments'}, "debug/", null, - function(name) { return name + '-comments'; }); -lessTester.runTestSet({strictMath: true, dumpLineNumbers: 'mediaquery'}, "debug/", null, - function(name) { return name + '-mediaquery'; }); -lessTester.runTestSet({strictMath: true, dumpLineNumbers: 'all'}, "debug/", null, - function(name) { return name + '-all'; }); -lessTester.runTestSet({strictMath: true, relativeUrls: false, rootpath: "folder (1)/"}, "static-urls/"); -lessTester.runTestSet({strictMath: true, compress: true}, "compression/"); -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, baseFolder) { - if (type === "vars") { - return path.join(baseFolder, filename) + '.json'; - } - return path.join('test/sourcemaps', filename) + '.json'; - }); -lessTester.runTestSet({strictMath: true, strictUnits: true, sourceMap: {sourceMapFileInline: true}}, "sourcemaps-empty/", lessTester.testEmptySourcemap); -lessTester.runTestSet({globalVars: true, banner: "/**\n * Test\n */\n"}, "globalVars/", - null, null, null, function(name, type, baseFolder) { return path.join(baseFolder, name) + '.json'; }); -lessTester.runTestSet({modifyVars: true}, "modifyVars/", - 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.runTestSet({paths: 'test/data/'}, "include-path-string/"); -lessTester.runTestSet({plugin: 'test/plugins/postprocess/'}, "postProcessorPlugin/"); -lessTester.runTestSet({plugin: 'test/plugins/preprocess/'}, "preProcessorPlugin/"); -lessTester.runTestSet({plugin: 'test/plugins/visitor/'}, "visitorPlugin/"); -lessTester.runTestSet({plugin: 'test/plugins/filemanager/'}, "filemanagerPlugin/"); -lessTester.runTestSet({}, "no-strict-math/"); +var testMap = [ + [{strictMath: true, relativeUrls: true, silent: true, javascriptEnabled: true}], + [{strictMath: true, strictUnits: true, javascriptEnabled: true}, "errors/", + lessTester.testErrors, null, getErrorPathReplacementFunction("errors")], + [{strictMath: true, strictUnits: true, javascriptEnabled: false}, "no-js-errors/", + lessTester.testErrors, null, getErrorPathReplacementFunction("no-js-errors")], + [{strictMath: true, dumpLineNumbers: 'comments'}, "debug/", null, + function(name) { return name + '-comments'; }], + [{strictMath: true, dumpLineNumbers: 'mediaquery'}, "debug/", null, + function(name) { return name + '-mediaquery'; }], + [{strictMath: true, dumpLineNumbers: 'all'}, "debug/", null, + function(name) { return name + '-all'; }], + [{strictMath: true, relativeUrls: false, rootpath: "folder (1)/"}, "static-urls/"], + [{strictMath: true, compress: true}, "compression/"], + [{strictMath: true, strictUnits: true}, "strict-units/"], + [{}, "legacy/"], + [{strictMath: true, strictUnits: true, sourceMap: true, globalVars: true }, "sourcemaps/", + lessTester.testSourcemap, null, null, + function(filename, type, baseFolder) { + if (type === "vars") { + return path.join(baseFolder, filename) + '.json'; + } + return path.join('test/sourcemaps', filename) + '.json'; + }], + [{strictMath: true, strictUnits: true, sourceMap: {sourceMapFileInline: true}}, + "sourcemaps-empty/", lessTester.testEmptySourcemap], + [{globalVars: true, banner: "/**\n * Test\n */\n"}, "globalVars/", + null, null, null, function(name, type, baseFolder) { return path.join(baseFolder, name) + '.json'; }], + [{modifyVars: true}, "modifyVars/", + null, null, null, function(name, type, baseFolder) { return path.join(baseFolder, name) + '.json'; }], + [{urlArgs: '424242'}, "url-args/"], + [{paths: ['test/data/', 'test/less/import/']}, "include-path/"], + [{paths: 'test/data/'}, "include-path-string/"], + [{plugin: 'test/plugins/postprocess/'}, "postProcessorPlugin/"], + [{plugin: 'test/plugins/preprocess/'}, "preProcessorPlugin/"], + [{plugin: 'test/plugins/visitor/'}, "visitorPlugin/"], + [{plugin: 'test/plugins/filemanager/'}, "filemanagerPlugin/"], + [{}, "no-strict-math/"] +]; + +testMap.forEach(function(args) { + lessTester.runTestSet.apply(lessTester, args) +}); lessTester.testSyncronous({syncImport: true}, "import"); lessTester.testSyncronous({syncImport: true}, "css"); lessTester.testNoOptions();