Document 'errors' #61
Conversation
Display the content of the error object when the compilation fails
There was a problem hiding this comment.
In general, I think anything that is not regular output of the compiler should go to stderr. The fact whether the compilation was successful or not should be reported via the process exit status. If there are no errors (but warnings), exit status should report "success" but the stderr stream should be nonempty.
| @@ -59,6 +62,7 @@ for (var i = 0; i < files.length; i++) { | |||
| } | |||
| } | |||
|
|
|||
| console.log("Compiling with version: " + solc.version()); | |||
There was a problem hiding this comment.
Please do not clutter stdout
| spt.end(); | ||
| }); | ||
|
|
||
| t.test('--abi', function (st) { | ||
| t.test('--abi -o output', function (st) { |
There was a problem hiding this comment.
Please revert (see comment above).
| // only leave the console.error | ||
|
|
||
| if (res.errors[0].indexOf('Warning') > 0) { | ||
| console.log('Compiled with Warnings: ', res.errors); |
There was a problem hiding this comment.
Please output that on stderr.
|
@chevdor could you please provide a summary of the PR as the title (and commit message) instead of just mentioning the issue number? |
| process.exit(1); | ||
| } | ||
|
|
||
| if (!files.length) | ||
| abort('You must provide at least one file to compile.'); |
There was a problem hiding this comment.
This will need to be wrapper into {} to satisfy the lint.
| @@ -11,42 +11,42 @@ tape('CLI', function (t) { | |||
|
|
|||
| t.test('no parameters', function (st) { | |||
| var spt = spawn(st, './solcjs'); | |||
| spt.stderr.match(/^Must provide a file/); | |||
| spt.stderr.match(/^Must provide a file.*/); | |||
There was a problem hiding this comment.
The .* shouldn't be needed as there's no closing $ to mean end-of-match.
| // https://github.com/ethereum/solc-js/issues/53 | ||
| // only leave the console.error | ||
|
|
||
| if (res.errors[0].indexOf('Warning') > 0) { |
There was a problem hiding this comment.
I think res.errors[0] is wrong, it should check every single error for being a warning.
| if (res.errors[0].indexOf('Warning') > 0) { | ||
| console.log('Compiled with Warnings: ', res.errors); | ||
| } else { | ||
| console.error('Compiled with Errors: ', res.errors); |
|
@chevdor afaict you don't set the exit code to non-zero. |
| if (res.errors.filter(function (value) { | ||
| value.indexOf('Warning') > 0; | ||
| })) { | ||
| console.error('Compiled with Warnings: ', res.errors); |
There was a problem hiding this comment.
Wrapper is too low-level to be allowed to write to the console.
| process.exit(1); | ||
| } | ||
|
|
||
| if (!files.length) { | ||
| abort('You must provide at least one file to compile.'); |
| @@ -41,10 +41,14 @@ var files = argv._; | |||
| var destination = argv['output-dir'] || '.' | |||
|
|
|||
| function abort (msg) { | |||
| console.log(msg || 'Error occured'); | |||
| console.error(msg || 'Error occured'); | |||
There was a problem hiding this comment.
The error/log change is handled by #129.
|
Closing this as it seems everything has been addressed since. |
No description provided.