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.

Sign up for GitHub

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

Jump to bottom

Prettified timestamps and error reports in --pretty #20416

Merged
merged 4 commits into from
Dec 19, 2017
Merged

Prettified timestamps and error reports in --pretty #20416

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb5d8d0
Prettified timestamps and error reports in --pretty
Dec 3, 2017
ff7510d
Fixed prettyContextNotDebugAssertion.errors baseline
Dec 3, 2017
98b64db
Re-added blue color, with corrected 94m code
Dec 5, 2017
b315170
Converted to enum; marked internal
Dec 19, 2017
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
41 changes: 26 additions & 15 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,22 +241,27 @@ namespace ts {
return errorMessage;
}

const redForegroundEscapeSequence = "\u001b[91m";
const yellowForegroundEscapeSequence = "\u001b[93m";
const blueForegroundEscapeSequence = "\u001b[93m";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: the blue sequence was the same as yellow before. I'm guessing that's a behavior bug that was never exposed. I've removed the sequence but left the behavior of using the yellow color in getCategoryFormat.

Copy link
Member

@weswigham weswigham Dec 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Note: the blue sequence was the same as yellow before. I'm guessing that's a behavior bug that was never exposed.

We don't actually use ErrorCategory.Warning right now (and Message is only used for help output and quickfixes/refactorings), so it would never get noticed/changed until it mattered, like for #19126

IMO, we should preemptively change Message to dark blue or something, just in case.

/** @internal */
export const foregroundColorEscapeSequences = {
Copy link
Member

@DanielRosenwasser DanielRosenwasser Dec 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This can just be a const string enum ForegroundColorEscapeSequence, and it doesn't need to be exported at all

grey: "\u001b[90m",
red: "\u001b[91m",
yellow: "\u001b[93m",
cyan: "\u001b[96m"
};
const gutterStyleSequence = "\u001b[30;47m";
const gutterSeparator = " ";
const resetEscapeSequence = "\u001b[0m";
const ellipsis = "...";
function getCategoryFormat(category: DiagnosticCategory): string {
switch (category) {
case DiagnosticCategory.Warning: return yellowForegroundEscapeSequence;
case DiagnosticCategory.Error: return redForegroundEscapeSequence;
case DiagnosticCategory.Message: return blueForegroundEscapeSequence;
case DiagnosticCategory.Warning: return foregroundColorEscapeSequences.yellow;
case DiagnosticCategory.Error: return foregroundColorEscapeSequences.red;
case DiagnosticCategory.Message: return foregroundColorEscapeSequences.yellow;
}
}

function formatAndReset(text: string, formatStyle: string) {
/** @internal */
export function formatColorAndReset(text: string, formatStyle: string) {
return formatStyle + text + resetEscapeSequence;
}

Expand Down Expand Up @@ -289,7 +294,7 @@ namespace ts {
// 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) {
context += formatAndReset(padLeft(ellipsis, gutterWidth), gutterStyleSequence) + gutterSeparator + host.getNewLine();
context += formatColorAndReset(padLeft(ellipsis, gutterWidth), gutterStyleSequence) + gutterSeparator + host.getNewLine();
i = lastLine - 1;
}

Expand All @@ -300,12 +305,12 @@ namespace ts {
lineContent = lineContent.replace("\t", " "); // convert tabs to single spaces

// Output the gutter and the actual contents of the line.
context += formatAndReset(padLeft(i + 1 + "", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += formatColorAndReset(padLeft(i + 1 + "", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += lineContent + host.getNewLine();

// Output the gutter and the error span for the line using tildes.
context += formatAndReset(padLeft("", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += redForegroundEscapeSequence;
context += formatColorAndReset(padLeft("", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += foregroundColorEscapeSequences.red;
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.
Expand All @@ -324,13 +329,19 @@ namespace ts {
context += resetEscapeSequence;
}

output += host.getNewLine();
output += `${ relativeFileName }(${ firstLine + 1 },${ firstLineChar + 1 }): `;
output += formatColorAndReset(relativeFileName, foregroundColorEscapeSequences.cyan);
output += "(";
output += formatColorAndReset(`${ firstLine + 1 }`, foregroundColorEscapeSequences.yellow);
output += ",";
output += formatColorAndReset(`${ firstLineChar + 1 }`, foregroundColorEscapeSequences.yellow);
output += "): ";
}

const categoryColor = getCategoryFormat(diagnostic.category);
const category = DiagnosticCategory[diagnostic.category].toLowerCase();
output += `${ formatAndReset(category, categoryColor) } TS${ diagnostic.code }: ${ flattenDiagnosticMessageText(diagnostic.messageText, host.getNewLine()) }`;
output += formatColorAndReset(category, categoryColor);
output += formatColorAndReset(` TS${ diagnostic.code }: `, foregroundColorEscapeSequences.grey);
output += flattenDiagnosticMessageText(diagnostic.messageText, host.getNewLine());

if (diagnostic.file) {
output += host.getNewLine();
Expand All @@ -339,7 +350,7 @@ namespace ts {

output += host.getNewLine();
}
return output;
return output + host.getNewLine();
}

export function flattenDiagnosticMessageText(messageText: string | DiagnosticMessageChain, newLine: string): string {
Expand Down
10 changes: 9 additions & 1 deletion src/compiler/watch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ namespace ts {
};
}

export function createWatchDiagnosticReporterWithColor(system = sys): DiagnosticReporter {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I just saw this. It's unfortunate that we need a new reporter. This should probably be marked /** @internal */ though.

return diagnostic => {
let output = `[${ formatColorAndReset(new Date().toLocaleTimeString(), foregroundColorEscapeSequences.grey) }] `;
output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${system.newLine + system.newLine + system.newLine}`;
system.write(output);
};
}

export function reportDiagnostics(diagnostics: Diagnostic[], reportDiagnostic: DiagnosticReporter): void {
for (const diagnostic of diagnostics) {
reportDiagnostic(diagnostic);
Expand Down Expand Up @@ -131,7 +139,7 @@ namespace ts {
reportWatchDiagnostic?: DiagnosticReporter
): WatchingSystemHost {
reportDiagnostic = reportDiagnostic || createDiagnosticReporter(system, pretty ? reportDiagnosticWithColorAndContext : reportDiagnosticSimply);
reportWatchDiagnostic = reportWatchDiagnostic || createWatchDiagnosticReporter(system);
reportWatchDiagnostic = reportWatchDiagnostic || pretty ? createWatchDiagnosticReporterWithColor(system) : createWatchDiagnosticReporter(system);
parseConfigFile = parseConfigFile || ts.parseConfigFile;
return {
system,
Expand Down