Update the map between console color to VT sequences by daxian-dbw · Pull Request #11891 · PowerShell/PowerShell · GitHub
Skip to content

Update the map between console color to VT sequences#11891

Merged
adityapatwardhan merged 1 commit intoPowerShell:masterfrom
daxian-dbw:color
Feb 20, 2020
Merged

Update the map between console color to VT sequences#11891
adityapatwardhan merged 1 commit intoPowerShell:masterfrom
daxian-dbw:color

Conversation

@daxian-dbw
Copy link
Copy Markdown
Member

@daxian-dbw daxian-dbw commented Feb 19, 2020

PR Summary

Update the map between console color to VT sequences.
The ConsoleColor to VT Escape Sequences mapping in VTUtils.cs is not accurate, and thus is updated according to https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences#text-formatting

The update is needed as "Bright foreground color" (e.g. bright red \x1b[91m) is rendered differently from bold color (e.g. \x1b[1;31m) in some front-end client, such as the Jupyter Notebook client.
See the 3rd and 4th output below as an example:
image

PR Checklist

@daxian-dbw daxian-dbw added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Feb 19, 2020
@daxian-dbw daxian-dbw added this to the GA-consider milestone Feb 19, 2020
Copy link
Copy Markdown
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

Did you check the color rendering on Linux and/or macOS and Windows Terminal?

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Feb 20, 2020
@daxian-dbw
Copy link
Copy Markdown
Member Author

I did check on console host and win terminal, but not Linux and macOS yet. Will do that and update here.

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Feb 20, 2020
@daxian-dbw
Copy link
Copy Markdown
Member Author

daxian-dbw commented Feb 20, 2020

I tested the changes on console host, windows terminal, Linux terminal and macOS terminal, and all look good:

Windows console host

image

Windows terminal

image

Linux

image

macOS

Screen Shot 2020-02-20 at 11 40 15 AM

@adityapatwardhan
Copy link
Copy Markdown
Member

@SteveL-MSFT Please update your review

@TravisEz13
Copy link
Copy Markdown
Member

LGTM. Let's merge so we can start the build

@daxian-dbw
Copy link
Copy Markdown
Member Author

Since I have tested it on all platforms, I'm OK to merge this PR.

@adityapatwardhan
Copy link
Copy Markdown
Member

I am OK to merge this PR.

@adityapatwardhan adityapatwardhan dismissed SteveL-MSFT’s stale review February 20, 2020 22:27

The task asked in the review is complete

@adityapatwardhan adityapatwardhan merged commit da94afa into PowerShell:master Feb 20, 2020
@daxian-dbw daxian-dbw deleted the color branch February 20, 2020 22:33
@ghost
Copy link
Copy Markdown

ghost commented Feb 21, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants