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

Give min requirements to VS Code integrated terminal + debugger #77

Merged
merged 4 commits into from
Mar 2, 2018
Merged

Give min requirements to VS Code integrated terminal + debugger #77

merged 4 commits into from
Mar 2, 2018

Conversation

wuweiweiwu
Copy link
Contributor

@wuweiweiwu wuweiweiwu commented Feb 28, 2018

The debugger is not a full terminal emulator thus fails the stream.isTTY check. However, it still supports color. Thus by checking if the process is run through vscode through process.env.VSCODE_PID we can give the min color support to the debugger. (The terminal currently has level 1 support and the debugger has none)

// what the vscode integrated terminal has right now
{ supportsColor: [Function: getSupportLevel],
  stdout: { level: 1, hasBasic: true, has256: false, has16m: false },
  stderr: { level: 1, hasBasic: true, has256: false, has16m: false } }

microsoft/vscode#3006 (comment)

issue from Chalk:
chalk/chalk#254

index.js Outdated
@@ -47,6 +47,10 @@ function supportsColor(stream) {
return 2;
}

if (env.VSCODE_PID) {
Copy link

Choose a reason for hiding this comment

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

Doesn't the integrated terminal set the correct environment flags already? I thought it was only the debugger that didn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake. I'll move this check down to the if block on line 54 because the debugger doesnt have isTTY set

@Qix- Qix- merged commit b764af9 into chalk:master Mar 2, 2018
@Qix-
Copy link
Member

Qix- commented Mar 2, 2018

Thank you :)

@wuweiweiwu
Copy link
Contributor Author

No problem!

@jecot jecot mentioned this pull request May 1, 2020
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.

3 participants