Skip to content

match tsc's behaviour when printing diagnostics context #9

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

Merged
merged 1 commit into from
Jun 24, 2025

Conversation

k-yle
Copy link
Contributor

@k-yle k-yle commented Apr 3, 2025

When printing diagnostics, the typescript CLI first checks process.env.NO_COLOR and !process.stdout.isTTY to decide whether it should print with colours & context, or not.

Currently, this library always uses ts.formatDiagnosticsWithColorAndContext; so this PR updates the behaviour to match the tsc CLI.

Test plan:

  • run the CLI normally → it uses the pretty formatting
  • run the CLI with NO_COLOR=1 … → it uses the machine-readable formatting
  • run the CLI with … | cat → it uses the machine-readable formatting

@@ -119,6 +119,14 @@ export function findDiagnosticsScopeId(d: DiagnosticTsc, isStrictScope: boolean)
return `.${results.join('.')}`;
}

// if stdout is being piped to another destination, print diagnostics
// without pretty colours. This matches the behaviour of TSC:
// https://github.com/microsoft/TypeScript/blob/0652664e/src/compiler/executeCommandLine.ts#L161
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for providing this reference 🙏

Copy link
Member

Choose a reason for hiding this comment

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

Strange that they tsc considers NO_COLOR=0 to mean no color, but when in Rome 🤷‍♂️

@octogonz octogonz merged commit 32c77fd into tiktok:main Jun 24, 2025
2 checks passed
@k-yle k-yle deleted the kh/diags branch June 25, 2025 00:46
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.

2 participants