8000 Update the map between console color to VT sequences by daxian-dbw · Pull Request #11891 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@daxian-dbw
Copy link
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

private static readonly Dictionary<ConsoleColor, string> ConsoleColors = new Dictionary<ConsoleColor, string>
{
{ ConsoleColor.Black, "\x1b[2;30m" },
{ ConsoleColor.Gray, "\x1b[2;37m" },
{ ConsoleColor.Red, "\x1b[1;31m" },
{ ConsoleColor.Green, "\x1b[1;32m" },
{ ConsoleColor.Yellow, "\x1b[1;33m" },
{ ConsoleColor.Blue, "\x1b[1;34m" },
{ ConsoleColor.Magenta, "\x1b[1;35m" },
{ ConsoleColor.Cyan, "\x1b[1;36m" },
{ ConsoleColor.White, "\x1b[1;37m" },
{ ConsoleColor.DarkRed, "\x1b[2;31m" },
{ ConsoleColor.DarkGreen, "\x1b[2;32m" },
{ ConsoleColor.DarkYellow, "\x1b[2;33m" },
{ ConsoleColor.DarkBlue, "\x1b[2;34m" },
{ ConsoleColor.DarkMagenta, "\x1b[2;35m" },
{ ConsoleColor.DarkCyan, "\x1b[2;36m" },
{ ConsoleColor.DarkGray, "\x1b[1;30m" },
};

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
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
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
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
Member

@SteveL-MSFT Please update your review

@TravisEz13
Copy link
Member

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

@daxian-dbw
Copy link
Member Author

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

@adityapatwardhan
Copy link
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
ghost commented Feb 21, 2020

🎉v7.0.0-rc.3 has been released which incorporates this pull request.:tada:

Handy links:

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

0