Inconsistent error reporting when globalVars and/or modifyVars are being passed

[yatskevich]

Here is a simple demo of the issue:

index.js

var less = require('less');

var log = console.log;

less.render('Invalid content', { globalVars: {} }).then(log, log);
less.render('Invalid content', { modifyVars: {} }).then(log, log);
less.render('Invalid content', { globalVars: {}, modifyVars: {} }).then(log, log);
less.render('Invalid content', { }).then(log, log);

node index.js gives the following output:

{ [Error: Unrecognised input. Possibly missing something]
  type: 'Parse',
  filename: 'input',
  index: 16,
  line: 2,
  callLine: NaN,
  callExtract: undefined,
  column: 15,
  extract: [ '', 'Invalid content', undefined ],
  message: 'Unrecognised input. Possibly missing something',
  stack: undefined }
{ [Error: Unrecognised input. Possibly missing something]
  type: 'Parse',
  filename: 'input',
  index: 16,
  line: 2,
  callLine: NaN,
  callExtract: undefined,
  column: 0,
  extract: [ 'Invalid content', '', undefined ],
  message: 'Unrecognised input. Possibly missing something',
  stack: undefined }
{ [Error: Unrecognised input. Possibly missing something]
  type: 'Parse',
  filename: 'input',
  index: 17,
  line: 3,
  callLine: NaN,
  callExtract: undefined,
  column: 0,
  extract: [ 'Invalid content', '', undefined ],
  message: 'Unrecognised input. Possibly missing something',
  stack: undefined }
{ [Error: Unrecognised input. Possibly missing something]
  type: 'Parse',
  filename: 'input',
  index: 15,
  line: 1,
  callLine: NaN,
  callExtract: undefined,
  column: 15,
  extract: [ undefined, 'Invalid content', undefined ],
  message: 'Unrecognised input. Possibly missing something',
  stack: undefined }

Please note that reported line number, column and index change depending on passed globalVars and modifyVars.

[seven-phases-max]

The result is actually sort of expected since these inputs expand to the following Less code (with line numbers):
1:

1:
2: Invalid content

2:

1: Invalid content
2: 

3:

1: 
2: Invalid content
3:

1: Invalid content

As you can see every error points to the end of the input (where parser can't finally find anything recognizable). And, yes, globalVars does shift line numbers of the input (this can be fixed by not using line-break for each var there, but this creates another problem of a weird report on an invalid value passed with more than a few vars e.g. globalVars: {var: 1, foo: bar, <20 more variables here>, baz:./} -> error in (at best):@var: 1; @foo: bar; <20 more variables here> @baz: ./; <Actual Less code here>).

What "more consistent" report would you expect?

[yatskevich]

As a user I don't have access to the final source (which includes globalVars and modifyVars) which is used for compilation. That means seeing error on line 3 for a file containing just one line will confuse me.
Sure thing, one can use piece of code included in extract field to find an actual error in real source code. But still, compiler points to a line in phantom source file.

I see that an error may happen in globalVars itself and it has to be reported properly. Have you considered an option of pre-compiling globalVars and reporting related errors separately?

[seven-phases-max]

Have you considered an option of pre-compiling globalVars and reporting related errors separately?

That would be an overkill imho.

---

Sorry, I mislabeled by pressing wrong menu. Should be "Needs decision".

[kuus]

Have you considered an option of pre-compiling globalVars and reporting related errors separately?

I also like this idea if it doesn't impact on performances too much

[lukeapage]

We have the characters added, we should use that to adjust all error messages surely?

[richdougherty]

Right. Track the start and end of the user content, then adjust the reported error location accordingly.

[seven-phases-max]

Right. Track the start and end of the user content, then adjust the reported error location accordingly.

@richdougherty Right, ready to make a PR? ;)

[richdougherty]

:) I just meant to agree with the simpler option rather than the precompiling option; I didn't want to hassle anyone. In any case, I think @yatskevich is going to work around the issue.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.