Skip to content

Commit

Permalink
fix Runtime error reports from imported files
Browse files Browse the repository at this point in the history
  • Loading branch information
@cloudhead
cloudhead committed Jan 10, 2012
1 parent 2cc1b01 commit aefd310
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 20 deletions.
2 changes: 1 addition & 1 deletion lib/less/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ less.Parser.importer = function (file, paths, callback) {
paths: [path.dirname(pathname)].concat(paths),
filename: pathname
}).parse(data, function (e, root) {
callback(e, root);
callback(e, root, data);
});
});
} else {
Expand Down
33 changes: 22 additions & 11 deletions lib/less/parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ less.Parser = function Parser(env) {
paths: env && env.paths || [], // Search paths, when importing
queue: [], // Files which haven't been imported yet
files: {}, // Holds the imported parse trees
contents: {}, // Holds the imported file contents
mime: env && env.mime, // MIME type of .less files
error: null, // Error in parsing/evaluating an import
push: function (path, callback) {
Expand All @@ -82,9 +83,10 @@ less.Parser = function Parser(env) {
//
// Import a file asynchronously
//
less.Parser.importer(path, this.paths, function (e, root) {
less.Parser.importer(path, this.paths, function (e, root, contents) {
that.queue.splice(that.queue.indexOf(path), 1); // Remove the path from the queue
that.files[path] = root; // Store the root
that.contents[path] = contents;

if (e && !that.error) { that.error = e }
callback(e, root);
Expand Down Expand Up @@ -189,7 +191,15 @@ less.Parser = function Parser(env) {
}
}

function getLocation(index) {
function getInput(e, env) {
if (e.filename && env.filename && (e.filename !== env.filename)) {
return parser.imports.contents[e.filename];
} else {
return input;
}
}

function getLocation(index, input) {
for (var n = index, column = -1;
n >= 0 && input.charAt(n) !== '\n';
n--) { column++ }
Expand All @@ -199,18 +209,19 @@ less.Parser = function Parser(env) {
}

function LessError(e, env) {
var lines = input.split('\n'),
loc = getLocation(e.index),
var input = getInput(e, env),
loc = getLocation(e.index, input),
line = loc.line,
col = loc.column;
col = loc.column,
lines = input.split('\n');

this.type = e.type || 'SyntaxError';
this.type = e.type || 'Syntax';
this.message = e.message;
this.filename = e.filename || env.filename;
this.index = e.index;
this.line = typeof(line) === 'number' ? line + 1 : null;
this.callLine = e.call && (getLocation(e.call) + 1);
this.callExtract = lines[getLocation(e.call)];
this.callLine = e.call && (getLocation(e.call, input) + 1);
this.callExtract = lines[getLocation(e.call, input)];
this.stack = e.stack;
this.column = col;
this.extract = [
Expand Down Expand Up @@ -559,7 +570,7 @@ less.Parser = function Parser(env) {

if (! $(')')) return;

if (name) { return new(tree.Call)(name, args, index) }
if (name) { return new(tree.Call)(name, args, index, env.filename) }
},
arguments: function () {
var args = [], arg;
Expand Down Expand Up @@ -635,7 +646,7 @@ less.Parser = function Parser(env) {
var name, index = i;

if (input.charAt(i) === '@' && (name = $(/^@@?[\w-]+/))) {
return new(tree.Variable)(name, index);
return new(tree.Variable)(name, index, env.filename);
}
},

Expand Down Expand Up @@ -746,7 +757,7 @@ less.Parser = function Parser(env) {
}

if (elements.length > 0 && ($(';') || peek('}'))) {
return new(tree.mixin.Call)(elements, args, index, important);
return new(tree.mixin.Call)(elements, args, index, env.filename, important);
}
},

Expand Down
5 changes: 3 additions & 2 deletions lib/less/tree/call.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
//
// A function call node.
//
tree.Call = function (name, args, index) {
tree.Call = function (name, args, index, filename) {
this.name = name;
this.args = args;
this.index = index;
this.filename = filename;
};
tree.Call.prototype = {
//
Expand All @@ -31,7 +32,7 @@ tree.Call.prototype = {
throw { type: e.type || "Runtime",
message: "error evaluating function `" + this.name + "`" +
(e.message ? ': ' + e.message : ''),
index: this.index };
index: this.index, filename: this.filename };
}
} else { // 2.
return new(tree.Anonymous)(this.name +
Expand Down
9 changes: 5 additions & 4 deletions lib/less/tree/mixin.js
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
(function (tree) {

tree.mixin = {};
tree.mixin.Call = function (elements, args, index, important) {
tree.mixin.Call = function (elements, args, index, filename, important) {
this.selector = new(tree.Selector)(elements);
this.arguments = args;
this.index = index;
this.filename = filename;
this.important = important;
};
tree.mixin.Call.prototype = {
Expand All @@ -21,7 +22,7 @@ tree.mixin.Call.prototype = {
rules, mixins[m].eval(env, this.arguments, this.important).rules);
match = true;
} catch (e) {
throw { message: e.message, index: e.index, stack: e.stack, call: this.index };
throw { message: e.message, index: e.index, filename: this.filename, stack: e.stack, call: this.index };
}
}
}
Expand All @@ -34,13 +35,13 @@ tree.mixin.Call.prototype = {
this.arguments.map(function (a) {
return a.toCSS();
}).join(', ') + ")`",
index: this.index };
index: this.index, filename: this.filename };
}
}
}
throw { type: 'Name',
message: this.selector.toCSS().trim() + " is undefined",
index: this.index };
index: this.index, filename: this.filename };
}
};

Expand Down
6 changes: 4 additions & 2 deletions lib/less/tree/variable.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(function (tree) {

tree.Variable = function (name, index) { this.name = name, this.index = index };
tree.Variable = function (name, index, file) { this.name = name, this.index = index, this.file = file };
tree.Variable.prototype = {
eval: function (env) {
var variable, v, name = this.name;
Expand All @@ -15,7 +15,9 @@ tree.Variable.prototype = {
}
})) { return variable }
else {
throw { message: "variable " + name + " is undefined",
throw { type: 'Name',
message: "variable " + name + " is undefined",
filename: this.file,
index: this.index };
}
}
Expand Down

5 comments on commit aefd310

@trkoch
Copy link

@trkoch trkoch commented on aefd310 Jan 11, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid I'm a victim of version chaos. Using 1.2.0 from the dist folder, or from lesscss.org, I get an error when importing less files using @import. The error itself is not very helpful: Cannot call method 'slice' of undefined (using Chrome).

Did this commit make it into the 1.2.0 hotfix? Why did you choose to update in place? I wonder, what (remote) file do you recommend to include? Sure, I could copy the file, but in my case loading from remote is easier, at least for the time being.

@cloudhead
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't make it in 1.2.0, it is in 1.2.1 however.

@paulogaspar7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running from the command prompt, from this commit on, a syntax error on an import is producing a big exception like this:

TypeError: Cannot call method 'charAt' of undefined
    at getLocation (/some-dir/less.js/lib/less/parser.js:204:34)
    at new LessError (/some-dir/less.js/lib/less/parser.js:213:19)
    at Object.toCSS (/some-dir/less.js/lib/less/parser.js:379:31)
    at /some-dir/less.js/bin/lessc:103:28
    at /some-dir/less.js/lib/less/parser.js:428:40
    at /some-dir/less.js/lib/less/parser.js:94:48
    at /some-dir/less.js/lib/less/index.js:113:15
    at /some-dir/less.js/lib/less/parser.js:428:40
    at /some-dir/less.js/lib/less/parser.js:94:48
    at /some-dir/less.js/lib/less/index.js:113:15

This happens because the getLocation() function is called for an undefined "input" argument.

The degradation of reporting syntax errors at imported files when running "lessc" started already with a first step at commit "6d4516e6fc". Since that commit:

  • if I introduce a syntax error on a first imported less file...
  • ...which causes an "undefined variable" error on a second imported less file...
  • ...only this second error gets reported.

Before commit ""6d4516e6fc", both errors would be reported by "lessc", as I already reported at:

I have been tweaking with the code and managed to have a version reporting both errors, but my current solution is awful, as nothing to do with version 1.2.0 code (which ran ok) and therefore, I am pretty sure there is a better way to do it.

Cheers!

@cloudhead
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, could you try 1.2.2? I remember fixing something related to this.

@paulogaspar7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try with 1.2.2, same problem.

Ok, moved my answer to the related issue:

Please sign in to comment.