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
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ba7fade
Add support to tell whether output directs to a terminal emulator.
DanielRosenwasser Jul 1, 2015
f660bd3
Got "prettier" error printing working.
DanielRosenwasser Jul 1, 2015
17f443a
Just squiggle the entire line for middle lines (sans trailing space).
DanielRosenwasser Jul 1, 2015
fdbea8b
Give the new reporter a more reasonable name.
DanielRosenwasser Jul 1, 2015
d8e462d
Make output reliant on whether stdout redirects to a terminal; use fo…
DanielRosenwasser Jul 1, 2015
cc6b04c
Bring back 'reportDiagnostics'.
DanielRosenwasser Jul 1, 2015
99f6985
Don't expand tabs; just use a space.
DanielRosenwasser Jul 17, 2015
85b54e8
Fixed out of bounds error; made index start directly at the line number.
DanielRosenwasser Jul 17, 2015
dda058b
Use 'process.stdout.write' to ensure colors get displayed correctly o…
DanielRosenwasser Jul 24, 2015
3c04dff
Limit errors to 5 lines, fixed some other issues.
DanielRosenwasser Aug 18, 2015
ea1882d
Merge branch 'master' into iFeelPrettyErr
DanielRosenwasser Aug 18, 2015
44f3c0c
Stylize gutter.
DanielRosenwasser Aug 19, 2015
ac5bed8
Added '--diagnosticStyle' compiler argument with options 'simple' and…
DanielRosenwasser Sep 19, 2015
7c73a66
Merge branch 'master' into iFeelPrettyErr
DanielRosenwasser Oct 6, 2015
2b4febe
Moved JSX diagnostics to a more sensical spot.
DanielRosenwasser Oct 6, 2015
c3e00a2
Only use colors if we are certain we are using a pseudoterminal.
DanielRosenwasser Oct 6, 2015
5c5fca6
Just make the compiler option internal.
DanielRosenwasser Oct 6, 2015
87554cb
Merge branch 'master' into iFeelPrettyErr
DanielRosenwasser Oct 6, 2015
fa2e614
Merge branch 'master' into iFeelPrettyErr
DanielRosenwasser Oct 31, 2015
48c2bb1
Fixed 'tsconfig.json' ordering.
DanielRosenwasser Oct 31, 2015
4219c5f
Added colors to diagnostic types, addressed some CR feedback.
DanielRosenwasser Nov 2, 2015
654cbd9
Just name the option 'pretty' for now.
DanielRosenwasser Nov 2, 2015
40f10ab
Forget about tty detection. It won't work in build tools.
DanielRosenwasser Nov 2, 2015
e5f1053
Merge branch 'master' into iFeelPrettyErr
DanielRosenwasser Nov 2, 2015
32b259e
Merge branch 'master' into iFeelPrettyErr
DanielRosenwasser Nov 2, 2015
ce24bcb
;
DanielRosenwasser Nov 2, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ namespace ts {
name: "diagnostics",
type: "boolean",
},
{
name: "diagnosticStyle",
paramType: Diagnostics.KIND,
description: Diagnostics.Specify_diagnostic_printing_style_Colon_simple_default_or_pretty,
type: {
"simple": DiagnosticStyle.Simple,
"pretty": DiagnosticStyle.Pretty,
},
error: Diagnostics.Argument_for_diagnosticStyle_must_be_simple_or_pretty,
experimental: true,
},
{
name: "emitBOM",
type: "boolean"
Expand Down Expand Up @@ -500,4 +511,4 @@ namespace ts {
return fileNames;
}
}
}
}
25 changes: 17 additions & 8 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2258,14 +2258,6 @@
"code": 6063
},

"Specify JSX code generation: 'preserve' or 'react'": {
"category": "Message",
"code": 6080
},
"Argument for '--jsx' must be 'preserve' or 'react'.": {
"category": "Message",
"code": 6081
},
"Enables experimental support for ES7 decorators.": {
"category": "Message",
"code": 6065
Expand Down Expand Up @@ -2298,6 +2290,23 @@
"category": "Message",
"code": 6072
},
"Specify diagnostic printing style: 'simple' (default) or 'pretty'.": {
"category": "Message",
"code": 6073
},
"Argument for '--diagnosticStyle' must be 'simple' or 'pretty'.": {
"category": "Error",
"code": 6074
},

"Specify JSX code generation: 'preserve' or 'react'": {
"category": "Message",
"code": 6080
},
"Argument for '--jsx' must be 'preserve' or 'react'.": {
"category": "Message",
"code": 6081
},

"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
Expand Down
13 changes: 4 additions & 9 deletions src/compiler/sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace ts {
newLine: string;
useCaseSensitiveFileNames: boolean;
write(s: string): void;
writesToTty?(): boolean;
readFile(path: string, encoding?: string): string;
writeFile(path: string, data: string, writeByteOrderMark?: boolean): void;
watchFile?(path: string, callback: (path: string, removed: boolean) => void): FileWatcher;
Expand Down Expand Up @@ -191,6 +192,7 @@ namespace ts {
const _fs = require("fs");
const _path = require("path");
const _os = require("os");
const _tty = require("tty");

const platform: string = _os.platform();
// win32\win64 are case insensitive platforms, MacOS (darwin) by default is also case insensitive
Expand Down Expand Up @@ -271,16 +273,9 @@ namespace ts {
newLine: _os.EOL,
useCaseSensitiveFileNames: useCaseSensitiveFileNames,
write(s: string): void {
const buffer = new Buffer(s, "utf8");
let offset = 0;
let toWrite: number = buffer.length;
let written = 0;
// 1 is a standard descriptor for stdout
while ((written = _fs.writeSync(1, buffer, offset, toWrite)) < toWrite) {
offset += written;
toWrite -= written;
}
process.stdout.write(s);
},
writesToTty: () => _tty.isatty(1),
readFile,
writeFile,
watchFile: (fileName, callback) => {
Expand Down
93 changes: 87 additions & 6 deletions src/compiler/tsc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ namespace ts {
fileWatcher?: FileWatcher;
}

let reportDiagnostic = reportDiagnosticSimply;

function reportDiagnostics(diagnostics: Diagnostic[]): void {
for (let diagnostic of diagnostics) {
reportDiagnostic(diagnostic);
}
}

/**
* Checks to see if the locale is in the appropriate format,
* and if it is, attempts to set the appropriate language.
Expand Down Expand Up @@ -81,12 +89,13 @@ namespace ts {
return <string>diagnostic.messageText;
}

function reportDiagnostic(diagnostic: Diagnostic) {
function reportDiagnosticSimply(diagnostic: Diagnostic): void {
let output = "";

if (diagnostic.file) {
let loc = getLineAndCharacterOfPosition(diagnostic.file, diagnostic.start);
output += `${ diagnostic.file.fileName }(${ loc.line + 1 },${ loc.character + 1 }): `;
let { line, character } = getLineAndCharacterOfPosition(diagnostic.file, diagnostic.start);

output += `${ diagnostic.file.fileName }(${ line + 1 },${ character + 1 }): `;
}

let category = DiagnosticCategory[diagnostic.category].toLowerCase();
Expand All @@ -95,10 +104,78 @@ namespace ts {
sys.write(output);
}

function reportDiagnostics(diagnostics: Diagnostic[]) {
for (let i = 0; i < diagnostics.length; i++) {
reportDiagnostic(diagnostics[i]);
const shouldUseColors = sys.writesToTty && sys.writesToTty();
const redForegroundEscapeSequence = shouldUseColors ? "\u001b[91m" : "";
const gutterStyleSequence = shouldUseColors ? "\u001b[100;30m" : "";
const gutterSeparator = shouldUseColors ? " " : " | ";
const resetEscapeSequence = shouldUseColors ? "\u001b[0m" : "";

function reportDiagnosticWithColorAndContext(diagnostic: Diagnostic): void {
let output = "";

if (diagnostic.file) {
let { start, length, file } = diagnostic;
let { line: firstLine, character: firstLineChar } = getLineAndCharacterOfPosition(file, start);
let { line: lastLine, character: lastLineChar } = getLineAndCharacterOfPosition(file, start + length);
const lastLineInFile = getLineAndCharacterOfPosition(file, file.text.length).line;

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.

}

output += sys.newLine;
for (let i = firstLine; i <= lastLine; i++) {
// If the error spans over 5 lines, we'll only show the first 2 and last 2 lines,
// so we'll skip ahead to the second-to-last line.
if (hasMoreThanFiveLines && firstLine + 1 < i && i < lastLine - 1) {
output += gutterStyleSequence + padLeft("...", gutterWidth) + resetEscapeSequence + gutterSeparator + sys.newLine;
i = lastLine - 1;
}

let lineStart = getPositionOfLineAndCharacter(file, i, 0);
let lineEnd = i < lastLineInFile ? getPositionOfLineAndCharacter(file, i + 1, 0) : file.text.length;
let lineContent = file.text.slice(lineStart, lineEnd);
lineContent = lineContent.replace(/\s+$/g, ""); // trim from end
lineContent = lineContent.replace("\t", " "); // convert tabs to single spaces

// Output the gutter and the actual contents of the line.
output += gutterStyleSequence + padLeft(i + 1 + "", gutterWidth) + resetEscapeSequence + gutterSeparator;
output += lineContent + sys.newLine;

// Output the gutter and the error span for the line using tildes.
output += gutterStyleSequence + padLeft("", gutterWidth) + resetEscapeSequence + gutterSeparator;
output += redForegroundEscapeSequence;
if (i === firstLine) {
// If we're on the last line, then limit it to the last character of the last line.
// Otherwise, we'll just squiggle the rest of the line, giving 'slice' no end position.
let lastCharForLine = i === lastLine ? lastLineChar : undefined;

output += lineContent.slice(0, firstLineChar).replace(/\S/g, " ");
output += lineContent.slice(firstLineChar, lastCharForLine).replace(/./g, "~");
}
else if (i === lastLine) {
output += lineContent.slice(0, lastLineChar).replace(/./g, "~");
Copy link
Contributor

Choose a reason for hiding this comment

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

sound like a long way to say repeat("~", lastLineChar), where repeat is just a simple for loop.

}
else {
// Squiggle the entire line.
output += lineContent.replace(/./g, "~");
}
output += resetEscapeSequence;

output += sys.newLine;
}

output += sys.newLine;
output += `${ file.fileName }(${ firstLine + 1 },${ firstLineChar + 1 }): `;
}

let category = DiagnosticCategory[diagnostic.category].toLowerCase();
output += `${ category } TS${ diagnostic.code }: ${ flattenDiagnosticMessageText(diagnostic.messageText, sys.newLine) }`;
output += sys.newLine + sys.newLine;

sys.write(output);
}

function reportWatchDiagnostic(diagnostic: Diagnostic) {
Expand Down Expand Up @@ -252,6 +329,10 @@ namespace ts {
compilerHost.getSourceFile = getSourceFile;
}

if (compilerOptions.diagnosticStyle === DiagnosticStyle.Pretty) {
reportDiagnostic = reportDiagnosticWithColorAndContext;
}

let compileResult = compile(rootFileNames, compilerOptions, compilerHost);

if (!compilerOptions.watch) {
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,7 @@ namespace ts {
charset?: string;
declaration?: boolean;
diagnostics?: boolean;
/* @internal */diagnosticStyle?: DiagnosticStyle;
emitBOM?: boolean;
help?: boolean;
init?: boolean;
Expand Down Expand Up @@ -2128,6 +2129,12 @@ namespace ts {
JSX
}

/* @internal */
export const enum DiagnosticStyle {
Simple,
Pretty,
}

export interface ParsedCommandLine {
options: CompilerOptions;
fileNames: string[];
Expand Down