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

Stylize tsc's error messages with color and context #5140

Merged
merged 26 commits into from
Nov 3, 2015

Conversation

DanielRosenwasser
Copy link
Member

This change seeks to make our output a little easier on the eyes and addresses #3852. I suggest this PR's final changes be reviewed rather than going commit-by-commit.

Firstly, a new flag is introduced: --diagnosticStyle. It accepts the options pretty and simple. It is currently marked as experimental, and we can bikeshed over the names later.

If --diagnosticStyle simple is used, the user will get the same behavior as they experience today. Note that simple is left as the default.

If --diagnosticStyle pretty is used as an option, then we check if we are outputting to a terminal-style descriptor. If we are entirely sure, then we stylize the output with color. If not (e.g. output is being piped, forwarded to a file, etc.) we make the output readable so that no special control characters are used in the output.

Here's some screenshots/gifs to give you an idea of what this looks like right now.

watchpretty

Monokai
image

Solarized Light
image

Terminal Basic
image

My Stupid Terminal Theme
image

Cygwin's MinTTY
image

cmd.exe
image

GNOME Terminal Defaults in Ubuntu
image

@myitcv
Copy link

myitcv commented Oct 6, 2015

Note that simple is left as the default.

Thanks. This should mean that any tools that consume the output from tsc continue to work.

@DanielRosenwasser
Copy link
Member Author

@myitcv this was one of the original concerns I heard a lot of from other members of the team; however, I think it would be a good idea to make it a default option in tsconfig.json when using tsc --init.

@Gaelan
Copy link

Gaelan commented Oct 6, 2015

One thing I would really appreciate would be some sort of coloring for the "starting incremental recompilation" message, so we can more easily tell which errors are current.

@tinganho
Copy link
Contributor

tinganho commented Oct 7, 2015

Awesome that it shows the line numbers! I would also like the message when overflowed to be tabbed to the starting position. For example, the text part type 'I' was too long and began on the next line on column 0. It would be awesome if that part began in column 2 instead.

I guess you would add colors on the messages too?

@DanielRosenwasser
Copy link
Member Author

@tinganho I think terminal size detection/wrapping is better left for a separate issue/PR.

let hasMoreThanFiveLines = (lastLine - firstLine) >= 4;
let gutterWidth = (lastLine + 1 + "").length;
if (hasMoreThanFiveLines) {
gutterWidth = Math.max("...".length, gutterWidth);
Copy link
Contributor

Choose a reason for hiding this comment

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

capture the ellipses in a const, and so is its length.

Copy link
Contributor

Choose a reason for hiding this comment

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

also applies to the squiggle down below.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 2, 2015

LGTM. a few nits, but other than than that 👍

@mhegazy
Copy link
Contributor

mhegazy commented Nov 2, 2015

DanielRosenwasser added a commit that referenced this pull request Nov 3, 2015
Stylize tsc's error messages with color and context
@DanielRosenwasser DanielRosenwasser merged commit 38b990b into master Nov 3, 2015
@DanielRosenwasser DanielRosenwasser deleted the iFeelPrettyErr branch November 3, 2015 00:31
@mhegazy mhegazy mentioned this pull request Dec 9, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants