From 0db3fd3ae35d6f4d7df7dd4e297c0623f3afdff6 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Tue, 20 Dec 2016 16:22:44 +0100 Subject: [PATCH] Standardizes :: formatting. Why: * When a warning is raised we should give user as much info as possible, which includes a filename, line and column number. --- lib/optimizer/basic.js | 5 +++-- lib/optimizer/tidy-rules.js | 3 ++- lib/properties/break-up.js | 5 +++-- lib/tokenizer/tokenize.js | 6 ++++-- lib/utils/format-position.js | 11 +++++++++++ test/module-test.js | 12 ++++++------ test/properties/break-up-test.js | 4 ++-- test/tokenizer/tokenize-test.js | 2 +- 8 files changed, 32 insertions(+), 16 deletions(-) create mode 100644 lib/utils/format-position.js diff --git a/lib/optimizer/basic.js b/lib/optimizer/basic.js index 132e3dec..17754918 100644 --- a/lib/optimizer/basic.js +++ b/lib/optimizer/basic.js @@ -2,6 +2,7 @@ var tidyRules = require('./tidy-rules'); var tidyBlock = require('./tidy-block'); var tidyAtRule = require('./tidy-at-rule'); var split = require('../utils/split'); +var formatPosition = require('../utils/format-position'); var Token = require('../tokenizer/token'); var Marker = require('../tokenizer/marker'); @@ -346,7 +347,7 @@ function optimizeBody(properties, context) { if (property.value.length === 0) { propertyToken = property.all[property.position]; - context.warnings.push('Empty property \'' + name + '\' at line ' + propertyToken[1][2][0][0] + ', column ' + propertyToken[1][2][0][1] + '. Ignoring.'); + context.warnings.push('Empty property \'' + name + '\' at ' + formatPosition(propertyToken[1][2][0]) + '. Ignoring.'); property.unused = true; } @@ -374,7 +375,7 @@ function optimizeBody(properties, context) { if (valueIsUrl && !context.validator.isValidUrl(value)) { property.unused = true; - context.warnings.push('Broken URL \'' + value + '\' at line ' + property.value[j][2][0][0] + ', column ' + property.value[j][2][0][1] + '. Ignoring.'); + context.warnings.push('Broken URL \'' + value + '\' at ' + formatPosition(property.value[j][2][0]) + '. Ignoring.'); break; } diff --git a/lib/optimizer/tidy-rules.js b/lib/optimizer/tidy-rules.js index c3cd31e2..9ef6e2a1 100644 --- a/lib/optimizer/tidy-rules.js +++ b/lib/optimizer/tidy-rules.js @@ -1,4 +1,5 @@ var Marker = require('../tokenizer/marker'); +var formatPosition = require('../utils/format-position'); var RELATION_PATTERN = /[>\+~]/; var WHITESPACE_PATTERN = /\s/; @@ -146,7 +147,7 @@ function tidyRules(rules, removeUnsupported, adjacentSpace, warnings) { var reduced = rule[1]; if (hasInvalidCharacters(reduced)) { - warnings.push('Invalid selector \'' + rule[1] + '\' at line ' + rule[2][0][0] + ', column ' + rule[2][0][1] + '. Ignoring.'); + warnings.push('Invalid selector \'' + rule[1] + '\' at ' + formatPosition(rule[2][0]) + '. Ignoring.'); continue; } diff --git a/lib/properties/break-up.js b/lib/properties/break-up.js index 4b01aca0..66a342ab 100644 --- a/lib/properties/break-up.js +++ b/lib/properties/break-up.js @@ -2,6 +2,7 @@ var wrapSingle = require('./wrap-for-optimizing').single; var InvalidPropertyError = require('./invalid-property-error'); var Token = require('../tokenizer/token'); +var formatPosition = require('../utils/format-position'); var MULTIPLEX_SEPARATOR = ','; @@ -136,7 +137,7 @@ function background(property, compactable, validator) { origin.value = clip.value.slice(0); if (!anyValueSet) { - throw new InvalidPropertyError('Invalid background value.'); + throw new InvalidPropertyError('Invalid background value at ' + formatPosition(values[0][2][0]) + '. Ignoring.'); } return components; @@ -154,7 +155,7 @@ function borderRadius(property, compactable) { } if (splitAt === 0 || splitAt === values.length - 1) { - throw new InvalidPropertyError('Invalid border-radius value.'); + throw new InvalidPropertyError('Invalid border-radius value at ' + formatPosition(values[0][2][0]) + '. Ignoring.'); } var target = _wrapDefault(property.name, property, compactable); diff --git a/lib/tokenizer/tokenize.js b/lib/tokenizer/tokenize.js index e2a98c67..a88ee38b 100644 --- a/lib/tokenizer/tokenize.js +++ b/lib/tokenizer/tokenize.js @@ -1,6 +1,8 @@ var Marker = require('./marker'); var Token = require('./token'); +var formatPosition = require('../utils/format-position'); + var Level = { BLOCK: 'block', COMMENT: 'comment', @@ -309,7 +311,7 @@ function intoTokens(source, externalContext, internalContext, isNested) { seekingValue = false; } else if (character == Marker.CLOSE_BRACE && level == Level.BLOCK && !isNested && position.index <= source.length - 1) { // stray close brace at block level, e.g. a{color:red}color:blue}<-- - externalContext.warnings.push('Unexpected \'}\' at line ' + position.line + ', column ' + position.column + '.'); + externalContext.warnings.push('Unexpected \'}\' at ' + formatPosition([position.line, position.column, position.source]) + '.'); buffer.push(character); } else if (character == Marker.CLOSE_BRACE && level == Level.BLOCK) { // close brace at block level, e.g. @media screen {...}<-- @@ -386,7 +388,7 @@ function intoTokens(source, externalContext, internalContext, isNested) { } if (seekingValue) { - externalContext.warnings.push('Missing \'}\' at line ' + position.line + ', column ' + position.column + '.'); + externalContext.warnings.push('Missing \'}\' at ' + formatPosition([position.line, position.column, position.source]) + '.'); } if (seekingValue && buffer.length > 0) { diff --git a/lib/utils/format-position.js b/lib/utils/format-position.js new file mode 100644 index 00000000..0e3713c1 --- /dev/null +++ b/lib/utils/format-position.js @@ -0,0 +1,11 @@ +function formatPosition(metadata) { + var line = metadata[0]; + var column = metadata[1]; + var source = metadata[2]; + + return source ? + source + ':' + line + ':' + column : + line + ':' + column; +} + +module.exports = formatPosition; diff --git a/test/module-test.js b/test/module-test.js index b019427a..5f256b19 100644 --- a/test/module-test.js +++ b/test/module-test.js @@ -112,7 +112,7 @@ vows.describe('module tests').addBatch({ }, 'should raise one warning': function (error, minified) { assert.lengthOf(minified.warnings, 1); - assert.equal(minified.warnings[0], 'Unexpected \'}\' at line 1, column 16.'); + assert.equal(minified.warnings[0], 'Unexpected \'}\' at 1:16.'); } }, 'warnings on unexpected body': { @@ -127,8 +127,8 @@ vows.describe('module tests').addBatch({ }, 'should raise one warning': function (error, minified) { assert.lengthOf(minified.warnings, 2); - assert.equal(minified.warnings[0], 'Unexpected \'}\' at line 1, column 29.'); - assert.equal(minified.warnings[1], 'Invalid selector \'color:#535353}p\' at line 1, column 16. Ignoring.'); + assert.equal(minified.warnings[0], 'Unexpected \'}\' at 1:29.'); + assert.equal(minified.warnings[1], 'Invalid selector \'color:#535353}p\' at 1:16. Ignoring.'); } }, 'warnings on empty properties': { @@ -143,7 +143,7 @@ vows.describe('module tests').addBatch({ }, 'should raise one warning': function (error, minified) { assert.lengthOf(minified.warnings, 1); - assert.equal(minified.warnings[0], 'Empty property \'color\' at line 1, column 2. Ignoring.'); + assert.equal(minified.warnings[0], 'Empty property \'color\' at 1:2. Ignoring.'); } }, 'warnings on broken urls': { @@ -158,8 +158,8 @@ vows.describe('module tests').addBatch({ }, 'should raise one warning': function (error, minified) { assert.lengthOf(minified.warnings, 2); - assert.equal(minified.warnings[0], 'Missing \'}\' at line 1, column 24.'); - assert.equal(minified.warnings[1], 'Broken URL \'url(image/\' at line 1, column 13. Ignoring.'); + assert.equal(minified.warnings[0], 'Missing \'}\' at 1:24.'); + assert.equal(minified.warnings[1], 'Broken URL \'url(image/\' at 1:13. Ignoring.'); } }, 'no errors': { diff --git a/test/properties/break-up-test.js b/test/properties/break-up-test.js index df1ad5c3..86335e54 100644 --- a/test/properties/break-up-test.js +++ b/test/properties/break-up-test.js @@ -519,7 +519,7 @@ vows.describe(breakUp) [ 'property', ['property-name', 'border-radius'], - ['property-value', '0px'], + ['property-value', '0px', [[1, 20, undefined]]], ['property-value', '/'] ] ]); @@ -534,7 +534,7 @@ vows.describe(breakUp) [ 'property', ['property-name', 'border-radius'], - ['property-value', '/'], + ['property-value', '/', [[1, 20, undefined]]], ['property-value', '0px'] ] ]); diff --git a/test/tokenizer/tokenize-test.js b/test/tokenizer/tokenize-test.js index 0f726c2d..9bac5a3c 100644 --- a/test/tokenizer/tokenize-test.js +++ b/test/tokenizer/tokenize-test.js @@ -3436,7 +3436,7 @@ vows.describe(tokenize) return warnings; }, 'logs them correctly': function (warnings) { - assert.deepEqual(warnings, ['Missing \'}\' at line 1, column 15.']); + assert.deepEqual(warnings, ['Missing \'}\' at one.css:1:15.']); } } }) -- 2.34.1