Skip to content

Commit

Permalink
fix(rosetta): prints colors to log file (#3953)
Browse files Browse the repository at this point in the history
Rosetta error messages contain colors codes in the errors, which makes the errors produced by Rosetta look like garbage and very hard to parse.

Strip the colors if we detect a non-TTY input.

(Copied the regex from an MIT-licensed package to avoid adding extra dependencies)



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
rix0rrr authored Feb 28, 2023
1 parent 07f88d0 commit 0fc9229
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 8 deletions.
6 changes: 3 additions & 3 deletions packages/jsii-rosetta/bin/jsii-rosetta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ function handleDiagnostics(diagnostics: readonly RosettaDiagnostic[], fail: bool
if (fail !== false) {
// Fail on any diagnostic
if (diagnostics.length > 0) {
printDiagnostics(diagnostics, process.stderr);
printDiagnostics(diagnostics, process.stderr, process.stderr.isTTY);
logging.error(
[
`${diagnostics.length} diagnostics encountered in ${snippetCount} snippets`,
Expand All @@ -531,7 +531,7 @@ function handleDiagnostics(diagnostics: readonly RosettaDiagnostic[], fail: bool
// (so it's very clear what is failing the build), otherwise print everything.
const strictDiagnostics = diagnostics.filter((diag) => diag.isFromStrictAssembly);
if (strictDiagnostics.length > 0) {
printDiagnostics(strictDiagnostics, process.stderr);
printDiagnostics(strictDiagnostics, process.stderr, process.stderr.isTTY);
const remaining = diagnostics.length - strictDiagnostics.length;
logging.warn(
[
Expand All @@ -544,7 +544,7 @@ function handleDiagnostics(diagnostics: readonly RosettaDiagnostic[], fail: bool
}

if (diagnostics.length > 0) {
printDiagnostics(diagnostics, process.stderr);
printDiagnostics(diagnostics, process.stderr, process.stderr.isTTY);
logging.warn(`${diagnostics.length} diagnostics encountered in ${snippetCount} snippets`);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/commands/transliterate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export async function transliterateAssembly(
}
}

rosetta.printDiagnostics(process.stderr);
rosetta.printDiagnostics(process.stderr, process.stderr.isTTY);
if (rosetta.hasErrors && options.strict) {
throw new Error('Strict mode is enabled and some examples failed compilation!');
}
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-rosetta/lib/rosetta-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,8 @@ export class RosettaTabletReader {
);
}

public printDiagnostics(stream: NodeJS.WritableStream) {
printDiagnostics(this.diagnostics, stream);
public printDiagnostics(stream: NodeJS.WritableStream, colors = true) {
printDiagnostics(this.diagnostics, stream, colors);
}

public get hasErrors() {
Expand Down
17 changes: 15 additions & 2 deletions packages/jsii-rosetta/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ export interface File {
readonly fileName: string;
}

export function printDiagnostics(diags: readonly RosettaDiagnostic[], stream: NodeJS.WritableStream) {
export function printDiagnostics(diags: readonly RosettaDiagnostic[], stream: NodeJS.WritableStream, colors: boolean) {
// Don't print too much, at some point it just clogs up the log
const maxDiags = 50;

for (const diag of diags.slice(0, maxDiags)) {
stream.write(diag.formattedMessage);
stream.write(colors ? diag.formattedMessage : stripColorCodes(diag.formattedMessage));
}

if (diags.length > maxDiags) {
Expand Down Expand Up @@ -243,3 +243,16 @@ export async function pathExists(path: string): Promise<boolean> {
throw err;
}
}

// Copy/pasted from the 'ansi-regex' package to avoid taking a dependency for this one line that will never change
const ANSI_PATTERN = new RegExp(
[
'[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)',
'(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-nq-uy=><~]))',
].join('|'),
'g',
);

function stripColorCodes(x: string) {
return x.replace(ANSI_PATTERN, '');
}

0 comments on commit 0fc9229

Please sign in to comment.