Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(cli): unify display of errors from Rust and JS #5183

Merged
merged 4 commits into from
May 9, 2020

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented May 9, 2020

Currently error formatting is different for errors originating from JS and Rust:

// JS
error: Uncaught URIError: relative import path "bad-module.ts" not prefixed with / or ./ or ../ Imported from "file:https:///Users/biwanczuk/dev/deno/cli/tests/error_011_bad_module_specifier.ts"

// Rust
Remote module are not allowed to statically import local modules. Use dynamic import instead.

This PR changes formatting so both errors are prefixed with red error::

// JS
error: Uncaught URIError: relative import path "bad-module.ts" not prefixed with / or ./ or ../ Imported from "file:https:///Users/biwanczuk/dev/deno/cli/tests/error_011_bad_module_specifier.ts"

// Rust
error: Remote module are not allowed to statically import local modules. Use dynamic import instead.

There is also question of diagnostics which have completely different formatting and display as:

error TS2339: Property 'loadavg' does not exist on type 'typeof Deno'.
console.log(Deno.loadavg);

I propose to change it to:

error: TS2339 [ERROR]: Property 'loadavg' does not exist on type 'typeof Deno'.
console.log(Deno.loadavg);

The [ERROR] describes the category of diagnostic (it can be also warn/debug/info).

EDIT: Already went ahead and did suggested change for diagnostics

@bartlomieju
Copy link
Member Author

CC @ry @kitsonk

@bartlomieju bartlomieju changed the title refactor: unify display of errors from Rust and JS refactor(cli): unify display of errors from Rust and JS May 9, 2020
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju bartlomieju merged commit 1fddcc3 into denoland:master May 9, 2020
@bartlomieju bartlomieju deleted the unify_error_display branch May 9, 2020 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants