Standardizes <file>:<line>:<column> formatting.

Why:

* When a warning is raised we should give user as much info as
  possible, which includes a filename, line and column number.

diff --git a/lib/optimizer/basic.js b/lib/optimizer/basic.js
index 132e3de..1775491 100644 (file)
--- 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 c3cd31e..9ef6e2a 100644 (file)
--- 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 4b01aca..66a342a 100644 (file)
--- 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 e2a98c6..a88ee38 100644 (file)
--- 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 (file)
index 0000000..0e3713c
--- /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 b019427..5f256b1 100644 (file)
--- 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 df1ad5c..86335e5 100644 (file)
--- 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 0f726c2..9bac5a3 100644 (file)
--- 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.']);
       }
     }
   })